[U-Boot] [PATCHv3 10/11] pci: ls_pcie_g4: Add Workaround for A-011451
Z.q. Hou
zhiqiang.hou at nxp.com
Tue Feb 26 03:10:30 UTC 2019
Hi Bin,
> -----Original Message-----
> From: Bin Meng <bmeng.cn at gmail.com>
> Sent: 2019年2月26日 10:48
> To: Z.q. Hou <zhiqiang.hou at nxp.com>
> Cc: u-boot at lists.denx.de; albert.u.boot at aribaud.net; Priyanka Jain
> <priyanka.jain at nxp.com>; York Sun <york.sun at nxp.com>;
> sriram.dash at nxp.com; yamada.masahiro at socionext.com; Prabhakar
> Kushwaha <prabhakar.kushwaha at nxp.com>; Mingkai Hu
> <mingkai.hu at nxp.com>; M.h. Lian <minghuan.lian at nxp.com>
> Subject: Re: [U-Boot] [PATCHv3 10/11] pci: ls_pcie_g4: Add Workaround for
> A-011451
>
> Hi Zhiqiang,
>
> On Tue, Feb 26, 2019 at 10:40 AM Z.q. Hou <zhiqiang.hou at nxp.com> wrote:
> >
> > Hi Bin,
> >
> > Thanks a lot for your comments!
> >
> > > -----Original Message-----
> > > From: Bin Meng <bmeng.cn at gmail.com>
> > > Sent: 2019年2月21日 18:26
> > > To: Z.q. Hou <zhiqiang.hou at nxp.com>
> > > Cc: u-boot at lists.denx.de; albert.u.boot at aribaud.net; Priyanka Jain
> > > <priyanka.jain at nxp.com>; York Sun <york.sun at nxp.com>;
> > > sriram.dash at nxp.com; yamada.masahiro at socionext.com; Prabhakar
> > > Kushwaha <prabhakar.kushwaha at nxp.com>; Mingkai Hu
> > > <mingkai.hu at nxp.com>; M.h. Lian <minghuan.lian at nxp.com>
> > > Subject: Re: [U-Boot] [PATCHv3 10/11] pci: ls_pcie_g4: Add
> > > Workaround for
> > > A-011451
> > >
> > > Hi Zhiqiang,
> > >
> > > On Mon, Feb 18, 2019 at 9:34 AM Z.q. Hou <zhiqiang.hou at nxp.com>
> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > Thanks a lot for your comments!
> > > >
> > > > > -----Original Message-----
> > > > > From: Bin Meng <bmeng.cn at gmail.com>
> > > > > Sent: 2019年2月14日 14:17
> > > > > To: Z.q. Hou <zhiqiang.hou at nxp.com>
> > > > > Cc: u-boot at lists.denx.de; albert.u.boot at aribaud.net; Priyanka
> > > > > Jain <priyanka.jain at nxp.com>; York Sun <york.sun at nxp.com>;
> > > > > sriram.dash at nxp.com; yamada.masahiro at socionext.com; Prabhakar
> > > > > Kushwaha <prabhakar.kushwaha at nxp.com>; Mingkai Hu
> > > > > <mingkai.hu at nxp.com>; M.h. Lian <minghuan.lian at nxp.com>
> > > > > Subject: Re: [U-Boot] [PATCHv3 10/11] pci: ls_pcie_g4: Add
> > > > > Workaround for
> > > > > A-011451
> > > > >
> > > > > Hi Zhiqiang,
> > > > >
> > > > > On Thu, Feb 14, 2019 at 11:52 AM Z.q. Hou <zhiqiang.hou at nxp.com>
> > > wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > Thanks a lot for your comments!
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Bin Meng <bmeng.cn at gmail.com>
> > > > > > > Sent: 2019年2月12日 11:45
> > > > > > > To: Z.q. Hou <zhiqiang.hou at nxp.com>
> > > > > > > Cc: u-boot at lists.denx.de; albert.u.boot at aribaud.net;
> > > > > > > Priyanka Jain <priyanka.jain at nxp.com>; York Sun
> > > > > > > <york.sun at nxp.com>; sriram.dash at nxp.com;
> > > > > > > yamada.masahiro at socionext.com; Prabhakar Kushwaha
> > > > > > > <prabhakar.kushwaha at nxp.com>; Mingkai Hu
> > > > > > > <mingkai.hu at nxp.com>; M.h. Lian <minghuan.lian at nxp.com>
> > > > > > > Subject: Re: [U-Boot] [PATCHv3 10/11] pci: ls_pcie_g4: Add
> > > > > > > Workaround for
> > > > > > > A-011451
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Fri, Jan 25, 2019 at 6:07 PM Z.q. Hou
> > > > > > > <zhiqiang.hou at nxp.com>
> > > wrote:
> > > > > > > >
> > > > > > > > From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > > > > > > >
> > > > > > > > When LAYERSCAPE Gen4 PCIe controller is sending multiple
> > > > > > > > split completions and ACK latency expires indicating that
> > > > > > > > ACK should be send at priority. But because of large
> > > > > > > > number of split completions and FC update DLLP, the
> > > > > > > > controller does not give priority to ACK transmission.
> > > > > > > > This results into ACK latency timer timeout error at the
> > > > > > > > link partner and the pending TLPs are replayed by the link partner
> again.
> > > > > > > >
> > > > > > > > Workaround:
> > > > > > > > 1. Reduce the ACK latency timeout value to a very small value.
> > > > > > > > 2. Restrict the number of completions from the PCIe controller
> > > > > > > > to 1, by changing the Max Read Request Size (MRRS) of link
> > > > > > > > partner to the same value as Max Packet size (MPS).
> > > > > > > >
> > > > > > > > This ERRATA is only for LX2160A Rev1.0 and will be fixed in
> Rev2.0.
> > > > > > > >
> > > > > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > > > > > > > ---
> > > > > > > > V3:
> > > > > > > > - New patch.
> > > > > > > >
> > > > > > > > drivers/pci/pci_auto.c | 34
> > > > > > > ++++++++++++++++++++++++++++++
> > > > > > > > drivers/pci/pcie_layerscape_gen4.c | 8 +++++++
> > > > > > > > drivers/pci/pcie_layerscape_gen4.h | 4 ++++
> > > > > > > > include/pci.h | 9 ++++++++
> > > > > > > > include/pci_ids.h | 1 +
> > > > > > > > 5 files changed, 56 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/pci/pci_auto.c
> > > > > > > > b/drivers/pci/pci_auto.c index d7237f6eee..37307bd54b
> > > > > > > > 100644
> > > > > > > > --- a/drivers/pci/pci_auto.c
> > > > > > > > +++ b/drivers/pci/pci_auto.c
> > > > > > > > @@ -22,6 +22,7 @@ void dm_pciauto_setup_device(struct
> > > > > > > > udevice *dev,
> > > > > > > int bars_num,
> > > > > > > > struct pci_region *prefetch,
> > > > > > > > struct
> > > > > > > pci_region *io,
> > > > > > > > bool enum_only) {
> > > > > > > > + struct udevice *rp = pci_get_controller(dev);
> > > > > > > > u32 bar_response;
> > > > > > > > pci_size_t bar_size;
> > > > > > > > u16 cmdstat = 0;
> > > > > > > > @@ -32,6 +33,9 @@ void dm_pciauto_setup_device(struct
> > > > > > > > udevice *dev,
> > > > > > > int bars_num,
> > > > > > > > struct pci_region *bar_res = NULL;
> > > > > > > > int found_mem64 = 0;
> > > > > > > > u16 class;
> > > > > > > > + int pos;
> > > > > > > > + u16 val, vendor, dev_id;
> > > > > > > > + u8 rev;
> > > > > > > >
> > > > > > > > dm_pci_read_config16(dev, PCI_COMMAND,
> &cmdstat);
> > > > > > > > cmdstat = (cmdstat & ~(PCI_COMMAND_IO |
> > > > > > > PCI_COMMAND_MEMORY)) |
> > > > > > > > @@ -161,6 +165,36 @@ void dm_pciauto_setup_device(struct
> > > > > > > > udevice
> > > > > > > *dev, int bars_num,
> > > > > > > > dm_pci_write_config8(dev, PCI_CACHE_LINE_SIZE,
> > > > > > > >
> > > > > CONFIG_SYS_PCI_CACHE_LINE_SIZE);
> > > > > > > > dm_pci_write_config8(dev, PCI_LATENCY_TIMER,
> > > > > > > > 0x80);
> > > > > > > > +
> > > > > > > > + /*
> > > > > > > > + * When NXP LAYERSCAPE Gen4 PCIe controller is
> > > > > > > > + sending
> > > > > > > multiple split
> > > > > > > > + * completions and ACK latency expires indicating
> > > > > > > > + that ACK should
> > > > > > > be
> > > > > > > > + * send at priority. But because of large number
> > > > > > > > + of split
> > > > > > > completions
> > > > > > > > + * and FC update DLLP, the controller does not
> > > > > > > > + give priority to
> > > > > ACK
> > > > > > > > + * transmission. This results into ACK latency
> > > > > > > > + timer timeout error
> > > > > > > at
> > > > > > > > + * the link partner and the pending TLPs are
> > > > > > > > + replayed by the
> > > > > link
> > > > > > > > + * partner again.
> > > > > > > > + *
> > > > > > > > + * The workaround:
> > > > > > > > + * Restrict the number of completions from the
> > > > > > > > + PCIe controller to
> > > > > > > 1,
> > > > > > > > + * by changing the Max Read Request Size (MRRS) of
> > > > > > > > + link partner
> > > > > > > to the
> > > > > > > > + * same value as Max Packet size (MPS).
> > > > > > > > + *
> > > > > > > > + * So, set both the MPS and MRRS to the minimum
> 128B.
> > > > > > > > + */
> > > > > > > > + dm_pci_read_config16(rp, PCI_VENDOR_ID, &vendor);
> > > > > > > > + dm_pci_read_config16(rp, PCI_DEVICE_ID, &dev_id);
> > > > > > > > + dm_pci_read_config8(rp, PCI_REVISION_ID, &rev);
> > > > > > > > + if (vendor == PCI_VENDOR_ID_FREESCALE &&
> > > > > > > > + dev_id == PCI_DEVICE_ID_LX2160A && rev ==
> > > > > > > > + 0x10)
> > > {
> > > > > > > > + pos = dm_pci_find_capability(dev,
> > > > > PCI_CAP_ID_EXP);
> > > > > > > > + if (pos) {
> > > > > > > > + dm_pci_read_config16(dev, pos +
> > > > > > > PCI_EXP_DEVCTL, &val);
> > > > > > > > + val &=
> ~(PCI_EXP_DEVCTL_READRQ |
> > > > > > > > +
> > > PCI_EXP_DEVCTL_PAYLOAD);
> > > > > > > > + dm_pci_write_config16(dev, pos +
> > > > > > > PCI_EXP_DEVCTL, val);
> > > > > > > > + }
> > > > > > > > + }
> > > > > > >
> > > > > > > Can this be done in the probe() routine of the driver codes?
> > > > > >
> > > > > > No, not only the RC itself but also all the EP devices need to
> > > > > > setup the same
> > > > > MPS and MRRS, it is hard to scan the PCIe EP devices in the RC
> > > > > driver (a platform driver).
> > > > > >
> > > > >
> > > > > If this is the case, I think we need split the patch into 2 commits:
> > > > >
> > > > > - 1 commit to update the PCI library to program MPS and MRRS.
> > > > > This can be generic though.
> > > > > - 1 commit to update the gen4 PCIe driver for the workaround
> > > >
> > > > Thanks for your suggestion! Will split it.
> > > >
> > >
> > > I was wondering whether we can introduce a quirk settings support in
> > > the PCI bus codes, something like how Linux handles.
> > >
> > > For this errata, is the timing of programming MPS and MRRS required
> > > to be happen in the middle of enumeration progress? If not, we can
> > > do it later by using the quirk settings (VID, DID).
> >
> > The problem is not the timing, it can be put anywhere before the
> enumeration complete, but the NXP Gen4 RC - PCI bridge - EP breadth, I mean
> the whole PCIe device tree's MPS and MRRS from RC to all the PCIe devices
> (PCI bridge and EP) need to be set, so the Linux quirk mode is not suitable for
> this.
> >
>
> Yes, I understand all devices's MPS/MRRS on the path should be programmed.
> Actually I have an x86 board that expose such an issue too and I am sure we
> need introduce the MPS/MRRS support in U-Boot but I have not worked on
> this.
I'm working on this and will add the MPS/MRRS APIs in v4 patches, and you can work out the x86 patch base on that.
>
> > >
> > > > >
> > > > > BTW: the default MPS and MRRS should be 128B per the PCIe spec.
> > > > > I am wondering why the gen4 PCIe RC does not follow the spec.
> > > >
> > > > The default MPS is 128B but default MRRS is 512B in the spec. And
> > > > this is a
> > > NXP Gen4 PCIe controller's ERRATA as descripted in the comments that
> > > result in the NXP RC does not work well with the default value.
> > > >
> > >
> > > OK, thanks for the clarification. So both MPS and MRRS have to be
> > > 128B on the NXP Gen3 PCIe controller in order to get it work.
> > >
>
> Regards,
> Bin
Thanks,
Zhiqiang
More information about the U-Boot
mailing list