* [PATCH resend] PCI: designware: fix register access before host init
@ 2016-10-18 8:25 Lucas Stach
2016-10-18 16:30 ` Jingoo Han
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Lucas Stach @ 2016-10-18 8:25 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jingoo Han, Pratyush Anand, linux-pci, Gary Bisson, kernel,
patchwork-lst
The function used to determine if iATU unroll is available will read
a RC register. This is not possible and will result in a system hang
if the RC is still in reset, so it is only safe to call this function
after host_init() has been called.
Fixes: a0601a470537 (PCI: designware: Add iATU Unroll feature)
Reported-by: Gary Bisson <gary.bisson@boundarydevices.com>
Tested-by: Gary Bisson <gary.bisson@boundarydevices.com>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/pci/host/pcie-designware.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 035f50c03281..74510508fafc 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -637,11 +637,11 @@ 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);
+ pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp);
+
pp->root_bus_nr = pp->busn->start;
if (IS_ENABLED(CONFIG_PCI_MSI)) {
bus = pci_scan_root_bus_msi(pp->dev, pp->root_bus_nr,
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH resend] PCI: designware: fix register access before host init 2016-10-18 8:25 [PATCH resend] PCI: designware: fix register access before host init Lucas Stach @ 2016-10-18 16:30 ` Jingoo Han 2016-10-18 18:36 ` Fabio Estevam 2016-10-21 15:31 ` Bjorn Helgaas 2 siblings, 0 replies; 6+ messages in thread From: Jingoo Han @ 2016-10-18 16:30 UTC (permalink / raw) To: 'Lucas Stach', 'Bjorn Helgaas' Cc: 'Pratyush Anand', linux-pci, 'Gary Bisson', kernel, patchwork-lst On Tuesday, October 18, 2016 4:26 AM, Lucas Stach wrote: > > The function used to determine if iATU unroll is available will read a RC register. > This is not possible and will result in a system hang if the RC is still in reset, > so it is only safe to call this function after host_init() has been called. > > Fixes: a0601a470537 (PCI: designware: Add iATU Unroll feature) > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> It looks good! Acked-by: Jingoo Han <jigoohan1@gmail.com> > --- > drivers/pci/host/pcie-designware.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 035f50c03281..74510508fafc 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -637,11 +637,11 @@ 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); > > + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); > + > pp->root_bus_nr = pp->busn->start; > if (IS_ENABLED(CONFIG_PCI_MSI)) { > bus = pci_scan_root_bus_msi(pp->dev, pp->root_bus_nr, > -- > 2.9.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] PCI: designware: fix register access before host init 2016-10-18 8:25 [PATCH resend] PCI: designware: fix register access before host init Lucas Stach 2016-10-18 16:30 ` Jingoo Han @ 2016-10-18 18:36 ` Fabio Estevam 2016-10-21 15:31 ` Bjorn Helgaas 2 siblings, 0 replies; 6+ messages in thread From: Fabio Estevam @ 2016-10-18 18:36 UTC (permalink / raw) To: Lucas Stach Cc: Bjorn Helgaas, Jingoo Han, Pratyush Anand, linux-pci@vger.kernel.org, Gary Bisson, Sascha Hauer, patchwork-lst On Tue, Oct 18, 2016 at 6:25 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > The function used to determine if iATU unroll is available will read > a RC register. This is not possible and will result in a system hang > if the RC is still in reset, so it is only safe to call this function > after host_init() has been called. > > Fixes: a0601a470537 (PCI: designware: Add iATU Unroll feature) > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> This fixes the hang on mx6, thanks: Tested-by: Fabio Estevam <fabio.estevam@nxp.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] PCI: designware: fix register access before host init 2016-10-18 8:25 [PATCH resend] PCI: designware: fix register access before host init Lucas Stach 2016-10-18 16:30 ` Jingoo Han 2016-10-18 18:36 ` Fabio Estevam @ 2016-10-21 15:31 ` Bjorn Helgaas 2016-10-21 16:22 ` Bjorn Helgaas 2 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2016-10-21 15:31 UTC (permalink / raw) To: Lucas Stach Cc: Bjorn Helgaas, Jingoo Han, Pratyush Anand, linux-pci, Gary Bisson, kernel, patchwork-lst On Tue, Oct 18, 2016 at 10:25:49AM +0200, Lucas Stach wrote: > The function used to determine if iATU unroll is available will read > a RC register. This is not possible and will result in a system hang > if the RC is still in reset, so it is only safe to call this function > after host_init() has been called. I'm sure Joao tested a0601a470537 somewhere, so I assume this hang only happens on certain systems? I'm wondering if this is suggesting something that's currently done differently across the drivers but could be done more similarly. > Fixes: a0601a470537 (PCI: designware: Add iATU Unroll feature) > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/pci/host/pcie-designware.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 035f50c03281..74510508fafc 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -637,11 +637,11 @@ 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); > > + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); > + > pp->root_bus_nr = pp->busn->start; > if (IS_ENABLED(CONFIG_PCI_MSI)) { > bus = pci_scan_root_bus_msi(pp->dev, pp->root_bus_nr, > -- > 2.9.3 > > -- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] PCI: designware: fix register access before host init 2016-10-21 15:31 ` Bjorn Helgaas @ 2016-10-21 16:22 ` Bjorn Helgaas 2016-10-21 16:44 ` João Pinto 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2016-10-21 16:22 UTC (permalink / raw) To: Lucas Stach Cc: Bjorn Helgaas, Jingoo Han, Pratyush Anand, linux-pci, Gary Bisson, kernel, patchwork-lst, Niklas Cassel, Carlos Palminha, jose.abreu, Jesper Nilsson, Joao Pinto [+cc Joao, Niklas, Carlos, Jose, Jesper] On Fri, Oct 21, 2016 at 10:31:42AM -0500, Bjorn Helgaas wrote: > On Tue, Oct 18, 2016 at 10:25:49AM +0200, Lucas Stach wrote: > > The function used to determine if iATU unroll is available will read > > a RC register. This is not possible and will result in a system hang > > if the RC is still in reset, so it is only safe to call this function > > after host_init() has been called. > > I'm sure Joao tested a0601a470537 somewhere, so I assume this hang only > happens on certain systems? > > I'm wondering if this is suggesting something that's currently done > differently across the drivers but could be done more similarly. > > > Fixes: a0601a470537 (PCI: designware: Add iATU Unroll feature) > > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > > Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > drivers/pci/host/pcie-designware.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > > index 035f50c03281..74510508fafc 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -637,11 +637,11 @@ 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); > > > > + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); > > + Niklas suggested this exact change earlier [1], but Joao pointed out the problem with calling dw_pcie_iatu_unroll_enabled() after calling pp->ops->host_init(): dw_pcie_host_init pp->ops->host_init dw_pcie_setup_rc dw_pcie_prog_outbound_atu if (pp->iatu_unroll_enabled) ... pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled() We check iatu_unroll_enabled in dw_pcie_prog_outbound_atu() before it has been set. Niklas' second patch [2] looks like a better fix. Can you guys take a look and see if you agree? [1] http://lkml.kernel.org/r/b53d5807-8199-5789-6184-e37324a1d79e@axis.com [2] http://lkml.kernel.org/r/1476482095-26289-1-git-send-email-niklass@axis.com > > pp->root_bus_nr = pp->busn->start; > > if (IS_ENABLED(CONFIG_PCI_MSI)) { > > bus = pci_scan_root_bus_msi(pp->dev, pp->root_bus_nr, > > -- > > 2.9.3 > > > > -- > > 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 > -- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] PCI: designware: fix register access before host init 2016-10-21 16:22 ` Bjorn Helgaas @ 2016-10-21 16:44 ` João Pinto 0 siblings, 0 replies; 6+ messages in thread From: João Pinto @ 2016-10-21 16:44 UTC (permalink / raw) To: Bjorn Helgaas Cc: Lucas Stach, Bjorn Helgaas, Jingoo Han, Pratyush Anand, linux-pci, Gary Bisson, kernel, patchwork-lst, Niklas Cassel, Carlos Palminha, jose.abreu, Jesper Nilsson Hi, Sorry for replying directly from gmail, I don't have the Linux environment ready yet :) Niklas patch is the one that should be applied, because in the host_init we need to know if the iATU is in Unroll or in Legacy mode. Thanks. 2016-10-21 17:22 GMT+01:00 Bjorn Helgaas <helgaas@kernel.org>: > [+cc Joao, Niklas, Carlos, Jose, Jesper] > > On Fri, Oct 21, 2016 at 10:31:42AM -0500, Bjorn Helgaas wrote: >> On Tue, Oct 18, 2016 at 10:25:49AM +0200, Lucas Stach wrote: >> > The function used to determine if iATU unroll is available will read >> > a RC register. This is not possible and will result in a system hang >> > if the RC is still in reset, so it is only safe to call this function >> > after host_init() has been called. >> >> I'm sure Joao tested a0601a470537 somewhere, so I assume this hang only >> happens on certain systems? >> >> I'm wondering if this is suggesting something that's currently done >> differently across the drivers but could be done more similarly. >> >> > Fixes: a0601a470537 (PCI: designware: Add iATU Unroll feature) >> > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> >> > Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> >> > --- >> > drivers/pci/host/pcie-designware.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c >> > index 035f50c03281..74510508fafc 100644 >> > --- a/drivers/pci/host/pcie-designware.c >> > +++ b/drivers/pci/host/pcie-designware.c >> > @@ -637,11 +637,11 @@ 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); >> > >> > + pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp); >> > + > > Niklas suggested this exact change earlier [1], but Joao pointed out the > problem with calling dw_pcie_iatu_unroll_enabled() after calling > pp->ops->host_init(): > > dw_pcie_host_init > pp->ops->host_init > dw_pcie_setup_rc > dw_pcie_prog_outbound_atu > if (pp->iatu_unroll_enabled) > ... > pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled() > > We check iatu_unroll_enabled in dw_pcie_prog_outbound_atu() before it > has been set. > > Niklas' second patch [2] looks like a better fix. Can you guys take a > look and see if you agree? > > [1] http://lkml.kernel.org/r/b53d5807-8199-5789-6184-e37324a1d79e@axis.com > [2] http://lkml.kernel.org/r/1476482095-26289-1-git-send-email-niklass@axis.com > >> > pp->root_bus_nr = pp->busn->start; >> > if (IS_ENABLED(CONFIG_PCI_MSI)) { >> > bus = pci_scan_root_bus_msi(pp->dev, pp->root_bus_nr, >> > -- >> > 2.9.3 >> > >> > -- >> > 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 >> -- >> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-21 16:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-18 8:25 [PATCH resend] PCI: designware: fix register access before host init Lucas Stach 2016-10-18 16:30 ` Jingoo Han 2016-10-18 18:36 ` Fabio Estevam 2016-10-21 15:31 ` Bjorn Helgaas 2016-10-21 16:22 ` Bjorn Helgaas 2016-10-21 16:44 ` João Pinto
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).