mbox series

[v3,0/2] fw_devlink overlay fix

Message ID 20240411235623.1260061-1-saravanak@google.com
Headers show
Series fw_devlink overlay fix | expand

Message

Saravana Kannan April 11, 2024, 11:56 p.m. UTC
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

Saravana Kannan (2):
  Revert "treewide: Fix probing of devices in DT overlays"
  of: dynamic: Fix overlayed devices not probing because of fw_devlink

 drivers/base/core.c       | 76 ++++++++++++++++++++++++++++++++++-----
 drivers/bus/imx-weim.c    |  6 ----
 drivers/i2c/i2c-core-of.c |  5 ---
 drivers/of/dynamic.c      |  1 -
 drivers/of/overlay.c      | 15 ++++++++
 drivers/of/platform.c     |  5 ---
 drivers/spi/spi.c         |  5 ---
 include/linux/fwnode.h    |  1 +
 8 files changed, 83 insertions(+), 31 deletions(-)

Comments

Herve Codina April 23, 2024, 8:39 a.m. UTC | #1
Hi Saravana,

On Thu, 11 Apr 2024 16:56:20 -0700
Saravana Kannan <saravanak@google.com> 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é
Geert Uytterhoeven April 24, 2024, 1:40 p.m. UTC | #2
Hi Saravana,

On Fri, Apr 12, 2024 at 1:56 AM Saravana Kannan <saravanak@google.com> 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?

Unfortunately it doesn't, and the result is worse than v2.

After applying the first patch (the revert), the issue reported in
[1] is back, as expected.

After applying both patches, that issue is not fixed, i.e. I still
need an add/rm/add cycle to instantiate the devices from the overlay.

/sys/class/devlink shows one extra link after the first add:
platform:e6060000.pinctrl--platform:e6e90000.spi ->
../../devices/virtual/devlink/platform:e6060000.pinctrl--platform:e6e90000.spi

> [1] - https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert
Herve Codina July 12, 2024, 1:34 p.m. UTC | #3
Hi Saravana,

On Tue, 23 Apr 2024 10:39:24 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Saravana,
> 
> On Thu, 11 Apr 2024 16:56:20 -0700
> Saravana Kannan <saravanak@google.com> 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.
> 

Have you made any progress on this topic ?

I haven't seen any updates.
Maybe I missed something.

Best regards,
Hervé