[U-Boot] [PATCHv3 10/11] pci: ls_pcie_g4: Add Workaround for A-011451
Bin Meng
bmeng.cn at gmail.com
Tue Feb 26 02:48:23 UTC 2019
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.
> >
> > > >
> > > > 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
More information about the U-Boot
mailing list