Hi,
On Fri, 30 Aug 2024 14:37:54 -0500
Rob Herring <robh@xxxxxxxxxx> wrote:
...
> > this view is much like Bootlin's approach, also my pci-ep-bus node now would look
> > like this:
> > ...
> > pci-ep-bus@0 {
> > ranges = <0xc0 0x40000000
> > 0x01 0x00 0x00000000
> > 0x00 0x00400000>;
> > ...
> > };
> >
> > and also the correct unit address here is 0 again, since the parent address in
> > ranges is 0x01 0x00 0x00000000 (0x01 is the flags and in this case represent
> > BAR1, I assume that for the unit address I should use only the address part that
> > is 0, right?).
>
> No, it should be 1 for BAR1. It's 1 node per BAR.
It should be 1 node per BAR but in some cases it is not.
Indeed, in the LAN966x case, the pci-ep-bus need to have access to several
BARs and we have:
...
pci-ep-bus@0 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
/*
* map @0xe2000000 (32MB) to BAR0 (CPU)
* map @0xe0000000 (16MB) to BAR1 (AMBA)
*/
ranges = <0xe2000000 0x00 0x00 0x00 0x2000000
0xe0000000 0x01 0x00 0x00 0x1000000>;
...
Some devices under this bus need to use both BARs and use two regs values
in their reg properties to access BAR0 and BAR1.
> > > > > The assumption so far with all of this is that you have some specific
> > > > > PCI device (and therefore a driver). The simple-buses under it are
> > > > > defined per BAR. Not really certain if that makes sense in all cases,
> > > > > but since the address assignment is dynamic, it may have to. I'm also
> > > > > not completely convinced we should reuse 'simple-bus' here or define
> > > > > something specific like 'pci-bar-bus' or something.
> > > >
> > > > Good point. Labeling a new bus for this kind of 'appliance' could be
> > > > beneficial to unify the dt overlay approach, and I guess it could be
> > > > adopted by the aforementioned Bootlin's Microchip patchset too.
> > > > However, since the difference with simple-bus would be basically non
> > > > existent, I believe that this could be done in a future patch due to
> > > > the fact that the dtbo is contained into the driver itself, so we do
> > > > not suffer from the proliferation that happens when dtb are managed
> > > > outside.
> > >
> > > It's an ABI, so we really need to decide first.
> >
> > Okay. How should we proceed?
>
> I think simple-bus where you have it is fine. It is really 1 level up
> that needs to be specified. Basically something that's referenced from
> the specific PCI device's schema (e.g. the RP1 schema (which you are
> missing)).
>
> That schema needs to roughly look like this:
>
> properties:
> "#address-cells":
> const: 3
> "#size-cells":
> const: 2
> ranges:
> minItems: 1
> maxItems: 6
> items:
> additionalItems: true
> items:
> - maximum: 5 # The BAR number
> - const: 0
> - const: 0
> - # TODO: valid PCI memory flags
>
> patternProperties:
> "^bar-bus@[0-5]$":
> type: object
> additionalProperties: true
> properties:
> compatible:
> const: simple-bus
> ranges: true
>
IMHO, the node should not have 'bar' in the name.
In the LAN966x PCI use case, multiple BARs have to be accessed by devices
under this simple-bus. That's why I choose pci-ep-bus for this node name.
Best regards,
Hervé
