On Mon, Apr 07, 2025 at 04:55:44PM +0200, Herve Codina wrote: > Add device-tree nodes needed to support SFPs. > Those nodes are: > - the clock controller > - the i2c controller > - the i2c mux > - the SFPs themselves and their related ports in the switch > > Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx> > --- > drivers/misc/lan966x_pci.dtso | 111 ++++++++++++++++++++++++++++++++++ > 1 file changed, 111 insertions(+) > > diff --git a/drivers/misc/lan966x_pci.dtso b/drivers/misc/lan966x_pci.dtso > index 94a967b384f3..a2015b46cd44 100644 > --- a/drivers/misc/lan966x_pci.dtso > +++ b/drivers/misc/lan966x_pci.dtso What exactly does this DTSO file represent? > @@ -47,6 +47,47 @@ sys_clk: clock-15625000 { > clock-frequency = <15625000>; /* System clock = 15.625MHz */ > }; > > + i2c0_emux: i2c0-emux { > + compatible = "i2c-mux-pinctrl"; > + #address-cells = <1>; > + #size-cells = <0>; > + i2c-parent = <&i2c0>; > + pinctrl-names = "i2c102", "i2c103", "idle"; > + pinctrl-0 = <&i2cmux_0>; > + pinctrl-1 = <&i2cmux_1>; > + pinctrl-2 = <&i2cmux_pins>; > + > + i2c102: i2c@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + i2c103: i2c@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + }; > + > + sfp2: sfp2 { > + compatible = "sff,sfp"; > + i2c-bus = <&i2c102>; > + tx-disable-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>; > + los-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>; > + mod-def0-gpios = <&gpio 18 GPIO_ACTIVE_LOW>; > + tx-fault-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>; DT files are generally hierarchical. There is a soc .dtsi file which describes everything internal to the SoC. And then we have .dts file which describes the board the SoC is placed on. We have a slightly different setup here. A PCI chip instead of a SoC. And a PCI card in a slot, which could be seen as the board. The SFP cage is on the board. How the GPIOs and i2c busses are wired to the SFP cage is a board property, not a SoC/PCI chip property. Different boards could wire them up differently. So to me, it seems wrong these nodes are here. They should be in a dtso file which represents the PCIe board in the slot, and that .dtso file imports the .dtso file which represents the PCIe chip. I suppose this comes down to, what do the PCIe IDs represent, since that is what is used for probing? The PCIe chip, or the board as a whole. When somebody purchases the chips from Microchip, and builds their own board, are they required to have their own PCIe IDs, and a .dtso file representing their board design? The PCIe chip part should be reusable, so we are talking about stacked dtso files, or at least included .dtso files. Andrew