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;
MIME-Version: 1.0
References: <20230703-pci-imx-regulator-cleanup-v1-1-b6c050ae2bad@kernel.org>
In-Reply-To: <20230703-pci-imx-regulator-cleanup-v1-1-b6c050ae2bad@kernel.org>
From:   Fabio Estevam <festevam@gmail.com>
Date:   Tue, 4 Jul 2023 08:50:15 -0300
Message-ID: <CAOMZO5AghPUsVqMRdByR9hOatwHmgx90mq1HyZYFGkw1WaAY_Q@mail.gmail.com>
Subject: Re: [PATCH] PCI: dwc: Provide deinit callback for i.MX
To:     Mark Brown <broonie@kernel.org>
Cc:     Richard Zhu <hongxing.zhu@nxp.com>,
        Lucas Stach <l.stach@pengutronix.de>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= <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>,
        NXP Linux Team <linux-imx@nxp.com>,
        linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Mark,

On Mon, Jul 3, 2023 at 1:30=E2=80=AFPM Mark Brown <broonie@kernel.org> wrot=
e:
>
> The i.MX integration for the DesignWare PCI controller has a _host_exit()
> operation which undoes everything that the _host_init() operation does bu=
t
> does not wire this up as the host_deinit callback for the core, or call i=
t
> 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 16=
G
> 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+0=
x110/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);