drivers: pci: imx: add imx_pcie_remove function
Subsystems (3)
PATCH 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 puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
--- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)
arch/arm/imx-common/cpu.c +3 -0
@@ -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 )
drivers/pci/pcie_imx.c +38 -0
@@ -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 ) {
include/pci.h +4 -0
@@ -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
Signature · 1 line
2.7.4
Fabio Estevam
reviewerPATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Hi Tim,
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 4 lines
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by Tim Harvey <[email protected]>
This looks good:
Added Peter on Cc in case he could send his Tested-by tag.
Quoted 101 lines
--- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)arch/arm/imx-common/cpu.c +3
-0@@ -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 )drivers/pci/pcie_imx.c +38
-0@@ -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 ) {include/pci.h +4
-1@@ -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
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Quoted 15 lines · Fabio Estevam
Hi Tim,
Quoted 12 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.
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Quoted 7 lines
Quoted 1 line
Signed-off-by Tim Harvey <[email protected]>This looks good:
Reviewed-by Fabio Estevam <[email protected]>Added Peter on Cc in case he could send his Tested-by tag.
I confirm this patch solves my issues. Thank you Tim!
Quoted 102 lines
Quoted 102 lines
--- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)arch/arm/imx-common/cpu.c +3
-0@@ -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 )drivers/pci/pcie_imx.c +38
-0@@ -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 ) {include/pci.h +4
-1@@ -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
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
In reply to Tim HarveyQuoted 102 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.
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by Tim Harvey <[email protected]>--- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)arch/arm/imx-common/cpu.c +3
-0@@ -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 )drivers/pci/pcie_imx.c +38
-0@@ -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 ) {include/pci.h +0
-0
include/pci.h int pci_last_busno(void); · line 763
Quoted 10 lines
include/pci.h +4
-0@@ -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
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 =====================================================================
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Quoted 119 lines · Stefano Babic
Quoted 102 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.
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by Tim Harvey <[email protected]>--- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)arch/arm/imx-common/cpu.c +3
-0@@ -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 )drivers/pci/pcie_imx.c +38
-0@@ -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 ) {include/pci.h +0
-0include/pci.h int pci_last_busno(void); · line 763
Quoted 10 lines
include/pci.h +4
-0@@ -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.
Quoted 3 lines
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
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Hi Tim,
Quoted 124 lines · Tim Harvey
Quoted 119 lines · Stefano Babic
Quoted 102 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.
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by Tim Harvey <[email protected]>--- arch/arm/imx-common/cpu.c | 3 +++ drivers/pci/pcie_imx.c | 38 ++++++++++++++++++++++++++++++++++++++ include/pci.h | 4 ++++ 3 files changed, 45 insertions(+)arch/arm/imx-common/cpu.c +3
-0@@ -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 )drivers/pci/pcie_imx.c +38
-0@@ -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 ) {include/pci.h +0
-0include/pci.h int pci_last_busno(void); · line 763
Quoted 10 lines
include/pci.h +4
-0@@ -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.
Quoted 3 lines
Best regards,
Stefano
Quoted 5 lines
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
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 =====================================================================
Stefano Babic
AppliedPATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Hi Tim,
Quoted 6 lines · Stefano Babic
Quoted 5 lines
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?
Quoted 6 lines
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
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 =====================================================================
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 6 lines
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by Tim Harvey <[email protected]>Reviewed-by Fabio Estevam <[email protected]>Tested-by Peter Senna Tschudin <[email protected]>
Stefano,
can we get this fix merged for v2017.07!?
Thanks,
Soeren
Quoted 19 lines · Soeren Moch
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 6 lines
This puts the PCIe controller back into a safe state for the kernel driver before launching the kernel.
Signed-off-by Tim Harvey <[email protected]>Reviewed-by Fabio Estevam <[email protected]>Tested-by Peter Senna Tschudin <[email protected]>
Quoted 7 lines
Tested-by Soeren Moch <[email protected]>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
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 =====================================================================
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
In reply to Tim HarveyHello
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.
drivers/pci/pcie_imx.c static int imx6_pcie_assert_core_reset(void) · line 468
Quoted 7 lines · Tim Harvey
drivers/pci/pcie_imx.c +7
-0+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)) {
drivers/pci/pcie_imx.c static int imx6_pcie_assert_core_reset(void) · line 478
Quoted 10 lines
drivers/pci/pcie_imx.c +10
-0+ 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
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Quoted 8 lines · David Müller (ELSOFT AG)
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 ?
Quoted 36 lines
While debugging it, I also noticed the two problems below.
drivers/pci/pcie_imx.c static int imx6_pcie_assert_core_reset(void) · line 468
Quoted 7 lines · Tim Harvey
drivers/pci/pcie_imx.c +7
-0+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)) {drivers/pci/pcie_imx.c static int imx6_pcie_assert_core_reset(void) · line 478
Quoted 10 lines
drivers/pci/pcie_imx.c +10
-0+ 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
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Quoted 7 lines · Marek Vasut
Quoted 4 lines · David Müller (ELSOFT AG)
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
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
Quoted 8 lines · David Müller (ELSOFT AG)
Quoted 6 lines · Marek Vasut
Quoted 4 lines · David Müller (ELSOFT AG)
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?
Quoted 4 lines
Quoted 1 line
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 ...
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
In reply to David Müller (ELSOFT AG)Quoted 3 lines · David Müller (ELSOFT AG)
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.
Quoted 19 lines
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.
drivers/pci/pcie_imx.c static int imx6_pcie_assert_core_reset(void) · line 468
Quoted 7 lines · Tim Harvey
drivers/pci/pcie_imx.c +7
-0+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 )) {
Quoted 1 line
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.
Quoted 13 lines
drivers/pci/pcie_imx.c static int imx6_pcie_assert_core_reset(void) · line 478
Quoted 11 lines
drivers/pci/pcie_imx.c +10
-0+ 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
Quoted 5 lines
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
Fabio Estevam
reviewerPATCH 1/1 drivers: pci: imx: add imx_pcie_remove function
In reply to David Müller (ELSOFT AG)Hi David,
Quoted 8 lines · David Müller (ELSOFT AG)
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.