On Tue, Dec 24, 2024 at 11:49:42AM +0200, Dmitry Baryshkov wrote:
> On Tue, Dec 24, 2024 at 02:41:10PM +0530, Krishna Chaitanya Chundru wrote:
> > On 12/5/2024 2:55 AM, Bjorn Helgaas wrote:
> > > On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote:
> > > > Add binding describing the Qualcomm PCIe switch, QPS615,
> > > > which provides Ethernet MAC integrated to the 3rd downstream port
> > > > and two downstream PCIe ports.
> > > > + pcie {
> > > > + #address-cells = <3>;
> > > > + #size-cells = <2>;
> > > > +
> > > > + pcie@0 {
> > > > + device_type = "pci";
> > > > + reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > +
> > > > + #address-cells = <3>;
> > > > + #size-cells = <2>;
> > > > + ranges;
> > > > + bus-range = <0x01 0xff>;
> > > > +
> > > > + pcie@0,0 {
> > > > + compatible = "pci1179,0623";
> > > > + reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > + device_type = "pci";
> > > > + #address-cells = <3>;
> > > > + #size-cells = <2>;
> > > > + ranges;
> > > > + bus-range = <0x02 0xff>;
> > >
> > > This binding describes a switch. I don't think bus-range should
> > > appear here at all because it is not a feature of the hardware (unless
> > > the switch ports are broken and their Secondary/Subordinate Bus
> > > Numbers are hard-wired).
> > >
> > > The Primary/Secondary/Subordinate Bus Numbers of all switch ports
> > > should be writable and the PCI core knows how to manage them.
> >
> > The dt binding check is throwing an error if we don't keep bus-range
> > property for that reason we added it, from dt binding perspective i think it
> > is mandatory to add this property.
>
> Could you please provide an error message? I don't see any of the PCIe
> bindingins declaring bus-range as mandatory. I might be missing it
> though.
I think the warning message is like this:
Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
and only happens if there's a device below a Root Port or a Switch.
In that case the device "reg" property apparently has to include the
bus/device/function.
IIUC, in this case, we're describing a Switch with an integrated
Ethernet MAC:
pcie@0 {
device_type = "pci";
reg = <0x0 0x0 0x0 0x0 0x0>; # 00:00.0 RP to [bus 01-ff]
bus-range = <0x01 0xff>;
pcie@0,0 {
compatible = "pci1179,0623";
reg = <0x10000 0x0 0x0 0x0 0x0>; # 01:00.0 Switch USP to [bus 02-ff]
device_type = "pci";
bus-range = <0x02 0xff>;
pcie@1,0 {
reg = <0x20800 0x0 0x0 0x0 0x0>; # 02:01.0 Switch DSP to [bus 03-ff]
device_type = "pci";
bus-range = <0x03 0xff>;
qcom,no-dfe-support;
};
pcie@2,0 {
reg = <0x21000 0x0 0x0 0x0 0x0>; # 02:02.0 Switch DSP to [bus 04-ff]
device_type = "pci";
bus-range = <0x04 0xff>;
qcom,nfts = <10>;
};
pcie@3,0 {
reg = <0x21800 0x0 0x0 0x0 0x0>; # 02:02.1 Switch DSP to [bus 05-ff]
device_type = "pci";
bus-range = <0x05 0xff>;
qcom,tx-amplitude-millivolt = <10>;
pcie@0,0 {
reg = <0x50000 0x0 0x0 0x0 0x0>; # 05:00.0 Ethernet MAC, I guess?
device_type = "pci";
qcom,l1-entry-delay-ns = <10>;
};
...
};
};
};
So I think the bus-range properties are needed to match the reg
properties of the downstream devices.
I do think the bus-ranges of the Switch Downstream Ports look bogus
because they all extend to bus ff, so they overlap. The Switch
wouldn't know how to route config transactions to the correct DSP.
I suppose the PCI core would fix these overlaps at boot time, but
it seems wrong to describe them this way here.
There's an example "reg" decoding and a couple URLs here:
https://lore.kernel.org/r/20250106230705.GA132316@bhelgaas
Bjorn
