Re: [PATCH v7 01/10] PCI: host: rcar: Add Renesas R-Car PCIe driver

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Thu, May 01, 2014 at 09:50:56AM +0000, Phil Edworthy wrote:

> > The logic should be:
> >  if (bus = primary)
> >     do io access to host bridge
> >  else if (bus = secondary)
> >     issue type 0 TLP on the wire
> >  else if (bus > secondary && bus <= subordinate)
> >     issue type 1 TLP on the wire
> >  else
> >     fail, invalid bus number
> > Where the three values come from the register in the PCI host bridge's
> > configuration space, and are kept in sync with the programming from
> > the Linux PCI core.
> > 
> > It is just a happy hapenstance that root_bus_nr equals the value the
> > PCI core programmed into secondary - that is not guarenteed, you must
> > use the primary value directly.

> For type0 TLPs, we are not checking that root_bus_nr equals the
> value the PCI core programmed into secondary, we are checking that
> the (root_bus_nr = bus->parent->number). The only way this wouldn't
> work is if root_bus_nr was not the root bus number.

Okay, that isn't as sketchy, but that process still ignores the
subordinate bus number and the failure case as required by PCI.

The goal here is to have the stuff below the drivers implement the PCI
spec so that the core code can assume everything below is
conformant. Drivers should not introduce gratuitous differences 'just
because'

There is no reason drivers should be using PCI core structures to make
decisions when the spec says those decisions are driven by config
space fields.

This way the PCI core code doesn't have to be aware of any weird
non-standard edge cases.. Such as not failing bus numbers beyond the
subordinate bus number.

> Since the Synopsys DW driver also saves off sys->busnr and later
> uses this to determine if accesses are for the host bridge, I guess
> that means it won't always work either, right? Or is it ok because
> the DW driver saves sys->busnr in its .scan function?

Sounds like it is making the same mistake, and nobody noticed. This is
another reason why it is important to implement correctly so people
copying copy the right stuff :)

> When would the PCI core change the root bus number to something
> other than set in sys->busnr?

I think the more likely scenario is that 'sys' in general is
architecture specific and its use is being discouraged so that host
drivers are not arch specific.

A domain driver like rcar should always place the root complex
integrated bus as bus 0 in the domain.

Jason