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 83328C433EF for ; Mon, 17 Jan 2022 23:19:04 +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: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:References: List-Owner; bh=bjWx+DI1WYZdi7gYBLm+vX3DJYCjnVkKKN6YlFHR2Rw=; b=mUuQZC+Nr5qqjp eCOTZ46cQuvC8GNGhZWgN7dOe/2yYhr5oatcMyqyI5gHb4u66OgGenca2/LXmqH0Ova1ky7szdNm7 rR0P01MA3MYiCzKOmwTknBy83kC2ctPfL1wgjgd375lrnnDHlO5w3KowHfkMcehpdXW/e/YjpYM+7 HPPYq1fcfiQ2GhYIbey2ImeD6mKqimcRRt5JcxWXQ1kMWC9PV1aRhkRe3z+Ap0io57jFkqsyG2duc mB5rxWRwTGLrzHmVM/JDiVSjMndktr4jfqO9h+X/DxzD9oaUc6XH2TFK8sermZhSehjktkO4INrfg kLa7AfsbMauKJFa5TJEw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9bFu-00Gasm-PM; Mon, 17 Jan 2022 23:17:42 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9bFr-00GasE-Bs for linux-arm-kernel@lists.infradead.org; Mon, 17 Jan 2022 23:17:40 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 47AE6611B6; Mon, 17 Jan 2022 23:17:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 51069C36AE7; Mon, 17 Jan 2022 23:17:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1642461457; bh=/H4hodNYNQX3IviAGezs1hAY921lAVGOrr7Ddgmplvo=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=ia/UvUI844gw+WPfwhpJ6CQYqC3Io+w357Z+aGKwEsO1sO8ICYc/ENa5nKvN4o2oN BKj58XNTesbzhY5wein2705/CG/tTJU/xv/5yvvaY34MCqjfQe19ubqEthQho8Pehy id0x+GZtfWNnAE7taY4Ezoq256Sh2ElZaGHI+GLWAJpZdMun5QDEJWL+3KTDSYWBl1 SfTMZm9mqqZr53WWoSw9yAujDqEi+P/3WPo+Nl+LTyULpCIPCDvEWKHCMOCzg3JL1Y 2Y3gt4LF17SICawmHmm12LXg8EIxriCiWKPK61Z+b2Z1HnfKp/1aG4JT57ygcrEv2q nTDv54d+coBKA== Date: Mon, 17 Jan 2022 17:17:35 -0600 From: Bjorn Helgaas To: Xiaolei Wang Cc: hongxing.zhu@nxp.com, l.stach@pengutronix.de, lorenzo.pieralisi@arm.com, robh@kernel.org, kw@linux.com, bhelgaas@google.com, shawnguo@kernel.org, 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: imx: disable reglator when imx6_pcie_probe fails Message-ID: <20220117231735.GA813924@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220117102137.3513439-1-xiaolei.wang@windriver.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220117_151739_533690_B502DD93 X-CRM114-Status: GOOD ( 28.19 ) 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 Mon, Jan 17, 2022 at 06:21:37PM +0800, Xiaolei Wang wrote: > From: wrsadmin I got two copies of this. This one changed: - Signed-off-by: wrsadmin + Signed-off-by: Xiaolei Wang This looks like a good change. You should also change the "From: wrsadmin" line above because that is what "git am" uses in the commit: commit 119fdfb8fb1b ("pci: imx: disable reglator when imx6_pcie_probe fails") Author: wrsadmin Date: Mon Jan 17 18:17:35 2022 +0800 Also, please run "git log --oneline drivers/pci/controller/dwc/pci-imx6.c" and make your subject line match in capitalization and style. Also, fix the "s/reglator/regulator" typo in subject and below and add "()" after the imx6_pcie_probe() function name. If you include a calltrace, remove timestamps and other non-relevant stuff from it and indent quoted material two spaces. But in this case, I don't think the calltrace is useful. The reason to do this is not to avoid a warning and the related calltrace. The reason is to fix the problem that *caused* the warning in the first place. So we should describe the underlying problem and the fix. > disable reglator when imx6_pcie_probe fails, > otherwise the following calltrace will appear > > [ 3.785075] ------------[ cut here ]------------ > [ 3.788142] Registering SWP/SWPB emulation handler > [ 3.789853] WARNING: CPU: 0 PID: 7 at drivers/regulator/core.c:2257 _regulator_put.part.0+0x1bc/0x1e0 > [ 3.795680] Loading compiled-in X.509 certificates > [ 3.803947] Modules linked in: > [ 3.811922] CPU: 0 PID: 7 Comm: kworker/u8:0 Not tainted 5.16.0-10645-g3c750c7b6143-dirty #9 > [ 3.820393] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > [ 3.826945] Workqueue: events_unbound async_run_entry_fn > [ 3.832304] unwind_backtrace from show_stack+0x10/0x14 > [ 3.837569] show_stack from dump_stack_lvl+0x58/0x70 > [ 3.842663] dump_stack_lvl from __warn+0xd8/0x114 > [ 3.847493] __warn from warn_slowpath_fmt+0x5c/0xc4 > [ 3.852490] warn_slowpath_fmt from _regulator_put.part.0+0x1bc/0x1e0 > [ 3.858968] _regulator_put.part.0 from regulator_put+0x2c/0x3c > [ 3.864918] regulator_put from release_nodes+0x50/0x178 > [ 3.870270] release_nodes from devres_release_all+0x80/0xd0 > [ 3.875968] devres_release_all from really_probe+0xdc/0x30c > [ 3.881661] really_probe from __driver_probe_device+0x80/0xe4 > [ 3.887522] __driver_probe_device from driver_probe_device+0x30/0xd4 > [ 3.893991] driver_probe_device from __driver_attach_async_helper+0x20/0x38 > [ 3.901068] __driver_attach_async_helper from async_run_entry_fn+0x20/0xb4 > [ 3.908059] async_run_entry_fn from process_one_work+0x298/0x7d0 > [ 3.914188] process_one_work from worker_thread+0x30/0x510 > [ 3.919792] worker_thread from kthread+0x128/0x14c > [ 3.924705] kthread from ret_from_fork+0x14/0x38 > [ 3.929443] Exception stack(0xc20cbfb0 to 0xc20cbff8) > [ 3.934521] bfa0: 00000000 00000000 00000000 00000000 > [ 3.942722] bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 3.950922] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ 3.957677] irq event stamp: 1207 > [ 3.961024] hardirqs last enabled at (1215): [] __up_console_sem+0x50/0x60 > [ 3.968974] hardirqs last disabled at (1224): [] __up_console_sem+0x3c/0x60 > [ 3.976911] softirqs last enabled at (1206): [] __do_softirq+0x2ec/0x5a4 > [ 3.984669] softirqs last disabled at (1197): [] irq_exit+0x18c/0x20c > [ 3.992021] ---[ end trace 45a52c023bf8fb33 ]--- > > Signed-off-by: Xiaolei Wang > --- > drivers/pci/controller/dwc/pci-imx6.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 6974bd5aa116..f8279a15463b 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -1216,7 +1216,7 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > ret = dw_pcie_host_init(&pci->pp); > if (ret < 0) > - return ret; > + goto err_vpcie; > > if (pci_msi_enabled()) { > u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI); > @@ -1226,6 +1226,11 @@ static int imx6_pcie_probe(struct platform_device *pdev) > } > > return 0; > + > +err_vpcie: > + regulator_disable(imx6_pcie->vpcie); I don't know the regulator code and I couldn't figure out how devm_regulator_get_optional() works because it passes OPTIONAL_GET in to _devm_regulator_get(), but nothing ever checks for that, and I couldn't be bothered to figure out what magic makes it work. But regulator_disable() definitely dereferences its argument, and I'm pretty sure it's possible to get here with "imx6_pcie->vpcie == NULL". Also, it looks like the same situation with imx6_pcie->vph, so why don't you clean that one up, too? > + return ret; > } > > static void imx6_pcie_shutdown(struct platform_device *pdev) > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel