[PATCH 0/3] Add PCI domain support to R-Car drivers

[Phil Edworthy]

Hi Liviu,

On 23 September 2014 12:10, Liviu wrote:
> On Tue, Sep 23, 2014 at 12:00:41PM +0100, Phil Edworthy wrote:
> > Hi Liviu,
> >
> > On 23 September 2014 11:32, Liviu wrote:
> > > On Tue, Sep 23, 2014 at 11:10:29AM +0100, Phil Edworthy wrote:
> > > > Hi Bjorn,
> > > >
> > > > On 22 September 2014 22:01, Bjorn wrote:
> > > > > On Mon, Sep 22, 2014 at 10:51:07AM +0100, Phil Edworthy wrote:
> > > > > > The Renesas R-Car devices (r8a7790 and r8a7791) use two PCI controller
> > > > > drivers,
> > > > > > one for an external PCIe slot, the other for an internal PCI bridge to USB
> > > > > > controllers.
> > > > > >
> > > > > > However, they currently do not work at the same time as they use the
> > > > > same PCI
> > > > > > domain and use the same root bus number. We can't use different root
> > > > > bus numbers
> > > > > > due to the way root bus numbers are assigned in pcibios_init_hw() in
> > > > > > arch/arm/kernel/bios32.c.
> > > > > >
> > > > > > Since the two PCI controllers are completely independent, I think it makes
> > > > > sense
> > > > > > to use different PCI domains for them.
> > > > > >
> > > > > > I've marked the third patch as RFC as I am not sure of the impact of
> > > > > enabling
> > > > > > PCI domains for all ARM devices. In the march to 'one kernel to rule them
> > > > > all',
> > > > > > I steered clear of mach specific changes.
> > > > > >
> > > > > > These patches require the following patch from Liviu Dudau:
> > > > > >   [PATCH v11 07/10] OF: Introduce helper function for getting PCI
> > > > > domain_nr
> > > > > > Based on comments on this patch from Jason Gunthorpe, there is still the
> > > > > issue
> > > > > > that the domain numbers may change depending on the ordering at probe
> > > > > time.
> > > > > > However, this can be fixed later on by adding the entries in the DT files.
> > > > > >
> > > > > >
> > > > > > Phil Edworthy (3):
> > > > > >   PCI: rcar-pcie: Add call to get domain nr
> > > > > >   PCI: rcar-internal-pci: Add call to get domain nr
> > > > > >   ARM: Enable PCI domains
> > > > >
> > > > > I'm deferring these for now because they depend on Liviu's work, which I
> > > > > haven't merged yet, and I suspect some minor adaptation will be required
> > > > > here.
> > > > >
> > > > > For what it's worth, I agree with Rob's hesitation about mixing lookup with
> > > > > domain number allocation in of_pci_get_domain_nr().  That seems
> > > > > unnecessarily complicated.
> > > > I could create patches to add an optional "pci-domain" property for the R-Car
> > > > PCI drivers, and just attempt to get the property in the drivers. If not found,
> > > > the drivers will assume the domain is 0.
> > > >
> > > > We would then have fixed PCI domain numbering and I don't have to worry about
> > > > Liviu's work.
> > >
> > > I will split the current of_pci_get_domain_nr() even further and replace it with
> > > two functions: pci_get_domain_nr() which will do just the allocation (still based
> > > on the boolean flag passed as parameter) and of_get_pci_domain_nr() that will
> > > retrieve a "linux,pci-domain" value from a property belonging to a given device
> > > node.
> > This doesn't solve the problem of different domain numbers based on different
> > probe ordering. If you have multiple domains then I think you must have the
> > "linux,pci-domain" property for each controller.
> 
> Correct, but I've said I'm going to leave for the moment the check to the host
> controller(s).
> So if you care about ordering or mandating the presence of "linux,pci-
> domain" in the DT, please add the check in your driver.
>
> >
> > If we assume the above, of_get_pci_domain_nr() can just return a bus number of
> > 0 if the "linux,pci-domain" property doesn't exist.
> 
> You mean *domain* number of 0. But that is still a valid value and you won't
> be able to distinguish between DT providing a "linux,pci-domain" value of zero and
> the property not being present.
Yes, I meant domain.
If a device only has a single PCI domain, no "linux,pci-domain" prop is needed
and a default domain number of 0 is ok.
If there is more than one domain, userspace needs to have consistent domain
numbering over kernel versions, so we have to specify the domain in the DT.

I am probably missing something, it wouldn’t be the first time, but I don’t see
why of_get_pci_domain_nr() needs to allocate domain numbers.
 
> >
> > I suppose you could have a temporary solution that allocates the domain numbers
> > until the necessary drivers have been updated.
> 
> And that is what pci_get_domain_nr() will do. If you don't care about ordering, one
> will use that one. If you care about ordering then "linux,pci-domain" is required
> and one can use of_get_pci_domain_nr() to retrieve it and fail if the information
> is missing.
> 
> Does that look like a workable solution?
> 
> Best regards,
> Liviu
> 
> >
> > > I plan to leave for the moment the check for mandatory presence of
> > > "linux,pci-domain"
> > > to the host bridge driver so that everyone can implement their own
> policies.
> > >
> > > How does that sound?
> >

Thanks
Phil