Re: PCIe device tree support

Re: PCIe device tree support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

[Adding Jesse Barnes and the linux-pci mailing list to CC.]

* Stephen Warren wrote:
> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
> > I would like to add device tree support for the Tegra2 PCIe controller. From
> > what I understand this will require the PCIe code to be rewritten as a proper
> > platform driver in order to be able to instantiate it via the device tree. Is
> > that correct?
> 
> That's probably the cleanest way, yes.
> 
> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
> code there might be nice if so, although IIRC when I asked Olof about
> that a number of months back, he said quite a few host controllers were
> in arch/...

Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
a specific reason for this? Would it be acceptable to put any new drivers
below drivers/pci/?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: PCIe device tree support

[Bjorn Helgaas]

On Wed, Feb 22, 2012 at 12:24 AM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> [Adding Jesse Barnes and the linux-pci mailing list to CC.]
>
> * Stephen Warren wrote:
>> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
>> > I would like to add device tree support for the Tegra2 PCIe controller. From
>> > what I understand this will require the PCIe code to be rewritten as a proper
>> > platform driver in order to be able to instantiate it via the device tree. Is
>> > that correct?
>>
>> That's probably the cleanest way, yes.
>>
>> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
>> code there might be nice if so, although IIRC when I asked Olof about
>> that a number of months back, he said quite a few host controllers were
>> in arch/...
>
> Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
> a specific reason for this? Would it be acceptable to put any new drivers
> below drivers/pci/?

PCI host bridges aren't architected, so discovering them and their
properties has historically been mostly architecture-specific.  ACPI
is one exception (with a driver in drivers/acpi/pci_root.c) and it
sounds like device tree might be a similar exception.  If you can make
a non-arch-specific driver and put it somewhere other than arch/, I'm
all in favor of that, especially if you can make it usable by other
arches that use device tree.  Where to put it?  I dunno.  drivers/pci/
sounds like a reasonable possibility.

Bjorn

Re: PCIe device tree support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

* Bjorn Helgaas wrote:
> On Wed, Feb 22, 2012 at 12:24 AM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > [Adding Jesse Barnes and the linux-pci mailing list to CC.]
> >
> > * Stephen Warren wrote:
> >> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
> >> > I would like to add device tree support for the Tegra2 PCIe controller. From
> >> > what I understand this will require the PCIe code to be rewritten as a proper
> >> > platform driver in order to be able to instantiate it via the device tree. Is
> >> > that correct?
> >>
> >> That's probably the cleanest way, yes.
> >>
> >> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
> >> code there might be nice if so, although IIRC when I asked Olof about
> >> that a number of months back, he said quite a few host controllers were
> >> in arch/...
> >
> > Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
> > a specific reason for this? Would it be acceptable to put any new drivers
> > below drivers/pci/?
> 
> PCI host bridges aren't architected, so discovering them and their
> properties has historically been mostly architecture-specific.  ACPI
> is one exception (with a driver in drivers/acpi/pci_root.c) and it
> sounds like device tree might be a similar exception.  If you can make
> a non-arch-specific driver and put it somewhere other than arch/, I'm
> all in favor of that, especially if you can make it usable by other
> arches that use device tree.  Where to put it?  I dunno.  drivers/pci/
> sounds like a reasonable possibility.

I don't think it would be possible to write a driver that works for all
device tree based boards or architectures. As you said the implementation is
very hardware specific and probably couldn't even be generalized among two
chipsets of the same architecture.

However there has recently been a lot of work to move out arch-specific
drivers for PWM, GPIO and others out of arch/ and into respective directories
below drivers/. The reason I asked was because I think the same could be done
for PCI/PCIe.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: PCIe device tree support

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2615 bytes --]

On Thu, 23 Feb 2012 07:03:55 +0100
Thierry Reding <thierry.reding@avionic-design.de> wrote:

> * Bjorn Helgaas wrote:
> > On Wed, Feb 22, 2012 at 12:24 AM, Thierry Reding
> > <thierry.reding@avionic-design.de> wrote:
> > > [Adding Jesse Barnes and the linux-pci mailing list to CC.]
> > >
> > > * Stephen Warren wrote:
> > >> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
> > >> > I would like to add device tree support for the Tegra2 PCIe controller. From
> > >> > what I understand this will require the PCIe code to be rewritten as a proper
> > >> > platform driver in order to be able to instantiate it via the device tree. Is
> > >> > that correct?
> > >>
> > >> That's probably the cleanest way, yes.
> > >>
> > >> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
> > >> code there might be nice if so, although IIRC when I asked Olof about
> > >> that a number of months back, he said quite a few host controllers were
> > >> in arch/...
> > >
> > > Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
> > > a specific reason for this? Would it be acceptable to put any new drivers
> > > below drivers/pci/?
> > 
> > PCI host bridges aren't architected, so discovering them and their
> > properties has historically been mostly architecture-specific.  ACPI
> > is one exception (with a driver in drivers/acpi/pci_root.c) and it
> > sounds like device tree might be a similar exception.  If you can make
> > a non-arch-specific driver and put it somewhere other than arch/, I'm
> > all in favor of that, especially if you can make it usable by other
> > arches that use device tree.  Where to put it?  I dunno.  drivers/pci/
> > sounds like a reasonable possibility.
> 
> I don't think it would be possible to write a driver that works for all
> device tree based boards or architectures. As you said the implementation is
> very hardware specific and probably couldn't even be generalized among two
> chipsets of the same architecture.
> 
> However there has recently been a lot of work to move out arch-specific
> drivers for PWM, GPIO and others out of arch/ and into respective directories
> below drivers/. The reason I asked was because I think the same could be done
> for PCI/PCIe.

I think that makes sense for stuff that may be shared between arches,
but host bridges are generally part of the core architecture of the
system, so factoring them out would just be code shuffling without any
real gain.

But for shared code, drivers/pci is definitely the right place.

Thanks,
Jesse

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: PCIe device tree support

[Olof Johansson]

Hi,

On Wed, Feb 22, 2012 at 10:03 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> * Bjorn Helgaas wrote:
>> On Wed, Feb 22, 2012 at 12:24 AM, Thierry Reding
>> <thierry.reding@avionic-design.de> wrote:
>> > [Adding Jesse Barnes and the linux-pci mailing list to CC.]
>> >
>> > * Stephen Warren wrote:
>> >> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
>> >> > I would like to add device tree support for the Tegra2 PCIe controller. From
>> >> > what I understand this will require the PCIe code to be rewritten as a proper
>> >> > platform driver in order to be able to instantiate it via the device tree. Is
>> >> > that correct?
>> >>
>> >> That's probably the cleanest way, yes.
>> >>
>> >> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
>> >> code there might be nice if so, although IIRC when I asked Olof about
>> >> that a number of months back, he said quite a few host controllers were
>> >> in arch/...
>> >
>> > Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
>> > a specific reason for this? Would it be acceptable to put any new drivers
>> > below drivers/pci/?
>>
>> PCI host bridges aren't architected, so discovering them and their
>> properties has historically been mostly architecture-specific.  ACPI
>> is one exception (with a driver in drivers/acpi/pci_root.c) and it
>> sounds like device tree might be a similar exception.  If you can make
>> a non-arch-specific driver and put it somewhere other than arch/, I'm
>> all in favor of that, especially if you can make it usable by other
>> arches that use device tree.  Where to put it?  I dunno.  drivers/pci/
>> sounds like a reasonable possibility.
>
> I don't think it would be possible to write a driver that works for all
> device tree based boards or architectures. As you said the implementation is
> very hardware specific and probably couldn't even be generalized among two
> chipsets of the same architecture.
>
> However there has recently been a lot of work to move out arch-specific
> drivers for PWM, GPIO and others out of arch/ and into respective directories
> below drivers/. The reason I asked was because I think the same could be done
> for PCI/PCIe.


Right. PCI/PCIe host bridges tend to be custom. Well, I'm sure some
vendors share some of the IP, but I'm not sure how much of the
programming interface comes with that and how much is actually their
own. So, either way, there's only a few ARM platforms so far with
PCI/PCIe, and I think we can keep the logic under arch/arm for them
for now. If more start to show up and we want to move it out, we can
do so.

Sorry for being silent on this thread as well -- I've been travelling
and offline a bit. It shouldn't be too hard to modify the tegra pcie
driver to probe through the device tree, to start with you pretty much
just need to describe the register range with a reg property and a
compatible field to bind against. Some of the powerpc platforms do
similar things, so take a look at how they do it.


-Olof

Re: PCIe device tree support

[Bjorn Helgaas]

On Wed, Feb 22, 2012 at 10:03 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> * Bjorn Helgaas wrote:
>> On Wed, Feb 22, 2012 at 12:24 AM, Thierry Reding
>> <thierry.reding@avionic-design.de> wrote:
>> > [Adding Jesse Barnes and the linux-pci mailing list to CC.]
>> >
>> > * Stephen Warren wrote:
>> >> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
>> >> > I would like to add device tree support for the Tegra2 PCIe controller. From
>> >> > what I understand this will require the PCIe code to be rewritten as a proper
>> >> > platform driver in order to be able to instantiate it via the device tree. Is
>> >> > that correct?
>> >>
>> >> That's probably the cleanest way, yes.
>> >>
>> >> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
>> >> code there might be nice if so, although IIRC when I asked Olof about
>> >> that a number of months back, he said quite a few host controllers were
>> >> in arch/...
>> >
>> > Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
>> > a specific reason for this? Would it be acceptable to put any new drivers
>> > below drivers/pci/?
>>
>> PCI host bridges aren't architected, so discovering them and their
>> properties has historically been mostly architecture-specific.  ACPI
>> is one exception (with a driver in drivers/acpi/pci_root.c) and it
>> sounds like device tree might be a similar exception.  If you can make
>> a non-arch-specific driver and put it somewhere other than arch/, I'm
>> all in favor of that, especially if you can make it usable by other
>> arches that use device tree.  Where to put it?  I dunno.  drivers/pci/
>> sounds like a reasonable possibility.
>
> I don't think it would be possible to write a driver that works for all
> device tree based boards or architectures. As you said the implementation is
> very hardware specific and probably couldn't even be generalized among two
> chipsets of the same architecture.

Host bridges may be hardware-specific, but the PCI core really only
needs an abstract description.  It needs the bus number aperture, the
I/O port apertures, the MMIO apertures, and associated offsets between
CPU and PCI bus addresses.  I don't know anything about device tree,
but I'd be surprised and disappointed if it encodes this basic
information in platform-specific ways.

I agree that if the code isn't shared at all, there's no point in
putting it in a generic place like drivers/pci/.

Bjorn

Re: PCIe device tree support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 212 bytes --]

* Bjorn Helgaas wrote:
> I agree that if the code isn't shared at all, there's no point in
> putting it in a generic place like drivers/pci/.

Alright, I'll keep the driver in arch/arm/mach-tegra/ then.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: PCIe device tree support

[Pratyush Anand]

I too had similar query.
I have written a PCIe host driver for SPEAr platform (ARM Arch).
This driver is registered on paltform bus.
It is for Designware IP, so I believe that most of its part
can be re-usaable for other person.
 have kept common and SPEAr specific part separately.
But,  I am not sure about its best location.
I believe best would have been to keep in driver/pci/pcie/hosts/.
But it uses pci_common_init function which is specific to ARM platform.
So it limits option to arch/arm.
But, if I keep it in arch/arm/mach-spear13xx/ , then it will limit others
from using synopsys specific code.

What do you suggest?

Regards
Pratyush


On Fri, Feb 24, 2012 at 12:29 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> * Bjorn Helgaas wrote:
>> I agree that if the code isn't shared at all, there's no point in
>> putting it in a generic place like drivers/pci/.
>
> Alright, I'll keep the driver in arch/arm/mach-tegra/ then.
>
> Thierry

Re: PCIe device tree support

[Bjorn Helgaas]

On Fri, Feb 24, 2012 at 9:16 AM, Pratyush Anand
<pratyush.anand@gmail.com> wrote:
> I too had similar query.
> I have written a PCIe host driver for SPEAr platform (ARM Arch).
> This driver is registered on paltform bus.
> It is for Designware IP, so I believe that most of its part
> can be re-usaable for other person.
>  have kept common and SPEAr specific part separately.
> But,  I am not sure about its best location.
> I believe best would have been to keep in driver/pci/pcie/hosts/.
> But it uses pci_common_init function which is specific to ARM platform.
> So it limits option to arch/arm.
> But, if I keep it in arch/arm/mach-spear13xx/ , then it will limit others
> from using synopsys specific code.
>
> What do you suggest?

If it uses ARM-specific functions, I think somewhere in arch/arm/
makes the most sense.  There are some arches (parisc comes to mind)
that do have arch-specific code in drivers/, but until it can be
usefully shared, that doesn't seem worthwhile to me.  We can always
move it later.

Bjorn

Re: PCIe device tree support

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Fri, 24 Feb 2012 22:46:06 +0530
Pratyush Anand <pratyush.anand@gmail.com> wrote:

> I too had similar query.
> I have written a PCIe host driver for SPEAr platform (ARM Arch).
> This driver is registered on paltform bus.
> It is for Designware IP, so I believe that most of its part
> can be re-usaable for other person.
>  have kept common and SPEAr specific part separately.
> But,  I am not sure about its best location.
> I believe best would have been to keep in driver/pci/pcie/hosts/.
> But it uses pci_common_init function which is specific to ARM platform.
> So it limits option to arch/arm.
> But, if I keep it in arch/arm/mach-spear13xx/ , then it will limit others
> from using synopsys specific code.
> 
> What do you suggest?

Maybe arch/arm/pci until there are non-ARM users of the IP?  In fact,
you may want to put off moving it to arch/arm/pci until you have
someone to share it with.  Premature sharing is like premature
optimization after all...

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: PCIe device tree support

[Bjorn Helgaas]

On Thu, Feb 23, 2012 at 10:49 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Feb 22, 2012 at 10:03 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
>> * Bjorn Helgaas wrote:
>>> On Wed, Feb 22, 2012 at 12:24 AM, Thierry Reding
>>> <thierry.reding@avionic-design.de> wrote:
>>> > [Adding Jesse Barnes and the linux-pci mailing list to CC.]
>>> >
>>> > * Stephen Warren wrote:
>>> >> Thierry Reding wrote at Monday, February 20, 2012 12:18 PM:
>>> >> > I would like to add device tree support for the Tegra2 PCIe controller. From
>>> >> > what I understand this will require the PCIe code to be rewritten as a proper
>>> >> > platform driver in order to be able to instantiate it via the device tree. Is
>>> >> > that correct?
>>> >>
>>> >> That's probably the cleanest way, yes.
>>> >>
>>> >> Is there a drivers/ directory for PCI/PCIe host controllers? Moving the
>>> >> code there might be nice if so, although IIRC when I asked Olof about
>>> >> that a number of months back, he said quite a few host controllers were
>>> >> in arch/...
>>> >
>>> > Most PCI/PCIe host controller drivers seem to currently be in arch/. Is there
>>> > a specific reason for this? Would it be acceptable to put any new drivers
>>> > below drivers/pci/?
>>>
>>> PCI host bridges aren't architected, so discovering them and their
>>> properties has historically been mostly architecture-specific.  ACPI
>>> is one exception (with a driver in drivers/acpi/pci_root.c) and it
>>> sounds like device tree might be a similar exception.  If you can make
>>> a non-arch-specific driver and put it somewhere other than arch/, I'm
>>> all in favor of that, especially if you can make it usable by other
>>> arches that use device tree.  Where to put it?  I dunno.  drivers/pci/
>>> sounds like a reasonable possibility.
>>
>> I don't think it would be possible to write a driver that works for all
>> device tree based boards or architectures. As you said the implementation is
>> very hardware specific and probably couldn't even be generalized among two
>> chipsets of the same architecture.
>
> Host bridges may be hardware-specific, but the PCI core really only
> needs an abstract description.  It needs the bus number aperture, the
> I/O port apertures, the MMIO apertures, and associated offsets between
> CPU and PCI bus addresses.  I don't know anything about device tree,
> but I'd be surprised and disappointed if it encodes this basic
> information in platform-specific ways.

This made me curious, so I poked around at the callers of
of_get_property("bus-range").  Most of these callers are basically PCI
host bridge drivers, though they aren't really structured as drivers.
They have a very consistent structure of the form:

    for_each_compatible_node(np, "pci", "mpc10x-pci")
        pcibios_alloc_controller()
        of_get_property(dev, "bus-range", &len)
        pci_process_bridge_OF_ranges()
            of_get_property(dev, "ranges", &rlen)
            ...
        pci_create_root_bus()

This basic pattern is used twenty or more times already!  There are
definitely variations and some platform-specific pieces, but I think
there's certainly the possibility of making some of this code more
generic, since the information used by the PCI core seems to be
described consistently across platforms.

Bjorn