[RFC] Architecture independent pcibios?

[RFC] Architecture independent pcibios?

[Liviu Dudau]

Hello,

I am working on adding PCIe support for a new architecture and looking
at various existing implementations of pcibios functions I've realised
that some architectures share a lot of common code. As I don't like to
repeat the pattern again without any good reasons, I am wondering if
there is any appetite for carving out those common functions into
a generic place under drivers/pci/pcibios.c where they can be reused.

Things that I am specifically looking at are pcibios_{alloc,free}_controller,
pci_process_bridge_OF_ranges and anything that will make adding support
for PCI/PCIe in a new architecture easier. Candidates for promoting
to a generic place are the functions found in both powerpc and microblaze
as they seem to be mostly identical, they support DT bindings and are
64bits ready.

Any comments on why that is (possibly not) a great idea and on the chance
that a patch will get reviewed/accepted?

Best regards,
Liviu Dudau

ARM Ltd Linux Platforms engineer

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC] Architecture independent pcibios?

[Bjorn Helgaas]

[+cc Grant, Rob, devicetree list]

On Tue, Oct 8, 2013 at 8:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> Hello,
>
> I am working on adding PCIe support for a new architecture and looking
> at various existing implementations of pcibios functions I've realised
> that some architectures share a lot of common code. As I don't like to
> repeat the pattern again without any good reasons, I am wondering if
> there is any appetite for carving out those common functions into
> a generic place under drivers/pci/pcibios.c where they can be reused.
>
> Things that I am specifically looking at are pcibios_{alloc,free}_controller,
> pci_process_bridge_OF_ranges and anything that will make adding support
> for PCI/PCIe in a new architecture easier. Candidates for promoting
> to a generic place are the functions found in both powerpc and microblaze
> as they seem to be mostly identical, they support DT bindings and are
> 64bits ready.

I'm very much in favor of unifying code to reduce duplication across
architectures.

In general, the pcibios_*() interfaces are things used by the PCI core
(drivers/pci) but implemented by the architecture.  The specific cases
you mentioned are not actually used by the PCI core, so my inclination
would be to name them something else and possibly put them somewhere
other than drivers/pci.

I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
drivers/of?  The implementations I looked at are mostly concerned with
parsing OF resources, and they don't have much to do with PCI
directly.

pcibios_alloc_controller() is implemented by microblaze, powerpc, and
xtensa.  Each of those arches defines its own "struct pci_controller",
so it won't be completely trivial to unify this.  I tried to start
some unification with the "struct pci_host_bridge" in the core, but
haven't made much progress there.

Bjorn

Re: [RFC] Architecture independent pcibios?

[Rob Herring]

On 10/08/2013 11:32 AM, Bjorn Helgaas wrote:
> [+cc Grant, Rob, devicetree list]
> 
> On Tue, Oct 8, 2013 at 8:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> Hello,
>>
>> I am working on adding PCIe support for a new architecture and looking
>> at various existing implementations of pcibios functions I've realised
>> that some architectures share a lot of common code. As I don't like to
>> repeat the pattern again without any good reasons, I am wondering if
>> there is any appetite for carving out those common functions into
>> a generic place under drivers/pci/pcibios.c where they can be reused.
>>
>> Things that I am specifically looking at are pcibios_{alloc,free}_controller,
>> pci_process_bridge_OF_ranges and anything that will make adding support
>> for PCI/PCIe in a new architecture easier. Candidates for promoting
>> to a generic place are the functions found in both powerpc and microblaze
>> as they seem to be mostly identical, they support DT bindings and are
>> 64bits ready.
> 
> I'm very much in favor of unifying code to reduce duplication across
> architectures.
> 
> In general, the pcibios_*() interfaces are things used by the PCI core
> (drivers/pci) but implemented by the architecture.  The specific cases
> you mentioned are not actually used by the PCI core, so my inclination
> would be to name them something else and possibly put them somewhere
> other than drivers/pci.
> 
> I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> drivers/of?  The implementations I looked at are mostly concerned with
> parsing OF resources, and they don't have much to do with PCI
> directly.

This was being done until Ben weighed in:

https://lkml.org/lkml/2013/5/4/103

Rob

> pcibios_alloc_controller() is implemented by microblaze, powerpc, and
> xtensa.  Each of those arches defines its own "struct pci_controller",
> so it won't be completely trivial to unify this.  I tried to start
> some unification with the "struct pci_host_bridge" in the core, but
> haven't made much progress there.
> 
> Bjorn
> 

Re: [RFC] Architecture independent pcibios?

[Andrew Murray]

On 8 October 2013 17:55, Rob Herring <robherring2@gmail.com> wrote:
> On 10/08/2013 11:32 AM, Bjorn Helgaas wrote:
>> [+cc Grant, Rob, devicetree list]
>>
>> On Tue, Oct 8, 2013 at 8:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>>> Hello,
>>>
>>> I am working on adding PCIe support for a new architecture and looking
>>> at various existing implementations of pcibios functions I've realised
>>> that some architectures share a lot of common code. As I don't like to
>>> repeat the pattern again without any good reasons, I am wondering if
>>> there is any appetite for carving out those common functions into
>>> a generic place under drivers/pci/pcibios.c where they can be reused.
>>>
>>> Things that I am specifically looking at are pcibios_{alloc,free}_controller,
>>> pci_process_bridge_OF_ranges and anything that will make adding support
>>> for PCI/PCIe in a new architecture easier. Candidates for promoting
>>> to a generic place are the functions found in both powerpc and microblaze
>>> as they seem to be mostly identical, they support DT bindings and are
>>> 64bits ready.
>>
>> I'm very much in favor of unifying code to reduce duplication across
>> architectures.
>>
>> In general, the pcibios_*() interfaces are things used by the PCI core
>> (drivers/pci) but implemented by the architecture.  The specific cases
>> you mentioned are not actually used by the PCI core, so my inclination
>> would be to name them something else and possibly put them somewhere
>> other than drivers/pci.
>>
>> I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
>> drivers/of?  The implementations I looked at are mostly concerned with
>> parsing OF resources, and they don't have much to do with PCI
>> directly.
>
> This was being done until Ben weighed in:
>
> https://lkml.org/lkml/2013/5/4/103
>
> Rob
>

Some of this work made it into the kernel...

The OF parts of pci_process_bridge_OF_ranges was split into
of_pci_range_parser and for_each_of_pci_range [1].

I then refactored some of the pci_process_bridge_OF_ranges functions
to use this parser, this was accepted by Microblaze [2] and has been
recently acked by Ralf for MIPS but not made it upstream yet [3]. I
think I submitted a patch for PowerPC at one point, but not sure what
happened to that.

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=29b635c00f3ebcdaf7a52c4948f6d948ad3757d3
[2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4f7b6de437544cd1e2e210919cb58cbe5cc3c393
[3] http://www.linux-mips.org/archives/linux-mips/2013-09/msg00125.html

I'd like to get involved, but don't have any time at the moment.

Thanks,

Andrew Murray

Re: [RFC] Architecture independent pcibios?

[Benjamin Herrenschmidt]

On Tue, 2013-10-08 at 18:20 +0100, Andrew Murray wrote:
> I then refactored some of the pci_process_bridge_OF_ranges functions
> to use this parser, this was accepted by Microblaze [2] and has been
> recently acked by Ralf for MIPS but not made it upstream yet [3]. I
> think I submitted a patch for PowerPC at one point, but not sure what
> happened to that.

Probably slipped through as I was doing some other changes in that
area on powerpc and things conflicted. I can have a look at introducing
the use of your helpers again whenever I manage to get my head out of
meetings and patch monkeying for more than 5mn...

Cheers,
Ben.

> [1]
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=29b635c00f3ebcdaf7a52c4948f6d948ad3757d3
> [2]
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4f7b6de437544cd1e2e210919cb58cbe5cc3c393
> [3]
> http://www.linux-mips.org/archives/linux-mips/2013-09/msg00125.html
> 
> I'd like to get involved, but don't have any time at the moment.

Re: [RFC] Architecture independent pcibios?

[Benjamin Herrenschmidt]

On Tue, 2013-10-08 at 11:55 -0500, Rob Herring wrote:

> > I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> > drivers/of?  The implementations I looked at are mostly concerned with
> > parsing OF resources, and they don't have much to do with PCI
> > directly.
> 
> This was being done until Ben weighed in:
> 
> https://lkml.org/lkml/2013/5/4/103

Well, I proposed an alternative (better) approach which I of course had
no time to actually implement yet :-)

I have done the changes I needed to do to powerpc
pci_process_bridge_OF_ranges so it would be possible to move that now to
a generic place, but I still think it's not a great idea. It means the
pci_controller structure with its resources will have to become generic
which somewhat overlaps with the pci_host_bridge that Bjorn introduced,
so that's really not great.

I still think an arch with DT and simpler PCI code that powerpc could
start looking at the transition to a better model that I hinted at...

Cheers,
Ben.

Re: [RFC] Architecture independent pcibios?

[Liviu Dudau]

On Tue, Oct 08, 2013 at 09:28:55PM +0100, Benjamin Herrenschmidt wrote:
> On Tue, 2013-10-08 at 11:55 -0500, Rob Herring wrote:
> 
> > > I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> > > drivers/of?  The implementations I looked at are mostly concerned with
> > > parsing OF resources, and they don't have much to do with PCI
> > > directly.
> > 
> > This was being done until Ben weighed in:
> > 
> > https://lkml.org/lkml/2013/5/4/103
> 
> Well, I proposed an alternative (better) approach which I of course had
> no time to actually implement yet :-)

In order to avoid any confusion, could you please point me again to the
relevant message(s) where you proposed your approach?

> 
> I have done the changes I needed to do to powerpc
> pci_process_bridge_OF_ranges so it would be possible to move that now to
> a generic place, but I still think it's not a great idea. It means the
> pci_controller structure with its resources will have to become generic
> which somewhat overlaps with the pci_host_bridge that Bjorn introduced,
> so that's really not great.
> 
> I still think an arch with DT and simpler PCI code that powerpc could
> start looking at the transition to a better model that I hinted at...

As tempting as it is to start anew and with a cleaner code, I am wary
that porting the existing platforms to the new code will take longer
that way. My intentions are to make the (probably infrequent) task of
adding a new architecture to the PCI infrastructure a simple and
straighforward task. But adding code for my platform is no guarantee
that new ones will have an easier job.

Best regards,
Liviu

> 
> Cheers,
> Ben.
> 
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC] Architecture independent pcibios?

[Liviu Dudau]

On Wed, Oct 09, 2013 at 10:09:23AM +0100, Liviu Dudau wrote:
> On Tue, Oct 08, 2013 at 09:28:55PM +0100, Benjamin Herrenschmidt wrote:
> > On Tue, 2013-10-08 at 11:55 -0500, Rob Herring wrote:
> > 
> > > > I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> > > > drivers/of?  The implementations I looked at are mostly concerned with
> > > > parsing OF resources, and they don't have much to do with PCI
> > > > directly.
> > > 
> > > This was being done until Ben weighed in:
> > > 
> > > https://lkml.org/lkml/2013/5/4/103
> > 
> > Well, I proposed an alternative (better) approach which I of course had
> > no time to actually implement yet :-)
> 
> In order to avoid any confusion, could you please point me again to the
> relevant message(s) where you proposed your approach?

Sorry, I'm being sloppy. I've made the wrong assumption that the link listed
above was only Ben talking about why it cannot get into powerpc. Now I see
that it is also the place where Ben talks about moving the "intermediary"
set of resources out of struct pci_controller.

Is there anything else that I might be missing?

Cheers,
Liviu

> 
> > 
> > I have done the changes I needed to do to powerpc
> > pci_process_bridge_OF_ranges so it would be possible to move that now to
> > a generic place, but I still think it's not a great idea. It means the
> > pci_controller structure with its resources will have to become generic
> > which somewhat overlaps with the pci_host_bridge that Bjorn introduced,
> > so that's really not great.
> > 
> > I still think an arch with DT and simpler PCI code that powerpc could
> > start looking at the transition to a better model that I hinted at...
> 
> As tempting as it is to start anew and with a cleaner code, I am wary
> that porting the existing platforms to the new code will take longer
> that way. My intentions are to make the (probably infrequent) task of
> adding a new architecture to the PCI infrastructure a simple and
> straighforward task. But adding code for my platform is no guarantee
> that new ones will have an easier job.
> 
> Best regards,
> Liviu
> 
> > 
> > Cheers,
> > Ben.
> > 
> > 
> > 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC] Architecture independent pcibios?

[Benjamin Herrenschmidt]

On Wed, 2013-10-09 at 10:09 +0100, Liviu Dudau wrote:
> On Tue, Oct 08, 2013 at 09:28:55PM +0100, Benjamin Herrenschmidt wrote:
> > On Tue, 2013-10-08 at 11:55 -0500, Rob Herring wrote:
> >
> > > > I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> > > > drivers/of?  The implementations I looked at are mostly concerned with
> > > > parsing OF resources, and they don't have much to do with PCI
> > > > directly.
> > >
> > > This was being done until Ben weighed in:
> > >
> > > https://lkml.org/lkml/2013/5/4/103
> >
> > Well, I proposed an alternative (better) approach which I of course had
> > no time to actually implement yet :-)
> 
> In order to avoid any confusion, could you please point me again to the
> relevant message(s) where you proposed your approach?

The one pointed to by the above URL ?

> > I have done the changes I needed to do to powerpc
> > pci_process_bridge_OF_ranges so it would be possible to move that now to
> > a generic place, but I still think it's not a great idea. It means the
> > pci_controller structure with its resources will have to become generic
> > which somewhat overlaps with the pci_host_bridge that Bjorn introduced,
> > so that's really not great.
> >
> > I still think an arch with DT and simpler PCI code that powerpc could
> > start looking at the transition to a better model that I hinted at...
> 
> As tempting as it is to start anew and with a cleaner code, I am wary
> that porting the existing platforms to the new code will take longer
> that way. My intentions are to make the (probably infrequent) task of
> adding a new architecture to the PCI infrastructure a simple and
> straighforward task. But adding code for my platform is no guarantee
> that new ones will have an easier job.

It still makes little sense to generalize a pci_controller with
resources & offset on top of the generic pci_host_bridge with apertures.

Ben.

> Best regards,
> Liviu
> 
> >
> > Cheers,
> > Ben.
> >
> >
> >
> 
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
> 
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
> 
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

Re: [RFC] Architecture independent pcibios?

[Liviu Dudau]

On Wed, Oct 09, 2013 at 11:37:48AM +0100, Benjamin Herrenschmidt wrote:
> On Wed, 2013-10-09 at 10:09 +0100, Liviu Dudau wrote:
> > On Tue, Oct 08, 2013 at 09:28:55PM +0100, Benjamin Herrenschmidt wrote:
> > > On Tue, 2013-10-08 at 11:55 -0500, Rob Herring wrote:
> > >
> > > > > I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> > > > > drivers/of?  The implementations I looked at are mostly concerned with
> > > > > parsing OF resources, and they don't have much to do with PCI
> > > > > directly.
> > > >
> > > > This was being done until Ben weighed in:
> > > >
> > > > https://lkml.org/lkml/2013/5/4/103
> > >
> > > Well, I proposed an alternative (better) approach which I of course had
> > > no time to actually implement yet :-)
> > 
> > In order to avoid any confusion, could you please point me again to the
> > relevant message(s) where you proposed your approach?
> 
> The one pointed to by the above URL ?
> 
> > > I have done the changes I needed to do to powerpc
> > > pci_process_bridge_OF_ranges so it would be possible to move that now to
> > > a generic place, but I still think it's not a great idea. It means the
> > > pci_controller structure with its resources will have to become generic
> > > which somewhat overlaps with the pci_host_bridge that Bjorn introduced,
> > > so that's really not great.
> > >
> > > I still think an arch with DT and simpler PCI code that powerpc could
> > > start looking at the transition to a better model that I hinted at...
> > 
> > As tempting as it is to start anew and with a cleaner code, I am wary
> > that porting the existing platforms to the new code will take longer
> > that way. My intentions are to make the (probably infrequent) task of
> > adding a new architecture to the PCI infrastructure a simple and
> > straighforward task. But adding code for my platform is no guarantee
> > that new ones will have an easier job.
> 
> It still makes little sense to generalize a pci_controller with
> resources & offset on top of the generic pci_host_bridge with apertures.

Agreed. I'm looking on how to move functions that use pci_controller to
use pci_host_bridge.

Semantic question: what is the perceived difference between a pci_controller
and a pci_host_bridge? Are they just historical naming artifacts of the
same concept? (person A deciding to do a cleanup of the code and picks a
new name in order not to upset the old APIs)

Best regards,
Liviu

> 
> Ben.
> 
> > Best regards,
> > Liviu
> > 
> > >
> > > Cheers,
> > > Ben.
> > >
> > >
> > >
> > 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC] Architecture independent pcibios?

[Bjorn Helgaas]

On Wed, Oct 9, 2013 at 4:45 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:

> Semantic question: what is the perceived difference between a pci_controller
> and a pci_host_bridge? Are they just historical naming artifacts of the
> same concept? (person A deciding to do a cleanup of the code and picks a
> new name in order not to upset the old APIs)

pci_controller is a typical name for the arch-dependent PCI host
bridge structure.  Some arches use different names, e.g.,
pci_sys_data, pci_hba_data, pci_channel, etc.

When I added support in the PCI core for host bridge address
translation, e.g., ACPI _TRA, I named the core structure
pci_host_bridge.

Some of the stuff in pci_controller and similar arch-dependent
structures could probably be moved into pci_host_bridge, given
appropriate PCI core interfaces to supply the information (or maybe
pcibios_*() functions the core could use to retrieve it).  But I doubt
we could completely remove the arch-dependent structures.

Bjorn

Re: [RFC] Architecture independent pcibios?

[Liviu Dudau]

On Wed, Oct 09, 2013 at 03:30:35PM +0100, Bjorn Helgaas wrote:
> On Wed, Oct 9, 2013 at 4:45 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> 
> > Semantic question: what is the perceived difference between a pci_controller
> > and a pci_host_bridge? Are they just historical naming artifacts of the
> > same concept? (person A deciding to do a cleanup of the code and picks a
> > new name in order not to upset the old APIs)
> 
> pci_controller is a typical name for the arch-dependent PCI host
> bridge structure.  Some arches use different names, e.g.,
> pci_sys_data, pci_hba_data, pci_channel, etc.
> 
> When I added support in the PCI core for host bridge address
> translation, e.g., ACPI _TRA, I named the core structure
> pci_host_bridge.
> 
> Some of the stuff in pci_controller and similar arch-dependent
> structures could probably be moved into pci_host_bridge, given
> appropriate PCI core interfaces to supply the information (or maybe
> pcibios_*() functions the core could use to retrieve it).  But I doubt
> we could completely remove the arch-dependent structures.

How about this as a battle plan:

  - create a "generic pci_controller struct" (actual name to be picked later)
    that parses the DT for ranges and creates the relevant
    pci_host_bridge_windows, then calls pci_scan_root_bus()

  - provide some weak function definitions that arch-dependent code
    can override for the cases where the "generic pci_controller struct"
    version does not fit the bill.

Best regards,
Liviu


> 
> Bjorn
> --
> 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
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC] Architecture independent pcibios?

[Grant Likely]

On Wed, 9 Oct 2013 10:09:23 +0100, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Tue, Oct 08, 2013 at 09:28:55PM +0100, Benjamin Herrenschmidt wrote:
> > On Tue, 2013-10-08 at 11:55 -0500, Rob Herring wrote:
> > 
> > > > I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> > > > drivers/of?  The implementations I looked at are mostly concerned with
> > > > parsing OF resources, and they don't have much to do with PCI
> > > > directly.
> > > 
> > > This was being done until Ben weighed in:
> > > 
> > > https://lkml.org/lkml/2013/5/4/103
> > 
> > Well, I proposed an alternative (better) approach which I of course had
> > no time to actually implement yet :-)
> 
> In order to avoid any confusion, could you please point me again to the
> relevant message(s) where you proposed your approach?
> 
> > 
> > I have done the changes I needed to do to powerpc
> > pci_process_bridge_OF_ranges so it would be possible to move that now to
> > a generic place, but I still think it's not a great idea. It means the
> > pci_controller structure with its resources will have to become generic
> > which somewhat overlaps with the pci_host_bridge that Bjorn introduced,
> > so that's really not great.
> > 
> > I still think an arch with DT and simpler PCI code that powerpc could
> > start looking at the transition to a better model that I hinted at...
> 
> As tempting as it is to start anew and with a cleaner code, I am wary
> that porting the existing platforms to the new code will take longer
> that way. My intentions are to make the (probably infrequent) task of
> adding a new architecture to the PCI infrastructure a simple and
> straighforward task. But adding code for my platform is no guarantee
> that new ones will have an easier job.

In the previous thread I Nacked an approach that creates new code
without making at least two existing platforms (microblaze & powerpc are
the easy targets because they are so similar) use said new code. I know
that makes things harder for you, but not migrating is a bigger
maintenance burden in the long run.

g.

Re: [RFC] Architecture independent pcibios?

[Liviu Dudau]

On Tue, Oct 08, 2013 at 05:32:57PM +0100, Bjorn Helgaas wrote:
> [+cc Grant, Rob, devicetree list]
> 
> On Tue, Oct 8, 2013 at 8:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > Hello,
> >
> > I am working on adding PCIe support for a new architecture and looking
> > at various existing implementations of pcibios functions I've realised
> > that some architectures share a lot of common code. As I don't like to
> > repeat the pattern again without any good reasons, I am wondering if
> > there is any appetite for carving out those common functions into
> > a generic place under drivers/pci/pcibios.c where they can be reused.
> >
> > Things that I am specifically looking at are pcibios_{alloc,free}_controller,
> > pci_process_bridge_OF_ranges and anything that will make adding support
> > for PCI/PCIe in a new architecture easier. Candidates for promoting
> > to a generic place are the functions found in both powerpc and microblaze
> > as they seem to be mostly identical, they support DT bindings and are
> > 64bits ready.
> 
> I'm very much in favor of unifying code to reduce duplication across
> architectures.
> 
> In general, the pcibios_*() interfaces are things used by the PCI core
> (drivers/pci) but implemented by the architecture.  The specific cases
> you mentioned are not actually used by the PCI core, so my inclination
> would be to name them something else and possibly put them somewhere
> other than drivers/pci.

There are at least a handful of architectures that seem to share the same
implementation for most (all?) of the pcibios_*() functions. (PowerPC and
microblaze the most obvious offenders, x86 to a certain extent).

> 
> I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
> drivers/of?  The implementations I looked at are mostly concerned with
> parsing OF resources, and they don't have much to do with PCI
> directly.

Andrew Murray did sent a patch that was placing pci_process_bridge_OF_ranges()
in the drivers/of. I can update that, as I have taken over his work.

> 
> pcibios_alloc_controller() is implemented by microblaze, powerpc, and
> xtensa.  Each of those arches defines its own "struct pci_controller",
> so it won't be completely trivial to unify this.  I tried to start
> some unification with the "struct pci_host_bridge" in the core, but
> haven't made much progress there.

Do you have anything that you could share? I would pretty much like to take
on that as well, as I don't want to create yet another
"struct pci_controller." BTW, powerpc and microblaze again share a very
similarly looking structure.

Best regards,
Liviu

> 
> Bjorn
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC] Architecture independent pcibios?

[Bjorn Helgaas]

On 10/8/13, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Tue, Oct 08, 2013 at 05:32:57PM +0100, Bjorn Helgaas wrote:
>> [+cc Grant, Rob, devicetree list]
>>
>> On Tue, Oct 8, 2013 at 8:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > Hello,
>> >
>> > I am working on adding PCIe support for a new architecture and looking
>> > at various existing implementations of pcibios functions I've realised
>> > that some architectures share a lot of common code. As I don't like to
>> > repeat the pattern again without any good reasons, I am wondering if
>> > there is any appetite for carving out those common functions into
>> > a generic place under drivers/pci/pcibios.c where they can be reused.
>> >
>> > Things that I am specifically looking at are
>> > pcibios_{alloc,free}_controller,
>> > pci_process_bridge_OF_ranges and anything that will make adding support
>> > for PCI/PCIe in a new architecture easier. Candidates for promoting
>> > to a generic place are the functions found in both powerpc and
>> > microblaze
>> > as they seem to be mostly identical, they support DT bindings and are
>> > 64bits ready.
>>
>> I'm very much in favor of unifying code to reduce duplication across
>> architectures.
>>
>> In general, the pcibios_*() interfaces are things used by the PCI core
>> (drivers/pci) but implemented by the architecture.  The specific cases
>> you mentioned are not actually used by the PCI core, so my inclination
>> would be to name them something else and possibly put them somewhere
>> other than drivers/pci.
>
> There are at least a handful of architectures that seem to share the same
> implementation for most (all?) of the pcibios_*() functions. (PowerPC and
> microblaze the most obvious offenders, x86 to a certain extent).

Yes.  We've unified some of these by putting the generic version in
drivers/pci as a weak function.

>> I wonder if pci_process_bridge_OF_ranges() would fit somewhere in
>> drivers/of?  The implementations I looked at are mostly concerned with
>> parsing OF resources, and they don't have much to do with PCI
>> directly.
>
> Andrew Murray did sent a patch that was placing
> pci_process_bridge_OF_ranges()
> in the drivers/of. I can update that, as I have taken over his work.
>
>>
>> pcibios_alloc_controller() is implemented by microblaze, powerpc, and
>> xtensa.  Each of those arches defines its own "struct pci_controller",
>> so it won't be completely trivial to unify this.  I tried to start
>> some unification with the "struct pci_host_bridge" in the core, but
>> haven't made much progress there.
>
> Do you have anything that you could share? I would pretty much like to take
> on that as well, as I don't want to create yet another
> "struct pci_controller." BTW, powerpc and microblaze again share a very
> similarly looking structure.

No, I don't have any code (or plans).  I think pci_domain_nr() and
pcibus_to_node() should be pretty easy to start unifying.  I'd be
thrilled if you started pushing on this.

Bjorn

Re: [RFC] Architecture independent pcibios?

[Benjamin Herrenschmidt]

On Tue, 2013-10-08 at 18:13 +0100, Liviu Dudau wrote:
> There are at least a handful of architectures that seem to share the same
> implementation for most (all?) of the pcibios_*() functions. (PowerPC and
> microblaze the most obvious offenders, x86 to a certain extent).

That's because microblaze started by copying the powerpc code :-) The
other part about x86 is mostly me over the year changing the powerpc
code to make it closer to x86 especially in the area of resource
management. This leads to less overall surprises and issues when the
generic code changes and in a few cases has made it easier to move bits
to the core.

The main remaining difference is our use of IORESOURCE_UNSET and our
flags to force reallocation.

Ben.

Re: [RFC] Architecture independent pcibios?

[Michal Simek]

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

On 10/08/2013 10:31 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2013-10-08 at 18:13 +0100, Liviu Dudau wrote:
>> There are at least a handful of architectures that seem to share the same
>> implementation for most (all?) of the pcibios_*() functions. (PowerPC and
>> microblaze the most obvious offenders, x86 to a certain extent).
> 
> That's because microblaze started by copying the powerpc code :-) 

yes and the reason was that xilinx ppc405 and then ppc440 used
the same pci IPs that's why it was easier just to use this code.

I am keeping microblaze pcie driver out of mainline but definitely
unification work has to be done.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]