On Wed, Jul 09, 2014 at 04:10:04PM +0100, Bjorn Helgaas wrote:

On Tue, Jul 8, 2014 at 4:48 PM, Liviu Dudau [off-list ref] wrote:

quoted

On Tue, Jul 08, 2014 at 07:41:50PM +0100, Bjorn Helgaas wrote:

quoted

On Tue, Jul 8, 2014 at 4:46 AM, Liviu Dudau [off-list ref] wrote:

quoted

On Tue, Jul 08, 2014 at 01:59:54AM +0100, Bjorn Helgaas wrote:

quoted

quoted

I wonder if it would help to make a weak pci_domain_nr() function that
returns "bridge->domain_nr".  Then each arch could individually drop its
pci_domain_nr() definition as it was converted, e.g., something like this:

  - Convert every arch pci_domain_nr() from a #define to a non-inline
    function
  - Add bridge.domain_nr, initialized from pci_domain_nr()
  - Add a weak generic pci_domain_nr() that returns bridge.domain_nr
  - Add a way to create a host bridge in a specified domain, so we can
    initialize bridge.domain_nr without using pci_domain_nr()
  - Convert each arch to use the new creation mechanism and drop its
    pci_domain_nr() implementation

I will try to propose a patch implementing this.

I think this is more of an extra credit, cleanup sort of thing.  I
don't think it advances your primary goal of (I think) getting arm64
PCI support in.  So my advice is to not worry about unifying domain
handling until later.

Getting arm64 supported *is* my main goal. But like you have stated in your
review of v7, you wanted to see another architecture converted as a guarantee
of "genericity" (for lack of a better word) for my patches. The one architecture
I've set my eyes on is microblaze, and that one uses pci_scan_root_bus()
rather than pci_create_root_bus() so I don't have any opportunity to pass the
domain number or any additional info (like the sysdata pointer that we were
talking about) to the pci_host_bridge structure unless I do this cleanup.

I think maybe I was too harsh about that, or maybe we had different
ideas about what "conversion" involved.  My comment was in response to
"pci: Introduce pci_register_io_range() helper function", and I don't
remember why I was concerned about that; it's not even in drivers/pci,
and it doesn't have an obvious connection to putting the domain number
in struct pci_host_bridge.

Well, to be honest I did move some of the code (as mentioned in the Changelog) from
drivers/pci into drivers/of. It makes more sense to be in OF, as it mostly concerns
architectures that use it.

The thing I'm more concerned about is adding new PCI interfaces, e.g.,
pci_create_root_bus_in_domain(), that are only used by one
architecture.  Then it's hard to be sure that it's going to be useful
for other arches.  If you can add arm64 using the existing PCI
interfaces, I don't any problem with that.

(No blame here or reproaches, I'm just restating the situation:) I (mostly) did try
that in my v7 series but it also got NAK-ed by Arnd and Catalin as it had too much
arm64 specific code in there.

I don't see a way out of adding new PCI interfaces if we want to have support in
the PCI framework for unifying existing architectures. Of course, there is the painful
alternative of changing the existing APIs and fixing arches in one go, but like you've
said is going to be messy. I don't think I (or the people and companies wanting PCIe
on arm64) should cop out and pick a quick fix that adds sysdata structure into arm64
just to avoid new APIs, as this is not going to help anyone in long term. What I can
do is to create a set of parallel APIs for pci_{scan,create}_root_bus() that take
a pci_host_bridge pointer and start converting architectures one by one to that API
while deprecating the existing one. That way we can add arm64 easily as it would be
the first architecture to use new code without breaking things *and* we provide a
migration path.

Best regards,
Liviu

Bjorn

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