Hi Saravana, On Thu, 11 Apr 2024 16:56:20 -0700 Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > Overlays don't work correctly with fw_devlink. This patch series fixes > it. This series is now ready for review and merging once Geert and Herve > give they Tested-by. > > Geert and Herve, > > This patch series should hopefully fix both of your use cases [1][2][3]. > Can you please check to make sure the device links created to/from the > overlay devices are to/from the right ones? > > Thanks, > Saravana > I tested the series. On my Microchip use case (i.e. DT overlay on a PCIe device), I observed that some driver removal were done in a wrong order. For instance, the onboard PCIe device interrupt controller (oic@e00c0120) was removed before its consumers. I enabled debug traces in core.c and observed that many links were dropped. These links are related to pinctrl, clock, reset, interrupts, ... I have the feeling that these links should not be dropped. Here are extracted traces: --- 8< --- [ 9.225355] mchp_lan966x 0000:01:00.0: enabling device (0000 -> 0002) [ 9.279024] device: 'd0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0': device_add [ 9.286555] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 [ 9.302168] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/reset@e200400c [ 9.317489] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064/tod_pins ... [ 9.403387] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/mdio@e200413c Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/reset@e200400c [ 9.418466] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth3 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 [ 9.433263] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth2 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 ... [ 9.495849] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 [ 9.512111] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 [ 9.527931] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 [ 9.544878] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064/fcb4-i2c-pins ... [ 9.562283] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/reset@e200400c [ 9.577563] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 ... [ 9.592813] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/cpu_clk [ 9.608258] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/ddr_clk [ 9.623731] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sys_clk [ 9.639269] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0 [ 9.651930] device: 'pci:0000:01:00.0--platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0': device_add [ 9.661716] platform d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0: Linked as a sync state only consumer to 0000:01:00.0 ... [ 9.803560] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth2 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 9.816827] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth2 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 [ 9.831309] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth3 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 9.844561] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth3 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 ... [ 9.919384] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 9.934028] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064/fcb4-i2c-pins [ 9.951138] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 9.965012] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064/tod_pins [ 9.980886] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 9.994831] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/reset@e200400c [ 10.009834] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/mdio@e200413c Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.023520] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/mdio@e200413c Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/reset@e200400c [ 10.038248] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/reset@e200400c [ 10.053160] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 [ 10.069820] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.083772] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 [ 10.099739] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Linked as a fwnode consumer to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.114471] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sys_clk [ 10.129648] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/ddr_clk [ 10.144818] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/clock-controller@e00c00a8 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/cpu_clk [ 10.159986] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/pinctrl@e2004064 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 [ 10.174802] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000/i2c@600 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 [ 10.190323] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/oic@e00c0120 [ 10.205098] simple-pm-bus d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0: Dropping the link to 0000:01:00.0 [ 10.214501] device: 'pci:0000:01:00.0--platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0': device_unregister [ 10.225187] device: 'ea000000.switch': device_add [ 10.230321] device: 'platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0--platform:ea000000.switch': device_add [ 10.240806] devices_kset: Moving ea000000.switch to end of list [ 10.246794] platform ea000000.switch: Linked as a consumer to d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0 [ 10.256399] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/switch@e0000000 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.270803] device: 'e800413c.mdio': device_add [ 10.275775] device: 'platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0--platform:e800413c.mdio': device_add [ 10.286036] devices_kset: Moving e800413c.mdio to end of list [ 10.291863] platform e800413c.mdio: Linked as a consumer to d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0 [ 10.301213] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/mdio@e200413c Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.315260] mscc-miim e800413c.mdio: error -EPROBE_DEFER: Failed to get reset ... [ 10.387676] device: 'd0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth3': device_add [ 10.388124] devices_kset: Moving ea000000.switch to end of list [ 10.402096] device: 'platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0--platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth3': device_add [ 10.416004] devices_kset: Moving d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth3 to end of list [ 10.425281] platform d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth3: Linked as a consumer to d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0 [ 10.437942] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth3 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.451352] devices_kset: Moving e800413c.mdio to end of list [ 10.451912] device: 'd0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth2': device_add [ 10.458161] mscc-miim e800413c.mdio: error -EPROBE_DEFER: Failed to get reset [ 10.466501] device: 'platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0--platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth2': device_add [ 10.485906] devices_kset: Moving d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth2 to end of list [ 10.495096] platform d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0:sfp-eth2: Linked as a consumer to d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0 [ 10.507855] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/sfp-eth2 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 ... [ 10.580486] device: 'ea040000.flexcom': device_add [ 10.585753] device: 'platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0--platform:ea040000.flexcom': device_add [ 10.596336] devices_kset: Moving ea040000.flexcom to end of list [ 10.602429] platform ea040000.flexcom: Linked as a consumer to d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0 [ 10.612031] /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0/flexcom@e0040000 Dropping the fwnode link to /soc/pcie@d0070000/pci@0,0/dev@0,0/pci-ep-bus@0 [ 10.626863] device: 'e8004064.pinctrl': device_add [ 10.632045] device: 'platform:d0070000.pcie:pci@0,0:dev@0,0:pci-ep-bus@0--platform:e8004064.pinctrl': device_add ... --- 8< --- Here is the related dtso applied at the PCIe device DT node: --- 8< --- / { fragment@0 { target-path=""; __overlay__ { #address-cells = <3>; #size-cells = <2>; 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>; oic: oic@e00c0120 { compatible = "microchip,lan966x-oic"; #interrupt-cells = <2>; interrupt-controller; interrupts = <0>; /* PCI INTx assigned interrupt */ reg = <0xe00c0120 0x190>; }; cpu_clk: cpu_clk { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <600000000>; // CPU clock = 600MHz }; ddr_clk: ddr_clk { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <30000000>; // Fabric clock = 30MHz }; sys_clk: sys_clk { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <15625000>; // System clock = 15.625MHz }; 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>; }; cpu_ctrl: syscon@e00c0000 { compatible = "microchip,lan966x-cpu-syscon", "syscon"; reg = <0xe00c0000 0xa8>; }; reset: reset@e200400c { compatible = "microchip,lan966x-switch-reset"; reg = <0xe200400c 0x4>; reg-names = "gcb"; #reset-cells = <1>; cpu-syscon = <&cpu_ctrl>; }; gpio: pinctrl@e2004064 { compatible = "microchip,lan966x-pinctrl"; reg = <0xe2004064 0xb4>, <0xe2010024 0x138>; resets = <&reset 0>; reset-names = "switch"; gpio-controller; #gpio-cells = <2>; gpio-ranges = <&gpio 0 0 78>; interrupt-parent = <&oic>; interrupt-controller; interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; #interrupt-cells = <2>; tod_pins: tod_pins { pins = "GPIO_36"; function = "ptpsync_1"; }; fc0_a_pins: fcb4-i2c-pins { /* RXD, TXD */ 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; }; }; 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>; i2c_lan966x: i2c@600 { compatible = "microchip,lan966x-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>; }; }; i2c0_emux: i2c0-emux@0 { compatible = "i2c-mux-pinctrl"; #address-cells = <1>; #size-cells = <0>; i2c-parent = <&i2c_lan966x>; pinctrl-names = "i2c102", "i2c103", "idle"; pinctrl-0 = <&i2cmux_0>; pinctrl-1 = <&i2cmux_1>; pinctrl-2 = <&i2cmux_pins>; i2c102: i2c_sfp1 { reg = <0>; #address-cells = <1>; #size-cells = <0>; }; i2c103: i2c_sfp2 { reg = <1>; #address-cells = <1>; #size-cells = <0>; }; }; sfp_eth2: sfp-eth2 { 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>; }; sfp_eth3: sfp-eth3 { 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>; }; serdes: serdes@e202c000 { compatible = "microchip,lan966x-serdes"; reg = <0xe202c000 0x9c>, <0xe2004010 0x4>; #phy-cells = <2>; }; mdio1: mdio@e200413c { #address-cells = <1>; #size-cells = <0>; compatible = "microchip,lan966x-miim"; reg = <0xe200413c 0x24>, <0xe2010020 0x4>; resets = <&reset 0>; reset-names = "switch"; lan966x_phy0: ethernet-lan966x_phy@1 { reg = <1>; }; lan966x_phy1: ethernet-lan966x_phy@2 { reg = <2>; }; }; switch: switch@e0000000 { compatible = "microchip,lan966x-switch"; reg = <0xe0000000 0x0100000>, <0xe2000000 0x0800000>; reg-names = "cpu", "gcb"; interrupt-parent = <&oic>; interrupts = <12 IRQ_TYPE_LEVEL_HIGH>, <9 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "xtr", "ana"; resets = <&reset 0>; reset-names = "switch"; pinctrl-names = "default"; pinctrl-0 = <&tod_pins>; ethernet-ports { #address-cells = <1>; #size-cells = <0>; port0: port@0 { phy-handle = <&lan966x_phy0>; reg = <0>; phy-mode = "gmii"; phys = <&serdes 0 CU(0)>; }; port1: port@1 { phy-handle = <&lan966x_phy1>; reg = <1>; phy-mode = "gmii"; phys = <&serdes 1 CU(1)>; }; port2: port@2 { reg = <2>; phy-mode = "sgmii"; phys = <&serdes 2 SERDES6G(0)>; sfp = <&sfp_eth2>; managed = "in-band-status"; }; port3: port@3 { reg = <3>; phy-mode = "sgmii"; phys = <&serdes 3 SERDES6G(1)>; sfp = <&sfp_eth3>; managed = "in-band-status"; }; }; }; }; }; }; }; --- 8< --- Best regards, Hervé