Hi Zhiqiang,

On Thu, Feb 14, 2019 at 11:52 AM Z.q. Hou <zhiqiang....@nxp.com> wrote:
>
> Hi Bin,
>
> Thanks a lot for your comments!
>
> > -----Original Message-----
> > From: Bin Meng <bmeng...@gmail.com>
> > Sent: 2019年2月12日 11:45
> > To: Z.q. Hou <zhiqiang....@nxp.com>
> > Cc: u-boot@lists.denx.de; albert.u.b...@aribaud.net; Priyanka Jain
> > <priyanka.j...@nxp.com>; York Sun <york....@nxp.com>;
> > sriram.d...@nxp.com; yamada.masah...@socionext.com; Prabhakar
> > Kushwaha <prabhakar.kushw...@nxp.com>; Mingkai Hu
> > <mingkai...@nxp.com>; M.h. Lian <minghuan.l...@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....@nxp.com> wrote:
> > >
> > > From: Hou Zhiqiang <zhiqiang....@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....@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

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.

Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to