Hi Zhiqiang, On Tue, Feb 26, 2019 at 10:40 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月21日 18:26 > > 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 Zhiqiang, > > > > On Mon, Feb 18, 2019 at 9:34 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月14日 14:17 > > > > 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 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 > > > > > > 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot