* [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).