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

quoted

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