Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v1] PCI: imx6: Move clock enable after core reset assertion
@ 2026-08-13  8:29 hongxing.zhu
  2026-08-13  8:36 ` sashiko-bot
  2026-08-13  8:39 ` Francesco Dolcini
  0 siblings, 2 replies; 4+ messages in thread
From: hongxing.zhu @ 2026-08-13  8:29 UTC (permalink / raw)
  To: leoreis.costa, fra.schnyder, francesco, frank.li, l.stach,
	lpieralisi, kwilczynski, mani, robh, bhelgaas, s.hauer, kernel,
	festevam
  Cc: linux-pci, linux-arm-kernel, imx, linux-kernel, Richard Zhu,
	stable

From: Richard Zhu <hongxing.zhu@nxp.com>

Commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
inadvertently moved clock enablement before core reset assertion, breaking
PCI device initialization on i.MX6Q Apalis platforms when
ASM1061/ASM1062 is connected.

Logs:
root@apalis-imx6-11486042:~# dmesg | grep pcie
imx6q-pcie 1ffc000.pcie: host bridge /soc/pcie@1ffc000 ranges:
imx6q-pcie 1ffc000.pcie:       IO 0x0001f80000..0x0001f8ffff -> 0x0000000000
imx6q-pcie 1ffc000.pcie:      MEM 0x0001000000..0x0001efffff -> 0x0001000000
imx6q-pcie 1ffc000.pcie: config reg[1] 0x01f00000 == cpu 0x01f00000
imx6q-pcie 1ffc000.pcie: iATU: unroll F, 4 ob, 4 ib, align 64K, limit 4G
imx6q-pcie 1ffc000.pcie: Link: Only Gen1 is enabled
imx6q-pcie 1ffc000.pcie: Link failed to come up. LTSSM: POLL_CONFIG
imx6q-pcie 1ffc000.pcie: probe with driver imx6q-pcie failed with error -110

To fix this regression, restore the original sequence where clocks are
enabled after asserting core reset and configuring the controller type.

Fixes: 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reported-by: Leonardo Costa <leoreis.costa@gmail.com>
Closes: https://lore.kernel.org/all/bl7i3obu2clzsgeoct2a4mtfhv6typcjdqmgneropf3hpgwve6@n2m5uhlduw57/T/#u
Reported-by: Franz Schnyder <fra.schnyder@gmail.com>
Closes: https://lore.kernel.org/all/t65y5d54axtksbfs7r4olcefqhwm6m4dz3njgnrnf7fcotj74i@o7avoznlafbj/
Cc: stable@vger.kernel.org # 7.1+
---
 drivers/pci/controller/dwc/pci-imx6.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b98dc..f7a2eb257c169 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1394,12 +1394,6 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 		}
 	}
 
-	ret = imx_pcie_clk_enable(imx_pcie);
-	if (ret) {
-		dev_err(dev, "unable to enable pcie clocks: %d\n", ret);
-		goto err_pwrctrl_power_off;
-	}
-
 	if (pp->bridge && imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT)) {
 		pp->bridge->enable_device = imx_pcie_enable_device;
 		pp->bridge->disable_device = imx_pcie_disable_device;
@@ -1415,6 +1409,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 
 	imx_pcie_configure_type(imx_pcie);
 
+	ret = imx_pcie_clk_enable(imx_pcie);
+	if (ret) {
+		dev_err(dev, "unable to enable pcie clocks: %d\n", ret);
+		goto err_pwrctrl_power_off;
+	}
+
 	if (imx_pcie->phy) {
 		ret = phy_init(imx_pcie->phy);
 		if (ret) {

base-commit: 28d012efb4327f9c75d5e042a7c91e9a542efa98
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] PCI: imx6: Move clock enable after core reset assertion
  2026-08-13  8:29 [PATCH v1] PCI: imx6: Move clock enable after core reset assertion hongxing.zhu
@ 2026-08-13  8:36 ` sashiko-bot
  2026-08-13  8:39 ` Francesco Dolcini
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-13  8:36 UTC (permalink / raw)
  To: hongxing.zhu; +Cc: Frank.Li, imx, linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of imx_pcie_host_init() is ignored in imx_add_pcie_ep(), leading to a kernel crash if hardware initialization fails.
--

--- Patch [1]: [PATCH v1] PCI: imx6: Move clock enable after core reset assertion ---
commit 6d9f9eb5750531f447a5eaee6a038ee84ba72010
Author: Richard Zhu <hongxing.zhu@nxp.com>

PCI: imx6: Move clock enable after core reset assertion

This patch reverts a sequence change by moving clock enablement after
asserting core reset and configuring the controller type. This restores
correct initialization on i.MX6Q Apalis platforms where PCI device
initialization was failing.

Link: https://lore.kernel.org/all/bl7i3obu2clzsgeoct2a4mtfhv6typcjdqmgneropf3hpgwve6@n2m5uhlduw57/T/#u
Link: https://lore.kernel.org/all/t65y5d54axtksbfs7r4olcefqhwm6m4dz3njgnrnf7fcotj74i@o7avoznlafbj/

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..f7a2eb257c169 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -1415,6 +1409,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  	imx_pcie_configure_type(imx_pcie);
>  
> +	ret = imx_pcie_clk_enable(imx_pcie);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie clocks: %d\n", ret);
> +		goto err_pwrctrl_power_off;
> +	}
> +
[Severity: High]
This is a pre-existing issue, but is it safe for imx_add_pcie_ep() to ignore
the return value of imx_pcie_host_init()?

Looking at imx_add_pcie_ep(), it calls imx_pcie_host_init() and immediately
proceeds without checking if it succeeded:

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
    ...
	imx_pcie_host_init(pp);
	ep = &pci->ep;
	ep->ops = &pcie_ep_ops;
    ...
	ret = dw_pcie_ep_init(ep);
    ...
}

If imx_pcie_host_init() fails here (for example, if imx_pcie_clk_enable()
returns an error), it cleans up its resources and returns an error code with
clocks disabled.

Execution would then continue into dw_pcie_ep_init(), which attempts
hardware register accesses. On i.MX hardware, accessing PCIe registers with
disabled clocks triggers a synchronous bus error (SError), resulting in a
kernel panic.

Could imx_add_pcie_ep() be updated to check the return value of
imx_pcie_host_init() and handle the failure gracefully?

>  	if (imx_pcie->phy) {
>  		ret = phy_init(imx_pcie->phy);
>  		if (ret) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813082900.270003-1-hongxing.zhu@oss.nxp.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] PCI: imx6: Move clock enable after core reset assertion
  2026-08-13  8:29 [PATCH v1] PCI: imx6: Move clock enable after core reset assertion hongxing.zhu
  2026-08-13  8:36 ` sashiko-bot
@ 2026-08-13  8:39 ` Francesco Dolcini
  2026-08-13  8:47   ` Hongxing Zhu (OSS)
  1 sibling, 1 reply; 4+ messages in thread
From: Francesco Dolcini @ 2026-08-13  8:39 UTC (permalink / raw)
  To: hongxing.zhu
  Cc: leoreis.costa, fra.schnyder, francesco, frank.li, l.stach,
	lpieralisi, kwilczynski, mani, robh, bhelgaas, s.hauer, kernel,
	festevam, linux-pci, linux-arm-kernel, imx, linux-kernel,
	Richard Zhu, stable

On Thu, Aug 13, 2026 at 04:29:00PM +0800, hongxing.zhu@oss.nxp.com wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
> inadvertently moved clock enablement before core reset assertion, breaking
> PCI device initialization on i.MX6Q Apalis platforms when
> ASM1061/ASM1062 is connected.
> 
> Logs:
> root@apalis-imx6-11486042:~# dmesg | grep pcie
> imx6q-pcie 1ffc000.pcie: host bridge /soc/pcie@1ffc000 ranges:
> imx6q-pcie 1ffc000.pcie:       IO 0x0001f80000..0x0001f8ffff -> 0x0000000000
> imx6q-pcie 1ffc000.pcie:      MEM 0x0001000000..0x0001efffff -> 0x0001000000
> imx6q-pcie 1ffc000.pcie: config reg[1] 0x01f00000 == cpu 0x01f00000
> imx6q-pcie 1ffc000.pcie: iATU: unroll F, 4 ob, 4 ib, align 64K, limit 4G
> imx6q-pcie 1ffc000.pcie: Link: Only Gen1 is enabled
> imx6q-pcie 1ffc000.pcie: Link failed to come up. LTSSM: POLL_CONFIG
> imx6q-pcie 1ffc000.pcie: probe with driver imx6q-pcie failed with error -110
> 
> To fix this regression, restore the original sequence where clocks are
> enabled after asserting core reset and configuring the controller type.
> 
> Fixes: 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reported-by: Leonardo Costa <leoreis.costa@gmail.com>
> Closes: https://lore.kernel.org/all/bl7i3obu2clzsgeoct2a4mtfhv6typcjdqmgneropf3hpgwve6@n2m5uhlduw57/T/#u
> Reported-by: Franz Schnyder <fra.schnyder@gmail.com>
> Closes: https://lore.kernel.org/all/t65y5d54axtksbfs7r4olcefqhwm6m4dz3njgnrnf7fcotj74i@o7avoznlafbj/
> Cc: stable@vger.kernel.org # 7.1+

7.2+, 7.1 does not have the broken commit

minor: it is not 100% clear if the issue is specific to the ASM1061/ASM1062
device or on the specific power-up sequence (reset vs cold-power-on).

Francesco


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v1] PCI: imx6: Move clock enable after core reset assertion
  2026-08-13  8:39 ` Francesco Dolcini
@ 2026-08-13  8:47   ` Hongxing Zhu (OSS)
  0 siblings, 0 replies; 4+ messages in thread
From: Hongxing Zhu (OSS) @ 2026-08-13  8:47 UTC (permalink / raw)
  To: Francesco Dolcini, Hongxing Zhu (OSS)
  Cc: leoreis.costa@gmail.com, fra.schnyder@gmail.com, Frank Li,
	l.stach@pengutronix.de, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, robh@kernel.org,
	bhelgaas@google.com, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	imx@lists.linux.dev, linux-kernel@vger.kernel.org, Hongxing Zhu,
	stable@vger.kernel.org


NXP Confidential
> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Thursday, August 13, 2026 4:39 PM
> To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> Cc: leoreis.costa@gmail.com; fra.schnyder@gmail.com; francesco@dolcini.it;
> Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
> bhelgaas@google.com; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> Hongxing Zhu <hongxing.zhu@nxp.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v1] PCI: imx6: Move clock enable after core reset assertion
>
> On Thu, Aug 13, 2026 at 04:29:00PM +0800, hongxing.zhu@oss.nxp.com wrote:
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> >
> > Commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling
> > regulators") inadvertently moved clock enablement before core reset
> > assertion, breaking PCI device initialization on i.MX6Q Apalis
> > platforms when
> > ASM1061/ASM1062 is connected.
> >
> > Logs:
> > root@apalis-imx6-11486042:~# dmesg | grep pcie imx6q-pcie
> > 1ffc000.pcie: host bridge /soc/pcie@1ffc000 ranges:
> > imx6q-pcie 1ffc000.pcie:       IO 0x0001f80000..0x0001f8ffff -> 0x0000000000
> > imx6q-pcie 1ffc000.pcie:      MEM 0x0001000000..0x0001efffff ->
> 0x0001000000
> > imx6q-pcie 1ffc000.pcie: config reg[1] 0x01f00000 == cpu 0x01f00000
> > imx6q-pcie 1ffc000.pcie: iATU: unroll F, 4 ob, 4 ib, align 64K, limit
> > 4G imx6q-pcie 1ffc000.pcie: Link: Only Gen1 is enabled imx6q-pcie
> > 1ffc000.pcie: Link failed to come up. LTSSM: POLL_CONFIG imx6q-pcie
> > 1ffc000.pcie: probe with driver imx6q-pcie failed with error -110
> >
> > To fix this regression, restore the original sequence where clocks are
> > enabled after asserting core reset and configuring the controller type.
> >
> > Fixes: 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling
> > regulators")
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reported-by: Leonardo Costa <leoreis.costa@gmail.com>
> > Closes:
> > https://lore.kernel.org/all/bl7i3obu2clzsgeoct2a4mtfhv6typcjdqmgneropf
> > 3hpgwve6@n2m5uhlduw57/T/#u
> > Reported-by: Franz Schnyder <fra.schnyder@gmail.com>
> > Closes:
> > https://lore.kernel.org/all/t65y5d54axtksbfs7r4olcefqhwm6m4dz3njgnrnf7
> > fcotj74i@o7avoznlafbj/
> > Cc: stable@vger.kernel.org # 7.1+
>
> 7.2+, 7.1 does not have the broken commit
>
> minor: it is not 100% clear if the issue is specific to the ASM1061/ASM1062
> device or on the specific power-up sequence (reset vs cold-power-on).
Thanks for your review comments.
Okay, I add this "minor" into commit message.

Richard Zhu
>
> Francesco


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-13  8:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  8:29 [PATCH v1] PCI: imx6: Move clock enable after core reset assertion hongxing.zhu
2026-08-13  8:36 ` sashiko-bot
2026-08-13  8:39 ` Francesco Dolcini
2026-08-13  8:47   ` Hongxing Zhu (OSS)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox