| Message ID | b3eb2df9-7675-ea52-a3b7-d2dfc4963c2d@axis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 10/14/2016 4:24 PM, Niklas Cassel wrote: > On 10/14/2016 03:02 PM, Joao Pinto wrote: >> Hi Niklas, >> >> >> On 10/14/2016 1:41 PM, Niklas Cassel wrote: >>> Hello >>> (snip) > } > } > > - pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); > - > if (pp->ops->host_init) > pp->ops->host_init(pp); > > @@ -809,6 +807,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > { > u32 val; > > + /* get iATU unroll support */ > + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); > + dev_dbg(pp->dev, "iATU unroll: %s\n", > + pp->iatu_unroll_enabled ? "enabled" : "disabled"); > + > /* set the number of lanes */ > val = dw_pcie_readl_rc(pp, PCIE_PORT_LINK_CONTROL); > val &= ~PORT_LINK_MODE_MASK; > Seems reasonable to me. Please make the an official patch and I get it tested for you. Thanks. > > > With my patch I get: > > [ 0.976044] OF: PCI: host bridge /pcie@f8050000 ranges: > [ 0.981307] OF: PCI: IO 0xc0002000..0xc0011fff -> 0x00000000 > [ 0.987240] OF: PCI: MEM 0xc0012000..0xdfffffff -> 0xc0012000 > [ 1.010590] artpec6-pcie f8050000.pcie: iATU unroll: disabled > [ 1.116381] artpec6-pcie f8050000.pcie: link up > [ 1.121044] artpec6-pcie f8050000.pcie: PCI host bridge to bus 0000:00 > > and no SIGBUS/imprecise external abort. > > > > The only users of dw_pcie_prog_outbound_atu is > dw_pcie_rd_conf, dw_pcie_wr_conf and dw_pcie_setup_rc. > > As long as dw_pcie_setup_rc calls dw_pcie_iatu_unroll_enabled > before calling dw_pcie_prog_outbound_atu, > we should be fine (as done in my patch). > > dw_pcie_rd_conf and dw_pcie_wr_conf is only used by > struct pci_ops dw_pcie_ops, which is only used as an argument > for pci_scan_root_bus_msi and pci_scan_root_bus > (both are called after pp->ops->host_init, i.e., > after dw_pcie_setup_rc). (My patch should be fine for > this code path too.) > > > The only other solution would be to break out some code > from artpec6_pcie_establish_link and move that to > artpec6_pcie_probe. > But in that case I would highly recommend that all other > dwc-based drivers verify that they are still working after > a0601a470537 ("PCI: designware: Add iATU Unroll feature"), > since they might also first enable their PCI Express interface > module in pp->ops->host_init(). > > >>> From the ARTPEC-6 SoC manual: >>> By default, the PCI Express interface shall be held in reset and clock-gated. >>> Software is required to enable the relevant modules >>> (turns on clocks and de-asserts reset) before these modules can be used. >>> >>> Turning on the clocks and de-asserting reset is done in pp->ops->host_init(). >>> We get an external abort when calling dw_pcie_iatu_unroll_enabled, >>> since that function does a read from the IP before we are allowed to do >>> AXI transfers (at least in the ARTPEC-6 case, might be the same for some >>> other SoCs). >>> >>> It appears that dw_pcie_iatu_unroll_enabled was actually called _before_ >>> host_init() in v4 of Joao's patch, but was changed to after host_init() in v5, >>> unfortunately the patch doesn't state a reason for the move. >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
FYI: I added this report to the list of regressions for Linux 4.9. I'll watch this thread for further updates on this issue to document progress in my weekly reports. Please let me know via regressions@leemhuis.info in case the discussion moves to a different place (bugzilla or another mail thread for example). tia! Current status (afaics): Patch available (https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1249922.html ) Ciao, Thorsten On 14.10.2016 18:11, Joao Pinto wrote: > On 10/14/2016 4:24 PM, Niklas Cassel wrote: >> On 10/14/2016 03:02 PM, Joao Pinto wrote: >>> Hi Niklas, >>> >>> >>> On 10/14/2016 1:41 PM, Niklas Cassel wrote: >>>> Hello >>>> > > (snip) > >> } >> } >> >> - pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); >> - >> if (pp->ops->host_init) >> pp->ops->host_init(pp); >> >> @@ -809,6 +807,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) >> { >> u32 val; >> >> + /* get iATU unroll support */ >> + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); >> + dev_dbg(pp->dev, "iATU unroll: %s\n", >> + pp->iatu_unroll_enabled ? "enabled" : "disabled"); >> + >> /* set the number of lanes */ >> val = dw_pcie_readl_rc(pp, PCIE_PORT_LINK_CONTROL); >> val &= ~PORT_LINK_MODE_MASK; >> > > Seems reasonable to me. Please make the an official patch and I get it tested > for you. > > Thanks. > >> >> >> With my patch I get: >> >> [ 0.976044] OF: PCI: host bridge /pcie@f8050000 ranges: >> [ 0.981307] OF: PCI: IO 0xc0002000..0xc0011fff -> 0x00000000 >> [ 0.987240] OF: PCI: MEM 0xc0012000..0xdfffffff -> 0xc0012000 >> [ 1.010590] artpec6-pcie f8050000.pcie: iATU unroll: disabled >> [ 1.116381] artpec6-pcie f8050000.pcie: link up >> [ 1.121044] artpec6-pcie f8050000.pcie: PCI host bridge to bus 0000:00 >> >> and no SIGBUS/imprecise external abort. >> >> >> >> The only users of dw_pcie_prog_outbound_atu is >> dw_pcie_rd_conf, dw_pcie_wr_conf and dw_pcie_setup_rc. >> >> As long as dw_pcie_setup_rc calls dw_pcie_iatu_unroll_enabled >> before calling dw_pcie_prog_outbound_atu, >> we should be fine (as done in my patch). >> >> dw_pcie_rd_conf and dw_pcie_wr_conf is only used by >> struct pci_ops dw_pcie_ops, which is only used as an argument >> for pci_scan_root_bus_msi and pci_scan_root_bus >> (both are called after pp->ops->host_init, i.e., >> after dw_pcie_setup_rc). (My patch should be fine for >> this code path too.) >> >> >> The only other solution would be to break out some code >> from artpec6_pcie_establish_link and move that to >> artpec6_pcie_probe. >> But in that case I would highly recommend that all other >> dwc-based drivers verify that they are still working after >> a0601a470537 ("PCI: designware: Add iATU Unroll feature"), >> since they might also first enable their PCI Express interface >> module in pp->ops->host_init(). >> >> >>>> From the ARTPEC-6 SoC manual: >>>> By default, the PCI Express interface shall be held in reset and clock-gated. >>>> Software is required to enable the relevant modules >>>> (turns on clocks and de-asserts reset) before these modules can be used. >>>> >>>> Turning on the clocks and de-asserting reset is done in pp->ops->host_init(). >>>> We get an external abort when calling dw_pcie_iatu_unroll_enabled, >>>> since that function does a read from the IP before we are allowed to do >>>> AXI transfers (at least in the ARTPEC-6 case, might be the same for some >>>> other SoCs). >>>> >>>> It appears that dw_pcie_iatu_unroll_enabled was actually called _before_ >>>> host_init() in v4 of Joao's patch, but was changed to after host_init() in v5, >>>> unfortunately the patch doesn't state a reason for the move. >>>> >> > > > http://news.gmane.org/find-root.php?message_id=99a01d19-2b5e-19e4-7e73-286acf1684c4%40synopsys.com > http://mid.gmane.org/99a01d19-2b5e-19e4-7e73-286acf1684c4%40synopsys.com > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 035f50c03281..09eca2c5601d 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -637,8 +637,6 @@ int dw_pcie_host_init(struct pcie_port *pp) } } - pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); - if (pp->ops->host_init) pp->ops->host_init(pp); @@ -809,6 +807,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) { u32 val; + /* get iATU unroll support */ + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); + dev_dbg(pp->dev, "iATU unroll: %s\n", + pp->iatu_unroll_enabled ? "enabled" : "disabled"); + /* set the number of lanes */ val = dw_pcie_readl_rc(pp, PCIE_PORT_LINK_CONTROL); val &= ~PORT_LINK_MODE_MASK;