From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 89B79CD342F for ; Tue, 3 Sep 2024 06:30:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=/RMJxsOK4Y6eGOWdB1h2LGOXOp0A9WbAObwDhsltTEo=; b=AF2Ul5Oe339EUqRbrIJv5R0iIP /v1+gzWLJr8wzOzifa1tw3MafKdgf5CCsev7unc4d87VD8XEIig0XJcZhVOSY5rBeNuODRffuHpLk 6euxQUiNhz8nBzoo2f8OGoOAX4DFIG4kMCob+PULJHPLZKTV58+dxpJnFVjXjPrma65e4U5ymjkFR mwSOy4hwl59n8K2+BwN+5L1cYCnJORJ3xaslAvTj5m4hYAmeIDG1gHBNdZHgICMjFTDTPZwXfGQoA nw4J4tQt6MT+so6GFmYiYombj+b8o2fRkWnHt+TxvmrsIQBp3vMZ0rvj43L1k1ndYlS9ZdKrbAtGx OdpoZ82w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1slN3n-0000000GTDN-1VnZ; Tue, 03 Sep 2024 06:30:39 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1slN2r-0000000GT2M-4ACk for linux-arm-kernel@lists.infradead.org; Tue, 03 Sep 2024 06:29:43 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id DC5C75C59BB; Tue, 3 Sep 2024 06:29:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 66154C4CEC5; Tue, 3 Sep 2024 06:29:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1725344980; bh=10+jhI/6ImbS3ZFkXBIC2SnK6R1MLziOBOnLAq6QFJs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oG3FmULU+/8/xnITT69vNFuShKPWGm96zQV0TF/w6tzAgzzHXvDwx63HT7w8KE85t bNOAGaEjVYSwfyWZEpI7naviFAf3MZJNxzeVMPST9nd3zo3f1C066nUcxSuOGTJsh+ ZTDz6sUwC02OwBk9F41JpePipOzUUnXkfUDCerTM= Date: Tue, 3 Sep 2024 08:29:36 +0200 From: Greg Kroah-Hartman To: Yuesong Li Cc: Krzysztof Kozlowski , Jakub Kicinski , Geert Uytterhoeven , "David S. Miller" , Mark Brown , Paolo Abeni , Daniel Mack , Haojian Zhuang , Robert Jarzmik , linux-arm-kernel@lists.infradead.org, linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, opensource.kernel@vivo.com, Andy Shevchenko , Yang Ruibin <11162571@vivo.com> Subject: Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return Message-ID: <2024090358-settling-blimp-fb4c@gregkh> References: <20240829033511.1917015-1-11162571@vivo.com> <4e2ad62b-b11e-40db-9cd9-a26f7642c735@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240902_232942_190665_79E82452 X-CRM114-Status: GOOD ( 43.72 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Sep 02, 2024 at 03:15:48PM +0800, Yuesong Li wrote: > > > On 2024/8/31 1:10, Krzysztof Kozlowski wrote: > > On 30/08/2024 10:55, Geert Uytterhoeven wrote: > > > Hi Yang, > > > > > > On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote: > > > > Increase the reference count by calling pci_get_slot(), and remember to > > > > decrement the reference count by calling pci_dev_put(). > > > > > > > > Signed-off-by: Yang Ruibin <11162571@vivo.com> > > > > > > Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi: > > > Insert the missing pci_dev_put()before return") in spi/for-next. > > > > > > > --- a/drivers/spi/spi-pxa2xx-pci.c > > > > +++ b/drivers/spi/spi-pxa2xx-pci.c > > > > @@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c) > > > > c->num_chipselect = 1; > > > > > > > > ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000); > > > > - if (ret) > > > > + if (ret) { > > > > + pci_dev_put(dma_dev); > > > > > > dma_dev is still uninitialized at this point. > > > > > > > return ret; > > > > + } > > > > > > > > dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0)); > > > > > > dma_dev is initialized only here... > > > > > > > ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev); > > > > > > ... and freed automatically by lpss_dma_put_device() in case of > > > any later failures since commit 609d7ffdc42199a0 ("spi: pxa2xx-pci: > > > Balance reference count for PCI DMA device") in v5.18. > > > > > > > @@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c) > > > > } > > > > > > > > ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000); > > > > - if (ret) > > > > + if (ret) { > > > > + pci_dev_put(dma_dev); > > > > return ret; > > > > + } > > > > > > > > dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0)); > > > > ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev); > > > > > > Likewise. > > > > > > Hence this patch is not needed, and introduced two bugs. > > > > Cc Greg, Jakub, David and Paolo, > > > > It seems Vivo (at least two persons from vivo.com) is sending patches > > generated through some sort of automation without really knowing what > > they were doing. All of the patches look like innocent > > cleanups/simplifications/fixes, but they do more. > > > > This patch here looks like introducing two bugs. > > > > These patches: > > 1. https://lore.kernel.org/all/20240830033251.232992-1-yujiaoliang@vivo.com/ > > > > 2. https://lore.kernel.org/all/20240828122650.1324246-1-11162571@vivo.com/ > > (I sent a revert for this) > > > > 3. https://lore.kernel.org/all/20240829072016.2329466-1-11162571@vivo.com/ > > > > and probably more... > > > > introduce dev_err_probe() outside of probe path which is not desired, > > because it marks a probed (working) device as deferred. > > > > The patches look trivial and/or helpful, so people tend to accept them > > through default trust. > > > > I kindly suggest reverse - do not trust them by default and instead do a > > thorough review before accepting any cleanup/trivial patch from @vivo.com. > > > > Best regards, > > Krzysztof > > > > > > Dear Geert, Krzysztof, and the Linux Kernel Community, > > I hope this message finds you well. My name is Yuesong Li, and I am writing > on behalf of VIVO to sincerely apologize for the recent issues caused by the > patches submitted by our team members. We deeply regret the problems that > these submissions have introduced and the concerns they have raised within > the community. > > We recognize that the patches submitted were not up to the standards > expected by the Linux kernel community. It is clear that our team members > did not fully understand the implications of their contributions, leading to > errors and the need for reverts. This is entirely our responsibility, and we > are committed to ensuring that this does not happen again. > > To address these issues, VIVO is taking the following steps: > > 1.Training for employees: We are implementing a comprehensive training > program for all employees who contribute to open source projects. This > training will focus on understanding the intricacies of the Linux kernel, > best practices for code submissions, and the importance of thorough testing > and review before submitting patches. > > 2.Enhanced Internal Review Process: Moving forward, we will enforce a more > rigorous internal review process for all patches before they are submitted > to the community. This will involve senior developers with experience in the > open source community who will guide and review the work of less experienced > contributors. > > We value the open-source community and the collaborative spirit that drives > it. VIVO is committed to contributing positively and responsibly moving > forward. We kindly ask for your forgiveness for the mistakes we've made and > your understanding as we take concrete steps to improve. > > Thank you for your continued dedication to the Linux kernel, and please feel > free to reach out if there are any further concerns or if you have > suggestions on how we can better align with the community's expectations. Thanks for doing this. I've now dropped all pending vivo patches that were in my review queues and will wait for this process to happen so that they can be resubmitted after proper review by your internal groups. good luck! greg k-h