On Mon, Jun 9, 2025 at 1:03 AM Hongxing Zhu [off-list ref] wrote:

quoted

-----Original Message-----
From: Tim Harvey <tharvey@gateworks.com>
Sent: 2025年6月7日 5:04
To: Hongxing Zhu <hongxing.zhu@nxp.com>
Cc: l.stach@pengutronix.de; bhelgaas@google.com; lpieralisi@kernel.org;
kw@linux.com; manivannan.sadhasivam@linaro.org; robh@kernel.org;
krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org; Frank Li
[off-list ref]; s.hauer@pengutronix.de; festevam@gmail.com;
imx@lists.linux.dev; kernel@pengutronix.de; linux-pci@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 05/10] PCI: imx6: Deassert apps_reset in
imx_pcie_deassert_core_reset()

On Tue, Nov 26, 2024 at 12:03 AM Richard Zhu [off-list ref]
wrote:

quoted

Since the apps_reset is asserted in imx_pcie_assert_core_reset(), it
should be deasserted in imx_pcie_deassert_core_reset().

Fixes: 9b3fe6796d7c ("PCI: imx6: Add code to support i.MX7D")
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Manivannan Sadhasivam

[off-list ref]

quoted

Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 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 3538440601a7..413db182ce9f 100644

--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c

@@ -776,6 +776,7 @@ static void imx_pcie_assert_core_reset(struct

imx_pcie *imx_pcie)  static int imx_pcie_deassert_core_reset(struct
imx_pcie *imx_pcie)  {
        reset_control_deassert(imx_pcie->pciephy_reset);
+       reset_control_deassert(imx_pcie->apps_reset);

        if (imx_pcie->drvdata->core_reset)
                imx_pcie->drvdata->core_reset(imx_pcie, false);
--
2.37.1

Hi Richard,

I've found that this patch causes a regression on i.MX8MM and i.MX8MP
boards with hotplug capable bridges:
i.MX8MM+PI7C9X2G404EV (this switch does not support hotplug) - no issues
i.MX8MM+PI7C9X2G608GP (hotplug) - fails to reliably enumerate
downstream devices about 80% of the time ^^^ when this occurs
PCI_PRIMARY_BUS (0x18) for the root complex
0000:00:00.0 reads 0x00000000 instead of 0x00ff0100
(PCI_SECONDARY_BUS is 0 instead of 1 and PCI_SUBBORDINATE_BUS is 0
instead of 0xff) i.MX8MP+PI7C9X2G608GP (hotplug) - hangs at
imx_pcie_ltssm_enable deassert apps_reset

In both cases here reverting ef61c7d8d032 ("PCI: imx6: Deassert apps_reset
in imx_pcie_deassert_core_reset()") resolves this.

[Richard Zhu] I'm afraid that the ltssm_en bit assert to 1b'1 in
 imx_pcie_deassert_core_reset() is not correct in your use case.

Hi Richard,

Thanks for your quick response. Do you mean not correct for newer IP
core in i.MX8MM/i.MX8MP or in the case of a bridge?

Yes this resolves the regression of failing to reliably enumerate
downstream devices. I think that should be submitted with a fixes tag.

The i.MX8MP+PI7C9X2G608GP switch hanging issue was hardware related...
i was sadly testing on an old board with a defect. I did previously
have a hang issue there discussed previously here [1] but it was
resolved with commit 9c03e30e3ade ("PCI: imx6: Skip link up workaround
for newer platforms").

How much testing is done with i.MX8M{M,P} board with a switch? I feel
like I'm the only one with these SoC's and a switch and I need to get
better at monitoring patches to the IMX6 PCI controller driver and
testing these scenarios.

Best Regards,

Tim
[1] https://www.spinics.net/lists/linux-pci/msg142764.html

quoted

I notice the sequence of events here is:
imx_pcie_assert_core_reset asserts apps_reset (disables LTSSM)
imx_pcie_deassert_core_reset deasserts apps_reset (enables LTSSM)
imx_pcie_ltssm_enable deasserts apps_reset (enables LTSSM; this is where it
hangs on imx8mp)

Is there perhaps some issue with de-asserting this (enabling LTSSM) when it's
already in this state?

[Richard Zhu]The apps_reset is updated by src driver by regmap_update_bits().
I can't find the exceptions to update one bit, already has the according value.

Best Regards
Richard Zhu

quoted

In the case where downstream devices do not enumerate some investigation
points to them not being happy that the link drops so perhaps deasserting
apps_reset when its already asserted drops the link and restarts it?

Best Regards,

Tim