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

[Tim Harvey]

On Thu, May 18, 2017 at 2:12 AM, Stefano Babic <sbabic at 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 at 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