Message ID | 20230703-pci-imx-regulator-cleanup-v1-1-b6c050ae2bad@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f45:0:b0:3ea:f831:8777 with SMTP id v5csp652492vqx; Mon, 3 Jul 2023 09:44:40 -0700 (PDT) X-Google-Smtp-Source: APBJJlEXH7ycCaAvcnEQssUxyQCZiKUhnaserGUg8OSDYLFSWOBBRxtljqUV/+337cik/yK4AY2Q X-Received: by 2002:a05:6a00:3103:b0:67f:ff0a:1bbb with SMTP id bi3-20020a056a00310300b0067fff0a1bbbmr12167769pfb.1.1688402679774; Mon, 03 Jul 2023 09:44:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688402679; cv=none; d=google.com; s=arc-20160816; b=m4Tu6LPMZ4TutlxxcGvTFYCdQc0HB7xGduUyXHDPwXHpo29o6p7cD1UmpSktbYjhmg TI/JfrC5qcP81GH/RTBkfoyUBG0i51wHboZjz1L7TmoqlcUBLF6qCgKnrTPZqZgh383c AlxVHKfd4WXTbdpP+H54GvhY53QV09hrH6bOT2vyPiEWv/TaPk7Xnx3oR93dsmbQw091 IjQd6HAOva0O4ItC8KASfKbM+Jek+tKx6zdWYDzB/6xjDahihWqtWB2gkwVc1FNC6VjR S+ps0T3jdAl7rinw8xYK7Pyw30jP9K6s64lZkrH+JHDg7HQ+xLjKOixc8viu4PMrOZWT Um+A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:message-id:content-transfer-encoding :mime-version:subject:date:from:dkim-signature; bh=rknymOG7c1FxueK50H/I/xKGJwN0MzRJ7WhewKkvSv0=; fh=BgQ/1wAgXYFSlM43M8FR/bi8HX9Awp9/7UVjplcIl2s=; b=G1hl66jQbr5Ds4KCz+uXEl6mIxWRXKEkqyK1HYcx0jQUKtvahTLRpcbC5jVdclFwhJ 2ohKNyVGGU4hQJfEW5fRbiOss6eidXe0mghGgFr3K1qFfXmQ8bqueeLkUNuhnogS4Q01 LlDrYzJB00Tr/dyeQUTLfF+A5b0GWqHx1oZ8CSVZU0rwxW4KMs7v6lLbJdoKC0x4jd60 RsCxX2vizJpXA5kI4e8WEK1hVkCsBfKUUso1Tfzap4fs1T0AcWNgpC+ssPcdk6788wv8 DHHFE9tQGDPOUkhaOj0W5e8LmNPJxVXpO47kz/pZnvGs0QEC9CszwEX+8u5YoyamLxBm lMbg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=bBPtlmYX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id cr5-20020a056a000f0500b006738dfbaf42si16830522pfb.374.2023.07.03.09.44.24; Mon, 03 Jul 2023 09:44:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=bBPtlmYX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231305AbjGCQaQ (ORCPT <rfc822;ivan.orlov0322@gmail.com> + 99 others); Mon, 3 Jul 2023 12:30:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52482 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229504AbjGCQaP (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 3 Jul 2023 12:30:15 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AD6A190; Mon, 3 Jul 2023 09:30:10 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 485B760F7B; Mon, 3 Jul 2023 16:30:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 916BAC433C7; Mon, 3 Jul 2023 16:30:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1688401809; bh=8ZJywgN7zX7HPIQiniWG85bY3ojzDG5dLyrGXUsgDdE=; h=From:Date:Subject:To:Cc:From; b=bBPtlmYXEp+MmXj+TTq77bJFmzgUBr1/B+iXmcLdcb5xZ9/a1Mv2J0O0bN+hw7UwR kcwAQcGEXSfAt/6jbyHy2OXep4xO9aKChH1y12pGV+Sg1Hv/sA3SE9rIc8zXOCBVX2 xzlPvYpGbjcJQ65OH/AMChnqdjWZLxy6VD1ovIQ7DAuYF3PiJDpzihWq79X7PqXKeK RL2OA6+4nh3ZALka3w0A5oxCwnFwXel1vs6KPGr7tY12LEYdfUcjojKBnUnLjYQyAW hHQc8CjG3hVzgTr8guGm28TB1w8d7FuGjd3GP9QBfJjjmJzQIdGHXSC7WU57lqDy4x c84iIS4ulRtUw== From: Mark Brown <broonie@kernel.org> Date: Mon, 03 Jul 2023 17:23:09 +0100 Subject: [PATCH] PCI: dwc: Provide deinit callback for i.MX MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20230703-pci-imx-regulator-cleanup-v1-1-b6c050ae2bad@kernel.org> X-B4-Tracking: v=1; b=H4sIAO31omQC/x2N0QqDMAxFf0XyvECriGO/MvYQY9Qw15Z0boL47 6t7PBfOPTtkMZUMt2oHk49mjaGAv1TAM4VJUIfCULu6cZ1rMLGivjY0mdaF3tGQF6GwJiTf8XU cfCvcQvF7yoK9UeD5fPhGe55zMhl1+yfvj+P4AXxsmEWCAAAA To: Richard Zhu <hongxing.zhu@nxp.com>, Lucas Stach <l.stach@pengutronix.de>, Lorenzo Pieralisi <lpieralisi@kernel.org>, =?utf-8?q?Krzysztof_Wilczy=C5=84?= =?utf-8?q?ski?= <kw@linux.com>, Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team <kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, NXP Linux Team <linux-imx@nxp.com> Cc: linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org> X-Mailer: b4 0.13-dev-099c9 X-Developer-Signature: v=1; a=openpgp-sha256; l=1683; i=broonie@kernel.org; h=from:subject:message-id; bh=8ZJywgN7zX7HPIQiniWG85bY3ojzDG5dLyrGXUsgDdE=; b=owEBbQGS/pANAwAKASTWi3JdVIfQAcsmYgBkoveMNRJ5ZaJVV2xDxJg/bNmgjsnBBv0SXgdm9jjz L1k3vY6JATMEAAEKAB0WIQSt5miqZ1cYtZ/in+ok1otyXVSH0AUCZKL3jAAKCRAk1otyXVSH0AWyB/ 0SUGEefHTTN8YS90X8Fn/ghC+xZkL23oIxw85nsyJ2dZgPwb1Txh3vHr/WSqrKMJhRrE7xcP5zCE/z 4Td/4VUtE8jFfpmbqiM9ZNYtpW1SjBasiwNqkvZbgd+Fd82VA9jO2ZyvR17oT01q8xJX+d1qCpW5If 2X5E9QpMGVkBbWwkhhwCe6+PK9rOalVWUNrs+ED7OeQ4PNKwAH3WccpauzRje5xx8xuLm5eU5ay5KC RcQxZ+BMMee90Qh35J79LRXvcaVk2oa7GILyGHx6HeuFtHLtddCnsRydvk+XPn+Ta/Ph4NdkGpgwCh dN9a4/iM7ZOUogL29hhzTT4xHYqVDb X-Developer-Key: i=broonie@kernel.org; a=openpgp; fpr=3F2568AAC26998F9E813A1C5C3F436CA30F5D8EB X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1770418528256939963?= X-GMAIL-MSGID: =?utf-8?q?1770418528256939963?= |
Series |
PCI: dwc: Provide deinit callback for i.MX
|
|
Commit Message
Mark Brown
July 3, 2023, 4:23 p.m. UTC
The i.MX integration for the DesignWare PCI controller has a _host_exit()
operation which undoes everything that the _host_init() operation does but
does not wire this up as the host_deinit callback for the core, or call it
in any path other than suspend. This means that if we ever unwind the
initial probe of the device, for example because it fails, the regulator
core complains that the regulators for the device were left enabled:
imx6q-pcie 33800000.pcie: iATU: unroll T, 4 ob, 4 ib, align 64K, limit 16G
imx6q-pcie 33800000.pcie: Phy link never came up
imx6q-pcie 33800000.pcie: Phy link never came up
imx6q-pcie: probe of 33800000.pcie failed with error -110
------------[ cut here ]------------
WARNING: CPU: 2 PID: 46 at drivers/regulator/core.c:2396 _regulator_put+0x110/0x128
Wire up the callback so that the core can clean up after itself.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/pci/controller/dwc/pci-imx6.c | 1 +
1 file changed, 1 insertion(+)
---
base-commit: 6995e2de6891c724bfeb2db33d7b87775f913ad1
change-id: 20230703-pci-imx-regulator-cleanup-a17c8fd15ec5
Best regards,
Comments
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: 2023年7月4日 0:23 > To: Hongxing Zhu <hongxing.zhu@nxp.com>; Lucas Stach > <l.stach@pengutronix.de>; Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof > Wilczyński <kw@linux.com>; Rob Herring <robh@kernel.org>; Bjorn Helgaas > <bhelgaas@google.com>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Pengutronix Kernel Team <kernel@pengutronix.de>; > Fabio Estevam <festevam@gmail.com>; dl-linux-imx <linux-imx@nxp.com> > Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; Mark Brown <broonie@kernel.org> > Subject: [PATCH] PCI: dwc: Provide deinit callback for i.MX > > The i.MX integration for the DesignWare PCI controller has a _host_exit() > operation which undoes everything that the _host_init() operation does but does > not wire this up as the host_deinit callback for the core, or call it in any path other > than suspend. This means that if we ever unwind the initial probe of the device, > for example because it fails, the regulator core complains that the regulators for > the device were left enabled: > > imx6q-pcie 33800000.pcie: iATU: unroll T, 4 ob, 4 ib, align 64K, limit 16G > imx6q-pcie 33800000.pcie: Phy link never came up imx6q-pcie 33800000.pcie: > Phy link never came up > imx6q-pcie: probe of 33800000.pcie failed with error -110 ------------[ cut > here ]------------ > WARNING: CPU: 2 PID: 46 at drivers/regulator/core.c:2396 > _regulator_put+0x110/0x128 > > Wire up the callback so that the core can clean up after itself. > > Signed-off-by: Mark Brown <broonie@kernel.org> Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com> Thanks. Best Regards Richard Zhu > --- > drivers/pci/controller/dwc/pci-imx6.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > b/drivers/pci/controller/dwc/pci-imx6.c > index 52906f999f2b..e4942bd2598d 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -1039,6 +1039,7 @@ static void imx6_pcie_host_exit(struct dw_pcie_rp > *pp) > > static const struct dw_pcie_host_ops imx6_pcie_host_ops = { > .host_init = imx6_pcie_host_init, > + .host_deinit = imx6_pcie_host_exit, > }; > > static const struct dw_pcie_ops dw_pcie_ops = { > > --- > base-commit: 6995e2de6891c724bfeb2db33d7b87775f913ad1 > change-id: 20230703-pci-imx-regulator-cleanup-a17c8fd15ec5 > > Best regards, > -- > Mark Brown <broonie@kernel.org>
Hi Mark, On Mon, Jul 3, 2023 at 1:30 PM Mark Brown <broonie@kernel.org> wrote: > > The i.MX integration for the DesignWare PCI controller has a _host_exit() > operation which undoes everything that the _host_init() operation does but > does not wire this up as the host_deinit callback for the core, or call it > in any path other than suspend. This means that if we ever unwind the > initial probe of the device, for example because it fails, the regulator > core complains that the regulators for the device were left enabled: > > imx6q-pcie 33800000.pcie: iATU: unroll T, 4 ob, 4 ib, align 64K, limit 16G > imx6q-pcie 33800000.pcie: Phy link never came up > imx6q-pcie 33800000.pcie: Phy link never came up > imx6q-pcie: probe of 33800000.pcie failed with error -110 > ------------[ cut here ]------------ > WARNING: CPU: 2 PID: 46 at drivers/regulator/core.c:2396 _regulator_put+0x110/0x128 Your change looks good, but I have a more basic question. I recall seeing this regulator warning before: f81f095e8771 ("PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails") I think the real issue is why does the probe fail when the link is not up? It should not fail to probe. At least, this was the original intention with Rob's commit: 886a9c134755 ("PCI: dwc: Move link handling into common code") Should we do something like this? diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index cf61733bf78d..6bd6ffef900d 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -509,6 +509,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) err_stop_link: dw_pcie_stop_link(pci); + return 0; err_remove_edma: dw_pcie_edma_remove(pci);
On Tue, Jul 04, 2023 at 08:50:15AM -0300, Fabio Estevam wrote: > I recall seeing this regulator warning before: > f81f095e8771 ("PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails") > I think the real issue is why does the probe fail when the link is not up? Well, they're both issues - if someone unloads or unbinds the driver we can still trigger this, even if the probe succeeded. > It should not fail to probe. At least, this was the original intention > with Rob's commit: > 886a9c134755 ("PCI: dwc: Move link handling into common code") > + return 0; > > err_remove_edma: > dw_pcie_edma_remove(pci); I have no idea about doing this, all the PCI on the board that I care about seems to work fine (and worked fine even with the failure, I'm not sure this is being generated by a link that's in use).
On Tue, Jul 4, 2023 at 8:55 AM Mark Brown <broonie@kernel.org> wrote: > Well, they're both issues - if someone unloads or unbinds the driver we > can still trigger this, even if the probe succeeded. Yes, for your change: Tested-by: Fabio Estevam <festevam@gmail.com> > > It should not fail to probe. At least, this was the original intention > > with Rob's commit: > > > 886a9c134755 ("PCI: dwc: Move link handling into common code") > > > + return 0; > > > > err_remove_edma: > > dw_pcie_edma_remove(pci); > > I have no idea about doing this, all the PCI on the board that I care > about seems to work fine (and worked fine even with the failure, I'm not > sure this is being generated by a link that's in use). The probe failure when link is not up happens in linux-next due to commit: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started") Prior this commit the PCI driver probed successfully even when the link was down.
On Tue, Jul 04, 2023 at 09:13:09AM -0300, Fabio Estevam wrote: > On Tue, Jul 4, 2023 at 8:55 AM Mark Brown <broonie@kernel.org> wrote: > > I have no idea about doing this, all the PCI on the board that I care > > about seems to work fine (and worked fine even with the failure, I'm not > > sure this is being generated by a link that's in use). > The probe failure when link is not up happens in linux-next due to commit: > da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started") > Prior this commit the PCI driver probed successfully even when the > link was down. Ah, I see. I don't know what's going on here enough to say if that's good or bad but it does at least seem plausible.
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 52906f999f2b..e4942bd2598d 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -1039,6 +1039,7 @@ static void imx6_pcie_host_exit(struct dw_pcie_rp *pp) static const struct dw_pcie_host_ops imx6_pcie_host_ops = { .host_init = imx6_pcie_host_init, + .host_deinit = imx6_pcie_host_exit, }; static const struct dw_pcie_ops dw_pcie_ops = {