* [PATCH 0/2] PCI: ls_pcie_g4: add 2 workarounds @ 2018-12-02 13:32 Z.q. Hou 2018-12-02 13:32 ` [PATCH 1/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 Z.q. Hou 2018-12-02 13:32 ` [PATCH 2/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 Z.q. Hou 0 siblings, 2 replies; 5+ messages in thread From: Z.q. Hou @ 2018-12-02 13:32 UTC (permalink / raw) To: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, l.subrahmanya Cc: Leo Li, M.h. Lian, Xiaowei Bao, Z.q. Hou From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> This patchset adds 2 workarounds for NXP Layerscape Gen4 PCIe controller driver. Hou Zhiqiang (2): PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 depends on patchset: https://patchwork.ozlabs.org/project/linux-pci/list/?series=76942 .../controller/mobiveil/pci-layerscape-gen4.c | 38 +++++++++++++++++++ .../controller/mobiveil/pcie-mobiveil-host.c | 13 ++++++- .../pci/controller/mobiveil/pcie-mobiveil.h | 6 +++ 3 files changed, 56 insertions(+), 1 deletion(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 2018-12-02 13:32 [PATCH 0/2] PCI: ls_pcie_g4: add 2 workarounds Z.q. Hou @ 2018-12-02 13:32 ` Z.q. Hou 2018-12-03 21:06 ` Bjorn Helgaas 2018-12-02 13:32 ` [PATCH 2/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 Z.q. Hou 1 sibling, 1 reply; 5+ messages in thread From: Z.q. Hou @ 2018-12-02 13:32 UTC (permalink / raw) To: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, l.subrahmanya Cc: Leo Li, M.h. Lian, Xiaowei Bao, Z.q. Hou From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> PCIe configuration access to non-existent function triggered SERROR interrupt exception. Workaround: Disable error reporting on AXI bus during the Vendor ID read transactions in enumeration. This ERRATA is only for LX2160A Rev1.0, and it will be fixed in Rev2.0. Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> --- .../controller/mobiveil/pci-layerscape-gen4.c | 24 +++++++++++++++++++ .../controller/mobiveil/pcie-mobiveil-host.c | 13 +++++++++- .../pci/controller/mobiveil/pcie-mobiveil.h | 2 ++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c index 174cbcac4059..1fe56532b288 100644 --- a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c +++ b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c @@ -24,6 +24,9 @@ /* LUT and PF control registers */ #define PCIE_LUT_OFF (0x80000) +#define PCIE_LUT_GCR (0x28) +#define PCIE_LUT_GCR_RRE (0) + #define PCIE_PF_OFF (0xc0000) #define PCIE_PF_INT_STAT (0x18) #define PF_INT_STAT_PABRST (31) @@ -188,8 +191,29 @@ static void ls_pcie_g4_reset(struct work_struct *work) ls_pcie_g4_reinit_hw(pcie); } +static int ls_pcie_g4_read_other_conf(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *val) +{ + struct mobiveil_pcie *pci = bus->sysdata; + struct ls_pcie_g4 *pcie = to_ls_pcie_g4(pci); + int ret; + + if (where == PCI_VENDOR_ID) + ls_pcie_g4_lut_writel(pcie, PCIE_LUT_GCR, + 0 << PCIE_LUT_GCR_RRE); + + ret = pci_generic_config_read(bus, devfn, where, size, val); + + if (where == PCI_VENDOR_ID) + ls_pcie_g4_lut_writel(pcie, PCIE_LUT_GCR, + 1 << PCIE_LUT_GCR_RRE); + + return ret; +} + static struct mobiveil_rp_ops ls_pcie_g4_rp_ops = { .interrupt_init = ls_pcie_g4_interrupt_init, + .read_other_conf = ls_pcie_g4_read_other_conf, }; static const struct mobiveil_pab_ops ls_pcie_g4_pab_ops = { diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c b/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c index c85f00d3cfcf..8b6db38320d7 100644 --- a/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c @@ -79,9 +79,20 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus, return pcie->rp.config_axi_slave_base + where; } +static int mobiveil_pcie_config_read(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *val) +{ + struct mobiveil_pcie *pcie = bus->sysdata; + struct root_port *rp = &pcie->rp; + + if (bus->number > rp->root_bus_nr && rp->ops->read_other_conf) + return rp->ops->read_other_conf(bus, devfn, where, size, val); + + return pci_generic_config_read(bus, devfn, where, size, val); +} static struct pci_ops mobiveil_pcie_ops = { .map_bus = mobiveil_pcie_map_bus, - .read = pci_generic_config_read, + .read = mobiveil_pcie_config_read, .write = pci_generic_config_write, }; diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h b/drivers/pci/controller/mobiveil/pcie-mobiveil.h index 0ccd6cee5f8f..ef93b41f4419 100644 --- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h @@ -145,6 +145,8 @@ struct mobiveil_msi { /* MSI information */ struct mobiveil_rp_ops { int (*interrupt_init)(struct mobiveil_pcie *pcie); + int (*read_other_conf)(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *val); }; struct root_port { -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 2018-12-02 13:32 ` [PATCH 1/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 Z.q. Hou @ 2018-12-03 21:06 ` Bjorn Helgaas 0 siblings, 0 replies; 5+ messages in thread From: Bjorn Helgaas @ 2018-12-03 21:06 UTC (permalink / raw) To: Z.q. Hou Cc: linux-pci, linux-kernel, lorenzo.pieralisi, l.subrahmanya, Leo Li, M.h. Lian, Xiaowei Bao On Sun, Dec 02, 2018 at 01:32:42PM +0000, Z.q. Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> Can we pick one driver name (either "mobiveil" or "ls_pcie_g4" (this seems excessively long and excessively specific), or something else)? I don't want to waste the space of "PCI: mobiveil: ls_pcie_g4:" in every future subject line. Then "Add workaround for ...". I assume the "A-011577" part is meaningful inside NXP, but it's not useful to anybody else. Move that to the changelog proper and say something about the actual issue in the subject. > PCIe configuration access to non-existent function triggered > SERROR interrupt exception. > > Workaround: > Disable error reporting on AXI bus during the Vendor ID read > transactions in enumeration. > > This ERRATA is only for LX2160A Rev1.0, and it will be fixed > in Rev2.0. > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > .../controller/mobiveil/pci-layerscape-gen4.c | 24 +++++++++++++++++++ > .../controller/mobiveil/pcie-mobiveil-host.c | 13 +++++++++- > .../pci/controller/mobiveil/pcie-mobiveil.h | 2 ++ > 3 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c > index 174cbcac4059..1fe56532b288 100644 > --- a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c > +++ b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c > @@ -24,6 +24,9 @@ > > /* LUT and PF control registers */ > #define PCIE_LUT_OFF (0x80000) > +#define PCIE_LUT_GCR (0x28) > +#define PCIE_LUT_GCR_RRE (0) > + > #define PCIE_PF_OFF (0xc0000) > #define PCIE_PF_INT_STAT (0x18) > #define PF_INT_STAT_PABRST (31) > @@ -188,8 +191,29 @@ static void ls_pcie_g4_reset(struct work_struct *work) > ls_pcie_g4_reinit_hw(pcie); > } > > +static int ls_pcie_g4_read_other_conf(struct pci_bus *bus, unsigned int devfn, > + int where, int size, u32 *val) > +{ > + struct mobiveil_pcie *pci = bus->sysdata; > + struct ls_pcie_g4 *pcie = to_ls_pcie_g4(pci); > + int ret; > + > + if (where == PCI_VENDOR_ID) > + ls_pcie_g4_lut_writel(pcie, PCIE_LUT_GCR, > + 0 << PCIE_LUT_GCR_RRE); > + > + ret = pci_generic_config_read(bus, devfn, where, size, val); > + > + if (where == PCI_VENDOR_ID) > + ls_pcie_g4_lut_writel(pcie, PCIE_LUT_GCR, > + 1 << PCIE_LUT_GCR_RRE); 1) As a general style rule, it's better to "clear, then restore" than to "clear, then set" the bit. That way if somebody elsewhere decides that PCIE_LUT_GCR_RRE should be cleared by default, this code won't stomp on that decision. E.g., gcr = ls_pcie_g4_lut_readl(...); ls_pcie_g4_lut_writel(..., 0 << PCIE_LUT_GCR_RRE); ret = pci_generic_config_read(...); ls_pcie_g4_lut_writel(..., gcr); 2) I don't *think* the PCIe spec requires that the first access to a device be a read of the Vendor ID, so this is a 99% solution, not a 100% solution. A 100% solution would be to handle the SERROR so it's not fatal. But I'm pretty sure Linux always does read the Vendor ID first (except after a reset, and when we do config reads after a reset, we already know the device *exists*), so this is probably pretty safe. > + return ret; > +} > + > static struct mobiveil_rp_ops ls_pcie_g4_rp_ops = { > .interrupt_init = ls_pcie_g4_interrupt_init, > + .read_other_conf = ls_pcie_g4_read_other_conf, > }; > > static const struct mobiveil_pab_ops ls_pcie_g4_pab_ops = { > diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c b/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c > index c85f00d3cfcf..8b6db38320d7 100644 > --- a/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c > +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c > @@ -79,9 +79,20 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus, > return pcie->rp.config_axi_slave_base + where; > } > > +static int mobiveil_pcie_config_read(struct pci_bus *bus, unsigned int devfn, > + int where, int size, u32 *val) > +{ > + struct mobiveil_pcie *pcie = bus->sysdata; > + struct root_port *rp = &pcie->rp; > + > + if (bus->number > rp->root_bus_nr && rp->ops->read_other_conf) > + return rp->ops->read_other_conf(bus, devfn, where, size, val); > + > + return pci_generic_config_read(bus, devfn, where, size, val); > +} > static struct pci_ops mobiveil_pcie_ops = { > .map_bus = mobiveil_pcie_map_bus, > - .read = pci_generic_config_read, > + .read = mobiveil_pcie_config_read, > .write = pci_generic_config_write, > }; > > diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h b/drivers/pci/controller/mobiveil/pcie-mobiveil.h > index 0ccd6cee5f8f..ef93b41f4419 100644 > --- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h > +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h > @@ -145,6 +145,8 @@ struct mobiveil_msi { /* MSI information */ > > struct mobiveil_rp_ops { > int (*interrupt_init)(struct mobiveil_pcie *pcie); > + int (*read_other_conf)(struct pci_bus *bus, unsigned int devfn, > + int where, int size, u32 *val); > }; > > struct root_port { > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 2018-12-02 13:32 [PATCH 0/2] PCI: ls_pcie_g4: add 2 workarounds Z.q. Hou 2018-12-02 13:32 ` [PATCH 1/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 Z.q. Hou @ 2018-12-02 13:32 ` Z.q. Hou 2018-12-03 22:06 ` Bjorn Helgaas 1 sibling, 1 reply; 5+ messages in thread From: Z.q. Hou @ 2018-12-02 13:32 UTC (permalink / raw) To: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, l.subrahmanya Cc: Leo Li, M.h. Lian, Xiaowei Bao, Z.q. Hou From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> When LX2 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 LX2 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 patch implemented part 1, the part 2 can be set by kernel parameter 'pci=pcie_bus_perf' This ERRATA is only for LX2160A Rev1.0, and it will be fixed in Rev2.0. Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> --- .../pci/controller/mobiveil/pci-layerscape-gen4.c | 14 ++++++++++++++ drivers/pci/controller/mobiveil/pcie-mobiveil.h | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c index 1fe56532b288..ef43033e1c2a 100644 --- a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c +++ b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c @@ -220,6 +220,18 @@ static const struct mobiveil_pab_ops ls_pcie_g4_pab_ops = { .link_up = ls_pcie_g4_link_up, }; +static void workaround_A011451(struct ls_pcie_g4 *pcie) +{ + struct mobiveil_pcie *mv_pci = pcie->pci; + u32 val; + + /* Set ACK latency timeout */ + val = csr_readl(mv_pci, GPEX_ACK_REPLAY_TO); + val &= ~(ACK_LAT_TO_VAL_MASK << ACK_LAT_TO_VAL_SHIFT); + val |= (4 << ACK_LAT_TO_VAL_SHIFT); + csr_writel(mv_pci, val, GPEX_ACK_REPLAY_TO); +} + static int __init ls_pcie_g4_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -259,6 +271,8 @@ static int __init ls_pcie_g4_probe(struct platform_device *pdev) if (!ls_pcie_g4_is_bridge(pcie)) return -ENODEV; + workaround_A011451(pcie); + return 0; } diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h b/drivers/pci/controller/mobiveil/pcie-mobiveil.h index ef93b41f4419..c75b7c304c46 100644 --- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h @@ -85,6 +85,10 @@ #define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR(0x0bac, win) #define PAB_INTP_AXI_PIO_CLASS 0x474 +#define GPEX_ACK_REPLAY_TO 0x438 +#define ACK_LAT_TO_VAL_MASK 0x1fff +#define ACK_LAT_TO_VAL_SHIFT 0 + #define PAB_PEX_AMAP_CTRL(win) PAB_REG_ADDR(0x4ba0, win) #define AMAP_CTRL_EN_SHIFT 0 #define AMAP_CTRL_TYPE_SHIFT 1 -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 2018-12-02 13:32 ` [PATCH 2/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 Z.q. Hou @ 2018-12-03 22:06 ` Bjorn Helgaas 0 siblings, 0 replies; 5+ messages in thread From: Bjorn Helgaas @ 2018-12-03 22:06 UTC (permalink / raw) To: Z.q. Hou Cc: linux-pci, linux-kernel, lorenzo.pieralisi, l.subrahmanya, Leo Li, M.h. Lian, Xiaowei Bao On Sun, Dec 02, 2018 at 01:32:45PM +0000, Z.q. Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > When LX2 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 LX2 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 patch implemented part 1, the part 2 can be set by kernel parameter > 'pci=pcie_bus_perf' So you're saying that users of this controller must boot with "pci=pcie_bus_perf"? That's a little unfriendly to users. When they forget to use that parameter and some mysterious PCIe error occurs, they will not thank you. We should be able to figure this out automatically via some sort of quirk in the driver, and then do the right thing in the MPS/MRRS configuration. That would also give us a chance to make sure that when the MPS/MRRS code changes, it can be done in a way that keeps this Rev1.0 controller working. If you depend on users booting with "pci=pcie_bus_perf", there's no connection in the code, and if we change or remove that parameter, we would have no clue that you depend on it. > This ERRATA is only for LX2160A Rev1.0, and it will be fixed > in Rev2.0. > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > .../pci/controller/mobiveil/pci-layerscape-gen4.c | 14 ++++++++++++++ > drivers/pci/controller/mobiveil/pcie-mobiveil.h | 4 ++++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c > index 1fe56532b288..ef43033e1c2a 100644 > --- a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c > +++ b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c > @@ -220,6 +220,18 @@ static const struct mobiveil_pab_ops ls_pcie_g4_pab_ops = { > .link_up = ls_pcie_g4_link_up, > }; > > +static void workaround_A011451(struct ls_pcie_g4 *pcie) > +{ > + struct mobiveil_pcie *mv_pci = pcie->pci; > + u32 val; > + > + /* Set ACK latency timeout */ > + val = csr_readl(mv_pci, GPEX_ACK_REPLAY_TO); > + val &= ~(ACK_LAT_TO_VAL_MASK << ACK_LAT_TO_VAL_SHIFT); > + val |= (4 << ACK_LAT_TO_VAL_SHIFT); > + csr_writel(mv_pci, val, GPEX_ACK_REPLAY_TO); > +} > + > static int __init ls_pcie_g4_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -259,6 +271,8 @@ static int __init ls_pcie_g4_probe(struct platform_device *pdev) > if (!ls_pcie_g4_is_bridge(pcie)) > return -ENODEV; > > + workaround_A011451(pcie); > + > return 0; > } > > diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h b/drivers/pci/controller/mobiveil/pcie-mobiveil.h > index ef93b41f4419..c75b7c304c46 100644 > --- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h > +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h > @@ -85,6 +85,10 @@ > #define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR(0x0bac, win) > #define PAB_INTP_AXI_PIO_CLASS 0x474 > > +#define GPEX_ACK_REPLAY_TO 0x438 > +#define ACK_LAT_TO_VAL_MASK 0x1fff > +#define ACK_LAT_TO_VAL_SHIFT 0 > + > #define PAB_PEX_AMAP_CTRL(win) PAB_REG_ADDR(0x4ba0, win) > #define AMAP_CTRL_EN_SHIFT 0 > #define AMAP_CTRL_TYPE_SHIFT 1 > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-03 22:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-02 13:32 [PATCH 0/2] PCI: ls_pcie_g4: add 2 workarounds Z.q. Hou 2018-12-02 13:32 ` [PATCH 1/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 Z.q. Hou 2018-12-03 21:06 ` Bjorn Helgaas 2018-12-02 13:32 ` [PATCH 2/2] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 Z.q. Hou 2018-12-03 22:06 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).