| Message ID | 20250407145546.270683-16-herve.codina@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/16] Revert "treewide: Fix probing of devices in DT overlays" | expand |
Andrew, Hervé, On Tue Apr 8, 2025 at 4:26 PM CEST, Herve Codina wrote: >> What exactly does this DTSO file represent? > > The dsto represents de board connected to the PCI slot and identified > by its PCI vendor/device IDs. If I may extend on that by providing what I believe is a more accurate/precise definition. The DTSO doesn't represent the board, rather it describes the HW topology of the devices inside the PCI endpoint. Indeed, the PCI endpoint is a full-blown SoC with lots of different HW blocks that already have drivers in the kernel (because the same chip can be used with Linux running on an ARM core embedded in the SoC, rather than access as a PCI endpoint). So the DTSO describes the full topology of the HW blocks inside this complex PCI endpoint, just like the DTS describes the full topology of the HW blocks inside an SoC. Please see: https://lpc.events/event/17/contributions/1421/attachments/1337/2680/LPC2023%20Non-discoverable%20devices%20in%20PCI.pdf And most notably slide 6. Best regards, Thomas
On Tue Apr 8, 2025 at 5:38 PM CEST, Andrew Lunn wrote: > "HW blocks inside an SoC." That would be the SoC .dtsi file. Anything > outside of the SoC is in the .dts file. OEM vendors take the SoC, > build a board around it, and name there .dts file after the board, > describing how the board components are connected to the SoC. > > So.. > > So by PCI endpoint, you mean the PCIe chip? So it sounds like there > should be a .dtsi file describing the chip. > > Everything outside of the chip, like the SFP cages, are up to the > vendor building the board. I would say that should be described in a > .dtso file, which describes how the board components are connected to > the PCIe chip? And that .dtso file should be named after the board, > since there are going to many of them, from different OEM vendors. Indeed, that makes sense. So if I get correctly your suggestion, instead of having a .dtso that describes everything, it should be split between: - A .dtsi that describes what's inside the LAN996x when used in PCI endpoint mode - A .dtso that includes the above .dtsi, and that describes what on the PCI board around the LAN966x. Correct? Thomas
On Wed, 9 Apr 2025 at 09:44, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > On Tue Apr 8, 2025 at 5:38 PM CEST, Andrew Lunn wrote: > > "HW blocks inside an SoC." That would be the SoC .dtsi file. Anything > > outside of the SoC is in the .dts file. OEM vendors take the SoC, > > build a board around it, and name there .dts file after the board, > > describing how the board components are connected to the SoC. > > > > So.. > > > > So by PCI endpoint, you mean the PCIe chip? So it sounds like there > > should be a .dtsi file describing the chip. > > > > Everything outside of the chip, like the SFP cages, are up to the > > vendor building the board. I would say that should be described in a > > .dtso file, which describes how the board components are connected to > > the PCIe chip? And that .dtso file should be named after the board, > > since there are going to many of them, from different OEM vendors. > > Indeed, that makes sense. So if I get correctly your suggestion, > instead of having a .dtso that describes everything, it should be > split between: > > - A .dtsi that describes what's inside the LAN996x when used in PCI > endpoint mode > > - A .dtso that includes the above .dtsi, and that describes what on > the PCI board around the LAN966x. > > Correct? Sounds good to me! Gr{oetje,eeting}s, Geert
On Wed, Apr 09, 2025 at 09:44:25AM +0200, Thomas Petazzoni wrote: > On Tue Apr 8, 2025 at 5:38 PM CEST, Andrew Lunn wrote: > > > "HW blocks inside an SoC." That would be the SoC .dtsi file. Anything > > outside of the SoC is in the .dts file. OEM vendors take the SoC, > > build a board around it, and name there .dts file after the board, > > describing how the board components are connected to the SoC. > > > > So.. > > > > So by PCI endpoint, you mean the PCIe chip? So it sounds like there > > should be a .dtsi file describing the chip. > > > > Everything outside of the chip, like the SFP cages, are up to the > > vendor building the board. I would say that should be described in a > > .dtso file, which describes how the board components are connected to > > the PCIe chip? And that .dtso file should be named after the board, > > since there are going to many of them, from different OEM vendors. > > Indeed, that makes sense. So if I get correctly your suggestion, > instead of having a .dtso that describes everything, it should be > split between: > > - A .dtsi that describes what's inside the LAN996x when used in PCI > endpoint mode > > - A .dtso that includes the above .dtsi, and that describes what on > the PCI board around the LAN966x. > > Correct? Yes. And you need some way to map the PCI ID to the correct .dtso file. Maybe that is just a lookup table in the driver, or maybe you can pack the .dtso file into a kernel module with the correct MODULE_DEVICE_TABLE(pci, ...) so that PCI probing pulls in the specific driver module with the .dtso, which via dependencies pulls in the core driver which can actually make use of the .dtso? Andrew
On Wed, 9 Apr 2025 16:04:51 +0200 Andrew Lunn <andrew@lunn.ch> wrote: > And you need some way to map the PCI ID to the correct .dtso file. > Maybe that is just a lookup table in the driver, or maybe you can pack > the .dtso file into a kernel module with the correct > MODULE_DEVICE_TABLE(pci, ...) so that PCI probing pulls in the > specific driver module with the .dtso, which via dependencies pulls in > the core driver which can actually make use of the .dtso? Well, check the already upstream driver: https://elixir.bootlin.com/linux/v6.13.7/source/drivers/misc/lan966x_pci.c It indeed binds on the PCI ID, and the driver bundles the .dtbo. Best regards, Thomas
On Wed, Apr 09, 2025 at 04:14:44PM +0200, Thomas Petazzoni wrote: > On Wed, 9 Apr 2025 16:04:51 +0200 > Andrew Lunn <andrew@lunn.ch> wrote: > > > And you need some way to map the PCI ID to the correct .dtso file. > > Maybe that is just a lookup table in the driver, or maybe you can pack > > the .dtso file into a kernel module with the correct > > MODULE_DEVICE_TABLE(pci, ...) so that PCI probing pulls in the > > specific driver module with the .dtso, which via dependencies pulls in > > the core driver which can actually make use of the .dtso? > > Well, check the already upstream driver: > > https://elixir.bootlin.com/linux/v6.13.7/source/drivers/misc/lan966x_pci.c > > It indeed binds on the PCI ID, and the driver bundles the .dtbo. So it only supports a single .dtbo. In its current form it does not scale to multiple .dtso files for multiple different boards built around the PCIe chip. At the moment, that is not really an issue, but when the second board comes along, some refactoring will be needed. Andrew
On Wed, 9 Apr 2025 17:03:45 +0200 Andrew Lunn <andrew@lunn.ch> wrote: > So it only supports a single .dtbo. In its current form it does not > scale to multiple .dtso files for multiple different boards built > around the PCIe chip. > > At the moment, that is not really an issue, but when the second board > comes along, some refactoring will be needed. Indeed, but that's really an implementation detail. It doesn't change anything to the overall approach. The only thing that would have to change is how the driver gets the .dtbo. We could bundle several .dtbos in the driver, we could fall back to request_firmware(), etc. Best regards, Thomas
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 @@ -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>; + }; + + sfp3: sfp3 { + compatible = "sff,sfp"; + i2c-bus = <&i2c103>; + tx-disable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>; + los-gpios = <&gpio 26 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&gpio 19 GPIO_ACTIVE_LOW>; + tx-fault-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>; + }; + pci-ep-bus@0 { compatible = "simple-bus"; #address-cells = <1>; @@ -95,6 +136,50 @@ port1: port@1 { phy-mode = "gmii"; phys = <&serdes 1 CU(1)>; }; + + port2: port@2 { + reg = <2>; + phy-mode = "sgmii"; + phys = <&serdes 2 SERDES6G(0)>; + sfp = <&sfp2>; + managed = "in-band-status"; + }; + + port3: port@3 { + reg = <3>; + phy-mode = "sgmii"; + phys = <&serdes 3 SERDES6G(1)>; + sfp = <&sfp3>; + managed = "in-band-status"; + }; + }; + }; + + flx0: flexcom@e0040000 { + compatible = "atmel,sama5d2-flexcom"; + reg = <0xe0040000 0x100>; + clocks = <&clks GCK_ID_FLEXCOM0>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0xe0040000 0x800>; + + atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; + + i2c0: i2c@600 { + compatible = "microchip,sam9x60-i2c"; + reg = <0x600 0x200>; + interrupt-parent = <&oic>; + interrupts = <48 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&clks GCK_ID_FLEXCOM0>; + assigned-clocks = <&clks GCK_ID_FLEXCOM0>; + assigned-clock-rates = <20000000>; + pinctrl-0 = <&fc0_a_pins>; + pinctrl-names = "default"; + i2c-analog-filter; + i2c-digital-filter; + i2c-digital-filter-width-ns = <35>; }; }; @@ -103,6 +188,14 @@ cpu_ctrl: syscon@e00c0000 { reg = <0xe00c0000 0xa8>; }; + clks: clock-controller@e00c00a8 { + compatible = "microchip,lan966x-gck"; + #clock-cells = <1>; + clocks = <&cpu_clk>, <&ddr_clk>, <&sys_clk>; + clock-names = "cpu", "ddr", "sys"; + reg = <0xe00c00a8 0x38>, <0xe00c02cc 0x4>; + }; + oic: oic@e00c0120 { compatible = "microchip,lan966x-oic"; #interrupt-cells = <2>; @@ -143,6 +236,24 @@ fc0_a_pins: fcb4-i2c-pins { pins = "GPIO_9", "GPIO_10"; function = "fc0_a"; }; + + i2cmux_pins: i2cmux-pins { + pins = "GPIO_76", "GPIO_77"; + function = "twi_slc_gate"; + output-low; + }; + + i2cmux_0: i2cmux-0 { + pins = "GPIO_76"; + function = "twi_slc_gate"; + output-high; + }; + + i2cmux_1: i2cmux-1 { + pins = "GPIO_77"; + function = "twi_slc_gate"; + output-high; + }; }; mdio1: mdio@e200413c {
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@bootlin.com> --- drivers/misc/lan966x_pci.dtso | 111 ++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+)