[PATCH v3 00/28] lan966x pci device: Add support for SFPs

[PATCH v3 00/28] lan966x pci device: Add support for SFPs

[Herve Codina]

Hi,

This series add support for SFPs ports available on the LAN966x PCI
device. In order to have the SFPs supported, additional devices are
needed such as clock controller and I2C.

As a reminder, the LAN966x PCI device driver use a device-tree overlay
to describe devices available on the PCI board. Adding support for SFPs
ports consists in adding more devices in the already existing
device-tree overlay.

With those devices added, the device-tree overlay is more complex and
some consumer/supplier relationship are needed in order to remove
devices in correct order when the LAN966x PCI driver is removed.

Those links are typically provided by fw_devlink and we faced some
issues with fw_devlink and overlays.

This series gives the big picture related to the SFPs support from
fixing issues to adding new devices. Of course, it can be split if
needed.

The first part of the series (patch 1, 2 and 3) fixes fw_devlink when it
is used with overlay. Patches 1 and 3 were previously sent by Saravana
[0]. I just rebased them on top of v6.15-rc1 and added patch 2 in order
to take into account feedback received on the series sent by Saravana.

Those modification were not sufficient in our case and so, on top of
that, patch 4 and 5 fix some more issues related to fw_devlink.

Patches 6 to 12 introduce and use fw_devlink_set_device() in already
existing code.

Patches 13 and 14 are related also to fw_devlink but specific to PCI and
the device-tree nodes created during enumeration.

Patches 15, 15 and 17 are related fw_devlink too but specific to I2C
muxes. Patches purpose is to correctly set a link between an adapter
supplier and its consumer. Indeed, an i2c mux adapter's parent is not
the i2c mux supplier but the adapter the i2c mux is connected to. Adding
a new link between the adapter supplier involved when i2c muxes are used
avoid a freeze observed during device removal.

Patch 18 adds support for fw_delink on x86. fw_devlink is needed to have
the consumer/supplier relationship between devices in order to ensure a
correct device removal order. Adding fw_devlink support for x86 has been
tried in the past but was reverted [1] because it broke some systems.
Instead of enabling fw_devlink on *all* x86 system or on *all* x86
system except on those where it leads to issue, enable it only on system
where it is needed.

Patches 19 and 20 allow to build clock and i2c controller used by the
LAN966x PCI device when the LAN966x PCI device is enabled.

Patches 21 to 25 are specific to the LAN966x. They touch the current
dtso, split it in dtsi/dtso files, rename the dtso and improve the
driver to allow easier support for other boards.

The next patch (patch 26) update the LAN966x device-tree overlay itself
to have the SPF ports and devices they depends on described.

The last two patches (patches 27 and 28) sort the existing drivers in
the needed driver list available in the Kconfig help and add new drivers
in this list keep the list up to date with the devices described in the
device-tree overlay.

Once again, this series gives the big picture and can be split if
needed. Let me know.

[0] https://lore.kernel.org/lkml/20240411235623.1260061-1-saravanak@google.com/
[1] https://lore.kernel.org/lkml/3c1f2473-92ad-bfc4-258e-a5a08ad73dd0@web.de/

Compare to previous iteration, this v3 series mainly:
 - Rebase of top of v6.16-rc1
 - Fix typos
 - Improve code and add a missing check
 - Add 'Acked-by' and 'Reviewed-by' tags

Best regards,
Hervé

Changes:

v2 -> v3
  v2: https://lore.kernel.org/all/20250507071315.394857-1-herve.codina@bootlin.com/

  - Patch 1:
    Add 'Acked-by: Mark Brown'

  - Patch 2 and 3:
    No changes

  - Patch 4:
    Rewrite the WARN_ON() condition to avoid an additional 'if'

  - Patch 5:
    Fix typos in commit log
    Update a comment
    Remove the unneeded check before calling of_platform_depopulate()

  - Patches 6 to 11:
    No changes

  - Patch 12 (new in v3)
    Tag the fwnode dev member as private

  - Patch 13 (12 in v2)
    Fix a typo in the commit log

  - Patches 14 to 16 (13 to 15 in v2)
    No changes

  - Patch 17 (16 in v2)
    Check parent_physdev for NULL

  - Patch 18 (17 in v2)
    Capitalize "Link:"
    Add 'Reviewed-by: Andy Shevchenko'

  - Patch 19 (18 in v2)
    No changes

  - Patch 20 (19 in v2)
    Add 'Acked-by: Andi Shyti'

  - Patch 21 (20 in v2)
    No changes

  - Patch 22 (21 in v2)
    Add 'Reviewed-by: Andrew Lunn'

  - Patch 23 (22 in v2)
    Add 'Reviewed-by: Andrew Lunn'

  - Patch 24 (new in v3)
    Introduce PCI_DEVICE_ID_EFAR_LAN9662, the LAN966x PCI device ID

  - Patch 25 (23 in v2)
    Add 'Reviewed-by: Andrew Lunn'
    Use PCI_DEVICE_DATA() with PCI_DEVICE_ID_EFAR_LAN9662 instead of
    PCI_VDEVICE()

  - Patch 26 to 28 (24 to 26 in v2)
    No changes

v1 -> v2
  v1: https://lore.kernel.org/lkml/20250407145546.270683-1-herve.codina@bootlin.com/

  - Patch 1 and 3
    Remove 'From' tag from the commit log

  - Patch 2
    Add 'Reviewed-by: Andy Shevchenko'
    Add 'Reviewed-by: Saravana Kannan'
    Add 'Reviewed-by: Luca Ceresoli'

  - Patch 4 and 5
    No changes

  - Patch 6 (new in v2)
    Introduce fw_devlink_set_device()

  - Patch 7 (new in v2)
    Use existing device_set_node() helper.

  - Patch 8 to 11 (new in v2)
    Use fw_devlink_set_device() in existing code.

  - Patch 12 (6 in v1)
    Use fw_devlink_add_device()

  - Patch 13 (7 in v1)
    No changes

  - Patch 14 (8 in v1)
    Update commit log
    Use 'physdev' instead of 'supplier'
    Minor fixes in i2c_get_adapter_physdev() kdoc

  - Patch 15 and 16 (9 and 10 in v1)
    Use 'physdev' instead of 'supplier' (commit log, title and code)

  - Patch 17 (11 in v2)
    Enable fw_devlink on x86 only if PCI_DYNAMIC_OF_NODES is enabled.
    Rework commit log.

  - Patch 18, 19 and 20 (12, 13 and 14 in v1)
    No changes

  - Patch 21 (new in v2)
    Split dtso in dtsi/dtso

  - Patch 22 (new in v2)
    Rename lan966x_pci.dtso using the specific board name

  - Patch 23 (new in v2)
    Improve the driver introducing board specific data to ease support
    for other boards (avoid the direct dtbo reference in the function
    loading the dtbo).

  - Patch 24 (15 in v1)
    Refactor due to dtso split in dtsi/dtso

  - Patch 25 (new in v2)
    Sort exist driver list in Kconfig help

  - Patch 26 (16 in v1)
    Keep alphanumeric order for new drivers added in Kconfig help

Herve Codina (26):
  driver core: Rename get_dev_from_fwnode() wrapper to
    get_device_from_fwnode()
  driver core: Avoid warning when removing a device while its supplier
    is unbinding
  bus: simple-pm-bus: Populate child nodes at probe
  driver core: fw_devlink: Introduce fw_devlink_set_device()
  drivers: core: Use fw_devlink_set_device()
  pinctrl: cs42l43: Use fw_devlink_set_device()
  cxl/test: Use device_set_node()
  cxl/test: Use fw_devlink_set_device()
  PCI: of: Use fw_devlink_set_device()
  driver core: fw_devlink: Tag the fwnode dev member as private
  PCI: of: Set fwnode device of newly created PCI device nodes
  PCI: of: Remove fwnode_dev_initialized() call for a PCI root bridge
    node
  i2c: core: Introduce i2c_get_adapter_physdev()
  i2c: mux: Set adapter physical device
  i2c: mux: Create missing devlink between mux and adapter physical
    device
  of: property: Allow fw_devlink device-tree on x86 when PCI device-tree
    node creation is enabled
  clk: lan966x: Add MCHP_LAN966X_PCI dependency
  i2c: busses: at91: Add MCHP_LAN966X_PCI dependency
  misc: lan966x_pci: Fix dtso nodes ordering
  misc: lan966x_pci: Split dtso in dtsi/dtso
  misc: lan966x_pci: Rename lan966x_pci.dtso to
    lan966x_evb_lan9662_nic.dtso
  PCI: Add Microchip LAN9662 PCI Device ID
  misc: lan966x_pci: Introduce board specific data
  misc: lan966x_pci: Add dtsi/dtso nodes in order to support SFPs
  misc: lan966x_pci: Sort the drivers list in Kconfig help
  misc: lan966x_pci: Add drivers needed to support SFPs in Kconfig help

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

 MAINTAINERS                               |   3 +-
 drivers/base/core.c                       |  97 +++++++++---
 drivers/bus/imx-weim.c                    |   6 -
 drivers/bus/simple-pm-bus.c               |  23 +--
 drivers/clk/Kconfig                       |   2 +-
 drivers/i2c/busses/Kconfig                |   2 +-
 drivers/i2c/i2c-core-base.c               |  16 ++
 drivers/i2c/i2c-core-of.c                 |   5 -
 drivers/i2c/i2c-mux.c                     |  26 ++++
 drivers/misc/Kconfig                      |  11 +-
 drivers/misc/Makefile                     |   2 +-
 drivers/misc/lan966x_evb_lan9662_nic.dtso | 167 ++++++++++++++++++++
 drivers/misc/lan966x_pci.c                |  30 +++-
 drivers/misc/lan966x_pci.dtsi             | 172 +++++++++++++++++++++
 drivers/misc/lan966x_pci.dtso             | 177 ----------------------
 drivers/of/dynamic.c                      |   1 -
 drivers/of/overlay.c                      |  15 ++
 drivers/of/platform.c                     |   5 -
 drivers/of/property.c                     |   2 +-
 drivers/pci/of.c                          |  10 +-
 drivers/pci/quirks.c                      |   2 +-
 drivers/pinctrl/cirrus/pinctrl-cs42l43.c  |   2 +-
 drivers/spi/spi.c                         |   5 -
 include/linux/fwnode.h                    |   9 +-
 include/linux/i2c.h                       |   3 +
 include/linux/pci_ids.h                   |   1 +
 tools/testing/cxl/test/cxl.c              |   4 +-
 27 files changed, 547 insertions(+), 251 deletions(-)
 create mode 100644 drivers/misc/lan966x_evb_lan9662_nic.dtso
 create mode 100644 drivers/misc/lan966x_pci.dtsi
 delete mode 100644 drivers/misc/lan966x_pci.dtso

-- 
2.49.0

[PATCH v3 01/28] Revert "treewide: Fix probing of devices in DT overlays"

[Herve Codina]

From: Saravana Kannan <saravanak@google.com>

This reverts commit 1a50d9403fb90cbe4dea0ec9fd0351d2ecbd8924.

While the commit fixed fw_devlink overlay handling for one case, it
broke it for another case. So revert it and redo the fix in a separate
patch.

Fixes: 1a50d9403fb9 ("treewide: Fix probing of devices in DT overlays")
Reported-by: Herve Codina <herve.codina@bootlin.com>
Closes: https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/
Closes: https://lore.kernel.org/all/20240221095137.616d2aaa@bootlin.com/
Closes: https://lore.kernel.org/lkml/20240312151835.29ef62a0@bootlin.com/
Signed-off-by: Saravana Kannan <saravanak@google.com>
Link: https://lore.kernel.org/lkml/20240411235623.1260061-2-saravanak@google.com/
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
 drivers/bus/imx-weim.c    | 6 ------
 drivers/i2c/i2c-core-of.c | 5 -----
 drivers/of/dynamic.c      | 1 -
 drivers/of/platform.c     | 5 -----
 drivers/spi/spi.c         | 5 -----
 5 files changed, 22 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 83d623d97f5f..87070155b057 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -327,12 +327,6 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
 				 "Failed to setup timing for '%pOF'\n", rd->dn);
 
 		if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
-			/*
-			 * Clear the flag before adding the device so that
-			 * fw_devlink doesn't skip adding consumers to this
-			 * device.
-			 */
-			rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 			if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
 				dev_err(&pdev->dev,
 					"Failed to create child device '%pOF'\n",
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index eb7fb202355f..30b48a428c0b 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -176,11 +176,6 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
 			return NOTIFY_OK;
 		}
 
-		/*
-		 * Clear the flag before adding the device so that fw_devlink
-		 * doesn't skip adding consumers to this device.
-		 */
-		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		client = of_i2c_register_device(adap, rd->dn);
 		if (IS_ERR(client)) {
 			dev_err(&adap->dev, "failed to create client for '%pOF'\n",
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 0aba760f7577..6a117e1b6798 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -225,7 +225,6 @@ static void __of_attach_node(struct device_node *np)
 	np->sibling = np->parent->child;
 	np->parent->child = np;
 	of_node_clear_flag(np, OF_DETACHED);
-	np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
 
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f77cb19973a5..ef9445ba168b 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -739,11 +739,6 @@ static int of_platform_notify(struct notifier_block *nb,
 		if (of_node_check_flag(rd->dn, OF_POPULATED))
 			return NOTIFY_OK;
 
-		/*
-		 * Clear the flag before adding the device so that fw_devlink
-		 * doesn't skip adding consumers to this device.
-		 */
-		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		/* pdev_parent may be NULL when no bus platform device */
 		pdev_parent = of_find_device_by_node(parent);
 		pdev = of_platform_device_create(rd->dn, NULL,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 1bc0fdbb1bd7..f76b9ad1c63b 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4803,11 +4803,6 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
 			return NOTIFY_OK;
 		}
 
-		/*
-		 * Clear the flag before adding the device so that fw_devlink
-		 * doesn't skip adding consumers to this device.
-		 */
-		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		spi = of_register_spi_device(ctlr, rd->dn);
 		put_device(&ctlr->dev);
 
-- 
2.49.0

[PATCH v3 02/28] driver core: Rename get_dev_from_fwnode() wrapper to get_device_from_fwnode()

[Herve Codina]

get_dev_from_fwnode() calls get_device() and so it acquires a reference
on the device returned.

In order to be more obvious that this wrapper is a get_device() variant,
rename it to get_device_from_fwnode().

Suggested-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/lkml/CAGETcx97QjnjVR8Z5g0ndLHpK96hLd4aYSV=iEkKPNbNOccYmA@mail.gmail.com/
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Saravana Kannan <saravanak@google.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/base/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index cbc0099d8ef2..36ccee91ba9a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1881,7 +1881,7 @@ static void fw_devlink_unblock_consumers(struct device *dev)
 	device_links_write_unlock();
 }
 
-#define get_dev_from_fwnode(fwnode)	get_device((fwnode)->dev)
+#define get_device_from_fwnode(fwnode)	get_device((fwnode)->dev)
 
 static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
 {
@@ -1891,7 +1891,7 @@ static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
 	if (!(fwnode->flags & FWNODE_FLAG_INITIALIZED))
 		return false;
 
-	dev = get_dev_from_fwnode(fwnode);
+	dev = get_device_from_fwnode(fwnode);
 	ret = !dev || dev->links.status == DL_DEV_NO_DRIVER;
 	put_device(dev);
 
@@ -1960,7 +1960,7 @@ static struct device *fwnode_get_next_parent_dev(const struct fwnode_handle *fwn
 	struct device *dev;
 
 	fwnode_for_each_parent_node(fwnode, parent) {
-		dev = get_dev_from_fwnode(parent);
+		dev = get_device_from_fwnode(parent);
 		if (dev) {
 			fwnode_handle_put(parent);
 			return dev;
@@ -2016,8 +2016,8 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
 		goto out;
 	}
 
-	sup_dev = get_dev_from_fwnode(sup_handle);
-	con_dev = get_dev_from_fwnode(con_handle);
+	sup_dev = get_device_from_fwnode(sup_handle);
+	con_dev = get_device_from_fwnode(con_handle);
 	/*
 	 * If sup_dev is bound to a driver and @con hasn't started binding to a
 	 * driver, sup_dev can't be a consumer of @con. So, no need to check
@@ -2156,7 +2156,7 @@ static int fw_devlink_create_devlink(struct device *con,
 	if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE)
 		sup_dev = fwnode_get_next_parent_dev(sup_handle);
 	else
-		sup_dev = get_dev_from_fwnode(sup_handle);
+		sup_dev = get_device_from_fwnode(sup_handle);
 
 	if (sup_dev) {
 		/*
@@ -2225,7 +2225,7 @@ static void __fw_devlink_link_to_consumers(struct device *dev)
 		bool own_link = true;
 		int ret;
 
-		con_dev = get_dev_from_fwnode(link->consumer);
+		con_dev = get_device_from_fwnode(link->consumer);
 		/*
 		 * If consumer device is not available yet, make a "proxy"
 		 * SYNC_STATE_ONLY link from the consumer's parent device to
-- 
2.49.0

[PATCH v3 03/28] of: dynamic: Fix overlayed devices not probing because of fw_devlink

[Herve Codina]

From: Saravana Kannan <saravanak@google.com>

When an overlay is applied, if the target device has already probed
successfully and bound to a device, then some of the fw_devlink logic
that ran when the device was probed needs to be rerun. This allows newly
created dangling consumers of the overlayed device tree nodes to be
moved to become consumers of the target device.

Fixes: 1a50d9403fb9 ("treewide: Fix probing of devices in DT overlays")
Reported-by: Herve Codina <herve.codina@bootlin.com>
Closes: https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/
Closes: https://lore.kernel.org/all/20240221095137.616d2aaa@bootlin.com/
Closes: https://lore.kernel.org/lkml/20240312151835.29ef62a0@bootlin.com/
Signed-off-by: Saravana Kannan <saravanak@google.com>
Link: https://lore.kernel.org/lkml/20240411235623.1260061-3-saravanak@google.com/
[Herve: Rebase on top of recent kernel and use get_device_from_fwnode()]
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/base/core.c    | 78 ++++++++++++++++++++++++++++++++++++------
 drivers/of/overlay.c   | 15 ++++++++
 include/linux/fwnode.h |  1 +
 3 files changed, 83 insertions(+), 11 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 36ccee91ba9a..8fead097c404 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -47,6 +47,8 @@ static bool fw_devlink_drv_reg_done;
 static bool fw_devlink_best_effort;
 static struct workqueue_struct *device_link_wq;
 
+#define get_device_from_fwnode(fwnode)	get_device((fwnode)->dev)
+
 /**
  * __fwnode_link_add - Create a link between two fwnode_handles.
  * @con: Consumer end of the link.
@@ -235,6 +237,70 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
 		__fw_devlink_pickup_dangling_consumers(child, new_sup);
 }
 
+static void fw_devlink_pickup_dangling_consumers(struct device *dev)
+{
+	struct fwnode_handle *child;
+
+	guard(mutex)(&fwnode_link_lock);
+
+	fwnode_for_each_available_child_node(dev->fwnode, child)
+		__fw_devlink_pickup_dangling_consumers(child, dev->fwnode);
+	__fw_devlink_link_to_consumers(dev);
+}
+
+/**
+ * fw_devlink_refresh_fwnode - Recheck the tree under this firmware node
+ * @fwnode: The fwnode under which the fwnode tree has changed
+ *
+ * This function is mainly meant to adjust the supplier/consumer dependencies
+ * after a fwnode tree overlay has occurred.
+ */
+void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode)
+{
+	struct device *dev;
+
+	/*
+	 * Find the closest ancestor fwnode that has been converted to a device
+	 * that can bind to a driver (bus device).
+	 */
+	fwnode_handle_get(fwnode);
+	do {
+		if (fwnode->flags & FWNODE_FLAG_NOT_DEVICE)
+			continue;
+
+		dev = get_device_from_fwnode(fwnode);
+		if (!dev)
+			continue;
+
+		if (dev->bus)
+			break;
+
+		put_device(dev);
+	} while ((fwnode = fwnode_get_next_parent(fwnode)));
+
+	/*
+	 * If none of the ancestor fwnodes have (yet) been converted to a device
+	 * that can bind to a driver, there's nothing to fix up.
+	 */
+	if (!fwnode)
+		return;
+
+	WARN(device_is_bound(dev) && dev->links.status != DL_DEV_DRIVER_BOUND,
+	     "Don't multithread overlaying and probing the same device!\n");
+
+	/*
+	 * If the device has already bound to a driver, then we need to redo
+	 * some of the work that was done after the device was bound to a
+	 * driver. If the device hasn't bound to a driver, running thing too
+	 * soon would incorrectly pick up consumers that it shouldn't.
+	 */
+	if (dev->links.status == DL_DEV_DRIVER_BOUND)
+		fw_devlink_pickup_dangling_consumers(dev);
+
+	put_device(dev);
+	fwnode_handle_put(fwnode);
+}
+
 static DEFINE_MUTEX(device_links_lock);
 DEFINE_STATIC_SRCU(device_links_srcu);
 
@@ -1313,16 +1379,8 @@ void device_links_driver_bound(struct device *dev)
 	 * child firmware node.
 	 */
 	if (dev->fwnode && dev->fwnode->dev == dev) {
-		struct fwnode_handle *child;
-
 		fwnode_links_purge_suppliers(dev->fwnode);
-
-		guard(mutex)(&fwnode_link_lock);
-
-		fwnode_for_each_available_child_node(dev->fwnode, child)
-			__fw_devlink_pickup_dangling_consumers(child,
-							       dev->fwnode);
-		__fw_devlink_link_to_consumers(dev);
+		fw_devlink_pickup_dangling_consumers(dev);
 	}
 	device_remove_file(dev, &dev_attr_waiting_for_supplier);
 
@@ -1881,8 +1939,6 @@ static void fw_devlink_unblock_consumers(struct device *dev)
 	device_links_write_unlock();
 }
 
-#define get_device_from_fwnode(fwnode)	get_device((fwnode)->dev)
-
 static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
 {
 	struct device *dev;
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 1af6f52d0708..6bd93153d695 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -185,6 +185,15 @@ static int overlay_notify(struct overlay_changeset *ovcs,
 	return 0;
 }
 
+static void overlay_fw_devlink_refresh(struct overlay_changeset *ovcs)
+{
+	for (int i = 0; i < ovcs->count; i++) {
+		struct device_node *np = ovcs->fragments[i].target;
+
+		fw_devlink_refresh_fwnode(of_fwnode_handle(np));
+	}
+}
+
 /*
  * The values of properties in the "/__symbols__" node are paths in
  * the ovcs->overlay_root.  When duplicating the properties, the paths
@@ -951,6 +960,12 @@ static int of_overlay_apply(struct overlay_changeset *ovcs,
 		pr_err("overlay apply changeset entry notify error %d\n", ret);
 	/* notify failure is not fatal, continue */
 
+	/*
+	 * Needs to happen after changeset notify to give the listeners a chance
+	 * to finish creating all the devices they need to create.
+	 */
+	overlay_fw_devlink_refresh(ovcs);
+
 	ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_APPLY);
 	if (ret_tmp)
 		if (!ret)
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 097be89487bf..a921ca2fe940 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -228,6 +228,7 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
 		    u8 flags);
 void fwnode_links_purge(struct fwnode_handle *fwnode);
 void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
+void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode);
 bool fw_devlink_is_strict(void);
 
 #endif
-- 
2.49.0

[PATCH v3 04/28] driver core: Avoid warning when removing a device while its supplier is unbinding

[Herve Codina]

During driver removal, the following warning can appear:
   WARNING: CPU: 1 PID: 139 at drivers/base/core.c:1497 __device_links_no_driver+0xcc/0xfc
   ...
   Call trace:
     __device_links_no_driver+0xcc/0xfc (P)
     device_links_driver_cleanup+0xa8/0xf0
     device_release_driver_internal+0x208/0x23c
     device_links_unbind_consumers+0xe0/0x108
     device_release_driver_internal+0xec/0x23c
     device_links_unbind_consumers+0xe0/0x108
     device_release_driver_internal+0xec/0x23c
     device_links_unbind_consumers+0xe0/0x108
     device_release_driver_internal+0xec/0x23c
     driver_detach+0xa0/0x12c
     bus_remove_driver+0x6c/0xbc
     driver_unregister+0x30/0x60
     pci_unregister_driver+0x20/0x9c
     lan966x_pci_driver_exit+0x18/0xa90 [lan966x_pci]

This warning is triggered when a consumer is removed because the links
status of its supplier is not DL_DEV_DRIVER_BOUND and the link flag
DL_FLAG_SYNC_STATE_ONLY is not set.

The topology in terms of consumers/suppliers used was the following
(consumer ---> supplier):

      i2c -----------> OIC ----> PCI device
       |                ^
       |                |
       +---> pinctrl ---+

When the PCI device is removed, the OIC (interrupt controller) has to be
removed. In order to remove the OIC, pinctrl and i2c need to be removed
and to remove pinctrl, i2c need to be removed. The removal order is:
  1) i2c
  2) pinctrl
  3) OIC
  4) PCI device

In details, the removal sequence is the following (with 0000:01:00.0 the
PCI device):
  driver_detach: call device_release_driver_internal(0000:01:00.0)...
    device_links_busy(0000:01:00.0):
      links->status = DL_DEV_UNBINDING
    device_links_unbind_consumers(0000:01:00.0):
      0000:01:00.0--oic link->status = DL_STATE_SUPPLIER_UNBIND
      call device_release_driver_internal(oic)...
        device_links_busy(oic):
          links->status = DL_DEV_UNBINDING
        device_links_unbind_consumers(oic):
          oic--pinctrl link->status = DL_STATE_SUPPLIER_UNBIND
          call device_release_driver_internal(pinctrl)...
            device_links_busy(pinctrl):
              links->status = DL_DEV_UNBINDING
            device_links_unbind_consumers(pinctrl):
              pinctrl--i2c link->status = DL_STATE_SUPPLIER_UNBIND
              call device_release_driver_internal(i2c)...
                device_links_busy(i2c): links->status = DL_DEV_UNBINDING
                __device_links_no_driver(i2c)...
                  pinctrl--i2c link->status is DL_STATE_SUPPLIER_UNBIND
                  oic--i2c link->status is DL_STATE_ACTIVE
                  oic--i2c link->supplier->links.status is DL_DEV_UNBINDING

The warning is triggered by the i2c removal because the OIC (supplier)
links status is not DL_DEV_DRIVER_BOUND. Its links status is indeed set
to DL_DEV_UNBINDING.

It is perfectly legit to have the links status set to DL_DEV_UNBINDING
in that case. Indeed we had started to unbind the OIC which triggered
the consumer unbinding and didn't finish yet when the i2c is unbound.

Avoid the warning when the supplier links status is set to
DL_DEV_UNBINDING and thus support this removal sequence without any
warnings.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/base/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8fead097c404..ce367c44f642 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1494,7 +1494,8 @@ static void __device_links_no_driver(struct device *dev)
 		if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) {
 			WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
 		} else {
-			WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
+			WARN_ON(link->supplier->links.status != DL_DEV_UNBINDING &&
+				!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
 			WRITE_ONCE(link->status, DL_STATE_DORMANT);
 		}
 	}
-- 
2.49.0

[PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Herve Codina]

The simple-pm-bus driver handles several simple busses. When it is used
with busses other than a compatible "simple-pm-bus", it doesn't populate
its child devices during its probe.

This confuses fw_devlink and results in wrong or missing devlinks.

Once a driver is bound to a device and the probe() has been called,
device_links_driver_bound() is called.

This function performs operation based on the following assumption:
    If a child firmware node of the bound device is not added as a
    device, it will never be added.

Among operations done on fw_devlinks of those "never be added" devices,
device_links_driver_bound() changes their supplier.

With devices attached to a simple-bus compatible device, this change
leads to wrong devlinks where supplier of devices points to the device
parent (i.e. simple-bus compatible device) instead of the device itself
(i.e. simple-bus child).

When the device attached to the simple-bus is removed, because devlinks
are not correct, its consumers are not removed first.

In order to have correct devlinks created, make the simple-pm-bus driver
compliant with the devlink assumption and create its child devices
during its probe.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/bus/simple-pm-bus.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
index d8e029e7e53f..cfa05f9bd226 100644
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -42,14 +42,15 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
 	match = of_match_device(dev->driver->of_match_table, dev);
 	/*
 	 * These are transparent bus devices (not simple-pm-bus matches) that
-	 * have their child nodes populated automatically.  So, don't need to
-	 * do anything more. We only match with the device if this driver is
-	 * the most specific match because we don't want to incorrectly bind to
-	 * a device that has a more specific driver.
+	 * need to have their child nodes populated. So, don't need to do
+	 * anything more except populate child nodes during this probe(). We
+	 * only match with the device if this driver is the most specific match
+	 * because we don't want to incorrectly bind to a device that has a more
+	 * specific driver.
 	 */
 	if (match && match->data) {
 		if (of_property_match_string(np, "compatible", match->compatible) == 0)
-			return 0;
+			goto populate;
 		else
 			return -ENODEV;
 	}
@@ -64,13 +65,14 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(&pdev->dev, bus);
 
-	dev_dbg(&pdev->dev, "%s\n", __func__);
-
 	pm_runtime_enable(&pdev->dev);
 
+populate:
 	if (np)
 		of_platform_populate(np, NULL, lookup, &pdev->dev);
 
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
 	return 0;
 }
 
@@ -78,12 +80,15 @@ static void simple_pm_bus_remove(struct platform_device *pdev)
 {
 	const void *data = of_device_get_match_data(&pdev->dev);
 
-	if (pdev->driver_override || data)
+	if (pdev->driver_override)
 		return;
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
 
-	pm_runtime_disable(&pdev->dev);
+	of_platform_depopulate(&pdev->dev);
+
+	if (!data)
+		pm_runtime_disable(&pdev->dev);
 }
 
 static int simple_pm_bus_runtime_suspend(struct device *dev)
-- 
2.49.0

[PATCH v3 06/28] driver core: fw_devlink: Introduce fw_devlink_set_device()

[Herve Codina]

Setting fwnode->dev is specific to fw_devlink.

In order to avoid having a direct 'fwnode->dev = dev;' in several
place in the kernel, introduce fw_devlink_set_device() helper to perform
this operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 include/linux/fwnode.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index a921ca2fe940..a1345e274125 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -231,4 +231,10 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
 void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode);
 bool fw_devlink_is_strict(void);
 
+static inline void fw_devlink_set_device(struct fwnode_handle *fwnode,
+					 struct device *dev)
+{
+	fwnode->dev = dev;
+}
+
 #endif
-- 
2.49.0

[PATCH v3 07/28] drivers: core: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode->dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/base/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index ce367c44f642..3afe821f37f2 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3742,7 +3742,7 @@ int device_add(struct device *dev)
 	 * device and the driver sync_state callback is called for this device.
 	 */
 	if (dev->fwnode && !dev->fwnode->dev) {
-		dev->fwnode->dev = dev;
+		fw_devlink_set_device(dev->fwnode, dev);
 		fw_devlink_link_device(dev);
 	}
 
@@ -3902,7 +3902,7 @@ void device_del(struct device *dev)
 	device_unlock(dev);
 
 	if (dev->fwnode && dev->fwnode->dev == dev)
-		dev->fwnode->dev = NULL;
+		fw_devlink_set_device(dev->fwnode, NULL);
 
 	/* Notify clients of device removal.  This call must come
 	 * before dpm_sysfs_remove().
-- 
2.49.0

[PATCH v3 08/28] pinctrl: cs42l43: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode->dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pinctrl/cirrus/pinctrl-cs42l43.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/cirrus/pinctrl-cs42l43.c b/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
index 628b60ccc2b0..8df85ec5a02e 100644
--- a/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
+++ b/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
@@ -561,7 +561,7 @@ static int cs42l43_pin_probe(struct platform_device *pdev)
 		fwnode = fwnode_get_named_child_node(fwnode, "pinctrl");
 
 		if (fwnode && !fwnode->dev)
-			fwnode->dev = priv->dev;
+			fw_devlink_set_device(fwnode, priv->dev);
 	}
 
 	priv->gpio_chip.fwnode = fwnode;
-- 
2.49.0

[PATCH v3 09/28] cxl/test: Use device_set_node()

[Herve Codina]

The code set directly dev->fwnode.

Use the dedicated helper to perform this operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 tools/testing/cxl/test/cxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 8a5815ca870d..1db7732a70df 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1046,7 +1046,7 @@ static void mock_companion(struct acpi_device *adev, struct device *dev)
 {
 	device_initialize(&adev->dev);
 	fwnode_init(&adev->fwnode, NULL);
-	dev->fwnode = &adev->fwnode;
+	device_set_node(dev, &adev->fwnode);
 	adev->fwnode.dev = dev;
 }
 
-- 
2.49.0

[PATCH v3 10/28] cxl/test: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode.dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 tools/testing/cxl/test/cxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 1db7732a70df..80e6fb3c9a43 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1047,7 +1047,7 @@ static void mock_companion(struct acpi_device *adev, struct device *dev)
 	device_initialize(&adev->dev);
 	fwnode_init(&adev->fwnode, NULL);
 	device_set_node(dev, &adev->fwnode);
-	adev->fwnode.dev = dev;
+	fw_devlink_set_device(&adev->fwnode, dev);
 }
 
 #ifndef SZ_64G
-- 
2.49.0

[PATCH v3 11/28] PCI: of: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode.dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..95baa29d4c2d 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -803,7 +803,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
 	 * bus. Avoid any new device creation.
 	 */
 	of_node_set_flag(np, OF_POPULATED);
-	np->fwnode.dev = &bridge->dev;
+	fw_devlink_set_device(&np->fwnode, &bridge->dev);
 	fwnode_dev_initialized(&np->fwnode, true);
 
 	ret = of_changeset_apply(cset);
-- 
2.49.0

[PATCH v3 12/28] driver core: fw_devlink: Tag the fwnode dev member as private

[Herve Codina]

The dev member in the fwnode structure should be accessed only by the
fwnode core. The fw_devlink_set_device() helper has been introduced to
avoid users to set this member directly. All users have been converted
and use fw_devlink_set_device() to set the field.

To avoid future abusers who might set the field directly and allow
sparse to detect them, tag the dev field as a private field.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 include/linux/fwnode.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index a1345e274125..5762914fdbf2 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -54,7 +54,7 @@ struct fwnode_handle {
 	const struct fwnode_operations *ops;
 
 	/* The below is used solely by device links, don't use otherwise */
-	struct device *dev;
+	struct device *__private dev;
 	struct list_head suppliers;
 	struct list_head consumers;
 	u8 flags;
@@ -234,7 +234,7 @@ bool fw_devlink_is_strict(void);
 static inline void fw_devlink_set_device(struct fwnode_handle *fwnode,
 					 struct device *dev)
 {
-	fwnode->dev = dev;
+	ACCESS_PRIVATE(fwnode, dev) = dev;
 }
 
 #endif
-- 
2.49.0

[PATCH v3 13/28] PCI: of: Set fwnode device of newly created PCI device nodes

[Herve Codina]

Device-tree node can be created when CONFIG_PCI_DYNAMIC_OF_NODES. Those
node are created and filled based on PCI core information but the
fwnode device field is not set.

When later an overlay is applied, this confuses fw_devlink. Indeed,
without any device attached to the node, fw_devlink considers that this
node will never become a device. When this node is pointed as a
supplier, devlink looks at its ancestors in order to find a node with a
device that could be used as the supplier.

In the PCI use case, this leads to links that wrongly use the PCI root
bridge device as the supplier instead of the expected PCI device.

Setting the fwnode device to the device of the PCI device allows devlink
to use this device as a supplier and so, correct links are created.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 95baa29d4c2d..bbbab115d47f 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -709,6 +709,13 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 	if (ret)
 		goto out_free_node;
 
+	/*
+	 * Set the fwnode device in order to have fw_devlink creating links
+	 * pointing to this PCI device instead of walking up to the PCI host
+	 * bridge.
+	 */
+	fw_devlink_set_device(&np->fwnode, &pdev->dev);
+
 	ret = of_changeset_apply(cset);
 	if (ret)
 		goto out_free_node;
-- 
2.49.0

[PATCH v3 14/28] PCI: of: Remove fwnode_dev_initialized() call for a PCI root bridge node

[Herve Codina]

During the instantiation of devices described by a device-tree overlay
applied on a PCI device, devlink displays the following kind of debug
messages instead of creating the expected links:
   'Not linking xxxx - might never become dev'

Without those expected links, the device removal order cannot be
correct.

Those debug traces are printed by fw_devlink_create_devlink(). In our
use case, they are all printed because the supplier of the link has at
least one of its ancestor with its fwnode flag FWNODE_FLAG_INITIALIZED
set.

The culprit ancestor is the PCI root bridge.

The fwnode related to the PCI root bridge is created dynamically by the
of_pci_make_host_bridge_node() function. During this creation
fwnode_dev_initialized() is called which set the FWNODE_FLAG_INITIALIZED
flag.

Calling fwnode_dev_initialized() tells devlink that the device related
to this node is handled out of the driver core. This is not correct in
our case. Indeed the device related to this firmware node is handled
using driver core mechanisms and is fully compliant devlink
expectations.

Simply remove the fwnode_dev_initialized() call. With that done, the
devlink debug messages are no more displayed and links that were missing
are correctly created.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index bbbab115d47f..6b45eca63904 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -811,7 +811,6 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
 	 */
 	of_node_set_flag(np, OF_POPULATED);
 	fw_devlink_set_device(&np->fwnode, &bridge->dev);
-	fwnode_dev_initialized(&np->fwnode, true);
 
 	ret = of_changeset_apply(cset);
 	if (ret)
-- 
2.49.0

[PATCH v3 15/28] i2c: core: Introduce i2c_get_adapter_physdev()

[Herve Codina]

The physical device providing an I2C adapter is the device that calls
i2c_add_adapter() or variants and i2c_del_adapter().

Most of the time this physical device is the parent of the adapter
device.

Exceptions exist with i2c muxes. Indeed, in case of i2c muxes, the
parent of the mux adapter device points to the adapter device the mux is
connected to instead of the physical of this mux adapter.

Introduce i2c_get_adapter_physdev() and a new physdev field in the
adapter structure in order to ease the adapter physical device
retrieval.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/i2c/i2c-core-base.c | 16 ++++++++++++++++
 include/linux/i2c.h         |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 2ad2b1838f0f..86cd6da25024 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1916,6 +1916,22 @@ struct i2c_adapter *i2c_get_adapter_by_fwnode(struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL(i2c_get_adapter_by_fwnode);
 
+/**
+ * i2c_get_adapter_physdev() - Get the physical device of an adapter
+ * @adapter: the adapter to get the physical device from
+ *
+ * Return:
+ * Look up and return the &struct device corresponding to the device supplying
+ * this @adapter.
+ *
+ * The user must call put_device() once done with the physical device returned.
+ */
+struct device *i2c_get_adapter_physdev(struct i2c_adapter *adapter)
+{
+	return get_device(adapter->physdev ?: adapter->dev.parent);
+}
+EXPORT_SYMBOL(i2c_get_adapter_physdev);
+
 static void i2c_parse_timing(struct device *dev, char *prop_name, u32 *cur_val_p,
 			    u32 def_val, bool use_def)
 {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 20fd41b51d5c..dff04d20cafe 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -744,6 +744,7 @@ struct i2c_adapter {
 	int timeout;			/* in jiffies */
 	int retries;
 	struct device dev;		/* the adapter device */
+	struct device *physdev;		/* the physical device */
 	unsigned long locked_flags;	/* owned by the I2C core */
 #define I2C_ALF_IS_SUSPENDED		0
 #define I2C_ALF_SUSPEND_REPORTED	1
@@ -911,6 +912,8 @@ struct i2c_adapter *i2c_get_adapter(int nr);
 void i2c_put_adapter(struct i2c_adapter *adap);
 unsigned int i2c_adapter_depth(struct i2c_adapter *adapter);
 
+struct device *i2c_get_adapter_physdev(struct i2c_adapter *adap);
+
 void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_defaults);
 
 /* Return the functionality mask */
-- 
2.49.0

[PATCH v3 16/28] i2c: mux: Set adapter physical device

[Herve Codina]

For i2c muxes, the parent of the mux adapter device is the adapter
device the mux is connected to.

This parent is not the physical device related to the mux adapter.
Indeed, the physical device of the mux adapter is the mux device itself.

Fill the adap.physdev with the mux device.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/i2c/i2c-mux.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index fda72e8be885..3bf2035f485f 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -318,6 +318,7 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	priv->adap.algo = &priv->algo;
 	priv->adap.algo_data = priv;
 	priv->adap.dev.parent = &parent->dev;
+	priv->adap.physdev = muxc->dev;
 	priv->adap.retries = parent->retries;
 	priv->adap.timeout = parent->timeout;
 	priv->adap.quirks = parent->quirks;
-- 
2.49.0

[PATCH v3 17/28] i2c: mux: Create missing devlink between mux and adapter physical device

[Herve Codina]

When removing an i2c controller device handling an i2c bus where an i2c
mux is connected to, the removal process hangs and is stuck in the
wait_completion() call done in i2c_del_adapter().

The i2c_del_adapter() tries to removed the i2c adapter related to the
i2c controller device and the wait_completion() is waiting for the i2c
adapter device release. This release is performed when the device is no
more used (i.e. refcount reaches zero).

When an i2c mux is involved in an i2c path, the struct dev topology is
the following:
    +----------------+                +-------------------+
    | i2c controller |                |      i2c mux      |
    |     device     |                |      device       |
    |       ^        |                |                   |
    |       |        |                |                   |
    |  dev's parent  |                |                   |
    |       |        |                |                   |
    |   i2c adapter  |                | i2c adapter chanX |
    |     device  <---- dev's parent ------  device       |
    |   (no driver)  |                |    (no driver)    |
    +----------------+                +-------------------+

When an i2c mux device creates an i2c adapter for its downstream
channel, a reference is taken to its adapter dev's parent. This parent
is the i2c mux upstream adapter device.

No relationship exists between the i2c mux device itself and the i2c
controller device (physical device) in order to have the i2c mux device
calling i2c_del_adapter() to remove its downtream adapters and so,
release references taken to the upstream adapter.

This consumer/supplier relationship is typically a devlink relationship.

Also, i2c muxes can be chained and so, the upstream adapter can be
supplied by either an i2c controller device or an other i2c mux device.

In order to get the physical device of the adapter a mux is connected
to, rely on the newly introduced i2c_adapter_get_physdev() and create
the missing devlink between the i2c mux device and the physical
device of the adapter the mux is connected to.

With that done, the i2c mux device is removed before the device
handling the upstream i2c adapter (i2c controller device or i2c mux
device). All references are released and the i2c_del_adapter() call
performed by driver handling the upstream adapter device is not blocking
anymore.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/i2c/i2c-mux.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 3bf2035f485f..bcbca3135073 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -271,7 +271,9 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 			u32 force_nr, u32 chan_id)
 {
 	struct i2c_adapter *parent = muxc->parent;
+	struct device *parent_physdev;
 	struct i2c_mux_priv *priv;
+	struct device_link *dl;
 	char symlink_name[20];
 	int ret;
 
@@ -378,6 +380,29 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 				      ACPI_COMPANION(muxc->dev),
 				      chan_id);
 
+	/*
+	 * There is no relationship set between the mux device and the physical
+	 * device handling the parent adapter. Create this missing relationship
+	 * in order to remove the i2c mux device (consumer) and so the dowstream
+	 * channel adapters before removing the physical device (supplier) which
+	 * handles the i2c mux upstream adapter.
+	 */
+	parent_physdev = i2c_get_adapter_physdev(parent);
+	if (!parent_physdev) {
+		dev_err(muxc->dev, "failed to get the parent physical device\n");
+		ret = -EINVAL;
+		goto err_free_priv;
+	}
+	dl = device_link_add(muxc->dev, parent_physdev, DL_FLAG_AUTOREMOVE_CONSUMER);
+	if (!dl) {
+		dev_err(muxc->dev, "failed to create device link to %s\n",
+			dev_name(parent_physdev));
+		put_device(parent_physdev);
+		ret = -EINVAL;
+		goto err_free_priv;
+	}
+	put_device(parent_physdev);
+
 	if (force_nr) {
 		priv->adap.nr = force_nr;
 		ret = i2c_add_numbered_adapter(&priv->adap);
-- 
2.49.0

[PATCH v3 18/28] of: property: Allow fw_devlink device-tree on x86 when PCI device-tree node creation is enabled

[Herve Codina]

PCI drivers can use a device-tree overlay to describe the hardware
available on the PCI board. This is the case, for instance, of the
LAN966x PCI device driver.

Adding some more nodes in the device-tree overlay adds some more
consumer/supplier relationship between devices instantiated from this
overlay.

Those fw_node consumer/supplier relationships are handled by fw_devlink
and are created based on the device-tree parsing done by the
of_fwnode_add_links() function.

Those consumer/supplier links are needed in order to ensure a correct PM
runtime management and a correct removal order between devices.

For instance, without those links a supplier can be removed before its
consumers is removed leading to all kind of issue if this consumer still
want the use the already removed supplier.

The support for the usage of an overlay from a PCI driver has been added
on x86 systems in commit 1f340724419ed ("PCI: of: Create device tree PCI
host bridge node").

In the past, support for fw_devlink on x86 had been tried but this
support has been removed in commit 4a48b66b3f52 ("of: property: Disable
fw_devlink DT support for X86"). Indeed, this support was breaking some
x86 systems such as OLPC system and the regression was reported in [0].

Instead of disabling this support for all x86 system, a first approach
would be to use a finer grain and disable this support only for the
possible problematic subset of x86 systems (at least OLPC and CE4100).

This first approach could still leads to issues. Indeed, the list of
possible problematic system and the way to identify them using Kconfig
symbols is not well defined and so some system can be missed leading to
kernel regressions on those missing systems.

Use an other way and enable the support on x86 system only when this
support is needed by some specific feature. The usage of a device-tree
overlay by a PCI driver and thus the creation of PCI device-tree nodes
is a feature that needs it.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Link: https://lore.kernel.org/lkml/3c1f2473-92ad-bfc4-258e-a5a08ad73dd0@web.de/ [0]
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/of/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index c1feb631e383..8b5cfee696e2 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1605,7 +1605,7 @@ static int of_fwnode_add_links(struct fwnode_handle *fwnode)
 	const struct property *p;
 	struct device_node *con_np = to_of_node(fwnode);
 
-	if (IS_ENABLED(CONFIG_X86))
+	if (IS_ENABLED(CONFIG_X86) && !IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES))
 		return 0;
 
 	if (!con_np)
-- 
2.49.0

[PATCH v3 19/28] clk: lan966x: Add MCHP_LAN966X_PCI dependency

[Herve Codina]

The lan966x clock controller depends on the LAN969x architecture or the
LAN966x SoC.

This clock controller can be used by the LAN966x PCI device and so it
needs to be available when the LAN966x PCI device is enabled.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/clk/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 19c1ed280fd7..97ae87d384b7 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -270,7 +270,7 @@ config COMMON_CLK_LAN966X
 	tristate "Generic Clock Controller driver for LAN966X SoC"
 	depends on HAS_IOMEM
 	depends on OF
-	depends on SOC_LAN966 || ARCH_LAN969X || COMPILE_TEST
+	depends on SOC_LAN966 || ARCH_LAN969X || MCHP_LAN966X_PCI || COMPILE_TEST
 	help
 	  This driver provides support for Generic Clock Controller(GCK) on
 	  LAN966X SoC. GCK generates and supplies clock to various peripherals
-- 
2.49.0

[PATCH v3 20/28] i2c: busses: at91: Add MCHP_LAN966X_PCI dependency

[Herve Codina]

The AT91 I2C driver depends on ARCH_AT91.

This I2C controller can be used by the LAN966x PCI device and so
it needs to be available when the LAN966x PCI device is enabled.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Acked-by: Andi Shyti <andi.shyti@kernel.org>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 48c5ab832009..99dc414e3100 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -414,7 +414,7 @@ config I2C_ASPEED
 
 config I2C_AT91
 	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
-	depends on ARCH_AT91 || COMPILE_TEST
+	depends on ARCH_AT91 || MCHP_LAN966X_PCI || COMPILE_TEST
 	help
 	  This supports the use of the I2C interface on Atmel AT91
 	  processors.
-- 
2.49.0

[PATCH v3 21/28] misc: lan966x_pci: Fix dtso nodes ordering

[Herve Codina]

Nodes available in the dtso are not ordered by their unit address.

Fix that re-ordering them according to their unit address.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/misc/lan966x_pci.dtso | 99 +++++++++++++++++------------------
 1 file changed, 49 insertions(+), 50 deletions(-)

diff --git a/drivers/misc/lan966x_pci.dtso b/drivers/misc/lan966x_pci.dtso
index 7b196b0a0eb6..94a967b384f3 100644
--- a/drivers/misc/lan966x_pci.dtso
+++ b/drivers/misc/lan966x_pci.dtso
@@ -59,6 +59,50 @@ pci-ep-bus@0 {
 				ranges = <0xe2000000 0x00 0x00 0x00 0x2000000
 				          0xe0000000 0x01 0x00 0x00 0x1000000>;
 
+				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)>;
+						};
+					};
+				};
+
+				cpu_ctrl: syscon@e00c0000 {
+					compatible = "microchip,lan966x-cpu-syscon", "syscon";
+					reg = <0xe00c0000 0xa8>;
+				};
+
 				oic: oic@e00c0120 {
 					compatible = "microchip,lan966x-oic";
 					#interrupt-cells = <2>;
@@ -67,11 +111,6 @@ oic: oic@e00c0120 {
 					reg = <0xe00c0120 0x190>;
 				};
 
-				cpu_ctrl: syscon@e00c0000 {
-					compatible = "microchip,lan966x-cpu-syscon", "syscon";
-					reg = <0xe00c0000 0xa8>;
-				};
-
 				reset: reset@e200400c {
 					compatible = "microchip,lan966x-switch-reset";
 					reg = <0xe200400c 0x4>, <0xe00c0000 0xa8>;
@@ -104,14 +143,6 @@ fc0_a_pins: fcb4-i2c-pins {
 						pins = "GPIO_9", "GPIO_10";
 						function = "fc0_a";
 					};
-
-				};
-
-				serdes: serdes@e202c000 {
-					compatible = "microchip,lan966x-serdes";
-					reg = <0xe202c000 0x9c>,
-					      <0xe2004010 0x4>;
-					#phy-cells = <2>;
 				};
 
 				mdio1: mdio@e200413c {
@@ -133,43 +164,11 @@ lan966x_phy1: ethernet-lan966x_phy@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)>;
-						};
-					};
+				serdes: serdes@e202c000 {
+					compatible = "microchip,lan966x-serdes";
+					reg = <0xe202c000 0x9c>,
+					      <0xe2004010 0x4>;
+					#phy-cells = <2>;
 				};
 			};
 		};
-- 
2.49.0

[PATCH v3 22/28] misc: lan966x_pci: Split dtso in dtsi/dtso

[Herve Codina]

The lan966x_pci.dtso file contains descriptions related to both the
LAN966x PCI device chip and the LAN966x PCI device board where the chip
is soldered.

Split the file in order to have:
  - lan966x_pci.dtsi
    The description related to the PCI chip.

  - lan966x_pci.dtso
    The description of the PCI board.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 MAINTAINERS                   |   1 +
 drivers/misc/lan966x_pci.dtsi | 130 +++++++++++++++++++++++++
 drivers/misc/lan966x_pci.dtso | 174 +++++++---------------------------
 3 files changed, 166 insertions(+), 139 deletions(-)
 create mode 100644 drivers/misc/lan966x_pci.dtsi

diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa16..a7b1e4c42d9d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16278,6 +16278,7 @@ MICROCHIP LAN966X PCI DRIVER
 M:	Herve Codina <herve.codina@bootlin.com>
 S:	Maintained
 F:	drivers/misc/lan966x_pci.c
+F:	drivers/misc/lan966x_pci.dtsi
 F:	drivers/misc/lan966x_pci.dtso
 
 MICROCHIP LAN969X ETHERNET DRIVER
diff --git a/drivers/misc/lan966x_pci.dtsi b/drivers/misc/lan966x_pci.dtsi
new file mode 100644
index 000000000000..170298084fa5
--- /dev/null
+++ b/drivers/misc/lan966x_pci.dtsi
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Microchip UNG
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+cpu_clk: clock-600000000 {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <600000000>;  /* CPU clock = 600MHz */
+};
+
+ddr_clk: clock-30000000 {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <30000000>;  /* Fabric clock = 30MHz */
+};
+
+sys_clk: clock-15625000 {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <15625000>;  /* System clock = 15.625MHz */
+};
+
+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>;
+
+	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";
+		status = "disabled";
+
+		ethernet-ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port0: port@0 {
+				reg = <0>;
+				status = "disabled";
+			};
+
+			port1: port@1 {
+				reg = <1>;
+				status = "disabled";
+			};
+		};
+	};
+
+	cpu_ctrl: syscon@e00c0000 {
+		compatible = "microchip,lan966x-cpu-syscon", "syscon";
+		reg = <0xe00c0000 0xa8>;
+	};
+
+	oic: oic@e00c0120 {
+		compatible = "microchip,lan966x-oic";
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		interrupts = <0>; /* PCI INTx assigned interrupt */
+		reg = <0xe00c0120 0x190>;
+	};
+
+	reset: reset@e200400c {
+		compatible = "microchip,lan966x-switch-reset";
+		reg = <0xe200400c 0x4>, <0xe00c0000 0xa8>;
+		reg-names = "gcb","cpu";
+		#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>;
+	};
+
+	mdio1: mdio@e200413c {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "microchip,lan966x-miim";
+		reg = <0xe200413c 0x24>,
+		      <0xe2010020 0x4>;
+		resets = <&reset 0>;
+		reset-names = "switch";
+		status = "disabled";
+
+		lan966x_phy0: ethernet-lan966x_phy@1 {
+			reg = <1>;
+			status = "disabled";
+		};
+
+		lan966x_phy1: ethernet-lan966x_phy@2 {
+			reg = <2>;
+			status = "disabled";
+		};
+	};
+
+	serdes: serdes@e202c000 {
+		compatible = "microchip,lan966x-serdes";
+		reg = <0xe202c000 0x9c>,
+		      <0xe2004010 0x4>;
+		#phy-cells = <2>;
+	};
+};
diff --git a/drivers/misc/lan966x_pci.dtso b/drivers/misc/lan966x_pci.dtso
index 94a967b384f3..b3de5f14d9cb 100644
--- a/drivers/misc/lan966x_pci.dtso
+++ b/drivers/misc/lan966x_pci.dtso
@@ -3,10 +3,7 @@
  * Copyright (C) 2022 Microchip UNG
  */
 
-#include <dt-bindings/clock/microchip,lan966x.h>
 #include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/interrupt-controller/irq.h>
-#include <dt-bindings/mfd/atmel-flexcom.h>
 #include <dt-bindings/phy/phy-lan966x-serdes.h>
 
 /dts-v1/;
@@ -29,148 +26,47 @@ __overlay__ {
 			#address-cells = <3>;
 			#size-cells = <2>;
 
-			cpu_clk: clock-600000000 {
-				compatible = "fixed-clock";
-				#clock-cells = <0>;
-				clock-frequency = <600000000>;  /* CPU clock = 600MHz */
-			};
+			#include "lan966x_pci.dtsi"
 
-			ddr_clk: clock-30000000 {
-				compatible = "fixed-clock";
-				#clock-cells = <0>;
-				clock-frequency = <30000000>;  /* Fabric clock = 30MHz */
-			};
-
-			sys_clk: clock-15625000 {
-				compatible = "fixed-clock";
-				#clock-cells = <0>;
-				clock-frequency = <15625000>;  /* System clock = 15.625MHz */
-			};
-
-			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>;
-
-				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)>;
-						};
-					};
-				};
-
-				cpu_ctrl: syscon@e00c0000 {
-					compatible = "microchip,lan966x-cpu-syscon", "syscon";
-					reg = <0xe00c0000 0xa8>;
-				};
-
-				oic: oic@e00c0120 {
-					compatible = "microchip,lan966x-oic";
-					#interrupt-cells = <2>;
-					interrupt-controller;
-					interrupts = <0>; /* PCI INTx assigned interrupt */
-					reg = <0xe00c0120 0x190>;
-				};
-
-				reset: reset@e200400c {
-					compatible = "microchip,lan966x-switch-reset";
-					reg = <0xe200400c 0x4>, <0xe00c0000 0xa8>;
-					reg-names = "gcb","cpu";
-					#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";
-					};
+&gpio {
+	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";
-					};
-				};
+&lan966x_phy0 {
+	status = "okay";
+};
 
-				mdio1: mdio@e200413c {
-					#address-cells = <1>;
-					#size-cells = <0>;
-					compatible = "microchip,lan966x-miim";
-					reg = <0xe200413c 0x24>,
-					      <0xe2010020 0x4>;
+&lan966x_phy1 {
+	status = "okay";
+};
 
-					resets = <&reset 0>;
-					reset-names = "switch";
+&mdio1 {
+	status = "okay";
+};
 
-					lan966x_phy0: ethernet-lan966x_phy@1 {
-						reg = <1>;
-					};
+&port0 {
+	phy-handle = <&lan966x_phy0>;
+	phy-mode = "gmii";
+	phys = <&serdes 0 CU(0)>;
+	status = "okay";
+};
 
-					lan966x_phy1: ethernet-lan966x_phy@2 {
-						reg = <2>;
-					};
-				};
+&port1 {
+	phy-handle = <&lan966x_phy1>;
+	phy-mode = "gmii";
+	phys = <&serdes 1 CU(1)>;
+	status = "okay";
+};
 
-				serdes: serdes@e202c000 {
-					compatible = "microchip,lan966x-serdes";
-					reg = <0xe202c000 0x9c>,
-					      <0xe2004010 0x4>;
-					#phy-cells = <2>;
-				};
-			};
-		};
-	};
+&switch {
+	pinctrl-names = "default";
+	pinctrl-0 = <&tod_pins>;
+	status = "okay";
 };
-- 
2.49.0

[PATCH v3 23/28] misc: lan966x_pci: Rename lan966x_pci.dtso to lan966x_evb_lan9662_nic.dtso

[Herve Codina]

The lan966x_pci.dtso describes the Microchip EVB-LAN9662-NIC board [0]

This PCI board embeds a LAN9962 PCI device chip, part of the LAN966x
family.

Rename the lan966x_pci.dtso accordingly.

Link: https://www.microchip.com/en-us/development-tool/EV53U25A [0]
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 MAINTAINERS                                               | 2 +-
 drivers/misc/Makefile                                     | 2 +-
 .../{lan966x_pci.dtso => lan966x_evb_lan9662_nic.dtso}    | 0
 drivers/misc/lan966x_pci.c                                | 8 ++++----
 4 files changed, 6 insertions(+), 6 deletions(-)
 rename drivers/misc/{lan966x_pci.dtso => lan966x_evb_lan9662_nic.dtso} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index a7b1e4c42d9d..bea712ad4f6e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16277,9 +16277,9 @@ F:	drivers/irqchip/irq-lan966x-oic.c
 MICROCHIP LAN966X PCI DRIVER
 M:	Herve Codina <herve.codina@bootlin.com>
 S:	Maintained
+F:	drivers/misc/lan966x_evb_lan9662_nic.dtso
 F:	drivers/misc/lan966x_pci.c
 F:	drivers/misc/lan966x_pci.dtsi
-F:	drivers/misc/lan966x_pci.dtso
 
 MICROCHIP LAN969X ETHERNET DRIVER
 M:	Daniel Machon <daniel.machon@microchip.com>
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 917b9a7183aa..1e0da2d20528 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -71,7 +71,7 @@ obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
 obj-$(CONFIG_NSM)		+= nsm.o
 obj-$(CONFIG_MARVELL_CN10K_DPI)	+= mrvl_cn10k_dpi.o
 lan966x-pci-objs		:= lan966x_pci.o
-lan966x-pci-objs		+= lan966x_pci.dtbo.o
+lan966x-pci-objs		+= lan966x_evb_lan9662_nic.dtbo.o
 obj-$(CONFIG_MCHP_LAN966X_PCI)	+= lan966x-pci.o
 obj-y				+= keba/
 obj-y				+= amd-sbi/
diff --git a/drivers/misc/lan966x_pci.dtso b/drivers/misc/lan966x_evb_lan9662_nic.dtso
similarity index 100%
rename from drivers/misc/lan966x_pci.dtso
rename to drivers/misc/lan966x_evb_lan9662_nic.dtso
diff --git a/drivers/misc/lan966x_pci.c b/drivers/misc/lan966x_pci.c
index 9c79b58137e5..b28066c96534 100644
--- a/drivers/misc/lan966x_pci.c
+++ b/drivers/misc/lan966x_pci.c
@@ -19,8 +19,8 @@
 #include <linux/slab.h>
 
 /* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */
-extern char __dtbo_lan966x_pci_begin[];
-extern char __dtbo_lan966x_pci_end[];
+extern char __dtbo_lan966x_evb_lan9662_nic_begin[];
+extern char __dtbo_lan966x_evb_lan9662_nic_end[];
 
 struct pci_dev_intr_ctrl {
 	struct pci_dev *pci_dev;
@@ -125,8 +125,8 @@ struct lan966x_pci {
 
 static int lan966x_pci_load_overlay(struct lan966x_pci *data)
 {
-	u32 dtbo_size = __dtbo_lan966x_pci_end - __dtbo_lan966x_pci_begin;
-	void *dtbo_start = __dtbo_lan966x_pci_begin;
+	u32 dtbo_size = __dtbo_lan966x_evb_lan9662_nic_end - __dtbo_lan966x_evb_lan9662_nic_begin;
+	void *dtbo_start = __dtbo_lan966x_evb_lan9662_nic_begin;
 
 	return of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev));
 }
-- 
2.49.0

[PATCH v3 24/28] PCI: Add Microchip LAN9662 PCI Device ID

[Herve Codina]

Existing code uses the 0x9660 value (LAN9662 PCI Device ID) in several
places.

Avoid this direct use of the 0x9660 value replacing it by defined PCI
Device ID.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/misc/lan966x_pci.c | 2 +-
 drivers/pci/quirks.c       | 2 +-
 include/linux/pci_ids.h    | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/lan966x_pci.c b/drivers/misc/lan966x_pci.c
index b28066c96534..e60ab662b8f3 100644
--- a/drivers/misc/lan966x_pci.c
+++ b/drivers/misc/lan966x_pci.c
@@ -197,7 +197,7 @@ static void lan966x_pci_remove(struct pci_dev *pdev)
 }
 
 static struct pci_device_id lan966x_pci_ids[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_LAN9662) },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, lan966x_pci_ids);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index d7f4ee634263..bde077ce663a 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -6302,7 +6302,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0xa76e, dpc_log_size);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_LAN9662, of_pci_make_dev_node);
 
 /*
  * Devices known to require a longer delay before first config space access
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index e2d71b6fdd84..5d69fde7dd97 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -934,6 +934,7 @@
 #define PCI_VENDOR_ID_EFAR		0x1055
 #define PCI_DEVICE_ID_EFAR_SLC90E66_1	0x9130
 #define PCI_DEVICE_ID_EFAR_SLC90E66_3	0x9463
+#define PCI_DEVICE_ID_EFAR_LAN9662	0x9660
 
 #define PCI_VENDOR_ID_MOTOROLA		0x1057
 #define PCI_DEVICE_ID_MOTOROLA_MPC105	0x0001
-- 
2.49.0

[PATCH v3 25/28] misc: lan966x_pci: Introduce board specific data

[Herve Codina]

Only one device-tree overlay (lan966x_evb_lan9662_nic.dtbo) is handled
and this overlay is directly referenced in lan966x_pci_load_overlay().

This avoid to use the code for an other board.

In order to be more generic and to allow support for other boards (PCI
Vendor/Device IDs), introduce the lan966x_pci_info structure and attach
it to PCI Vendor/Device IDs handled by the driver.

This structure contains information related to the PCI board such as
information related to the dtbo describing the board we have to load.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/misc/lan966x_pci.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/lan966x_pci.c b/drivers/misc/lan966x_pci.c
index e60ab662b8f3..5fa6af531a53 100644
--- a/drivers/misc/lan966x_pci.c
+++ b/drivers/misc/lan966x_pci.c
@@ -18,10 +18,6 @@
 #include <linux/pci_ids.h>
 #include <linux/slab.h>
 
-/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */
-extern char __dtbo_lan966x_evb_lan9662_nic_begin[];
-extern char __dtbo_lan966x_evb_lan9662_nic_end[];
-
 struct pci_dev_intr_ctrl {
 	struct pci_dev *pci_dev;
 	struct irq_domain *irq_domain;
@@ -118,17 +114,23 @@ static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev)
 	return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl);
 }
 
+struct lan966x_pci_info {
+	void *dtbo_begin;
+	void *dtbo_end;
+};
+
 struct lan966x_pci {
 	struct device *dev;
 	int ovcs_id;
+	const struct lan966x_pci_info *info;
 };
 
 static int lan966x_pci_load_overlay(struct lan966x_pci *data)
 {
-	u32 dtbo_size = __dtbo_lan966x_evb_lan9662_nic_end - __dtbo_lan966x_evb_lan9662_nic_begin;
-	void *dtbo_start = __dtbo_lan966x_evb_lan9662_nic_begin;
+	const struct lan966x_pci_info *info = data->info;
 
-	return of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev));
+	return of_overlay_fdt_apply(info->dtbo_begin, info->dtbo_end - info->dtbo_begin,
+				    &data->ovcs_id, dev_of_node(data->dev));
 }
 
 static void lan966x_pci_unload_overlay(struct lan966x_pci *data)
@@ -169,6 +171,9 @@ static int lan966x_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 
 	pci_set_drvdata(pdev, data);
 	data->dev = dev;
+	data->info = (const struct lan966x_pci_info *)id->driver_data;
+	if (!data->info)
+		return -EINVAL;
 
 	ret = lan966x_pci_load_overlay(data);
 	if (ret)
@@ -196,8 +201,17 @@ static void lan966x_pci_remove(struct pci_dev *pdev)
 	lan966x_pci_unload_overlay(data);
 }
 
+/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */
+extern char __dtbo_lan966x_evb_lan9662_nic_begin[];
+extern char __dtbo_lan966x_evb_lan9662_nic_end[];
+
+static struct lan966x_pci_info evb_lan9662_nic_info = {
+	.dtbo_begin = __dtbo_lan966x_evb_lan9662_nic_begin,
+	.dtbo_end = __dtbo_lan966x_evb_lan9662_nic_end,
+};
+
 static struct pci_device_id lan966x_pci_ids[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_LAN9662) },
+	{ PCI_DEVICE_DATA(EFAR, LAN9662, &evb_lan9662_nic_info) },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, lan966x_pci_ids);
-- 
2.49.0

[PATCH v3 26/28] misc: lan966x_pci: Add dtsi/dtso nodes in order to support SFPs

[Herve Codina]

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_evb_lan9662_nic.dtso | 95 +++++++++++++++++++++++
 drivers/misc/lan966x_pci.dtsi             | 42 ++++++++++
 2 files changed, 137 insertions(+)

diff --git a/drivers/misc/lan966x_evb_lan9662_nic.dtso b/drivers/misc/lan966x_evb_lan9662_nic.dtso
index b3de5f14d9cb..20e1fe4f78bf 100644
--- a/drivers/misc/lan966x_evb_lan9662_nic.dtso
+++ b/drivers/misc/lan966x_evb_lan9662_nic.dtso
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/mfd/atmel-flexcom.h>
 #include <dt-bindings/phy/phy-lan966x-serdes.h>
 
 /dts-v1/;
@@ -28,15 +29,93 @@ __overlay__ {
 
 			#include "lan966x_pci.dtsi"
 
+			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>;
+			};
 		};
 	};
 };
 
+&flx0 {
+	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-0 = <&fc0_a_pins>;
+	pinctrl-names = "default";
+	i2c-analog-filter;
+	i2c-digital-filter;
+	i2c-digital-filter-width-ns = <35>;
+	status = "okay";
+};
+
 &gpio {
 	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;
+	};
 };
 
 &lan966x_phy0 {
@@ -65,6 +144,22 @@ &port1 {
 	status = "okay";
 };
 
+&port2 {
+	phy-mode = "sgmii";
+	phys = <&serdes 2 SERDES6G(0)>;
+	sfp = <&sfp2>;
+	managed = "in-band-status";
+	status = "okay";
+};
+
+&port3 {
+	phy-mode = "sgmii";
+	phys = <&serdes 3 SERDES6G(1)>;
+	sfp = <&sfp3>;
+	managed = "in-band-status";
+	status = "okay";
+};
+
 &switch {
 	pinctrl-names = "default";
 	pinctrl-0 = <&tod_pins>;
diff --git a/drivers/misc/lan966x_pci.dtsi b/drivers/misc/lan966x_pci.dtsi
index 170298084fa5..d5c2056e4e5c 100644
--- a/drivers/misc/lan966x_pci.dtsi
+++ b/drivers/misc/lan966x_pci.dtsi
@@ -3,6 +3,7 @@
  * Copyright (C) 2025 Microchip UNG
  */
 
+#include <dt-bindings/clock/microchip,lan966x.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 
 cpu_clk: clock-600000000 {
@@ -61,6 +62,39 @@ port1: port@1 {
 				reg = <1>;
 				status = "disabled";
 			};
+
+			port2: port@2 {
+				reg = <2>;
+				status = "disabled";
+			};
+
+			port3: port@3 {
+				reg = <3>;
+				status = "disabled";
+			};
+		};
+	};
+
+	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>;
+		status = "disabled";
+
+		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>;
+			status = "disabled";
 		};
 	};
 
@@ -69,6 +103,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>;
-- 
2.49.0

[PATCH v3 27/28] misc: lan966x_pci: Sort the drivers list in Kconfig help

[Herve Codina]

The LAN966X Kconfig help section mentions drivers related to
devices.

Sort this list alphabetically.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/misc/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b9ca56930003..b05fddff8f97 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -636,13 +636,13 @@ config MCHP_LAN966X_PCI
 	  Even if this driver does not depend on those other drivers, in order
 	  to have a fully functional board, the following drivers are needed:
 	    - fixed-clock (COMMON_CLK)
-	    - lan966x-oic (LAN966X_OIC)
 	    - lan966x-cpu-syscon (MFD_SYSCON)
-	    - lan966x-switch-reset (RESET_MCHP_SPARX5)
+	    - lan966x-miim (MDIO_MSCC_MIIM)
+	    - lan966x-oic (LAN966X_OIC)
 	    - lan966x-pinctrl (PINCTRL_OCELOT)
 	    - lan966x-serdes (PHY_LAN966X_SERDES)
-	    - lan966x-miim (MDIO_MSCC_MIIM)
 	    - lan966x-switch (LAN966X_SWITCH)
+	    - lan966x-switch-reset (RESET_MCHP_SPARX5)
 
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
-- 
2.49.0

[PATCH v3 28/28] misc: lan966x_pci: Add drivers needed to support SFPs in Kconfig help

[Herve Codina]

Recently, new device-tree nodes were added in the overlay to add support
for SFPs on LAN966x PCI device.

The LAN966X Kconfig help section mentions drivers related to devices
added based on the overlay description.

Add drivers related to devices described by those new nodes in the
already existing driver list.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/misc/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b05fddff8f97..ab6a94b201be 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -636,13 +636,18 @@ config MCHP_LAN966X_PCI
 	  Even if this driver does not depend on those other drivers, in order
 	  to have a fully functional board, the following drivers are needed:
 	    - fixed-clock (COMMON_CLK)
+	    - i2c-mux-pinctrl (I2C_MUX_PINCTRL)
 	    - lan966x-cpu-syscon (MFD_SYSCON)
+	    - lan966x-gck (COMMON_CLK_LAN966X)
 	    - lan966x-miim (MDIO_MSCC_MIIM)
 	    - lan966x-oic (LAN966X_OIC)
 	    - lan966x-pinctrl (PINCTRL_OCELOT)
 	    - lan966x-serdes (PHY_LAN966X_SERDES)
 	    - lan966x-switch (LAN966X_SWITCH)
 	    - lan966x-switch-reset (RESET_MCHP_SPARX5)
+	    - sam9x60-i2c (I2C_AT91)
+	    - sama5d2-flexcom (MFD_ATMEL_FLEXCOM)
+	    - sfp (SFP)
 
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
-- 
2.49.0

Re: [PATCH v3 09/28] cxl/test: Use device_set_node()

[Dave Jiang]

On 6/13/25 6:47 AM, Herve Codina wrote:
> The code set directly dev->fwnode.
> 
> Use the dedicated helper to perform this operation.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  tools/testing/cxl/test/cxl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 8a5815ca870d..1db7732a70df 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -1046,7 +1046,7 @@ static void mock_companion(struct acpi_device *adev, struct device *dev)
>  {
>  	device_initialize(&adev->dev);
>  	fwnode_init(&adev->fwnode, NULL);
> -	dev->fwnode = &adev->fwnode;
> +	device_set_node(dev, &adev->fwnode);
>  	adev->fwnode.dev = dev;
>  }
>  

Re: [PATCH v3 10/28] cxl/test: Use fw_devlink_set_device()

[Dave Jiang]

On 6/13/25 6:47 AM, Herve Codina wrote:
> The code set directly fwnode.dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  tools/testing/cxl/test/cxl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 1db7732a70df..80e6fb3c9a43 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -1047,7 +1047,7 @@ static void mock_companion(struct acpi_device *adev, struct device *dev)
>  	device_initialize(&adev->dev);
>  	fwnode_init(&adev->fwnode, NULL);
>  	device_set_node(dev, &adev->fwnode);
> -	adev->fwnode.dev = dev;
> +	fw_devlink_set_device(&adev->fwnode, dev);
>  }
>  
>  #ifndef SZ_64G

Re: [PATCH v3 06/28] driver core: fw_devlink: Introduce fw_devlink_set_device()

[Saravana Kannan]

On Fri, Jun 13, 2025 at 6:49 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> Setting fwnode->dev is specific to fw_devlink.
>
> In order to avoid having a direct 'fwnode->dev = dev;' in several
> place in the kernel, introduce fw_devlink_set_device() helper to perform
> this operation.
>

This should not be set anywhere outside the driver core files. I'll
get to reviewing the series, but until then, NACK to this.

Is there a specific patch that explain why we need to set this outside
driver core?

-Saravana

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  include/linux/fwnode.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index a921ca2fe940..a1345e274125 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -231,4 +231,10 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
>  void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode);
>  bool fw_devlink_is_strict(void);
>
> +static inline void fw_devlink_set_device(struct fwnode_handle *fwnode,
> +                                        struct device *dev)
> +{
> +       fwnode->dev = dev;
> +}
> +
>  #endif
> --
> 2.49.0
>

Re: [PATCH v3 24/28] PCI: Add Microchip LAN9662 PCI Device ID

[Bjorn Helgaas]

On Fri, Jun 13, 2025 at 03:48:04PM +0200, Herve Codina wrote:
> Existing code uses the 0x9660 value (LAN9662 PCI Device ID) in several
> places.
> 
> Avoid this direct use of the 0x9660 value replacing it by defined PCI
> Device ID.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/misc/lan966x_pci.c | 2 +-
>  drivers/pci/quirks.c       | 2 +-
>  include/linux/pci_ids.h    | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/lan966x_pci.c b/drivers/misc/lan966x_pci.c
> index b28066c96534..e60ab662b8f3 100644
> --- a/drivers/misc/lan966x_pci.c
> +++ b/drivers/misc/lan966x_pci.c
> @@ -197,7 +197,7 @@ static void lan966x_pci_remove(struct pci_dev *pdev)
>  }
>  
>  static struct pci_device_id lan966x_pci_ids[] = {
> -	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_LAN9662) },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(pci, lan966x_pci_ids);
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index d7f4ee634263..bde077ce663a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6302,7 +6302,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0xa76e, dpc_log_size);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_LAN9662, of_pci_make_dev_node);
>  
>  /*
>   * Devices known to require a longer delay before first config space access
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index e2d71b6fdd84..5d69fde7dd97 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -934,6 +934,7 @@
>  #define PCI_VENDOR_ID_EFAR		0x1055
>  #define PCI_DEVICE_ID_EFAR_SLC90E66_1	0x9130
>  #define PCI_DEVICE_ID_EFAR_SLC90E66_3	0x9463
> +#define PCI_DEVICE_ID_EFAR_LAN9662	0x9660
>  
>  #define PCI_VENDOR_ID_MOTOROLA		0x1057
>  #define PCI_DEVICE_ID_MOTOROLA_MPC105	0x0001
> -- 
> 2.49.0
> 

Re: [PATCH v3 06/28] driver core: fw_devlink: Introduce fw_devlink_set_device()

[Herve Codina]

Hi Saravana,

On Fri, 13 Jun 2025 14:13:49 -0700
Saravana Kannan <saravanak@google.com> wrote:

> On Fri, Jun 13, 2025 at 6:49 AM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > Setting fwnode->dev is specific to fw_devlink.
> >
> > In order to avoid having a direct 'fwnode->dev = dev;' in several
> > place in the kernel, introduce fw_devlink_set_device() helper to perform
> > this operation.
> >  
> 
> This should not be set anywhere outside the driver core files. I'll
> get to reviewing the series, but until then, NACK to this.
> 
> Is there a specific patch that explain why we need to set this outside
> driver core?

We need to set it in case of creating device-tree node for PCI.

Usually, fwnode are created (based on DT or ACPI) and then, dev are
created.

In the PCI DT node creation case, device are already created and then, based
on information already computed by the kernel, DT node are created.

You can see that on patch 11 (dev setting was already upstream and it is
replace by a call to the helper for PCI host bridge) and on patch 13 (PCI
device).

Other patches (8, 9 and 10) replace the existing direct setting of the dev
member by a call to the helper.

Best regards,
Hervé

Re: [PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:
> The simple-pm-bus driver handles several simple busses. When it is used
> with busses other than a compatible "simple-pm-bus", it doesn't populate
> its child devices during its probe.
> 
> This confuses fw_devlink and results in wrong or missing devlinks.
> 
> Once a driver is bound to a device and the probe() has been called,
> device_links_driver_bound() is called.
> 
> This function performs operation based on the following assumption:
>     If a child firmware node of the bound device is not added as a
>     device, it will never be added.
> 
> Among operations done on fw_devlinks of those "never be added" devices,
> device_links_driver_bound() changes their supplier.
> 
> With devices attached to a simple-bus compatible device, this change
> leads to wrong devlinks where supplier of devices points to the device
> parent (i.e. simple-bus compatible device) instead of the device itself
> (i.e. simple-bus child).
> 
> When the device attached to the simple-bus is removed, because devlinks
> are not correct, its consumers are not removed first.
> 
> In order to have correct devlinks created, make the simple-pm-bus driver
> compliant with the devlink assumption and create its child devices
> during its probe.

...

>  	if (match && match->data) {
>  		if (of_property_match_string(np, "compatible", match->compatible) == 0)
> -			return 0;
> +			goto populate;
>  		else
>  			return -ENODEV;
>  	}

A nit: seems that now we don't need to keep a symmetry in the branches and
hence the redundant 'else' can be dropped

		if (of_property_match_string(np, "compatible", match->compatible) != 0)
			return -ENODEV;
		goto populate;
	}

But this might be out of the scope here and can be done later on. In my opinion
it will make code slightly easier to follow.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 06/28] driver core: fw_devlink: Introduce fw_devlink_set_device()

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 02:13:49PM -0700, Saravana Kannan wrote:
> On Fri, Jun 13, 2025 at 6:49 AM Herve Codina <herve.codina@bootlin.com> wrote:

> > Setting fwnode->dev is specific to fw_devlink.
> >
> > In order to avoid having a direct 'fwnode->dev = dev;' in several
> > place in the kernel, introduce fw_devlink_set_device() helper to perform
> > this operation.
> 
> This should not be set anywhere outside the driver core files. I'll
> get to reviewing the series

Strictly speaking I agree with you, but this is not a some driver case,
it's very special and also we have some (ab)users of it.
I can relax the requirement to not set outside of a core functionality,
(like driver core, PCI core) which are tightly related to driver core
one.

Just my 2c.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 08/28] pinctrl: cs42l43: Use fw_devlink_set_device()

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 03:47:48PM +0200, Herve Codina wrote:
> The code set directly fwnode->dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 12/28] driver core: fw_devlink: Tag the fwnode dev member as private

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 03:47:52PM +0200, Herve Codina wrote:
> The dev member in the fwnode structure should be accessed only by the
> fwnode core. The fw_devlink_set_device() helper has been introduced to
> avoid users to set this member directly. All users have been converted
> and use fw_devlink_set_device() to set the field.
> 
> To avoid future abusers who might set the field directly and allow
> sparse to detect them, tag the dev field as a private field.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 07/28] drivers: core: Use fw_devlink_set_device()

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 03:47:47PM +0200, Herve Codina wrote:
> The code set directly fwnode->dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 11/28] PCI: of: Use fw_devlink_set_device()

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 03:47:51PM +0200, Herve Codina wrote:
> The code set directly fwnode.dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 19/28] clk: lan966x: Add MCHP_LAN966X_PCI dependency

[Stephen Boyd]

Quoting Herve Codina (2025-06-13 06:47:59)
> The lan966x clock controller depends on the LAN969x architecture or the
> LAN966x SoC.
> 
> This clock controller can be used by the LAN966x PCI device and so it
> needs to be available when the LAN966x PCI device is enabled.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v3 02/28] driver core: Rename get_dev_from_fwnode() wrapper to get_device_from_fwnode()

[Rob Herring]

On Fri, Jun 13, 2025 at 03:47:42PM +0200, Herve Codina wrote:
> get_dev_from_fwnode() calls get_device() and so it acquires a reference
> on the device returned.
> 
> In order to be more obvious that this wrapper is a get_device() variant,
> rename it to get_device_from_fwnode().
> 
> Suggested-by: Mark Brown <broonie@kernel.org>
> Link: https://lore.kernel.org/lkml/CAGETcx97QjnjVR8Z5g0ndLHpK96hLd4aYSV=iEkKPNbNOccYmA@mail.gmail.com/
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Saravana Kannan <saravanak@google.com>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/base/core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index cbc0099d8ef2..36ccee91ba9a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1881,7 +1881,7 @@ static void fw_devlink_unblock_consumers(struct device *dev)
>  	device_links_write_unlock();
>  }
>  
> -#define get_dev_from_fwnode(fwnode)	get_device((fwnode)->dev)
> +#define get_device_from_fwnode(fwnode)	get_device((fwnode)->dev)

In patch 3, you add the same define. Is there some reason to not move it 
to a header?

Rob

Re: [PATCH v3 02/28] driver core: Rename get_dev_from_fwnode() wrapper to get_device_from_fwnode()

[Herve Codina]

Hi Rob,

On Fri, 27 Jun 2025 09:18:46 -0500
Rob Herring <robh@kernel.org> wrote:

> On Fri, Jun 13, 2025 at 03:47:42PM +0200, Herve Codina wrote:
> > get_dev_from_fwnode() calls get_device() and so it acquires a reference
> > on the device returned.
> > 
> > In order to be more obvious that this wrapper is a get_device() variant,
> > rename it to get_device_from_fwnode().
> > 
> > Suggested-by: Mark Brown <broonie@kernel.org>
> > Link: https://lore.kernel.org/lkml/CAGETcx97QjnjVR8Z5g0ndLHpK96hLd4aYSV=iEkKPNbNOccYmA@mail.gmail.com/
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Reviewed-by: Saravana Kannan <saravanak@google.com>
> > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  drivers/base/core.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index cbc0099d8ef2..36ccee91ba9a 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -1881,7 +1881,7 @@ static void fw_devlink_unblock_consumers(struct device *dev)
> >  	device_links_write_unlock();
> >  }
> >  
> > -#define get_dev_from_fwnode(fwnode)	get_device((fwnode)->dev)
> > +#define get_device_from_fwnode(fwnode)	get_device((fwnode)->dev)  
> 
> In patch 3, you add the same define. Is there some reason to not move it 
> to a header?
> 

In this patch (patch 2), I rename the define. In patch 3, I move the define in
an other place in the same file (core.c) in order to have it available for the
function added (also in patch 3).

I don't think we need to move it to a header.

Best regards,
Hervé

Re: [PATCH v3 06/28] driver core: fw_devlink: Introduce fw_devlink_set_device()

[Herve Codina]

Hi Saravana,

On Mon, 16 Jun 2025 09:04:06 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Saravana,
> 
> On Fri, 13 Jun 2025 14:13:49 -0700
> Saravana Kannan <saravanak@google.com> wrote:
> 
> > On Fri, Jun 13, 2025 at 6:49 AM Herve Codina <herve.codina@bootlin.com> wrote:  
> > >
> > > Setting fwnode->dev is specific to fw_devlink.
> > >
> > > In order to avoid having a direct 'fwnode->dev = dev;' in several
> > > place in the kernel, introduce fw_devlink_set_device() helper to perform
> > > this operation.
> > >    
> > 
> > This should not be set anywhere outside the driver core files. I'll
> > get to reviewing the series, but until then, NACK to this.
> > 
> > Is there a specific patch that explain why we need to set this outside
> > driver core?  
> 
> We need to set it in case of creating device-tree node for PCI.
> 
> Usually, fwnode are created (based on DT or ACPI) and then, dev are
> created.
> 
> In the PCI DT node creation case, device are already created and then, based
> on information already computed by the kernel, DT node are created.
> 
> You can see that on patch 11 (dev setting was already upstream and it is
> replace by a call to the helper for PCI host bridge) and on patch 13 (PCI
> device).
> 
> Other patches (8, 9 and 10) replace the existing direct setting of the dev
> member by a call to the helper.
> 

Have you got time to look at the series, patches I pointed out and the reply
from Andy?

Are modifications still nacked on your side?
If so, what kind of modification would you like to see in order to move
forward?

Best regards,
Hervé

Re: [PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Rob Herring]

On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:
> The simple-pm-bus driver handles several simple busses. When it is used
> with busses other than a compatible "simple-pm-bus", it doesn't populate
> its child devices during its probe.
> 
> This confuses fw_devlink and results in wrong or missing devlinks.
> 
> Once a driver is bound to a device and the probe() has been called,
> device_links_driver_bound() is called.
> 
> This function performs operation based on the following assumption:
>     If a child firmware node of the bound device is not added as a
>     device, it will never be added.
> 
> Among operations done on fw_devlinks of those "never be added" devices,
> device_links_driver_bound() changes their supplier.
> 
> With devices attached to a simple-bus compatible device, this change
> leads to wrong devlinks where supplier of devices points to the device
> parent (i.e. simple-bus compatible device) instead of the device itself
> (i.e. simple-bus child).
> 
> When the device attached to the simple-bus is removed, because devlinks
> are not correct, its consumers are not removed first.
> 
> In order to have correct devlinks created, make the simple-pm-bus driver
> compliant with the devlink assumption and create its child devices
> during its probe.

IIRC, skipping child nodes was because there were problems with 
letting the driver handle 'simple-bus'. How does this avoid that now?

The root of_platform_populate() that created the simple-bus device that 
gets us to the probe here will continue descending into child nodes. 
Meanwhile, the probe here is also descending into those same child 
nodes. Best case, that's just redundant. Worst case, won't you still 
have the same problem if the first of_platform_populate() creates the 
devices first?

Rob

Re: [PATCH v3 00/28] lan966x pci device: Add support for SFPs

[Rob Herring]

On Fri, Jun 13, 2025 at 03:47:40PM +0200, Herve Codina wrote:
> Hi,
> 
> This series add support for SFPs ports available on the LAN966x PCI
> device. In order to have the SFPs supported, additional devices are
> needed such as clock controller and I2C.
> 
> As a reminder, the LAN966x PCI device driver use a device-tree overlay
> to describe devices available on the PCI board. Adding support for SFPs
> ports consists in adding more devices in the already existing
> device-tree overlay.
> 
> With those devices added, the device-tree overlay is more complex and
> some consumer/supplier relationship are needed in order to remove
> devices in correct order when the LAN966x PCI driver is removed.
> 
> Those links are typically provided by fw_devlink and we faced some
> issues with fw_devlink and overlays.
> 
> This series gives the big picture related to the SFPs support from
> fixing issues to adding new devices. Of course, it can be split if
> needed.
> 
> The first part of the series (patch 1, 2 and 3) fixes fw_devlink when it
> is used with overlay. Patches 1 and 3 were previously sent by Saravana
> [0]. I just rebased them on top of v6.15-rc1 and added patch 2 in order
> to take into account feedback received on the series sent by Saravana.
> 
> Those modification were not sufficient in our case and so, on top of
> that, patch 4 and 5 fix some more issues related to fw_devlink.
> 
> Patches 6 to 12 introduce and use fw_devlink_set_device() in already
> existing code.
> 
> Patches 13 and 14 are related also to fw_devlink but specific to PCI and
> the device-tree nodes created during enumeration.
> 
> Patches 15, 15 and 17 are related fw_devlink too but specific to I2C
> muxes. Patches purpose is to correctly set a link between an adapter
> supplier and its consumer. Indeed, an i2c mux adapter's parent is not
> the i2c mux supplier but the adapter the i2c mux is connected to. Adding
> a new link between the adapter supplier involved when i2c muxes are used
> avoid a freeze observed during device removal.
> 
> Patch 18 adds support for fw_delink on x86. fw_devlink is needed to have
> the consumer/supplier relationship between devices in order to ensure a
> correct device removal order. Adding fw_devlink support for x86 has been
> tried in the past but was reverted [1] because it broke some systems.
> Instead of enabling fw_devlink on *all* x86 system or on *all* x86
> system except on those where it leads to issue, enable it only on system
> where it is needed.
> 
> Patches 19 and 20 allow to build clock and i2c controller used by the
> LAN966x PCI device when the LAN966x PCI device is enabled.
> 
> Patches 21 to 25 are specific to the LAN966x. They touch the current
> dtso, split it in dtsi/dtso files, rename the dtso and improve the
> driver to allow easier support for other boards.
> 
> The next patch (patch 26) update the LAN966x device-tree overlay itself
> to have the SPF ports and devices they depends on described.
> 
> The last two patches (patches 27 and 28) sort the existing drivers in
> the needed driver list available in the Kconfig help and add new drivers
> in this list keep the list up to date with the devices described in the
> device-tree overlay.
> 
> Once again, this series gives the big picture and can be split if
> needed. Let me know.

Please suggest how you think this should get merged? There's 8 
maintainer trees involved here. Some parts can be merged independently? 
We need to spread over 2 cycles? Greg just takes it all?

Rob

Re: [PATCH v3 18/28] of: property: Allow fw_devlink device-tree on x86 when PCI device-tree node creation is enabled

[Rob Herring]

On Fri, Jun 13, 2025 at 03:47:58PM +0200, Herve Codina wrote:
> PCI drivers can use a device-tree overlay to describe the hardware
> available on the PCI board. This is the case, for instance, of the
> LAN966x PCI device driver.
> 
> Adding some more nodes in the device-tree overlay adds some more
> consumer/supplier relationship between devices instantiated from this
> overlay.
> 
> Those fw_node consumer/supplier relationships are handled by fw_devlink
> and are created based on the device-tree parsing done by the
> of_fwnode_add_links() function.
> 
> Those consumer/supplier links are needed in order to ensure a correct PM
> runtime management and a correct removal order between devices.
> 
> For instance, without those links a supplier can be removed before its
> consumers is removed leading to all kind of issue if this consumer still
> want the use the already removed supplier.
> 
> The support for the usage of an overlay from a PCI driver has been added
> on x86 systems in commit 1f340724419ed ("PCI: of: Create device tree PCI
> host bridge node").
> 
> In the past, support for fw_devlink on x86 had been tried but this
> support has been removed in commit 4a48b66b3f52 ("of: property: Disable
> fw_devlink DT support for X86"). Indeed, this support was breaking some
> x86 systems such as OLPC system and the regression was reported in [0].
> 
> Instead of disabling this support for all x86 system, a first approach
> would be to use a finer grain and disable this support only for the
> possible problematic subset of x86 systems (at least OLPC and CE4100).
> 
> This first approach could still leads to issues. Indeed, the list of
> possible problematic system and the way to identify them using Kconfig
> symbols is not well defined and so some system can be missed leading to
> kernel regressions on those missing systems.
> 
> Use an other way and enable the support on x86 system only when this
> support is needed by some specific feature. The usage of a device-tree
> overlay by a PCI driver and thus the creation of PCI device-tree nodes
> is a feature that needs it.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Link: https://lore.kernel.org/lkml/3c1f2473-92ad-bfc4-258e-a5a08ad73dd0@web.de/ [0]
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/of/property.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index c1feb631e383..8b5cfee696e2 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1605,7 +1605,7 @@ static int of_fwnode_add_links(struct fwnode_handle *fwnode)
>  	const struct property *p;
>  	struct device_node *con_np = to_of_node(fwnode);
>  
> -	if (IS_ENABLED(CONFIG_X86))
> +	if (IS_ENABLED(CONFIG_X86) && !IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES))

I really want CONFIG_PCI_DYNAMIC_OF_NODES to go away at some point, not 
add more users. 

I think this should instead check for specific platforms not with 
kconfig symbols but DT properties. For ce4100, you can just check the 
root compatible string. For OLPC, there isn't a root compatible (in the 
DT I have). You could check for /architecture == OLPC instead. There's 
some virtualization guests using DT now too. I would think their DT's 
are simple enough to avoid any fw_devlink issues. 

Alternatively, we could perhaps make x86 fw_devlink default off and then 
enable it only when you create nodes. Maybe it has to be restricted a 
sub tree of the DT to avoid any later interactions if devices are 
unbound and rebound. Not a fully fleshed out idea...

Rob

Re: [PATCH v3 18/28] of: property: Allow fw_devlink device-tree on x86 when PCI device-tree node creation is enabled

[Andy Shevchenko]

On Fri, Jun 27, 2025 at 11:22:45AM -0500, Rob Herring wrote:
> On Fri, Jun 13, 2025 at 03:47:58PM +0200, Herve Codina wrote:

...

> > -	if (IS_ENABLED(CONFIG_X86))
> > +	if (IS_ENABLED(CONFIG_X86) && !IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES))
> 
> I really want CONFIG_PCI_DYNAMIC_OF_NODES to go away at some point, not 
> add more users. 
> 
> I think this should instead check for specific platforms not with 
> kconfig symbols but DT properties. For ce4100, you can just check the 
> root compatible string. For OLPC, there isn't a root compatible (in the 
> DT I have). You could check for /architecture == OLPC instead. There's 
> some virtualization guests using DT now too. I would think their DT's 
> are simple enough to avoid any fw_devlink issues. 

I don't think this is good approach. The above check is more reliable in my
opinion.

> Alternatively, we could perhaps make x86 fw_devlink default off

For my (little) knowledge I believe this is not feasible anymore.
Some x86 code (drivers) relies on fw_devlink nowadays. But take
this with grain of salt, I may be way mistaken.

> and then enable it only when you create nodes. Maybe it has to be restricted
> a sub tree of the DT to avoid any later interactions if devices are unbound
> and rebound. Not a fully fleshed out idea...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 18/28] of: property: Allow fw_devlink device-tree on x86 when PCI device-tree node creation is enabled

[Rob Herring]

On Fri, Jun 27, 2025 at 11:33 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Jun 27, 2025 at 11:22:45AM -0500, Rob Herring wrote:
> > On Fri, Jun 13, 2025 at 03:47:58PM +0200, Herve Codina wrote:
>
> ...
>
> > > -   if (IS_ENABLED(CONFIG_X86))
> > > +   if (IS_ENABLED(CONFIG_X86) && !IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES))
> >
> > I really want CONFIG_PCI_DYNAMIC_OF_NODES to go away at some point, not
> > add more users.
> >
> > I think this should instead check for specific platforms not with
> > kconfig symbols but DT properties. For ce4100, you can just check the
> > root compatible string. For OLPC, there isn't a root compatible (in the
> > DT I have). You could check for /architecture == OLPC instead. There's
> > some virtualization guests using DT now too. I would think their DT's
> > are simple enough to avoid any fw_devlink issues.
>
> I don't think this is good approach. The above check is more reliable in my
> opinion.

I'm fine with any solution that doesn't add a
CONFIG_PCI_DYNAMIC_OF_NODES which we can't remove. Adding it was a
kick the can down the road to merge the support worry the mixed
usecase (on ACPI systems) later. It's now later.

> > Alternatively, we could perhaps make x86 fw_devlink default off
>
> For my (little) knowledge I believe this is not feasible anymore.
> Some x86 code (drivers) relies on fw_devlink nowadays. But take
> this with grain of salt, I may be way mistaken.

Doesn't the CONFIG_X86 check disable it?

Rob

Re: [PATCH v3 02/28] driver core: Rename get_dev_from_fwnode() wrapper to get_device_from_fwnode()

[Rafael J. Wysocki]

On Fri, Jun 13, 2025 at 3:48 PM Herve Codina <herve.codina@bootlin.com> wrote:
>
> get_dev_from_fwnode() calls get_device() and so it acquires a reference
> on the device returned.
>
> In order to be more obvious that this wrapper is a get_device() variant,
> rename it to get_device_from_fwnode().
>
> Suggested-by: Mark Brown <broonie@kernel.org>
> Link: https://lore.kernel.org/lkml/CAGETcx97QjnjVR8Z5g0ndLHpK96hLd4aYSV=iEkKPNbNOccYmA@mail.gmail.com/
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Saravana Kannan <saravanak@google.com>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/base/core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index cbc0099d8ef2..36ccee91ba9a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1881,7 +1881,7 @@ static void fw_devlink_unblock_consumers(struct device *dev)
>         device_links_write_unlock();
>  }
>
> -#define get_dev_from_fwnode(fwnode)    get_device((fwnode)->dev)
> +#define get_device_from_fwnode(fwnode) get_device((fwnode)->dev)
>
>  static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
>  {
> @@ -1891,7 +1891,7 @@ static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
>         if (!(fwnode->flags & FWNODE_FLAG_INITIALIZED))
>                 return false;
>
> -       dev = get_dev_from_fwnode(fwnode);
> +       dev = get_device_from_fwnode(fwnode);
>         ret = !dev || dev->links.status == DL_DEV_NO_DRIVER;
>         put_device(dev);
>
> @@ -1960,7 +1960,7 @@ static struct device *fwnode_get_next_parent_dev(const struct fwnode_handle *fwn
>         struct device *dev;
>
>         fwnode_for_each_parent_node(fwnode, parent) {
> -               dev = get_dev_from_fwnode(parent);
> +               dev = get_device_from_fwnode(parent);
>                 if (dev) {
>                         fwnode_handle_put(parent);
>                         return dev;
> @@ -2016,8 +2016,8 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
>                 goto out;
>         }
>
> -       sup_dev = get_dev_from_fwnode(sup_handle);
> -       con_dev = get_dev_from_fwnode(con_handle);
> +       sup_dev = get_device_from_fwnode(sup_handle);
> +       con_dev = get_device_from_fwnode(con_handle);
>         /*
>          * If sup_dev is bound to a driver and @con hasn't started binding to a
>          * driver, sup_dev can't be a consumer of @con. So, no need to check
> @@ -2156,7 +2156,7 @@ static int fw_devlink_create_devlink(struct device *con,
>         if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE)
>                 sup_dev = fwnode_get_next_parent_dev(sup_handle);
>         else
> -               sup_dev = get_dev_from_fwnode(sup_handle);
> +               sup_dev = get_device_from_fwnode(sup_handle);
>
>         if (sup_dev) {
>                 /*
> @@ -2225,7 +2225,7 @@ static void __fw_devlink_link_to_consumers(struct device *dev)
>                 bool own_link = true;
>                 int ret;
>
> -               con_dev = get_dev_from_fwnode(link->consumer);
> +               con_dev = get_device_from_fwnode(link->consumer);
>                 /*
>                  * If consumer device is not available yet, make a "proxy"
>                  * SYNC_STATE_ONLY link from the consumer's parent device to
> --

There are patches in flight that add new users of get_dev_from_fwnode(), see

https://lore.kernel.org/linux-pm/20250701114733.636510-1-ulf.hansson@linaro.org/T/#mc45a73769821cc520c35d8eb92d4bac75412bb7c

Re: [PATCH v3 04/28] driver core: Avoid warning when removing a device while its supplier is unbinding

[Rafael J. Wysocki]

On Fri, Jun 13, 2025 at 3:48 PM Herve Codina <herve.codina@bootlin.com> wrote:
>
> During driver removal, the following warning can appear:
>    WARNING: CPU: 1 PID: 139 at drivers/base/core.c:1497 __device_links_no_driver+0xcc/0xfc
>    ...
>    Call trace:
>      __device_links_no_driver+0xcc/0xfc (P)
>      device_links_driver_cleanup+0xa8/0xf0
>      device_release_driver_internal+0x208/0x23c
>      device_links_unbind_consumers+0xe0/0x108
>      device_release_driver_internal+0xec/0x23c
>      device_links_unbind_consumers+0xe0/0x108
>      device_release_driver_internal+0xec/0x23c
>      device_links_unbind_consumers+0xe0/0x108
>      device_release_driver_internal+0xec/0x23c
>      driver_detach+0xa0/0x12c
>      bus_remove_driver+0x6c/0xbc
>      driver_unregister+0x30/0x60
>      pci_unregister_driver+0x20/0x9c
>      lan966x_pci_driver_exit+0x18/0xa90 [lan966x_pci]
>
> This warning is triggered when a consumer is removed because the links
> status of its supplier is not DL_DEV_DRIVER_BOUND and the link flag
> DL_FLAG_SYNC_STATE_ONLY is not set.
>
> The topology in terms of consumers/suppliers used was the following
> (consumer ---> supplier):
>
>       i2c -----------> OIC ----> PCI device
>        |                ^
>        |                |
>        +---> pinctrl ---+
>
> When the PCI device is removed, the OIC (interrupt controller) has to be
> removed. In order to remove the OIC, pinctrl and i2c need to be removed
> and to remove pinctrl, i2c need to be removed. The removal order is:
>   1) i2c
>   2) pinctrl
>   3) OIC
>   4) PCI device
>
> In details, the removal sequence is the following (with 0000:01:00.0 the
> PCI device):
>   driver_detach: call device_release_driver_internal(0000:01:00.0)...
>     device_links_busy(0000:01:00.0):
>       links->status = DL_DEV_UNBINDING
>     device_links_unbind_consumers(0000:01:00.0):
>       0000:01:00.0--oic link->status = DL_STATE_SUPPLIER_UNBIND
>       call device_release_driver_internal(oic)...
>         device_links_busy(oic):
>           links->status = DL_DEV_UNBINDING
>         device_links_unbind_consumers(oic):
>           oic--pinctrl link->status = DL_STATE_SUPPLIER_UNBIND
>           call device_release_driver_internal(pinctrl)...
>             device_links_busy(pinctrl):
>               links->status = DL_DEV_UNBINDING
>             device_links_unbind_consumers(pinctrl):
>               pinctrl--i2c link->status = DL_STATE_SUPPLIER_UNBIND
>               call device_release_driver_internal(i2c)...
>                 device_links_busy(i2c): links->status = DL_DEV_UNBINDING
>                 __device_links_no_driver(i2c)...
>                   pinctrl--i2c link->status is DL_STATE_SUPPLIER_UNBIND
>                   oic--i2c link->status is DL_STATE_ACTIVE
>                   oic--i2c link->supplier->links.status is DL_DEV_UNBINDING
>
> The warning is triggered by the i2c removal because the OIC (supplier)
> links status is not DL_DEV_DRIVER_BOUND. Its links status is indeed set
> to DL_DEV_UNBINDING.
>
> It is perfectly legit to have the links status set to DL_DEV_UNBINDING
> in that case. Indeed we had started to unbind the OIC which triggered
> the consumer unbinding and didn't finish yet when the i2c is unbound.
>
> Avoid the warning when the supplier links status is set to
> DL_DEV_UNBINDING and thus support this removal sequence without any
> warnings.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

The change is OK, so

Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>

but it will clash with

https://lore.kernel.org/linux-pm/2793309.mvXUDI8C0e@rjwysocki.net/

that's been queued up by Greg AFAICS.

> ---
>  drivers/base/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 8fead097c404..ce367c44f642 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1494,7 +1494,8 @@ static void __device_links_no_driver(struct device *dev)
>                 if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) {
>                         WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
>                 } else {
> -                       WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
> +                       WARN_ON(link->supplier->links.status != DL_DEV_UNBINDING &&
> +                               !(link->flags & DL_FLAG_SYNC_STATE_ONLY));
>                         WRITE_ONCE(link->status, DL_STATE_DORMANT);
>                 }
>         }
> --
> 2.49.0
>

Re: [PATCH v3 02/28] driver core: Rename get_dev_from_fwnode() wrapper to get_device_from_fwnode()

[Danilo Krummrich]

On 6/27/25 4:18 PM, Rob Herring wrote:
> On Fri, Jun 13, 2025 at 03:47:42PM +0200, Herve Codina wrote:
>> get_dev_from_fwnode() calls get_device() and so it acquires a reference
>> on the device returned.
>>
>> In order to be more obvious that this wrapper is a get_device() variant,
>> rename it to get_device_from_fwnode().
>>
>> Suggested-by: Mark Brown <broonie@kernel.org>
>> Link: https://lore.kernel.org/lkml/CAGETcx97QjnjVR8Z5g0ndLHpK96hLd4aYSV=iEkKPNbNOccYmA@mail.gmail.com/
>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Reviewed-by: Saravana Kannan <saravanak@google.com>
>> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> ---
>>   drivers/base/core.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index cbc0099d8ef2..36ccee91ba9a 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -1881,7 +1881,7 @@ static void fw_devlink_unblock_consumers(struct device *dev)
>>   	device_links_write_unlock();
>>   }
>>   
>> -#define get_dev_from_fwnode(fwnode)	get_device((fwnode)->dev)
>> +#define get_device_from_fwnode(fwnode)	get_device((fwnode)->dev)
> 
> In patch 3, you add the same define. Is there some reason to not move it
> to a header?

AFAIK, the struct device pointer in struct fwnode_handle is not backed by a
reference count, which means that it's the callers responsibility to ensure that
it's guaranteed that the pointer in struct fwnode_handle is still valid.

Besides some driver-core internals the pointer shouldn't be used, and hence this
helper shouldn't be available through a public header.

Re: [PATCH v3 04/28] driver core: Avoid warning when removing a device while its supplier is unbinding

[Saravana Kannan]

On Wed, Jul 2, 2025 at 11:22 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Jun 13, 2025 at 3:48 PM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > During driver removal, the following warning can appear:
> >    WARNING: CPU: 1 PID: 139 at drivers/base/core.c:1497 __device_links_no_driver+0xcc/0xfc
> >    ...
> >    Call trace:
> >      __device_links_no_driver+0xcc/0xfc (P)
> >      device_links_driver_cleanup+0xa8/0xf0
> >      device_release_driver_internal+0x208/0x23c
> >      device_links_unbind_consumers+0xe0/0x108
> >      device_release_driver_internal+0xec/0x23c
> >      device_links_unbind_consumers+0xe0/0x108
> >      device_release_driver_internal+0xec/0x23c
> >      device_links_unbind_consumers+0xe0/0x108
> >      device_release_driver_internal+0xec/0x23c
> >      driver_detach+0xa0/0x12c
> >      bus_remove_driver+0x6c/0xbc
> >      driver_unregister+0x30/0x60
> >      pci_unregister_driver+0x20/0x9c
> >      lan966x_pci_driver_exit+0x18/0xa90 [lan966x_pci]
> >
> > This warning is triggered when a consumer is removed because the links
> > status of its supplier is not DL_DEV_DRIVER_BOUND and the link flag
> > DL_FLAG_SYNC_STATE_ONLY is not set.
> >
> > The topology in terms of consumers/suppliers used was the following
> > (consumer ---> supplier):
> >
> >       i2c -----------> OIC ----> PCI device
> >        |                ^
> >        |                |
> >        +---> pinctrl ---+
> >
> > When the PCI device is removed, the OIC (interrupt controller) has to be
> > removed. In order to remove the OIC, pinctrl and i2c need to be removed
> > and to remove pinctrl, i2c need to be removed. The removal order is:
> >   1) i2c
> >   2) pinctrl
> >   3) OIC
> >   4) PCI device
> >
> > In details, the removal sequence is the following (with 0000:01:00.0 the
> > PCI device):
> >   driver_detach: call device_release_driver_internal(0000:01:00.0)...
> >     device_links_busy(0000:01:00.0):
> >       links->status = DL_DEV_UNBINDING
> >     device_links_unbind_consumers(0000:01:00.0):
> >       0000:01:00.0--oic link->status = DL_STATE_SUPPLIER_UNBIND
> >       call device_release_driver_internal(oic)...
> >         device_links_busy(oic):
> >           links->status = DL_DEV_UNBINDING
> >         device_links_unbind_consumers(oic):
> >           oic--pinctrl link->status = DL_STATE_SUPPLIER_UNBIND
> >           call device_release_driver_internal(pinctrl)...
> >             device_links_busy(pinctrl):
> >               links->status = DL_DEV_UNBINDING
> >             device_links_unbind_consumers(pinctrl):
> >               pinctrl--i2c link->status = DL_STATE_SUPPLIER_UNBIND
> >               call device_release_driver_internal(i2c)...
> >                 device_links_busy(i2c): links->status = DL_DEV_UNBINDING
> >                 __device_links_no_driver(i2c)...
> >                   pinctrl--i2c link->status is DL_STATE_SUPPLIER_UNBIND
> >                   oic--i2c link->status is DL_STATE_ACTIVE
> >                   oic--i2c link->supplier->links.status is DL_DEV_UNBINDING
> >
> > The warning is triggered by the i2c removal because the OIC (supplier)
> > links status is not DL_DEV_DRIVER_BOUND. Its links status is indeed set
> > to DL_DEV_UNBINDING.
> >
> > It is perfectly legit to have the links status set to DL_DEV_UNBINDING
> > in that case. Indeed we had started to unbind the OIC which triggered
> > the consumer unbinding and didn't finish yet when the i2c is unbound.
> >
> > Avoid the warning when the supplier links status is set to
> > DL_DEV_UNBINDING and thus support this removal sequence without any
> > warnings.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
>
> The change is OK, so
>
> Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>

You can add mine too.

Reviewed-by: Saravana Kannan <saravanak@google.com>

-Saravana

>
> but it will clash with
>
> https://lore.kernel.org/linux-pm/2793309.mvXUDI8C0e@rjwysocki.net/
>
> that's been queued up by Greg AFAICS.
>
> > ---
> >  drivers/base/core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 8fead097c404..ce367c44f642 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -1494,7 +1494,8 @@ static void __device_links_no_driver(struct device *dev)
> >                 if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) {
> >                         WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
> >                 } else {
> > -                       WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
> > +                       WARN_ON(link->supplier->links.status != DL_DEV_UNBINDING &&
> > +                               !(link->flags & DL_FLAG_SYNC_STATE_ONLY));
> >                         WRITE_ONCE(link->status, DL_STATE_DORMANT);
> >                 }
> >         }
> > --
> > 2.49.0
> >

Re: [PATCH v3 18/28] of: property: Allow fw_devlink device-tree on x86 when PCI device-tree node creation is enabled

[Herve Codina]

Hi Rob, Andy,

On Fri, 27 Jun 2025 12:49:36 -0500
Rob Herring <robh@kernel.org> wrote:

> On Fri, Jun 27, 2025 at 11:33 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Jun 27, 2025 at 11:22:45AM -0500, Rob Herring wrote:  
> > > On Fri, Jun 13, 2025 at 03:47:58PM +0200, Herve Codina wrote:  
> >
> > ...
> >  
> > > > -   if (IS_ENABLED(CONFIG_X86))
> > > > +   if (IS_ENABLED(CONFIG_X86) && !IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES))  
> > >
> > > I really want CONFIG_PCI_DYNAMIC_OF_NODES to go away at some point, not
> > > add more users.
> > >
> > > I think this should instead check for specific platforms not with
> > > kconfig symbols but DT properties. For ce4100, you can just check the
> > > root compatible string. For OLPC, there isn't a root compatible (in the
> > > DT I have). You could check for /architecture == OLPC instead. There's
> > > some virtualization guests using DT now too. I would think their DT's
> > > are simple enough to avoid any fw_devlink issues.  
> >
> > I don't think this is good approach. The above check is more reliable in my
> > opinion.  
> 
> I'm fine with any solution that doesn't add a
> CONFIG_PCI_DYNAMIC_OF_NODES which we can't remove. Adding it was a
> kick the can down the road to merge the support worry the mixed
> usecase (on ACPI systems) later. It's now later.
> 
> > > Alternatively, we could perhaps make x86 fw_devlink default off  
> >
> > For my (little) knowledge I believe this is not feasible anymore.
> > Some x86 code (drivers) relies on fw_devlink nowadays. But take
> > this with grain of salt, I may be way mistaken.  
> 
> Doesn't the CONFIG_X86 check disable it?
> 
> Rob

Filtering out by Kconfig seems a no-go:
  - Check for CONFIG_OLPC of CONFIG_X86_INTEL_CE as proposed in v1
    (https://lore.kernel.org/lkml/20250407145546.270683-12-herve.codina@bootlin.com/)
    was a no-go from Andy

  - Check for CONFIG_PCI_DYNAMIC_OF_NODES as proposed here is a no-go from
    Rob

I will follow Rob's suggestion based on DT properties. With a DT property
list, it would be easier to add more x86 fw_delink broken system in the list
of the system to exclude.

Best regards,
Hervé

Re: [PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Herve Codina]

Hi Rob,

On Fri, 27 Jun 2025 10:52:00 -0500
Rob Herring <robh@kernel.org> wrote:

> On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:
> > The simple-pm-bus driver handles several simple busses. When it is used
> > with busses other than a compatible "simple-pm-bus", it doesn't populate
> > its child devices during its probe.
> > 
> > This confuses fw_devlink and results in wrong or missing devlinks.
> > 
> > Once a driver is bound to a device and the probe() has been called,
> > device_links_driver_bound() is called.
> > 
> > This function performs operation based on the following assumption:
> >     If a child firmware node of the bound device is not added as a
> >     device, it will never be added.
> > 
> > Among operations done on fw_devlinks of those "never be added" devices,
> > device_links_driver_bound() changes their supplier.
> > 
> > With devices attached to a simple-bus compatible device, this change
> > leads to wrong devlinks where supplier of devices points to the device
> > parent (i.e. simple-bus compatible device) instead of the device itself
> > (i.e. simple-bus child).
> > 
> > When the device attached to the simple-bus is removed, because devlinks
> > are not correct, its consumers are not removed first.
> > 
> > In order to have correct devlinks created, make the simple-pm-bus driver
> > compliant with the devlink assumption and create its child devices
> > during its probe.  
> 
> IIRC, skipping child nodes was because there were problems with 
> letting the driver handle 'simple-bus'. How does this avoid that now?

I don't know about the specific issues related to those problems. Do you
have some pointers about them?

> 
> The root of_platform_populate() that created the simple-bus device that 
> gets us to the probe here will continue descending into child nodes. 
> Meanwhile, the probe here is also descending into those same child 
> nodes. Best case, that's just redundant. Worst case, won't you still 
> have the same problem if the first of_platform_populate() creates the 
> devices first?
> 

Maybe we could simply avoid of_platform_populate() to be recursive when a
device populate by of_platform_populate() is one of devices handled by
the simple-bus driver and let the simple-bus driver do the job.

of_platform_populate will handle the first level. It will populate children
of the node given to of_platform_populate() and the children of those
children will be populate by the simple-bus driver.

I could try a modification in that way. Do you think it could be a correct
solution?

Best regards,
Hervé

Re: [PATCH v3 02/28] driver core: Rename get_dev_from_fwnode() wrapper to get_device_from_fwnode()

[Herve Codina]

On Wed, 2 Jul 2025 20:17:15 +0200
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> On Fri, Jun 13, 2025 at 3:48 PM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > get_dev_from_fwnode() calls get_device() and so it acquires a reference
> > on the device returned.
> >
> > In order to be more obvious that this wrapper is a get_device() variant,
> > rename it to get_device_from_fwnode().
> >
> > Suggested-by: Mark Brown <broonie@kernel.org>
> > Link: https://lore.kernel.org/lkml/CAGETcx97QjnjVR8Z5g0ndLHpK96hLd4aYSV=iEkKPNbNOccYmA@mail.gmail.com/
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Reviewed-by: Saravana Kannan <saravanak@google.com>
> > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  drivers/base/core.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >

...

> 
> There are patches in flight that add new users of get_dev_from_fwnode(), see
> 
> https://lore.kernel.org/linux-pm/20250701114733.636510-1-ulf.hansson@linaro.org/T/#mc45a73769821cc520c35d8eb92d4bac75412bb7c

Thanks for pointing out.

I will monitor the series you pointed out.

Once it is applied, I will update my modifications but I will probably send other
iterations in the meantime in order to move forward on the other topic without
waiting for the Ulf's series.

In my other iterations I will point the dependency in patches impacted i.e
patches 2 and 3.

Best regards,
Hervé

Re: [PATCH v3 00/28] lan966x pci device: Add support for SFPs

[Herve Codina]

Hi Rob,

On Fri, 27 Jun 2025 10:58:37 -0500
Rob Herring <robh@kernel.org> wrote:

> On Fri, Jun 13, 2025 at 03:47:40PM +0200, Herve Codina wrote:
> > Hi,
> > 
> > This series add support for SFPs ports available on the LAN966x PCI
> > device. In order to have the SFPs supported, additional devices are
> > needed such as clock controller and I2C.
> > 
> > As a reminder, the LAN966x PCI device driver use a device-tree overlay
> > to describe devices available on the PCI board. Adding support for SFPs
> > ports consists in adding more devices in the already existing
> > device-tree overlay.
> > 
> > With those devices added, the device-tree overlay is more complex and
> > some consumer/supplier relationship are needed in order to remove
> > devices in correct order when the LAN966x PCI driver is removed.
> > 
> > Those links are typically provided by fw_devlink and we faced some
> > issues with fw_devlink and overlays.
> > 
> > This series gives the big picture related to the SFPs support from
> > fixing issues to adding new devices. Of course, it can be split if
> > needed.
> > 
> > The first part of the series (patch 1, 2 and 3) fixes fw_devlink when it
> > is used with overlay. Patches 1 and 3 were previously sent by Saravana
> > [0]. I just rebased them on top of v6.15-rc1 and added patch 2 in order
> > to take into account feedback received on the series sent by Saravana.
> > 
> > Those modification were not sufficient in our case and so, on top of
> > that, patch 4 and 5 fix some more issues related to fw_devlink.
> > 
> > Patches 6 to 12 introduce and use fw_devlink_set_device() in already
> > existing code.
> > 
> > Patches 13 and 14 are related also to fw_devlink but specific to PCI and
> > the device-tree nodes created during enumeration.
> > 
> > Patches 15, 15 and 17 are related fw_devlink too but specific to I2C
> > muxes. Patches purpose is to correctly set a link between an adapter
> > supplier and its consumer. Indeed, an i2c mux adapter's parent is not
> > the i2c mux supplier but the adapter the i2c mux is connected to. Adding
> > a new link between the adapter supplier involved when i2c muxes are used
> > avoid a freeze observed during device removal.
> > 
> > Patch 18 adds support for fw_delink on x86. fw_devlink is needed to have
> > the consumer/supplier relationship between devices in order to ensure a
> > correct device removal order. Adding fw_devlink support for x86 has been
> > tried in the past but was reverted [1] because it broke some systems.
> > Instead of enabling fw_devlink on *all* x86 system or on *all* x86
> > system except on those where it leads to issue, enable it only on system
> > where it is needed.
> > 
> > Patches 19 and 20 allow to build clock and i2c controller used by the
> > LAN966x PCI device when the LAN966x PCI device is enabled.
> > 
> > Patches 21 to 25 are specific to the LAN966x. They touch the current
> > dtso, split it in dtsi/dtso files, rename the dtso and improve the
> > driver to allow easier support for other boards.
> > 
> > The next patch (patch 26) update the LAN966x device-tree overlay itself
> > to have the SPF ports and devices they depends on described.
> > 
> > The last two patches (patches 27 and 28) sort the existing drivers in
> > the needed driver list available in the Kconfig help and add new drivers
> > in this list keep the list up to date with the devices described in the
> > device-tree overlay.
> > 
> > Once again, this series gives the big picture and can be split if
> > needed. Let me know.  
> 
> Please suggest how you think this should get merged? There's 8 
> maintainer trees involved here. Some parts can be merged independently? 
> We need to spread over 2 cycles? Greg just takes it all?
> 
> Rob

I will add this information in the next iteration.

I think, the merge strategy could be the following:
 - patches 1 to 14 could be merged by driver core maintainers in cycle N

 - patches 15 to 17 and 20 could be merged by I2C maintainers in cycle N
   without any dependency issues against other patches.

 - patch 18 could be merged by OF maintainers in cycle N without any
   dependency issues

 - patch 19 could be merged by clock maintainers in cycle N without any
   dependency issues.

 - patch 21 to 25 could be merged by misc maintainers in cycle N without any
   dependency issues.

 - patch 26 to 28, even if there is no compilation dependencies with other
   patches, they need the other patches applied to have a working system and
   so they could be merged in cycle N+1.

Also, as the big picture and the goal of this series has been shown, I can
extract patches from this series and send them alone depending on maintainers
preferences.

Maintainers, just tell me.

Best regards,
Hervé

Re: [PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Herve Codina]

Hi Rob,

On Thu, 3 Jul 2025 09:33:02 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Rob,
> 
> On Fri, 27 Jun 2025 10:52:00 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:  
> > > The simple-pm-bus driver handles several simple busses. When it is used
> > > with busses other than a compatible "simple-pm-bus", it doesn't populate
> > > its child devices during its probe.
> > > 
> > > This confuses fw_devlink and results in wrong or missing devlinks.
> > > 
> > > Once a driver is bound to a device and the probe() has been called,
> > > device_links_driver_bound() is called.
> > > 
> > > This function performs operation based on the following assumption:
> > >     If a child firmware node of the bound device is not added as a
> > >     device, it will never be added.
> > > 
> > > Among operations done on fw_devlinks of those "never be added" devices,
> > > device_links_driver_bound() changes their supplier.
> > > 
> > > With devices attached to a simple-bus compatible device, this change
> > > leads to wrong devlinks where supplier of devices points to the device
> > > parent (i.e. simple-bus compatible device) instead of the device itself
> > > (i.e. simple-bus child).
> > > 
> > > When the device attached to the simple-bus is removed, because devlinks
> > > are not correct, its consumers are not removed first.
> > > 
> > > In order to have correct devlinks created, make the simple-pm-bus driver
> > > compliant with the devlink assumption and create its child devices
> > > during its probe.    
> > 
> > IIRC, skipping child nodes was because there were problems with 
> > letting the driver handle 'simple-bus'. How does this avoid that now?  
> 
> I don't know about the specific issues related to those problems. Do you
> have some pointers about them?
> 
> > 
> > The root of_platform_populate() that created the simple-bus device that 
> > gets us to the probe here will continue descending into child nodes. 
> > Meanwhile, the probe here is also descending into those same child 
> > nodes. Best case, that's just redundant. Worst case, won't you still 
> > have the same problem if the first of_platform_populate() creates the 
> > devices first?
> >   
> 
> Maybe we could simply avoid of_platform_populate() to be recursive when a
> device populate by of_platform_populate() is one of devices handled by
> the simple-bus driver and let the simple-bus driver do the job.
> 
> of_platform_populate will handle the first level. It will populate children
> of the node given to of_platform_populate() and the children of those
> children will be populate by the simple-bus driver.
> 
> I could try a modification in that way. Do you think it could be a correct
> solution?
> 

I have started to look at this solution and it's going to be more complex
than than I thought.

Many MFD drivers uses a compatible of this kind (the same exist for bus
driver with "simple-bus"):
  compatible = "foo,bar", "simple-mfd";

Usually the last compatible string ("simple-mfd" here) is a last fallback
and the first string is the more specific one.

In the problematic case, "foo,bar" has a specific driver and the driver
performs some operations at probe() but doesn't call of_platform_populate()
and relies on the core to do the device creations (recursively) based on
the "simple,mfd" string present in the compatible property.

Some other calls of_platform_populate() in they probe (which I think is
correct) and in that case, the child device creation can be done at two
location: specific driver probe() and core.

You pointed out that the core could create devices before the specific
driver is probed. In that case, some of existing drivers calling
of_platform_populate() are going to have issues.

I can try to modify existing MFD and bus drivers (compatible fallback to
simple-mfd, simple-bus, ...) in order to have them call of_platform_populate()
in they probe() and after all problematic drivers are converted, the
recursive creation of devices done in the core could be removed.

Before starting in that way, can you confirm that this is the right
direction?

Best regards,
Hervé

Re: [PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Rob Herring]

On Fri, Jul 4, 2025 at 3:57 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi Rob,
>
> On Thu, 3 Jul 2025 09:33:02 +0200
> Herve Codina <herve.codina@bootlin.com> wrote:
>
> > Hi Rob,
> >
> > On Fri, 27 Jun 2025 10:52:00 -0500
> > Rob Herring <robh@kernel.org> wrote:
> >
> > > On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:
> > > > The simple-pm-bus driver handles several simple busses. When it is used
> > > > with busses other than a compatible "simple-pm-bus", it doesn't populate
> > > > its child devices during its probe.
> > > >
> > > > This confuses fw_devlink and results in wrong or missing devlinks.
> > > >
> > > > Once a driver is bound to a device and the probe() has been called,
> > > > device_links_driver_bound() is called.
> > > >
> > > > This function performs operation based on the following assumption:
> > > >     If a child firmware node of the bound device is not added as a
> > > >     device, it will never be added.
> > > >
> > > > Among operations done on fw_devlinks of those "never be added" devices,
> > > > device_links_driver_bound() changes their supplier.
> > > >
> > > > With devices attached to a simple-bus compatible device, this change
> > > > leads to wrong devlinks where supplier of devices points to the device
> > > > parent (i.e. simple-bus compatible device) instead of the device itself
> > > > (i.e. simple-bus child).
> > > >
> > > > When the device attached to the simple-bus is removed, because devlinks
> > > > are not correct, its consumers are not removed first.
> > > >
> > > > In order to have correct devlinks created, make the simple-pm-bus driver
> > > > compliant with the devlink assumption and create its child devices
> > > > during its probe.
> > >
> > > IIRC, skipping child nodes was because there were problems with
> > > letting the driver handle 'simple-bus'. How does this avoid that now?
> >
> > I don't know about the specific issues related to those problems. Do you
> > have some pointers about them?
> >
> > >
> > > The root of_platform_populate() that created the simple-bus device that
> > > gets us to the probe here will continue descending into child nodes.
> > > Meanwhile, the probe here is also descending into those same child
> > > nodes. Best case, that's just redundant. Worst case, won't you still
> > > have the same problem if the first of_platform_populate() creates the
> > > devices first?
> > >
> >
> > Maybe we could simply avoid of_platform_populate() to be recursive when a
> > device populate by of_platform_populate() is one of devices handled by
> > the simple-bus driver and let the simple-bus driver do the job.
> >
> > of_platform_populate will handle the first level. It will populate children
> > of the node given to of_platform_populate() and the children of those
> > children will be populate by the simple-bus driver.
> >
> > I could try a modification in that way. Do you think it could be a correct
> > solution?
> >
>
> I have started to look at this solution and it's going to be more complex
> than than I thought.
>
> Many MFD drivers uses a compatible of this kind (the same exist for bus
> driver with "simple-bus"):
>   compatible = "foo,bar", "simple-mfd";
>
> Usually the last compatible string ("simple-mfd" here) is a last fallback
> and the first string is the more specific one.
>
> In the problematic case, "foo,bar" has a specific driver and the driver
> performs some operations at probe() but doesn't call of_platform_populate()
> and relies on the core to do the device creations (recursively) based on
> the "simple,mfd" string present in the compatible property.
>
> Some other calls of_platform_populate() in they probe (which I think is
> correct) and in that case, the child device creation can be done at two
> location: specific driver probe() and core.
>
> You pointed out that the core could create devices before the specific
> driver is probed. In that case, some of existing drivers calling
> of_platform_populate() are going to have issues.
>
> I can try to modify existing MFD and bus drivers (compatible fallback to
> simple-mfd, simple-bus, ...) in order to have them call of_platform_populate()
> in they probe() and after all problematic drivers are converted, the
> recursive creation of devices done in the core could be removed.

The problem is how does a bus driver know if there is a specific MFD
driver or not? It doesn't. The MFD driver could be a module and loaded
any time later. We'd really need some sort of unbind the generic
driver and re-bind to a more specific driver when and if that driver
appears. We could perhaps have a list of devices with a driver because
in theory that should be a short list as the (broken) promise of
simple-mfd is the child nodes have no dependency on the parent node
which implies the parent doesn't have a driver. The specific
compatible is there in case that assumption turns out wrong.

Rob

Re: [PATCH v3 05/28] bus: simple-pm-bus: Populate child nodes at probe

[Herve Codina]

Hi Rob,

On Mon, 14 Jul 2025 12:44:22 -0500
Rob Herring <robh@kernel.org> wrote:

> On Fri, Jul 4, 2025 at 3:57 AM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > Hi Rob,
> >
> > On Thu, 3 Jul 2025 09:33:02 +0200
> > Herve Codina <herve.codina@bootlin.com> wrote:
> >  
> > > Hi Rob,
> > >
> > > On Fri, 27 Jun 2025 10:52:00 -0500
> > > Rob Herring <robh@kernel.org> wrote:
> > >  
> > > > On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:  
> > > > > The simple-pm-bus driver handles several simple busses. When it is used
> > > > > with busses other than a compatible "simple-pm-bus", it doesn't populate
> > > > > its child devices during its probe.
> > > > >
> > > > > This confuses fw_devlink and results in wrong or missing devlinks.
> > > > >
> > > > > Once a driver is bound to a device and the probe() has been called,
> > > > > device_links_driver_bound() is called.
> > > > >
> > > > > This function performs operation based on the following assumption:
> > > > >     If a child firmware node of the bound device is not added as a
> > > > >     device, it will never be added.
> > > > >
> > > > > Among operations done on fw_devlinks of those "never be added" devices,
> > > > > device_links_driver_bound() changes their supplier.
> > > > >
> > > > > With devices attached to a simple-bus compatible device, this change
> > > > > leads to wrong devlinks where supplier of devices points to the device
> > > > > parent (i.e. simple-bus compatible device) instead of the device itself
> > > > > (i.e. simple-bus child).
> > > > >
> > > > > When the device attached to the simple-bus is removed, because devlinks
> > > > > are not correct, its consumers are not removed first.
> > > > >
> > > > > In order to have correct devlinks created, make the simple-pm-bus driver
> > > > > compliant with the devlink assumption and create its child devices
> > > > > during its probe.  
> > > >
> > > > IIRC, skipping child nodes was because there were problems with
> > > > letting the driver handle 'simple-bus'. How does this avoid that now?  
> > >
> > > I don't know about the specific issues related to those problems. Do you
> > > have some pointers about them?
> > >  
> > > >
> > > > The root of_platform_populate() that created the simple-bus device that
> > > > gets us to the probe here will continue descending into child nodes.
> > > > Meanwhile, the probe here is also descending into those same child
> > > > nodes. Best case, that's just redundant. Worst case, won't you still
> > > > have the same problem if the first of_platform_populate() creates the
> > > > devices first?
> > > >  
> > >
> > > Maybe we could simply avoid of_platform_populate() to be recursive when a
> > > device populate by of_platform_populate() is one of devices handled by
> > > the simple-bus driver and let the simple-bus driver do the job.
> > >
> > > of_platform_populate will handle the first level. It will populate children
> > > of the node given to of_platform_populate() and the children of those
> > > children will be populate by the simple-bus driver.
> > >
> > > I could try a modification in that way. Do you think it could be a correct
> > > solution?
> > >  
> >
> > I have started to look at this solution and it's going to be more complex
> > than than I thought.
> >
> > Many MFD drivers uses a compatible of this kind (the same exist for bus
> > driver with "simple-bus"):
> >   compatible = "foo,bar", "simple-mfd";
> >
> > Usually the last compatible string ("simple-mfd" here) is a last fallback
> > and the first string is the more specific one.
> >
> > In the problematic case, "foo,bar" has a specific driver and the driver
> > performs some operations at probe() but doesn't call of_platform_populate()
> > and relies on the core to do the device creations (recursively) based on
> > the "simple,mfd" string present in the compatible property.
> >
> > Some other calls of_platform_populate() in they probe (which I think is
> > correct) and in that case, the child device creation can be done at two
> > location: specific driver probe() and core.
> >
> > You pointed out that the core could create devices before the specific
> > driver is probed. In that case, some of existing drivers calling
> > of_platform_populate() are going to have issues.
> >
> > I can try to modify existing MFD and bus drivers (compatible fallback to
> > simple-mfd, simple-bus, ...) in order to have them call of_platform_populate()
> > in they probe() and after all problematic drivers are converted, the
> > recursive creation of devices done in the core could be removed.  
> 
> The problem is how does a bus driver know if there is a specific MFD
> driver or not? It doesn't. The MFD driver could be a module and loaded
> any time later. We'd really need some sort of unbind the generic
> driver and re-bind to a more specific driver when and if that driver
> appears. We could perhaps have a list of devices with a driver because
> in theory that should be a short list as the (broken) promise of
> simple-mfd is the child nodes have no dependency on the parent node
> which implies the parent doesn't have a driver. The specific
> compatible is there in case that assumption turns out wrong.
> 

Hum, I see.

In my use case, I don't use MFD drivers but only simple-bus compatible.
I think your point is also relevant with simple-bus. Indeed how does a
parent bus driver know if there is a specific bus driver that handles
the child simple-bus compatible one in case of 'simple-bus' used as
fallback.

Related to your proposal related to the "list of devices with a driver",
what do you mean? I don't see how to set this kind of list. Can you give
me some pointers?

If I understood the discussion, the issue seems that 'simple-bus' can't
populate unconditionally children at his probe. The possible recursion
in creating devices done by of_platform_populate() should be kept and
'simple-bus' should rely on that.

The other solution that fixes my use case is to use an other compatible
string. Would you accept a new compatible string: "simple-platform-bus"?

In simple-pm-bus.c, this compatible would populate children at probe.
In fact, it will act the same way as 'simple-pm-bus' without looking at
clocks nor handling pm_runtime.

Best regards,
Hervé

Re: [PATCH v3 15/28] i2c: core: Introduce i2c_get_adapter_physdev()

[Andi Shyti]

Hi Herve,

On Fri, Jun 13, 2025 at 03:47:55PM +0200, Herve Codina wrote:
> The physical device providing an I2C adapter is the device that calls
> i2c_add_adapter() or variants and i2c_del_adapter().
> 
> Most of the time this physical device is the parent of the adapter
> device.
> 
> Exceptions exist with i2c muxes. Indeed, in case of i2c muxes, the
> parent of the mux adapter device points to the adapter device the mux is
> connected to instead of the physical of this mux adapter.
> 
> Introduce i2c_get_adapter_physdev() and a new physdev field in the
> adapter structure in order to ease the adapter physical device
> retrieval.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Makes sense to me,

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Thanks,
Andi