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 34667C6FA82 for ; Tue, 13 Sep 2022 07:34:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To: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=UhBY7gDbNZMXJ4x1alKUckF7YzFCd3d0Oij00VkMlQ8=; b=pwPrOXN1SVYqZd D3N7HMAj/ra4MQjVdodcvsIz4Twdvl4Dr+C2wNCKjWLyLUO8xBWt+6Tlx4kbSZvCjj+julBX3L2UL tnAtEBTAze7CmFk0tXimCdx4qrL4pwJF1QiWmEi9UHyxm62meaQXbnsBKNxn/prb9esbYGqd9pszb Fwsx3vsTLnjp3wFk31CYOKfM6Jf1JSd3KwGFVJqOhO4cuhlAVlUt0SEQjYv9VPvriJu31PllhvnDj H/bSLmHNGb/V5gDQtqChnuQacQlo8kmeFAWMFmte/xSy/D2R+xMV/3g0fSnPXoRRJBGOaqOqqf2Gx RUi2ULxbz0CuR1H+x9aQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oY0Ov-003acr-EO; Tue, 13 Sep 2022 07:32:10 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oY0Oq-003aY4-9V for linux-arm-kernel@lists.infradead.org; Tue, 13 Sep 2022 07:32:06 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=xglz0vjqr0RitGgMnGnkfLMxC6SCl6nGd91QSpoaNaI=; b=o7TxC+At2Mo+ZS3vjfpbAZySLL +4QjSd4wUUiVK0sp0IUeOKe/F7SKWYJl0wtwOsWlzf9kOkgMkN9OEfLRabuDwIvtMDWDGJbp+yZGq 5M2h0hXUUwvM1PsGhGpFnG3bk128hVOfsJ+Z6MRt+K+nBJpojyHhDEpXEPGkx4o+nNzrK0N3gmgHh BtjAAedwtM/bPGovll4HLeWpGCqpeDdDlKl1/c6l/YFuMIVrbZ3vrsdheTrPXExgOS6YsPkaAojLX EKXhG7F8MkXpdaCyOUGEGb6VKpn76YrNKwO6tM5v541igx3D2BITIn+t9XuHmTs1zX59ZcltfzONA ndAr3sfA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:34278) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1oY0OU-0002Y5-FU; Tue, 13 Sep 2022 08:31:42 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1oY0ON-0000UN-4W; Tue, 13 Sep 2022 08:31:35 +0100 Date: Tue, 13 Sep 2022 08:31:35 +0100 From: "Russell King (Oracle)" To: Tang Bin Cc: hongxing.zhu@nxp.com, l.stach@pengutronix.de, lorenzo.pieralisi@arm.com, robh@kernel.org, kw@linux.com, shawnguo@kernel.org, bhelgaas@google.com, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PCI: imx6: Fix wrong check in imx6_pcie_attach_pd() Message-ID: References: <20220913065910.15348-1-tangbin@cmss.chinamobile.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220913065910.15348-1-tangbin@cmss.chinamobile.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220913_003204_369407_3A4A81C8 X-CRM114-Status: GOOD ( 18.14 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 13, 2022 at 02:59:10PM +0800, Tang Bin wrote: > In the function imx6_pcie_attach_pd(), > dev_pm_domain_attach_by_name() may return NULL in some cases, > so IS_ERR() doesn't meet the requirements. Thus fix it. NAK. You are clearly doing a mechanical search and replace, and then throwing out patches without a care in the world for other people to then decide whether the changes are in fact appropriate or not. Please don't do that. Please read and understand the code before you waste reviewers and developers time - otherwise you will educate reviews and developers to ignore your efforts. > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 6619e3caf..65d6ebbba 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -337,8 +337,8 @@ static int imx6_pcie_attach_pd(struct device *dev) > return 0; > > imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie"); > - if (IS_ERR(imx6_pcie->pd_pcie)) > - return PTR_ERR(imx6_pcie->pd_pcie); > + if (IS_ERR_OR_NULL(imx6_pcie->pd_pcie)) > + return PTR_ERR(imx6_pcie->pd_pcie) ? : -ENODATA; > /* Do nothing when power domain missing */ > if (!imx6_pcie->pd_pcie) > return 0; Your change is incorrect, as can be seen by the following if() statement, which checks for NULL here. Clearly, the explicit intention is that if dev_pm_domain_attach_by_name() returns NULL, imx6_pcie_attach_pd() does _not_ fail. So you are likely creating a regression by your change. You are likely introducing a bug rather than fixing something. This is why you must always carefully review any mechanical change. > @@ -352,8 +352,8 @@ static int imx6_pcie_attach_pd(struct device *dev) > } > > imx6_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy"); > - if (IS_ERR(imx6_pcie->pd_pcie_phy)) > - return PTR_ERR(imx6_pcie->pd_pcie_phy); > + if (IS_ERR_OR_NULL(imx6_pcie->pd_pcie_phy)) > + return PTR_ERR(imx6_pcie->pd_pcie_phy) ? : -ENODATA; > > link = device_link_add(dev, imx6_pcie->pd_pcie_phy, > DL_FLAG_STATELESS | This change is unnecessary. If dev_pm_domain_attach_by_name() returns Null, then device_link_add() will also return NULL, and the check for a NULL link will then succeed. So the NULL case is already cleanly handled. So overall, your patch is unnecessary and introduces a bug rather than fixing it. Therefore, you can discard the patch in its entirety. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel