| Message ID | 1483558350-8169-1-git-send-email-m-karicheri2@ti.com |
|---|---|
| State | New |
| Headers | show |
On 01/04/2017 02:32 PM, Murali Karicheri wrote: > Recent fixes for iATU unroll support introduced a bug that causes > asynchronous external abort in Keystone PCIe h/w which doesn't have > ATU port and the corresponding register. So the check should be moved > below where dw_pcie_prog_outbound_atu() is called to avoid that > being called on keystine PCIe h/w. > > Here is the backtrace > > [ 0.771174] OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 > [ 0.778118] Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 > [ 0.785548] pgd = c0003000 > [ 0.788347] [00000000] *pgd=80000800004003, *pmd=00000000 > [ 0.793864] Internal error: : 1211 [#1] PREEMPT SMP ARM > [ 0.799197] Modules linked in: > [ 0.802351] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 > [ 0.810130] Hardware name: Keystone > [ 0.813717] task: eb878000 task.stack: eb866000 > [ 0.818356] PC is at dw_pcie_setup_rc+0x24/0x380 > [ 0.823083] LR is at ks_pcie_host_init+0x10/0x170 > > Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > Tested-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > - Applies to pci/master > - Bug was introduced in v4.9 > - Tested on K2E EVM with SATA and DRA7-EVM with an intel PCI ethernet card > > drivers/pci/host/pcie-designware.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index bed1999..af8f6e9 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -807,11 +807,6 @@ 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; > @@ -882,6 +877,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > * we should not program the ATU here. > */ > if (!pp->ops->rd_other_conf) { > + /* 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"); > + > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > PCIE_ATU_TYPE_MEM, pp->mem_base, > pp->mem_bus_addr, pp->mem_size); > Bjorn, A Gentle ping to merge this important fix to v4.10-rc kernel for Keystone PCI as it is broken since v4.9. Thanks -- Murali Karicheri Linux Kernel, Keystone
Hi, Às 4:41 PM de 1/9/2017, Murali Karicheri escreveu: > On 01/04/2017 02:32 PM, Murali Karicheri wrote: >> Recent fixes for iATU unroll support introduced a bug that causes >> asynchronous external abort in Keystone PCIe h/w which doesn't have >> ATU port and the corresponding register. So the check should be moved >> below where dw_pcie_prog_outbound_atu() is called to avoid that >> being called on keystine PCIe h/w. >> >> Here is the backtrace >> >> [ 0.771174] OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 >> [ 0.778118] Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 >> [ 0.785548] pgd = c0003000 >> [ 0.788347] [00000000] *pgd=80000800004003, *pmd=00000000 >> [ 0.793864] Internal error: : 1211 [#1] PREEMPT SMP ARM >> [ 0.799197] Modules linked in: >> [ 0.802351] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 >> [ 0.810130] Hardware name: Keystone >> [ 0.813717] task: eb878000 task.stack: eb866000 >> [ 0.818356] PC is at dw_pcie_setup_rc+0x24/0x380 >> [ 0.823083] LR is at ks_pcie_host_init+0x10/0x170 >> >> Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> Tested-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> - Applies to pci/master >> - Bug was introduced in v4.9 >> - Tested on K2E EVM with SATA and DRA7-EVM with an intel PCI ethernet card >> >> drivers/pci/host/pcie-designware.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c >> index bed1999..af8f6e9 100644 >> --- a/drivers/pci/host/pcie-designware.c >> +++ b/drivers/pci/host/pcie-designware.c >> @@ -807,11 +807,6 @@ 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; >> @@ -882,6 +877,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) >> * we should not program the ATU here. >> */ >> if (!pp->ops->rd_other_conf) { >> + /* 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"); >> + >> dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, >> PCIE_ATU_TYPE_MEM, pp->mem_base, >> pp->mem_bus_addr, pp->mem_size); >> > Bjorn, > > A Gentle ping to merge this important fix to v4.10-rc kernel for Keystone PCI as > it is broken since v4.9. > In my understanding your proposal will have no impact in platform using ATU, so for me is fine. Acked-By: Joao Pinto <jpinto@synopsys.com> > Thanks >
Hi Murali, On Wed, Jan 04, 2017 at 02:32:30PM -0500, Murali Karicheri wrote: > Recent fixes for iATU unroll support introduced a bug that causes > asynchronous external abort in Keystone PCIe h/w which doesn't have > ATU port and the corresponding register. So the check should be moved > below where dw_pcie_prog_outbound_atu() is called to avoid that > being called on keystine PCIe h/w. > > Here is the backtrace > > [ 0.771174] OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 > [ 0.778118] Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 > [ 0.785548] pgd = c0003000 > [ 0.788347] [00000000] *pgd=80000800004003, *pmd=00000000 > [ 0.793864] Internal error: : 1211 [#1] PREEMPT SMP ARM > [ 0.799197] Modules linked in: > [ 0.802351] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 > [ 0.810130] Hardware name: Keystone > [ 0.813717] task: eb878000 task.stack: eb866000 > [ 0.818356] PC is at dw_pcie_setup_rc+0x24/0x380 > [ 0.823083] LR is at ks_pcie_host_init+0x10/0x170 > > Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") I tentatively applied this to for-linus for v4.10, with a stable tag for v4.9+. The patch itself mostly makes sense in terms of the code: we only use iatu_unroll_enabled in dw_pcie_prog_outbound_atu(). We only call that when "!pp->ops->rd_other_conf" or "!pp->ops->wr_other_conf". So it makes sense that we only need to initialize iatu_unroll_enabled in those cases. But the current patch only initializes it if "!pp->ops->rd_other_conf". If we had "pp->ops->rd_other_conf && !pp->ops->wr_other_conf", we would use iatu_unroll_enabled uninitialized in the dw_pcie_wr_other_conf() path. I suppose that's an invalid configuration, but it'd be better if we didn't have to rely on the host drivers to avoid that configuration. It's not obvious how this is connected to 416379f9ebde, though. It *looks* like we call dw_pcie_iatu_unroll_enabled() on Keystone both before and after that commit, so it seems like the external abort should have happened even before it. > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > Tested-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > - Applies to pci/master > - Bug was introduced in v4.9 > - Tested on K2E EVM with SATA and DRA7-EVM with an intel PCI ethernet card > > drivers/pci/host/pcie-designware.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index bed1999..af8f6e9 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -807,11 +807,6 @@ 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; > @@ -882,6 +877,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > * we should not program the ATU here. > */ > if (!pp->ops->rd_other_conf) { > + /* 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"); > + > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > PCIE_ATU_TYPE_MEM, pp->mem_base, > pp->mem_bus_addr, pp->mem_size); > -- > 1.9.1 > > -- > 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
On 01/10/2017 10:12 AM, Bjorn Helgaas wrote: > Hi Murali, > > On Wed, Jan 04, 2017 at 02:32:30PM -0500, Murali Karicheri wrote: >> Recent fixes for iATU unroll support introduced a bug that causes >> asynchronous external abort in Keystone PCIe h/w which doesn't have >> ATU port and the corresponding register. So the check should be moved >> below where dw_pcie_prog_outbound_atu() is called to avoid that >> being called on keystine PCIe h/w. >> >> Here is the backtrace >> >> [ 0.771174] OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 >> [ 0.778118] Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 >> [ 0.785548] pgd = c0003000 >> [ 0.788347] [00000000] *pgd=80000800004003, *pmd=00000000 >> [ 0.793864] Internal error: : 1211 [#1] PREEMPT SMP ARM >> [ 0.799197] Modules linked in: >> [ 0.802351] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 >> [ 0.810130] Hardware name: Keystone >> [ 0.813717] task: eb878000 task.stack: eb866000 >> [ 0.818356] PC is at dw_pcie_setup_rc+0x24/0x380 >> [ 0.823083] LR is at ks_pcie_host_init+0x10/0x170 >> >> Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") > > I tentatively applied this to for-linus for v4.10, with a stable tag > for v4.9+. > Ok Thanks! > The patch itself mostly makes sense in terms of the code: we only use > iatu_unroll_enabled in dw_pcie_prog_outbound_atu(). We only call that > when "!pp->ops->rd_other_conf" or "!pp->ops->wr_other_conf". > > So it makes sense that we only need to initialize iatu_unroll_enabled > in those cases. But the current patch only initializes it if > "!pp->ops->rd_other_conf". > I think the code before also should have checked for both if ((!pp->ops->rd_other_conf) && (!pp->ops->wr_other_conf)) The assumption was if rd_other_conf is Null, the platform provides both rd_other_conf and wr_other_conf. I see I have added this API to support Keystone and the above assumption is true so far. It make sense to fix it if you agree. > If we had "pp->ops->rd_other_conf && !pp->ops->wr_other_conf", we > would use iatu_unroll_enabled uninitialized in the > dw_pcie_wr_other_conf() path. I suppose that's an invalid Yes. So we need to fix the above to make the code correct instead of leaving it exposed. > configuration, but it'd be better if we didn't have to rely on the > host drivers to avoid that configuration. You mean to introduce the check in the designware core code above right? Murali > > It's not obvious how this is connected to 416379f9ebde, though. It > *looks* like we call dw_pcie_iatu_unroll_enabled() on Keystone both > before and after that commit, so it seems like the external abort > should have happened even before it. > You are right. The problem was introduced by commit a0601a47053714eecec726aea5ebcd829f817497 Author: Joao Pinto <Joao.Pinto@synopsys.com> Date: Wed Aug 10 11:02:39 2016 +0100 PCI: designware: Add iATU Unroll feature which is fixed by commit 416379f9ebde and is again fixed by my commit. So probably I should have added both commits in my description. Murali >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> Tested-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> - Applies to pci/master >> - Bug was introduced in v4.9 >> - Tested on K2E EVM with SATA and DRA7-EVM with an intel PCI ethernet card >> >> drivers/pci/host/pcie-designware.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c >> index bed1999..af8f6e9 100644 >> --- a/drivers/pci/host/pcie-designware.c >> +++ b/drivers/pci/host/pcie-designware.c >> @@ -807,11 +807,6 @@ 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; >> @@ -882,6 +877,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) >> * we should not program the ATU here. >> */ >> if (!pp->ops->rd_other_conf) { >> + /* 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"); >> + >> dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, >> PCIE_ATU_TYPE_MEM, pp->mem_base, >> pp->mem_bus_addr, pp->mem_size); >> -- >> 1.9.1 >> >> -- >> 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 -- Murali Karicheri Linux Kernel, Keystone
On 01/10/2017 06:06 PM, Bjorn Helgaas wrote: > On Tue, Jan 10, 2017 at 12:18:29PM -0500, Murali Karicheri wrote: >> On 01/10/2017 10:12 AM, Bjorn Helgaas wrote: >>> Hi Murali, >>> >>> On Wed, Jan 04, 2017 at 02:32:30PM -0500, Murali Karicheri wrote: >>>> Recent fixes for iATU unroll support introduced a bug that causes >>>> asynchronous external abort in Keystone PCIe h/w which doesn't have >>>> ATU port and the corresponding register. So the check should be moved >>>> below where dw_pcie_prog_outbound_atu() is called to avoid that >>>> being called on keystine PCIe h/w. >>>> >>>> Here is the backtrace >>>> >>>> [ 0.771174] OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 >>>> [ 0.778118] Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 >>>> [ 0.785548] pgd = c0003000 >>>> [ 0.788347] [00000000] *pgd=80000800004003, *pmd=00000000 >>>> [ 0.793864] Internal error: : 1211 [#1] PREEMPT SMP ARM >>>> [ 0.799197] Modules linked in: >>>> [ 0.802351] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 >>>> [ 0.810130] Hardware name: Keystone >>>> [ 0.813717] task: eb878000 task.stack: eb866000 >>>> [ 0.818356] PC is at dw_pcie_setup_rc+0x24/0x380 >>>> [ 0.823083] LR is at ks_pcie_host_init+0x10/0x170 >>>> >>>> Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") >>> >>> I tentatively applied this to for-linus for v4.10, with a stable tag >>> for v4.9+. >>> >> >> Ok Thanks! >> >>> The patch itself mostly makes sense in terms of the code: we only use >>> iatu_unroll_enabled in dw_pcie_prog_outbound_atu(). We only call that >>> when "!pp->ops->rd_other_conf" or "!pp->ops->wr_other_conf". >>> >>> So it makes sense that we only need to initialize iatu_unroll_enabled >>> in those cases. But the current patch only initializes it if >>> "!pp->ops->rd_other_conf". >>> >> >> I think the code before also should have checked for both >> if ((!pp->ops->rd_other_conf) && (!pp->ops->wr_other_conf)) >> >> The assumption was if rd_other_conf is Null, the platform provides >> both rd_other_conf and wr_other_conf. I see I have added this API >> to support Keystone and the above assumption is true so far. >> It make sense to fix it if you agree. >> >>> If we had "pp->ops->rd_other_conf && !pp->ops->wr_other_conf", we >>> would use iatu_unroll_enabled uninitialized in the >>> dw_pcie_wr_other_conf() path. I suppose that's an invalid >> >> Yes. So we need to fix the above to make the code correct instead >> of leaving it exposed. >> >>> configuration, but it'd be better if we didn't have to rely on the >>> host drivers to avoid that configuration. >> >> You mean to introduce the check in the designware core code above >> right? > > We *could* make the code look like: > > if (!pp->ops->rd_other_conf || !pp->ops->rd_other_conf) > pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); > > but that seems a little clunky and the connection between ATU and the > ->*_other_conf() pointers is slightly obscure. I guess it just ends > up being more detailed than I personally really want to worry about, > so I'm fine with leaving it as-is for now. > >>> It's not obvious how this is connected to 416379f9ebde, though. It >>> *looks* like we call dw_pcie_iatu_unroll_enabled() on Keystone both >>> before and after that commit, so it seems like the external abort >>> should have happened even before it. >>> >> >> You are right. The problem was introduced by >> >> commit a0601a47053714eecec726aea5ebcd829f817497 >> Author: Joao Pinto <Joao.Pinto@synopsys.com> >> Date: Wed Aug 10 11:02:39 2016 +0100 >> >> PCI: designware: Add iATU Unroll feature >> >> which is fixed by commit 416379f9ebde and is again fixed by my >> commit. So probably I should have added both commits in >> my description. > > I added that. The reason I wanted the correct "Fixes" information is > to figure out which stable kernels need this fix. In this case, both: > > a0601a470537 ("PCI: designware: Add iATU Unroll feature") and > 416379f9ebde ("PCI: designware: Check for iATU unroll support after > initializing host") > > appeared in v4.9, so it doesn't change anything as far as the stable > tag is concerned. > > The below is what I have on my for-linus branch: > > commit 6fdb996a55684016de1ce639b9316b7092fde95f > Author: Murali Karicheri <m-karicheri2@ti.com> > Date: Wed Jan 4 14:32:30 2017 -0500 > > PCI: designware: Check for iATU unroll only on platforms that use ATU > > Previously we checked for iATU unroll support by reading PCIE_ATU_VIEWPORT > even on platforms, e.g., Keystone, that do not have ATU ports. This can > cause bad behavior such as asynchronous external aborts: > > OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 > Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 > pgd = c0003000 > [00000000] *pgd=80000800004003, *pmd=00000000 > Internal error: : 1211 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 > Hardware name: Keystone > task: eb878000 task.stack: eb866000 > PC is at dw_pcie_setup_rc+0x24/0x380 > LR is at ks_pcie_host_init+0x10/0x170 > > Move the dw_pcie_iatu_unroll_enabled() check so we only call it on > platforms that do not use the ATU. These platforms supply their own > ->rd_other_conf() and ->wr_other_conf() methods. > > [bhelgaas: changelog] > Fixes: a0601a470537 ("PCI: designware: Add iATU Unroll feature") > Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") > Tested-by: Kishon Vijay Abraham I <kishon@ti.com> > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > CC: stable@vger.kernel.org # v4.9+ > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index bed19994c1e9..af8f6e92e885 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -807,11 +807,6 @@ 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; > @@ -882,6 +877,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > * we should not program the ATU here. > */ > if (!pp->ops->rd_other_conf) { > + /* 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"); > + > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > PCIE_ATU_TYPE_MEM, pp->mem_base, > pp->mem_bus_addr, pp->mem_size); > Thanks Bjorn! Looks good. -- Murali Karicheri Linux Kernel, Keystone
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index bed1999..af8f6e9 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -807,11 +807,6 @@ 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; @@ -882,6 +877,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp) * we should not program the ATU here. */ if (!pp->ops->rd_other_conf) { + /* 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"); + dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, PCIE_ATU_TYPE_MEM, pp->mem_base, pp->mem_bus_addr, pp->mem_size);
Recent fixes for iATU unroll support introduced a bug that causes asynchronous external abort in Keystone PCIe h/w which doesn't have ATU port and the corresponding register. So the check should be moved below where dw_pcie_prog_outbound_atu() is called to avoid that being called on keystine PCIe h/w. Here is the backtrace [ 0.771174] OF: PCI: MEM 0x60000000..0x6fffffff -> 0x60000000 [ 0.778118] Unhandled fault: asynchronous external abort (0x1211) at 0x00000000 [ 0.785548] pgd = c0003000 [ 0.788347] [00000000] *pgd=80000800004003, *pmd=00000000 [ 0.793864] Internal error: : 1211 [#1] PREEMPT SMP ARM [ 0.799197] Modules linked in: [ 0.802351] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-00009-g6ff59d2-dirty #7 [ 0.810130] Hardware name: Keystone [ 0.813717] task: eb878000 task.stack: eb866000 [ 0.818356] PC is at dw_pcie_setup_rc+0x24/0x380 [ 0.823083] LR is at ks_pcie_host_init+0x10/0x170 Fixes: 416379f9ebde ("PCI: designware: Check for iATU unroll support after initializing host") Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> Tested-by: Kishon Vijay Abraham I <kishon@ti.com> --- - Applies to pci/master - Bug was introduced in v4.9 - Tested on K2E EVM with SATA and DRA7-EVM with an intel PCI ethernet card drivers/pci/host/pcie-designware.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 1.9.1