[LEDE-DEV] [PATCH v2] imx6: fix pcie scanning on boot

[Tim Harvey]

On Thu, Jan 4, 2018 at 8:23 AM, Koen Vandeputte
<koen.vandeputte at ncentric.com> wrote:
> By default, when the imx6 PCIe RC boots up, the subordinate is set
> equally to the secondary bus (1), and does not alter afterwards.
>
> This means that theoretically, the highest bus reachable downstream is
> bus 1.
>
> Before upstream commit a20c7f36bd3d ("PCI: Do not allocate more buses
> than available in parent"), the driver ignored the subord value and just
> allowed up to 0xff on each device downstream.
>
> This caused a lot of errors to be printed, as this is not logical
> according to spec. (but it worked ..)
>
> After this commit, the driver stopped scanning deeper when the last
> allocated busnr equals the subordinate of it's master, causing devices
> to be undiscovered (especially behind bridges), uncovering the impact of
> this bug.
>
> Before:
>
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
>         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 10b5:8604 (rev ba)
> ... stops after bus 1 ...
>
> After:
>
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> ...
>         Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 10b5:8604 (rev ba)
> 02:01.0 0604: 10b5:8604 (rev ba)
> 02:04.0 0604: 10b5:8604 (rev ba)
> 02:05.0 0604: 10b5:8604 (rev ba)
> 03:00.0 0280: 168c:0033 (rev 01)
> 05:00.0 0280: 168c:0033 (rev 01)
>
> This upstream commit was introduced in kernel 4.9.71
>
> Signed-off-by: Koen Vandeputte <koen.vandeputte at ncentric.com>
> ---
>
> V2:
>
> Proper fix for the real rootcause
> kernel 4.4 is not affected
>
>
>  .../patches-4.9/230-fix-pcie-enumeration.patch     | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 target/linux/imx6/patches-4.9/230-fix-pcie-enumeration.patch
>
> diff --git a/target/linux/imx6/patches-4.9/230-fix-pcie-enumeration.patch b/target/linux/imx6/patches-4.9/230-fix-pcie-enumeration.patch
> new file mode 100644
> index 000000000000..18f22347172b
> --- /dev/null
> +++ b/target/linux/imx6/patches-4.9/230-fix-pcie-enumeration.patch
> @@ -0,0 +1,73 @@
> +By default, when the imx6 PCIe RC boots up, the subordinate is set
> +equally to the secondary bus (1), and does not alter afterwards.
> +
> +This means that theoretically, the highest bus reachable downstream is
> +bus 1.
> +
> +Before commit a20c7f36bd3d ("PCI: Do not allocate more buses than
> +available in parent"), the driver ignored the subord value and just
> +allowed up to 0xff on each device downstream.
> +
> +This caused a lot of errors to be printed, as this is not logical
> +according to spec. (but it worked ..)
> +
> +After this commit, the driver stopped scanning deeper when the last
> +allocated busnr equals the subordinate of it's master, causing devices
> +to be undiscovered (especially behind bridges), uncovering the impact of
> +this bug.
> +
> +Before:
> +
> +00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 ...
> +       Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> +
> +00:00.0 0604: 16c3:abcd (rev 01)
> +01:00.0 0604: 10b5:8604 (rev ba)
> +... stops after bus 1 ...
> +
> +After:
> +
> +00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
> +...
> +       Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> +
> +00:00.0 0604: 16c3:abcd (rev 01)
> +01:00.0 0604: 10b5:8604 (rev ba)
> +02:01.0 0604: 10b5:8604 (rev ba)
> +02:04.0 0604: 10b5:8604 (rev ba)
> +02:05.0 0604: 10b5:8604 (rev ba)
> +03:00.0 0280: 168c:0033 (rev 01)
> +05:00.0 0280: 168c:0033 (rev 01)
> +
> +Signed-off-by: Koen Vandeputte <koen.vandeputte at ncentric.com>
> +
> +--- a/drivers/pci/host/pci-imx6.c
> ++++ b/drivers/pci/host/pci-imx6.c
> +@@ -64,6 +64,9 @@ struct imx6_pcie {
> +
> + #define PCIE_RC_LCSR                          0x80
> +
> ++#define PCIE_RC_BNR                           0x18
> ++#define PCIE_RC_BNR_MAX_SUBORDINATE           (0xff << 16)
> ++
> + /* PCIe Port Logic registers (memory-mapped) */
> + #define PL_OFFSET 0x700
> + #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
> +@@ -488,6 +491,17 @@ static int imx6_pcie_establish_link(stru
> +       int ret;
> +
> +       /*
> ++       * By default, the subordinate is set equally to the secondary
> ++       * bus (0x01) when the RC boots.
> ++       * This means that theoretically, only bus 1 is reachable from the RC.
> ++       * Force the PCIe RC subordinate to 0xff, otherwise no downstream
> ++       * devices will be detected behind bus 1.
> ++      */
> ++      tmp = dw_pcie_readl_rc(pp, PCIE_RC_BNR);
> ++      tmp |= PCIE_RC_BNR_MAX_SUBORDINATE;
> ++      dw_pcie_writel_rc(pp, PCIE_RC_BNR, tmp);
> ++
> ++      /*
> +        * Force Gen1 operation when starting the link.  In case the link is
> +        * started in Gen2 mode, there is a possibility the devices on the
> +        * bus will not be detected at all.  This happens with PCIe switches.
> --
> 2.7.4
>

Hi Koen,

Thanks for this info!

Have you asked the linux-pci group about this? I'm curious what the
patch to 4.9.71 was trying to do and if this is the proper place to
fix this. I assume if we set that value in the bootloader it would get
reset when the kernel does a PCI reset.

It seems like every couple of months there is some upstream breakage
dealing with the i.MX6 and a PCIe switch :(

Regards,

Tim