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

[Tim Harvey]

On Fri, Sep 22, 2017 at 7:00 AM, David Müller (ELSOFT AG)
<d.mueller at 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/drivers/pci/host/pci-imx6.c?h=v4.10#n270