Skip to contentSkip to conversation

drivers: pci: imx: add imx_pcie_remove function

RFC In review Tim Harvey 7 participants 13 messages May 10, 2017 – May 12, 2017

Subsystems (3)
Arm (32-bit) Arm i.MX and Layerscape PCI
lore
Share & tools
lore

Tim Harvey

author
#0 lore raw

RFC 1/1 drivers: pci: imx: add imx_pcie_remove function

There is no dedicated reset signal wired up for the MX6QDL thus if the bootloader enables the link we need some special handling to get the core back into a state where it is safe to touch it for configuration.

While there has been some special handling in the Linux kernel to do this, it was removed in 4.11 thus we need to do it properly in the bootloader and therefore without this if you enable PCI in the bootloader you will hang while booting the 4.11 kernel.

This was tested on a Gateworks Ventana board.

TODO: what is the right arch specific place to hook the remove function?

Signed-off-by Tim Harvey <[email protected]>
---
 board/gateworks/gw_ventana/gw_ventana.c |  3 +++
 drivers/pci/pcie_imx.c                  | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
board/gateworks/gw_ventana/gw_ventana.c +3 -0
index f404497..6762a71 100644
@@ -1163,6 +1163,9 @@ int ft_board_setup(void *blob, bd_t *bd)
 	int i;
 	char rev = 0;
 
+#if defined(CONFIG_CMD_PCI)
+	imx_pcie_remove();
+#endif
 	/* determine board revision */
 	for (i = sizeof(ventana_info.model) - 1; i > 0; i--) {
 		if (ventana_info.model[i] >= 'A') {
drivers/pci/pcie_imx.c +38 -0
index 732d59d..eab0a2b 100644
@@ -42,6 +42,9 @@
 
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
+#define PCIE_PL_PFLR (PL_OFFSET + 0x08)
+#define PCIE_PL_PFLR_LINK_STATE_MASK		(0x3f << 16)
+#define PCIE_PL_PFLR_FORCE_LINK			(1 << 15)
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
 #define PCIE_PHY_DEBUG_R1_LINK_UP		(1 << 4)

@@ -445,6 +448,36 @@ static int imx6_pcie_assert_core_reset(void)
 	/* Power up PCIe PHY */
 	setbits_le32(&gpc_regs->cntr, PCIE_PHY_PUP_REQ);
 #else
+	/*
+	 * If the bootloader already enabled the link we need some special
+	 * handling to get the core back into a state where it is safe to
+	 * touch it for configuration.  As there is no dedicated reset signal
+	 * wired up for MX6QDL, we need to manually force LTSSM into "detect"
+	 * state before completely disabling LTSSM, which is a prerequisite
+	 * for core configuration.
+	 *
+	 * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
+	 * indication that the bootloader activated the link.
+	 */
+	if (is_mx6dq()) {
+		u32 val, gpr1, gpr12;
+
+		gpr1 = readl(&iomuxc_regs->gpr[1]);
+		gpr12 = readl(&iomuxc_regs->gpr[12]);
+		if ((gpr1 & IOMUXC_GPR1_PCIE_REF_CLK_EN) &&
+		    (gpr12 & IOMUXC_GPR12_PCIE_CTL_2)) {
+			val = readl(MX6_DBI_ADDR + PCIE_PL_PFLR);
+			val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
+			val |= PCIE_PL_PFLR_FORCE_LINK;
+
+			imx_pcie_fix_dabt_handler(true);
+			writel(val, MX6_DBI_ADDR + PCIE_PL_PFLR);
+			imx_pcie_fix_dabt_handler(false);
+
+			gpr12 &= ~IOMUXC_GPR12_PCIE_CTL_2;
+			writel(val, &iomuxc_regs->gpr[12]);
+		}
+	}
 	setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
 	clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
 #endif

@@ -652,6 +685,11 @@ void imx_pcie_init(void)
 	}
 }
 
+void imx_pcie_remove(void)
+{
+	imx6_pcie_assert_core_reset();
+}
+
 /* Probe function. */
 void pci_init_board(void)
 {
Signature · 1 line
2.7.4
reviewer
#1 lore raw

Hi Tim,

Thanks for working on this!

[Adding Peter and Jagan on Cc]

Quoted 4 lines · Tim Harvey

There is no dedicated reset signal wired up for the MX6QDL thus if the bootloader enables the link we need some special handling to get the core back into a state where it is safe to touch it for configuration.

Quoted 8 lines

While there has been some special handling in the Linux kernel to do this, it was removed in 4.11 thus we need to do it properly in the bootloader and therefore without this if you enable PCI in the bootloader you will hang while booting the 4.11 kernel.

This was tested on a Gateworks Ventana board.

TODO: what is the right arch specific place to hook the remove function?

Ideally the pcie_imx.c driver should be converted to driver model and then the .remove can be simply added, just like in Barebox.

Take a look at the following PCI drivers that have already been converted to the driver model to use as reference:

$ git grep U_BOOT_DRIVER drivers/pci/
drivers/pci/pci-uclass.c:U_BOOT_DRIVER(pci_bridge_drv) = {
drivers/pci/pci-uclass.c:U_BOOT_DRIVER(pci_generic_drv) = {
drivers/pci/pci_sandbox.c:U_BOOT_DRIVER(pci_sandbox) = {
drivers/pci/pci_tegra.c:U_BOOT_DRIVER(pci_tegra) = {
drivers/pci/pci_x86.c:U_BOOT_DRIVER(pci_x86) = {
drivers/pci/pcie_dw_mvebu.c:U_BOOT_DRIVER(pcie_dw_mvebu) = {
drivers/pci/pcie_layerscape.c:U_BOOT_DRIVER(pci_layerscape) = {
drivers/pci/pcie_xilinx.c:U_BOOT_DRIVER(pcie_xilinx) = {
author
#2 lore raw
Quoted 16 lines · Fabio Estevam

Hi Tim,

Thanks for working on this!

[Adding Peter and Jagan on Cc]

Quoted 9 lines · Tim Harvey

There is no dedicated reset signal wired up for the MX6QDL thus if the bootloader enables the link we need some special handling to get the core back into a state where it is safe to touch it for configuration.

While there has been some special handling in the Linux kernel to do this, it was removed in 4.11 thus we need to do it properly in the bootloader and therefore without this if you enable PCI in the bootloader you will hang while booting the 4.11 kernel.

Quoted 7 lines
Quoted 3 lines

This was tested on a Gateworks Ventana board.

TODO: what is the right arch specific place to hook the remove function?

Ideally the pcie_imx.c driver should be converted to driver model and then the .remove can be simply added, just like in Barebox.

Fabio,

Unfortunately converting the IMX PCI driver to DM is a cleanup effort that I'm not permitted the time to do.

Regards,

Tim

#3 lore raw

RFC 1/1 drivers: pci: imx: add imx_pcie_remove function

In reply to Fabio Estevam

On Wed, May 10, 2017 at 03:10:37PM -0300, Fabio Estevam wrote:
Hi Tim,

Quoted 6 lines

Hi Tim,

Thanks for working on this!

[Adding Peter and Jagan on Cc]

[snip]

Thank you for working on this! Your patch worked for me, but I needed to add an "extern void imx_pcie_remove(void);" before calling the function, which probably means I'm missing something.

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch 4.11.0-next-20170510 do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

board/ge/bx50v3/bx50v3.c +3 -0
index cb49b58..7e03082 100644
@@ -829,6 +829,7 @@ static void remove_ethaddr_env_var(int index)
 	setenv(env_var_name, NULL);
 }
 
+extern void imx_pcie_remove(void);
 int last_stage_init(void) {
 	int i;
 

@@ -838,6 +839,8 @@ int last_stage_init(void) {
 		remove_ethaddr_env_var(i);
 	}
 
+	imx_pcie_remove();
+
 	return 0;
 }
drivers/pci/pcie_imx.c +38 -0
index 732d59d..eab0a2b 100644
@@ -42,6 +42,9 @@
 
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
+#define PCIE_PL_PFLR (PL_OFFSET + 0x08)
+#define PCIE_PL_PFLR_LINK_STATE_MASK		(0x3f << 16)
+#define PCIE_PL_PFLR_FORCE_LINK			(1 << 15)
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
 #define PCIE_PHY_DEBUG_R1_LINK_UP		(1 << 4)

@@ -445,6 +448,36 @@ static int imx6_pcie_assert_core_reset(void)
 	/* Power up PCIe PHY */
 	setbits_le32(&gpc_regs->cntr, PCIE_PHY_PUP_REQ);
 #else
+	/*
+	 * If the bootloader already enabled the link we need some special
+	 * handling to get the core back into a state where it is safe to
+	 * touch it for configuration.  As there is no dedicated reset signal
+	 * wired up for MX6QDL, we need to manually force LTSSM into "detect"
+	 * state before completely disabling LTSSM, which is a prerequisite
+	 * for core configuration.
+	 *
+	 * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
+	 * indication that the bootloader activated the link.
+	 */
+	if (is_mx6dq()) {
+		u32 val, gpr1, gpr12;
+
+		gpr1 = readl(&iomuxc_regs->gpr[1]);
+		gpr12 = readl(&iomuxc_regs->gpr[12]);
+		if ((gpr1 & IOMUXC_GPR1_PCIE_REF_CLK_EN) &&
+		    (gpr12 & IOMUXC_GPR12_PCIE_CTL_2)) {
+			val = readl(MX6_DBI_ADDR + PCIE_PL_PFLR);
+			val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
+			val |= PCIE_PL_PFLR_FORCE_LINK;
+
+			imx_pcie_fix_dabt_handler(true);
+			writel(val, MX6_DBI_ADDR + PCIE_PL_PFLR);
+			imx_pcie_fix_dabt_handler(false);
+
+			gpr12 &= ~IOMUXC_GPR12_PCIE_CTL_2;
+			writel(val, &iomuxc_regs->gpr[12]);
+		}
+	}
 	setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
 	clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
 #endif

@@ -652,6 +685,11 @@ void imx_pcie_init(void)
 	}
 }
 
+void imx_pcie_remove(void)
+{
+	imx6_pcie_assert_core_reset();
+}
+
 /* Probe function. */
 void pci_init_board(void)
 {
reviewer
#4 lore raw
Quoted 3 lines · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I needed to add an "extern void imx_pcie_remove(void);" before calling the function, which probably means I'm missing something.

The extern should be added into a header file.

Quoted 7 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch 4.11.0-next-20170510 do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:
https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

Please advise.

Thanks

Quoted 85 lines
board/ge/bx50v3/bx50v3.c +3 -0
index cb49b58..7e03082 100644
@@ -829,6 +829,7 @@ static void remove_ethaddr_env_var(int index)
        setenv(env_var_name, NULL);
 }
 
+extern void imx_pcie_remove(void);
 int last_stage_init(void) {
        int i;
 

@@ -838,6 +839,8 @@ int last_stage_init(void) {
                remove_ethaddr_env_var(i);
        }
 
+       imx_pcie_remove();
+
        return 0;
 }
drivers/pci/pcie_imx.c +38 -0
index 732d59d..eab0a2b 100644
@@ -42,6 +42,9 @@
 
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
+#define PCIE_PL_PFLR (PL_OFFSET + 0x08)
+#define PCIE_PL_PFLR_LINK_STATE_MASK           (0x3f << 16)
+#define PCIE_PL_PFLR_FORCE_LINK                        (1 << 15)
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
 #define PCIE_PHY_DEBUG_R1_LINK_UP              (1 << 4)

@@ -445,6 +448,36 @@ static int imx6_pcie_assert_core_reset(void)
        /* Power up PCIe PHY */
        setbits_le32(&gpc_regs->cntr, PCIE_PHY_PUP_REQ);
 #else
+       /*
+        * If the bootloader already enabled the link we need some special
+        * handling to get the core back into a state where it is safe to
+        * touch it for configuration.  As there is no dedicated reset signal
+        * wired up for MX6QDL, we need to manually force LTSSM into "detect"
+        * state before completely disabling LTSSM, which is a prerequisite
+        * for core configuration.
+        *
+        * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
+        * indication that the bootloader activated the link.
+        */
+       if (is_mx6dq()) {
+               u32 val, gpr1, gpr12;
+
+               gpr1 = readl(&iomuxc_regs->gpr[1]);
+               gpr12 = readl(&iomuxc_regs->gpr[12]);
+               if ((gpr1 & IOMUXC_GPR1_PCIE_REF_CLK_EN) &&
+                   (gpr12 & IOMUXC_GPR12_PCIE_CTL_2)) {
+                       val = readl(MX6_DBI_ADDR + PCIE_PL_PFLR);
+                       val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
+                       val |= PCIE_PL_PFLR_FORCE_LINK;
+
+                       imx_pcie_fix_dabt_handler(true);
+                       writel(val, MX6_DBI_ADDR + PCIE_PL_PFLR);
+                       imx_pcie_fix_dabt_handler(false);
+
+                       gpr12 &= ~IOMUXC_GPR12_PCIE_CTL_2;
+                       writel(val, &iomuxc_regs->gpr[12]);
+               }
+       }
        setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
        clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
 #endif

@@ -652,6 +685,11 @@ void imx_pcie_init(void)
        }
 }
 
+void imx_pcie_remove(void)
+{
+       imx6_pcie_assert_core_reset();
+}
+
 /* Probe function. */
 void pci_init_board(void)
 {
#5 lore raw
Quoted 26 lines · Fabio Estevam
Quoted 3 lines · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I needed to add an "extern void imx_pcie_remove(void);" before calling the function, which probably means I'm missing something.

The extern should be added into a header file.

Quoted 7 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch 4.11.0-next-20170510 do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

Quoted 7 lines

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:
https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

The DM conversion should be pretty trivial IMO, no ?

Signature · 2 lines
Best regards,
Marek Vasut
reviewer
#6 lore raw
Quoted 1 line · Marek Vasut

The DM conversion should be pretty trivial IMO, no ?

Any volunteers? ;-)

#7 lore raw
Quoted 6 lines · Fabio Estevam
Quoted 1 line · Marek Vasut

The DM conversion should be pretty trivial IMO, no ?

Any volunteers? ;-)

Tim ? :)

Signature · 2 lines
Best regards,
Marek Vasut
#8 lore raw
In reply to Fabio Estevam

Hi Fabio,

Quoted 31 lines · Fabio Estevam
Quoted 3 lines · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I needed to add an "extern void imx_pcie_remove(void);" before calling the function, which probably means I'm missing something.

The extern should be added into a header file.

Quoted 7 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch 4.11.0-next-20170510 do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:
https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

Quoted 4 lines

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

Please advise.

Ok - let wait for Tim's answer, if he has enough time to work on this to move to DM. If not, it will be ok for now to fix it in this way, hoping to move soon to DM.

Regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#9 lore raw
Quoted 37 lines · Stefano Babic

Hi Fabio,

Quoted 34 lines · Fabio Estevam
Quoted 3 lines · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I needed to add an "extern void imx_pcie_remove(void);" before calling the function, which probably means I'm missing something.

The extern should be added into a header file.

Quoted 7 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch 4.11.0-next-20170510 do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:
https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

Quoted 6 lines
Quoted 1 line

Please advise.

Ok - let wait for Tim's answer, if he has enough time to work on this to move to DM. If not, it will be ok for now to fix it in this way, hoping to move soon to DM.

If none, have time to work, I'll work for dm conversion and have board to test the same.

Jagan.

author
#10 lore raw
In reply to Stefano Babic
Quoted 40 lines · Stefano Babic

Hi Fabio,

Quoted 31 lines · Fabio Estevam
Quoted 3 lines · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I needed to add an "extern void imx_pcie_remove(void);" before calling the function, which probably means I'm missing something.

The extern should be added into a header file.

Quoted 7 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch 4.11.0-next-20170510 do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:
https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

Quoted 4 lines

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

Please advise.

Quoted 6 lines

Ok - let wait for Tim's answer, if he has enough time to work on this to move to DM. If not, it will be ok for now to fix it in this way, hoping to move soon to DM.

Regards,
Stefano

Stefano,

I don't have enough time to work on the DM conversion currently but it sounds like this would be a welcomed 'fix' that we should get in sooner than later.

I know the include is missing from a header but I didn't bother as I though there would be a better arch-specific place to put the call to the remove. I don't think putting it in ft_board_setup made sense. Do you have any suggestions where it should go?

Tim

#11 lore raw

RFC 1/1 drivers: pci: imx: add imx_pcie_remove function

Quoted 12 lines

Date: Thu, 11 May 2017 07:02:52 -0700
From: Tim Harvey <[email protected]>
To: Stefano Babic <[email protected]>
Cc: Marek Vasut <[email protected]>, U-Boot-Denx <[email protected]>,
Peter Senna Tschudin <[email protected]>, Lucas Stach <[email protected]>
Subject: Re: [U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove
function
Message-ID:
<CAJ+vNU2MOp_1bsh5MusGauJPdASB7+KDPQGdg+YU4gfbtBSBhQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"

Quoted 8 lines
Quoted 7 lines · Stefano Babic

Hi Fabio,

Quoted 4 lines · Fabio Estevam
Quoted 1 line · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I

needed to

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

add an "extern void imx_pcie_remove(void);" before calling the

function,

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line

which probably means I'm missing something.

The extern should be added into a header file.

Quoted 2 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch

4.11.0-next-20170510

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 5 lines

do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:

https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

Quoted 18 lines
Quoted 4 lines
Quoted 4 lines

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

Quoted 8 lines
Quoted 1 line

Please advise.

Ok - let wait for Tim's answer, if he has enough time to work on this to move to DM. If not, it will be ok for now to fix it in this way, hoping to move soon to DM.

Regards,
Stefano

Stefano,

I don't have enough time to work on the DM conversion currently but it sounds like this would be a welcomed 'fix' that we should get in sooner than later.

Quoted 6 lines

I know the include is missing from a header but I didn't bother as I though there would be a better arch-specific place to put the call to the remove. I don't think putting it in ft_board_setup made sense. Do you have any suggestions where it should go?

Tim

Tim,

this patch in addition to your original drivers/pci/pcie_imx.c changes works for me on a tbs2910 board.
Since I did not touch board-specific code, this should also work for all affected boards.

Regards,
Soeren

arch/arm/imx-common/cpu.c +5 -0
index 40fe813..7ed9ca0 100644
@@ -273,8 +273,13 @@ u32 get_ahb_clk(void)
 }
 #endif
 
+extern void imx_pcie_remove(void);
+
 void arch_preboot_os(void)
 {
+#if defined(CONFIG_PCIE_IMX)
+       imx_pcie_remove();
+#endif
 #if defined(CONFIG_CMD_SATA)
        sata_stop();
 #if defined(CONFIG_MX6)
author
#12 lore raw
Quoted 71 lines · Soeren Moch
Quoted 12 lines

Date: Thu, 11 May 2017 07:02:52 -0700
From: Tim Harvey <[email protected]>
To: Stefano Babic <[email protected]>
Cc: Marek Vasut <[email protected]>, U-Boot-Denx <[email protected]>,
Peter Senna Tschudin <[email protected]>, Lucas Stach <[email protected]>
Subject: Re: [U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove
function
Message-ID:
<CAJ+vNU2MOp_1bsh5MusGauJPdASB7+KDPQGdg+YU4gfbtBSBhQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"

Quoted 8 lines
Quoted 7 lines · Stefano Babic

Hi Fabio,

Quoted 4 lines · Fabio Estevam
Quoted 1 line · Peter Senna Tschudin

Thank you for working on this! Your patch worked for me, but I

needed to

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

add an "extern void imx_pcie_remove(void);" before calling the

function,

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line

which probably means I'm missing something.

The extern should be added into a header file.

Quoted 2 lines

After applying this patch to u-boot, u-boot can initialize PCI and 4.11.0-next-20170510 will boot. Without your patch

4.11.0-next-20170510

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 5 lines

do not boot if u-boot initialize the PCI bus.

Thanks a lot!

Here is the patch I'm using for our board:

Stefano,

The patches that Tim/Peter posted solve a kernel hang with kernel 4.11 on mx6q when U-Boot has PCI support.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Could you please take a look and let us know if there is a more generic way to fix this issue, like fixing it for all mx6qdl boards that have PCI support?

I understand that ideally the imx6 pci driver should be converted to driver model and then we add the .remove hook, like Lucas did for Barebox:

https://git.pengutronix.de/cgit/barebox/commit/?id=f1da98da2760c21487bbba8f7fb957c843a22896

Quoted 23 lines
Quoted 4 lines
Quoted 4 lines

While the imx pci driver is not converted to device model, would you be willing to accept such per board patches for the time being?

Quoted 8 lines
Quoted 1 line

Please advise.

Ok - let wait for Tim's answer, if he has enough time to work on this to move to DM. If not, it will be ok for now to fix it in this way, hoping to move soon to DM.

Regards,
Stefano

Stefano,

I don't have enough time to work on the DM conversion currently but it sounds like this would be a welcomed 'fix' that we should get in sooner than later.

I know the include is missing from a header but I didn't bother as I though there would be a better arch-specific place to put the call to the remove. I don't think putting it in ft_board_setup made sense. Do you have any suggestions where it should go?

Quoted 9 lines
Quoted 1 line

Tim

Tim,

this patch in addition to your original drivers/pci/pcie_imx.c changes works for me on a tbs2910 board.
Since I did not touch board-specific code, this should also work for all affected boards.

Soeren,

Perfect, that's what I was looking for 'arch_preboot_os' seems like a good place for it!

I will put together a non-RFC patch.

Tim