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

[Stefano Babic]

Hi Tim,

On 18/05/2017 16:22, Tim Harvey wrote:
> 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?

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

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