diff mbox

[Discussion] : ARM: PCIE: Setup bridges not happening with portbus driver enabled

Message ID C79B248886DD134989C8FF6B096A91AB92139701C7@BGMAIL01.nvidia.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Jay Agarwal Dec. 5, 2013, 5:44 a.m. UTC
Hi All,

I am seeing below issue on an ARM platform with CONFIG_PCIEPORTBUS enabled in kernel

ISSUE:  Any memory access by devices fails

FINDINGS:

1. No bridge windows like below are setup and probably this is not allowing any memory access by devices
[    1.280324] pci 0000:00:00.0:   bridge window [mem 0xXXX00000-0xXXXfffff]
[    1.280350] pci 0000:00:00.0:   bridge window [mem 0xXXX00000-0xXXXfffff pref]

2. On debugging, I see pci_enable_bridge(setup-bus.c) is not called because pci_is_enabled returns true. This is because pci_enable_device is already called for this bridge by portbus driver in pcie_port_device_register(portdrv_core.c).
3. Below patch is resolving this issue without any visible side effects. In fact, I verified AER(which needs PORTBUS) also with this.


3. This patch works without PORTBUS enabled also.

QUESTIONS:
1. Does anybody see any problem with this patch? Any reason for pci_is_enabled check above?

With best,
Jay
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
--
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

Comments

Pratyush ANAND Dec. 5, 2013, 6:48 a.m. UTC | #1
On Thu, Dec 05, 2013 at 01:44:53PM +0800, Jay Agarwal wrote:
> Hi All,
> 
> I am seeing below issue on an ARM platform with CONFIG_PCIEPORTBUS enabled in kernel
> 
> ISSUE:  Any memory access by devices fails
> 
> FINDINGS:
> 
> 1. No bridge windows like below are setup and probably this is not allowing any memory access by devices

Does your RC driver calls pci_assign_unassigned_resources after
pci_common_init?

Regards
Pratyush

> [    1.280324] pci 0000:00:00.0:   bridge window [mem 0xXXX00000-0xXXXfffff]
> [    1.280350] pci 0000:00:00.0:   bridge window [mem 0xXXX00000-0xXXXfffff pref]
> 
> 2. On debugging, I see pci_enable_bridge(setup-bus.c) is not called because pci_is_enabled returns true. This is because pci_enable_device is already called for this bridge by portbus driver in pcie_port_device_register(portdrv_core.c).
> 3. Below patch is resolving this issue without any visible side effects. In fact, I verified AER(which needs PORTBUS) also with this.
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 64a7de2..9b67fff 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1200,8 +1200,7 @@ void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
>  
>                 switch (dev->class >> 8) {
>                 case PCI_CLASS_BRIDGE_PCI:
> -                       if (!pci_is_enabled(dev))
> -                               pci_setup_bridge(b);
> +                       pci_setup_bridge(b);
>                         break;
>  
>                 case PCI_CLASS_BRIDGE_CARDBUS:
> 
> 3. This patch works without PORTBUS enabled also.
> 
> QUESTIONS:
> 1. Does anybody see any problem with this patch? Any reason for pci_is_enabled check above?
> 
> With best,
> Jay
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> --
> 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
Jay Agarwal Dec. 5, 2013, 7:38 a.m. UTC | #2
> On Thu, Dec 05, 2013 at 01:44:53PM +0800, Jay Agarwal wrote:
> > Hi All,
> >
> > I am seeing below issue on an ARM platform with CONFIG_PCIEPORTBUS
> > enabled in kernel
> >
> > ISSUE:  Any memory access by devices fails
> >
> > FINDINGS:
> >
> > 1. No bridge windows like below are setup and probably this is not
> > allowing any memory access by devices
> 
> Does your RC driver calls pci_assign_unassigned_resources after
> pci_common_init?
> 
> Regards
> Pratyush
> 
No it does not. Btw, I tried calling it but it also did not help and same problem. 
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
--
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
Yinghai Lu Dec. 5, 2013, 8:27 a.m. UTC | #3
On Wed, Dec 4, 2013 at 11:38 PM, Jay Agarwal <jagarwal@nvidia.com> wrote:
>> On Thu, Dec 05, 2013 at 01:44:53PM +0800, Jay Agarwal wrote:
>> > I am seeing below issue on an ARM platform with CONFIG_PCIEPORTBUS
>> > enabled in kernel
>> >
>> > ISSUE:  Any memory access by devices fails
>> >
>> > FINDINGS:
>> >
>> > 1. No bridge windows like below are setup and probably this is not
>> > allowing any memory access by devices
>>
>> Does your RC driver calls pci_assign_unassigned_resources after
>> pci_common_init?
>>
> No it does not. Btw, I tried calling it but it also did not help and same problem.

Please make sure you pci_assign_unassigned_resources get called via
fs_initcall().

Thanks

Yinghai
--
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
Jay Agarwal Dec. 5, 2013, 8:41 a.m. UTC | #4
> >> Does your RC driver calls pci_assign_unassigned_resources after
> >> pci_common_init?
> >>
> > No it does not. Btw, I tried calling it but it also did not help and same
> problem.
> 
> Please make sure you pci_assign_unassigned_resources get called via
> fs_initcall().
> 
Calling fs_initcall(pci_assign_unassigned_resources)  just after pci_common_init also did not help.
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
--
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
Thierry Reding Dec. 5, 2013, 9:12 a.m. UTC | #5
On Thu, Dec 05, 2013 at 12:27:40AM -0800, Yinghai Lu wrote:
> On Wed, Dec 4, 2013 at 11:38 PM, Jay Agarwal <jagarwal@nvidia.com> wrote:
> >> On Thu, Dec 05, 2013 at 01:44:53PM +0800, Jay Agarwal wrote:
> >> > I am seeing below issue on an ARM platform with CONFIG_PCIEPORTBUS
> >> > enabled in kernel
> >> >
> >> > ISSUE:  Any memory access by devices fails
> >> >
> >> > FINDINGS:
> >> >
> >> > 1. No bridge windows like below are setup and probably this is not
> >> > allowing any memory access by devices
> >>
> >> Does your RC driver calls pci_assign_unassigned_resources after
> >> pci_common_init?
> >>
> > No it does not. Btw, I tried calling it but it also did not help and same problem.
> 
> Please make sure you pci_assign_unassigned_resources get called via
> fs_initcall().

Why does this have to be called as fs_initcall() time? Does it have to
be exactly then or can it be at any later time?

The Tegra PCIe driver is actually a regular driver, and run at the time
of device_initcall(). In fact it may even be probed much later because
it can depend on regulators and such that only become available later
and therefore cause the PCIe driver to defer it's probe.

Thierry
Bjorn Helgaas Dec. 5, 2013, 6:54 p.m. UTC | #6
On Wed, Dec 4, 2013 at 10:44 PM, Jay Agarwal <jagarwal@nvidia.com> wrote:
> Hi All,
>
> I am seeing below issue on an ARM platform with CONFIG_PCIEPORTBUS enabled in kernel
>
> ISSUE:  Any memory access by devices fails

"Memory access by device fails" sounds like DMA doesn't work.  Is that
what you mean?

> FINDINGS:
>
> 1. No bridge windows like below are setup and probably this is not allowing any memory access by devices
> [    1.280324] pci 0000:00:00.0:   bridge window [mem 0xXXX00000-0xXXXfffff]
> [    1.280350] pci 0000:00:00.0:   bridge window [mem 0xXXX00000-0xXXXfffff pref]

These bridge windows are used for MMIO, i.e., accesses performed by
the CPU, routed toward the device.  CPU accesses to the device will
use addresses inside a window, and the bridge will forward those
accesses from the primary (CPU) side to the secondary (device) side.
Device accesses, i.e., DMA, will use addresses outside the windows,
and the bridge will forward those accesses from the secondary (device)
side to the primary (CPU/memory) side.

All pci_setup_bridge() does is write the window base/limit registers.
It doesn't even figure out what to *put* in those registers; it just
uses the values already in bus->resource[].  So if DMA doesn't work,
and calling pci_setup_bridge() fixes it, bus->resource[] must already
have valid windows, but they don't match the base/limit registers.

What is in the registers and what is in bus->resource[]?  Comparing
the dmesg from with and without your patch should tell us this.

One thing we have to check is whether this path is used during
hotplug.  If there can be other active devices below the bridge,
calling pci_setup_bridge() may cause failures for them because it
temporarily disables the windows.

> 2. On debugging, I see pci_enable_bridge(setup-bus.c) is not called because pci_is_enabled returns true. This is because pci_enable_device is already called for this bridge by portbus driver in pcie_port_device_register(portdrv_core.c).
> 3. Below patch is resolving this issue without any visible side effects. In fact, I verified AER(which needs PORTBUS) also with this.
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 64a7de2..9b67fff 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1200,8 +1200,7 @@ void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
>
>                 switch (dev->class >> 8) {
>                 case PCI_CLASS_BRIDGE_PCI:
> -                       if (!pci_is_enabled(dev))
> -                               pci_setup_bridge(b);
> +                       pci_setup_bridge(b);
>                         break;
>
>                 case PCI_CLASS_BRIDGE_CARDBUS:
>
> 3. This patch works without PORTBUS enabled also.
>
> QUESTIONS:
> 1. Does anybody see any problem with this patch? Any reason for pci_is_enabled check above?
>
> With best,
> Jay
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
--
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
Yinghai Lu Dec. 5, 2013, 10:50 p.m. UTC | #7
On Thu, Dec 5, 2013 at 1:12 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Dec 05, 2013 at 12:27:40AM -0800, Yinghai Lu wrote:
>>
>> Please make sure you pci_assign_unassigned_resources get called via
>> fs_initcall().
>
> Why does this have to be called as fs_initcall() time? Does it have to
> be exactly then or can it be at any later time?

No, just need to make sure it is
before any consumer like pcie driver try to use pci bar resources.

Yinghai
--
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 mbox

Patch

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 64a7de2..9b67fff 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1200,8 +1200,7 @@  void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
 
                switch (dev->class >> 8) {
                case PCI_CLASS_BRIDGE_PCI:
-                       if (!pci_is_enabled(dev))
-                               pci_setup_bridge(b);
+                       pci_setup_bridge(b);
                        break;
 
                case PCI_CLASS_BRIDGE_CARDBUS: