[U-Boot] [PATCH] 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 40fe813..74bdd24 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -275,6 +275,9 @@ u32 get_ahb_clk(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) 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) { diff --git a/include/pci.h b/include/pci.h index d3c955e..c8ef997 100644 --- a/include/pci.h +++ b/include/pci.h @@ -754,6 +754,10 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#ifdef CONFIG_PCIE_IMX +extern void imx_pcie_remove(void); +#endif + #if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /** * pci_write_bar32() - Write the address of a BAR including control bits
Hi Tim,
On Fri, May 12, 2017 at 4:58 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com
This looks good:
Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Added Peter on Cc in case he could send his Tested-by tag.
arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 40fe813..74bdd24 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -275,6 +275,9 @@ u32 get_ahb_clk(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) 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) { diff --git a/include/pci.h b/include/pci.h index d3c955e..c8ef997 100644 --- a/include/pci.h +++ b/include/pci.h @@ -754,6 +754,10 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#ifdef CONFIG_PCIE_IMX +extern void imx_pcie_remove(void); +#endif
#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /**
- pci_write_bar32() - Write the address of a BAR including control bits
-- 2.7.4
On Fri, May 12, 2017 at 05:25:22PM -0300, Fabio Estevam wrote:
Hi Tim,
On Fri, May 12, 2017 at 4:58 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com
This looks good:
Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Added Peter on Cc in case he could send his Tested-by tag.
I confirm this patch solves my issues. Thank you Tim!
Tested-by: Peter Senna Tschudin peter.senna@collabora.com
arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 40fe813..74bdd24 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -275,6 +275,9 @@ u32 get_ahb_clk(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) 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) { diff --git a/include/pci.h b/include/pci.h index d3c955e..c8ef997 100644 --- a/include/pci.h +++ b/include/pci.h @@ -754,6 +754,10 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#ifdef CONFIG_PCIE_IMX +extern void imx_pcie_remove(void); +#endif
#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /**
- pci_write_bar32() - Write the address of a BAR including control bits
-- 2.7.4
On 12/05/2017 21:58, Tim Harvey 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com
arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 40fe813..74bdd24 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -275,6 +275,9 @@ u32 get_ahb_clk(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) 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) { diff --git a/include/pci.h b/include/pci.h index d3c955e..c8ef997 100644 --- a/include/pci.h +++ b/include/pci.h @@ -754,6 +754,10 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#ifdef CONFIG_PCIE_IMX +extern void imx_pcie_remove(void); +#endif
#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /**
- pci_write_bar32() - Write the address of a BAR including control bits
Ok, I see - now the question to Jagan. Tim has not time to move to DM, and you propose yourself as volunteer (welcome !) to do this job. Of course, I will not let things broken if move cannot be done, but I will prefer to wait having a proper long time fix.
Best regards, Stefano
On Thu, May 18, 2017 at 2:12 AM, Stefano Babic sbabic@denx.de wrote:
On 12/05/2017 21:58, Tim Harvey 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com
arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 40fe813..74bdd24 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -275,6 +275,9 @@ u32 get_ahb_clk(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) 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) { diff --git a/include/pci.h b/include/pci.h index d3c955e..c8ef997 100644 --- a/include/pci.h +++ b/include/pci.h @@ -754,6 +754,10 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#ifdef CONFIG_PCIE_IMX +extern void imx_pcie_remove(void); +#endif
#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /**
- pci_write_bar32() - Write the address of a BAR including control bits
Ok, I see - now the question to Jagan. Tim has not time to move to DM, and you propose yourself as volunteer (welcome !) to do this job. Of course, I will not let things broken if move cannot be done, but I will prefer to wait having a proper long time fix.
Best regards, Stefano
Stefano,
My patch is not intrusive and it appears several people are wanting it. Why not accept my patch which will end up getting re-written once the PCI driver is moved to DM?
Tim
Hi Tim,
On 18/05/2017 16:22, Tim Harvey wrote:
On Thu, May 18, 2017 at 2:12 AM, Stefano Babic sbabic@denx.de wrote:
On 12/05/2017 21:58, Tim Harvey 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com
arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 40fe813..74bdd24 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -275,6 +275,9 @@ u32 get_ahb_clk(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) 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) { diff --git a/include/pci.h b/include/pci.h index d3c955e..c8ef997 100644 --- a/include/pci.h +++ b/include/pci.h @@ -754,6 +754,10 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#ifdef CONFIG_PCIE_IMX +extern void imx_pcie_remove(void); +#endif
#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /**
- pci_write_bar32() - Write the address of a BAR including control bits
Ok, I see - now the question to Jagan. Tim has not time to move to DM, and you propose yourself as volunteer (welcome !) to do this job. Of course, I will not let things broken if move cannot be done, but I will prefer to wait having a proper long time fix.
Best regards, Stefano
Stefano,
My patch is not intrusive and it appears several people are wanting it. Why not accept my patch which will end up getting re-written once the PCI driver is moved to DM?
Well, most drivers are already moved to DM and there is a clear path to go on in this direction. It is not strange or something special with PCI driver. I am just asking if there is a volunteer to take over the job (I understand that we are all quite busy, but Jagan offers himself as volunteer). I fully agree that it is bad to let it broken, and I will merge this as it is if there will not a patch in time for release.
Best regards, Stefano
Hi Tim,
On 18/05/2017 16:35, Stefano Babic wrote:
Stefano,
My patch is not intrusive and it appears several people are wanting it. Why not accept my patch which will end up getting re-written once the PCI driver is moved to DM?
Well, most drivers are already moved to DM and there is a clear path to go on in this direction. It is not strange or something special with PCI driver. I am just asking if there is a volunteer to take over the job (I understand that we are all quite busy, but Jagan offers himself as volunteer). I fully agree that it is bad to let it broken, and I will merge this as it is if there will not a patch in time for release.
As promised: I merge it and moving to DM can be done in a follow-up patch when someone has time for it. In the meantime, this fix a grave issue.
Applied to -master, thanks !
Best regards, Stefano Babic
On 12.05.2017 21:58, Tim Harvey 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com Tested-by: Peter Senna Tschudin peter.senna@collabora.com
Tested-by: Soeren Moch smoch@web.de
Stefano,
can we get this fix merged for v2017.07!?
Thanks, Soeren
On 26/06/2017 16:07, Soeren Moch wrote:
On 12.05.2017 21:58, Tim Harvey 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by: Tim Harvey tharvey@gateworks.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com Tested-by: Peter Senna Tschudin peter.senna@collabora.com
Tested-by: Soeren Moch smoch@web.de
Stefano,
can we get this fix merged for v2017.07!?
Yes - this is merged in u-boot-imx, I am checking now which pending patches are bug fixes that can flow in, and I send later my PR to Tom.
Regards, Stefano
Hello
Does the code below really work?
On my custom MX6Q board, the code hangs on the read of the "PCIE_PL_PFLR". Please note that this code sequence is not entered the first time after a power up; I have to execute a U-Boot reset to actually trigger the hang. Any ideas what is going wrong?
While debugging it, I also noticed the two problems below.
Tim Harvey wrote:
- 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)) {
This could be (gpr12 & IOMUXC_GPR12_APPS_LTSSM_ENABLE)) {
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]);
I think this should be writel(gpr12, &iomuxc_regs->gpr[12]);
or even better clrbits_le32(&iomuxc_regs->gpr[12], IOMUXC_GPR12_APPS_LTSSM_ENABLE);
as in the rest of the file.
Dave
On 09/22/2017 04:00 PM, David Müller (ELSOFT AG) wrote:
Hello
Does the code below really work?
On my custom MX6Q board, the code hangs on the read of the "PCIE_PL_PFLR". Please note that this code sequence is not entered the first time after a power up; I have to execute a U-Boot reset to actually trigger the hang. Any ideas what is going wrong?
MX6Q PCIe reset breakage strikes again ?
While debugging it, I also noticed the two problems below.
Tim Harvey wrote:
- 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)) {This could be (gpr12 & IOMUXC_GPR12_APPS_LTSSM_ENABLE)) {
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]);I think this should be writel(gpr12, &iomuxc_regs->gpr[12]);
or even better clrbits_le32(&iomuxc_regs->gpr[12], IOMUXC_GPR12_APPS_LTSSM_ENABLE);
as in the rest of the file.
Dave
Marek Vasut wrote:
On 09/22/2017 04:00 PM, David Müller (ELSOFT AG) wrote:
On my custom MX6Q board, the code hangs on the read of the "PCIE_PL_PFLR". Please note that this code sequence is not entered the first time after a power up; I have to execute a U-Boot reset to actually trigger the hang. Any ideas what is going wrong?
MX6Q PCIe reset breakage strikes again ?
I thought this piece of code was added as work-around to fix the "MX6Q PCIe reset" problem. Or are you talking about something different?
Dave
On 09/25/2017 11:05 AM, David Müller (ELSOFT AG) wrote:
Marek Vasut wrote:
On 09/22/2017 04:00 PM, David Müller (ELSOFT AG) wrote:
On my custom MX6Q board, the code hangs on the read of the "PCIE_PL_PFLR". Please note that this code sequence is not entered the first time after a power up; I have to execute a U-Boot reset to actually trigger the hang. Any ideas what is going wrong?
MX6Q PCIe reset breakage strikes again ?
I thought this piece of code was added as work-around to fix the "MX6Q PCIe reset" problem. Or are you talking about something different?
That's exactly what I'm talking about ...
On Fri, Sep 22, 2017 at 7:00 AM, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Hello
Does the code below really work?
Hi Dave,
This patch does work to resolve the issue stated in the commit log which was to fix failing to boot on a 4.11+ kernel which stems from the fact that we have no reliable way to reset the PCIe host controller for IMX6SDL/DQ. This code used to be in the kernel [1] but was removed in 4.11 because it was seen as a workaround for the bootloader failing to put the PCIe host controller back in a sane state (something normally not needed to be done if these hardware blocks have a reset available).
However, your right you do end up hanging on a 'soft reset' likely for the same reason the kernel used to - the PCIe host controller is not left in a sane state because this code isn't run in that case.
On my custom MX6Q board, the code hangs on the read of the "PCIE_PL_PFLR". Please note that this code sequence is not entered the first time after a power up; I have to execute a U-Boot reset to actually trigger the hang. Any ideas what is going wrong?
While debugging it, I also noticed the two problems below.
Tim Harvey wrote:
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)) {This could be (gpr12 & IOMUXC_GPR12_APPS_LTSSM_ENABLE)) {
Currently IOMUXC_GPR12_PCIE_CTL_2 is defined as GPR12[10] likely because that's how it was defined in an early reference manual but I would agree it should be changed for readability - its not a bug.
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]);I think this should be writel(gpr12, &iomuxc_regs->gpr[12]);
or even better clrbits_le32(&iomuxc_regs->gpr[12], IOMUXC_GPR12_APPS_LTSSM_ENABLE);
Yes this is a bug for sure, however it doesn't appear to resolve your issue.
Lucas, does barebox suffer from a hang on PCI init on chip-level reset and if not do you know what could be causing the read of PCIE_PL_PFLR to hang here?
Regards,
Tim
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
Hi David,
On Fri, Sep 22, 2017 at 11:00 AM, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Hello
Does the code below really work?
On my custom MX6Q board, the code hangs on the read of the "PCIE_PL_PFLR". Please note that this code sequence is not entered the first time after a power up; I have to execute a U-Boot reset to actually trigger the hang. Any ideas what is going wrong?
Just sent a patch that should probably fix the PCIE_PL_PFLR hang problem.
Please give it a try.
participants (7)
-
David Müller (ELSOFT AG) -
Fabio Estevam -
Marek Vasut -
Peter Senna Tschudin -
Soeren Moch -
Stefano Babic -
Tim Harvey