Re: [PATCH v2 19/27] pci: PCIe driver for Marvell Armada 370/XP systems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 07, 2013 at 11:08:45AM -0700, Bjorn Helgaas wrote:
> On Thu, Feb 7, 2013 at 9:00 AM, Thomas Petazzoni
> <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> wrote:
> > Dear Bjorn Helgaas,
> >
> > On Thu, 7 Feb 2013 08:46:06 -0700, Bjorn Helgaas wrote:
> >
> >> Can you post the entire dmesg log, ideally with CONFIG_PCI_DEBUG=y?
> >> That should have more information about the enumeration process,
> >> including what we think the XHCI BARs are and the apertures leading to
> >> them.
> >
> > Sure, see below.
> >
> >> The PCI core assumes that we know the host bridge apertures up front,
> >> and I'm not sure that is true on your platform, so maybe we'll need
> >> some changes to accommodate that.
> >
> > In this hardware, we need to set up the address decoding windows. So
> > there shouldn't be any access to a PCI device memory or I/O region
> > until the addresses have been assigned in the PCI-to-PCI bridge.
> 
> I think this is the path where the crash happens (this is the same as
> the backtrace you included below):
> 
>     mvebu_pcie_scan_bus
>       pci_scan_root_bus
>         pci_create_root_bus
>         pci_scan_child_bus
>         pci_bus_add_devices
>           pci_bus_add_device
>             pci_fixup_device(pci_fixup_final)
>               quirk_usb_early_handoff           # pci_fixup_final
>                 quirk_usb_handoff_xhci
> 
> The problem is that we haven't assigned resources anywhere.  Normally
> this is done by pci_bus_assign_resources() or
> pci_assign_unassigned_bus_resources(), but I don't think there's
> anything in the path above that does this.
> 
> This is not really a problem in your code; it's a generic PCI core
> problem.  pci_scan_root_bus() does everything including creating the
> root bus, scanning it, and adding the devices we find.  At the point
> where we add a device (pci_bus_add_device()), it should be ready for a
> driver to claim it -- all resource assignment should already be done.
> 
> I don't think it's completely trivial to fix this in the PCI core yet
> (but we're moving in that direction) because we have some boot-time
> ordering issues, e.g., x86 scans the root buses before we know about
> the address space consumed by ACPI devices, so we can't just assign
> the resources when we scan the bus.
> 
> I think the way you'll have to fix this in the meantime is to use
> pci_create_root_bus() directly so you can do things in this sequence:
> 
>   pci_create_root_bus
>   pci_scan_child_bus
>   pci_bus_assign_resources
>   pci_bus_add_devices

The last two are already done by ARM's pci_common_init(). On Tegra I've
got by having a custom .scan_bus() implementation that does the first
two. Back when I used the pci_scan_root_bus() we were seeing some issues
with resource conflicts and such and there was a similar discussion at
that time.

Thomas, have you tried using the same .scan_bus() that I use on Tegra?
It should be easy to port it to Marvell, the only Tegra-specific bit is
how to get at the struct tegra_pcie.

Thierry

Attachment: pgpe1Li4I3fap.pgp
Description: PGP signature


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux