[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[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.

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 <tharvey@gateworks.com>
---
 board/gateworks/gw_ventana/gw_ventana.c |  3 +++
 drivers/pci/pcie_imx.c                  | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c
index f404497..6762a71 100644
--- a/board/gateworks/gw_ventana/gw_ventana.c
+++ b/board/gateworks/gw_ventana/gw_ventana.c
@@ -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') {
diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
index 732d59d..eab0a2b 100644
--- a/drivers/pci/pcie_imx.c
+++ b/drivers/pci/pcie_imx.c
@@ -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)
 {
-- 
2.7.4

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Fabio Estevam]

Hi Tim,

Thanks for working on this!

[Adding Peter and Jagan on Cc]

On Wed, May 10, 2017 at 1:43 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> 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?

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) = {

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Tim Harvey]

On Wed, May 10, 2017 at 11:10 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Tim,
>
> Thanks for working on this!
>
> [Adding Peter and Jagan on Cc]
>
> On Wed, May 10, 2017 at 1:43 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> 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?
>
> 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

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Peter Senna Tschudin]

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

> 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:

diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c
index cb49b58..7e03082 100644
--- a/board/ge/bx50v3/bx50v3.c
+++ b/board/ge/bx50v3/bx50v3.c
@@ -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;
 }
 
diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
index 732d59d..eab0a2b 100644
--- a/drivers/pci/pcie_imx.c
+++ b/drivers/pci/pcie_imx.c
@@ -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)
 {

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Fabio Estevam]

On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
<peter.senna@collabora.com> wrote:

> 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.

> 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

>
> diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c
> index cb49b58..7e03082 100644
> --- a/board/ge/bx50v3/bx50v3.c
> +++ b/board/ge/bx50v3/bx50v3.c
> @@ -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;
>  }
>
> diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
> index 732d59d..eab0a2b 100644
> --- a/drivers/pci/pcie_imx.c
> +++ b/drivers/pci/pcie_imx.c
> @@ -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)
>  {

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Marek Vasut]

On 05/11/2017 01:06 PM, Fabio Estevam wrote:
> On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
> <peter.senna@collabora.com> wrote:
> 
>> 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.
> 
>> 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?

The DM conversion should be pretty trivial IMO, no ?

-- 
Best regards,
Marek Vasut

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Fabio Estevam]

On Thu, May 11, 2017 at 8:47 AM, Marek Vasut <marex@denx.de> wrote:

> The DM conversion should be pretty trivial IMO, no ?

Any volunteers? ;-)

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Marek Vasut]

On 05/11/2017 03:08 PM, Fabio Estevam wrote:
> On Thu, May 11, 2017 at 8:47 AM, Marek Vasut <marex@denx.de> wrote:
> 
>> The DM conversion should be pretty trivial IMO, no ?
> 
> Any volunteers? ;-)
> 
Tim ? :)

-- 
Best regards,
Marek Vasut

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Stefano Babic]

Hi Fabio,

On 11/05/2017 13:06, Fabio Estevam wrote:
> On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
> <peter.senna@collabora.com> wrote:
> 
>> 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.
> 
>> 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.


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


-- 
=====================================================================
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
=====================================================================

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Jagan Teki]

On Thu, May 11, 2017 at 6:48 PM, Stefano Babic <sbabic@denx.de> wrote:
> Hi Fabio,
>
> On 11/05/2017 13:06, Fabio Estevam wrote:
>> On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
>> <peter.senna@collabora.com> wrote:
>>
>>> 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.
>>
>>> 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.
>
>
> 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.

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Tim Harvey]

On Thu, May 11, 2017 at 6:18 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi Fabio,
>
> On 11/05/2017 13:06, Fabio Estevam wrote:
>> On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
>> <peter.senna@collabora.com> wrote:
>>
>>> 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.
>>
>>> 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.
>
>
> 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

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Soeren Moch]

> Date: Thu, 11 May 2017 07:02:52 -0700
> From: Tim Harvey <tharvey@gateworks.com>
> To: Stefano Babic <sbabic@denx.de>
> Cc: Marek Vasut <marex@denx.de>, U-Boot-Denx <u-boot@lists.denx.de>,
>     Peter Senna Tschudin <peter.senna@collabora.com>, Lucas Stach
>     <l.stach@pengutronix.de>
> 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"
>
> On Thu, May 11, 2017 at 6:18 AM, Stefano Babic <sbabic@denx.de> wrote:
> > Hi Fabio,
> >
> > On 11/05/2017 13:06, Fabio Estevam wrote:
> >> On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
> >> <peter.senna@collabora.com> wrote:
> >>
> >>> 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.
> >>
> >>> 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.
> >
> > 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

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


diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c
index 40fe813..7ed9ca0 100644
--- a/arch/arm/imx-common/cpu.c
+++ b/arch/arm/imx-common/cpu.c
@@ -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)

[U-Boot] [RFC] drivers: pci: imx: add imx_pcie_remove function

[Tim Harvey]

On Fri, May 12, 2017 at 12:27 PM, Soeren Moch <smoch@web.de> wrote:
>> Date: Thu, 11 May 2017 07:02:52 -0700
>> From: Tim Harvey <tharvey@gateworks.com>
>> To: Stefano Babic <sbabic@denx.de>
>> Cc: Marek Vasut <marex@denx.de>, U-Boot-Denx <u-boot@lists.denx.de>,
>>     Peter Senna Tschudin <peter.senna@collabora.com>, Lucas Stach
>>     <l.stach@pengutronix.de>
>> 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"
>>
>> On Thu, May 11, 2017 at 6:18 AM, Stefano Babic <sbabic@denx.de> wrote:
>> > Hi Fabio,
>> >
>> > On 11/05/2017 13:06, Fabio Estevam wrote:
>> >> On Thu, May 11, 2017 at 6:01 AM, Peter Senna Tschudin
>> >> <peter.senna@collabora.com> wrote:
>> >>
>> >>> 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.
>> >>
>> >>> 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.
>> >
>> > 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
>
> 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