Re: [PATCH] driver core: hand off fwnode ownership when shared fwnode owner is rejected

[Sudeep Holla <sudeep.holla@kernel.org>]

On Tue, Sep 29, 2026 at 09:35:21AM +0100, Sudeep Holla wrote:
> On Tue, Sep 29, 2026 at 09:47:42AM +0800, Peng Fan wrote:
> > Hi Sudeep,
> > 
> > On Mon, Sep 28, 2026 at 01:09:08PM +0100, Sudeep Holla wrote:
> > >On Mon, Sep 28, 2026 at 07:49:01PM +0800, Peng Fan (OSS) wrote:
> > >> From: Peng Fan <peng.fan@nxp.com>
> > >> 
> > >> When multiple devices share the same fwnode (e.g. the SCMI bus creates
> > >> both "pinctrl" and "pinctrl-imx" devices for SCMI_PROTOCOL_PINCTRL), only
> > >> the first device registered becomes the fwnode owner (fwnode->dev, set in
> > >
> > >This has been rejected in the past. Apart from the trigger in -next,
> > >anything else has changed ?
> > 
> > When both pinctrl-scmi.c and pinctrl-imx-scmi.c are built into the
> > kernel Image, both drivers call module_scmi_driver() at init time, which
> > goes through:
> >   scmi_driver_register()
> >     → scmi_protocol_table_register(id_table)
> >       → scmi_protocol_device_request()
> >         → scmi_device_request_notifier()
> >           → scmi_create_protocol_devices(fwnode, ..., "pinctrl-imx")
> > So the on-demand path already creates both "pinctrl" and "pinctrl-imx"
> 
> Yes, but this particular device is i.MX specific issue as it should
> have never been there. So I will discard that in any future discussion.
> Use the SCMI pinmux driver and get rid of SCMI pinctrl-imx.
> 

Anyways, I cooked up a patch that may prevent the issue with pinmux.
See if something like below on top -next helps.

Regards,
Sudeep

-->8
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 25197197db8e..cee7cc777724 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -60,13 +60,6 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
 	pr_debug("Requesting SCMI device (%s) for protocol %x\n",
 		 id_table->name, id_table->protocol_id);

-	if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) &&
-	    !IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX)) {
-		pr_warn("SCMI Raw mode active. Rejecting '%s'/0x%02X\n",
-			id_table->name, id_table->protocol_id);
-		return -EINVAL;
-	}
-
 	/*
 	 * Find the matching protocol rdev list and then search of any
 	 * existent equally named device...fails if any duplicate found.
@@ -181,12 +174,35 @@ static void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table
 	}
 }

+static bool scmi_device_id_in_std_id_table(const struct scmi_device_id *id)
+{
+	for (int i = 0; scmi_std_id_table[i].name[0]; i++) {
+		if (scmi_std_id_table[i].protocol_id == id->protocol_id &&
+		    !strcmp(scmi_std_id_table[i].name, id->name))
+			return true;
+	}
+
+	return false;
+}
+
 static int scmi_protocol_table_register(const struct scmi_device_id *id_table)
 {
 	const struct scmi_device_id *entry;
 	int ret;

 	for (entry = id_table; entry->name[0]; entry++) {
+		if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) &&
+		    !IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX)) {
+			pr_warn("SCMI Raw mode active. Rejecting '%s'/0x%02X\n",
+				entry->name, entry->protocol_id);
+			ret = -EINVAL;
+			goto err_unrequest;
+		}
+
+		/* Standard devices are created independently of their drivers. */
+		if (scmi_device_id_in_std_id_table(entry))
+			continue;
+
 		ret = scmi_protocol_device_request(entry);
 		if (ret)
 			goto err_unrequest;
@@ -195,8 +211,11 @@ static int scmi_protocol_table_register(const struct scmi_device_id *id_table)
 	return 0;

 err_unrequest:
-	while (entry != id_table)
-		scmi_protocol_device_unrequest(--entry);
+	while (entry != id_table) {
+		--entry;
+		if (!scmi_device_id_in_std_id_table(entry))
+			scmi_protocol_device_unrequest(entry);
+	}

 	return ret;
 }
@@ -206,8 +225,10 @@ scmi_protocol_table_unregister(const struct scmi_device_id *id_table)
 {
 	const struct scmi_device_id *entry;

-	for (entry = id_table; entry->name[0]; entry++)
-		scmi_protocol_device_unrequest(entry);
+	for (entry = id_table; entry->name[0]; entry++) {
+		if (!scmi_device_id_in_std_id_table(entry))
+			scmi_protocol_device_unrequest(entry);
+	}
 }

 static bool scmi_device_is_transport(const struct scmi_device *scmi_dev)
@@ -545,21 +566,9 @@ static const struct scmi_device_id scmi_std_id_table[] = {
 	{ SCMI_PROTOCOL_VOLTAGE, "regulator" },
 	{ SCMI_PROTOCOL_POWERCAP, "powercap" },
 	{ SCMI_PROTOCOL_PINCTRL, "pinctrl" },
-	{ SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" },
 	{ },
 };

-static bool scmi_device_id_in_std_id_table(const struct scmi_device_id *id)
-{
-	for (int i = 0; scmi_std_id_table[i].name[0]; i++) {
-		if (scmi_std_id_table[i].protocol_id == id->protocol_id &&
-		    !strcmp(scmi_std_id_table[i].name, id->name))
-			return true;
-	}
-
-	return false;
-}
-
 /**
  * scmi_device_create  - A method to create one or more SCMI devices
  *
diff --git a/drivers/pinctrl/freescale/pinctrl-imx-scmi.c b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
index 613552e35070..b259a526ccc1 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
@@ -354,7 +354,7 @@ static int scmi_pinctrl_imx_probe(struct scmi_device *sdev)
 }

 static const struct scmi_device_id scmi_id_table[] = {
-	{ SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" },
+	{ SCMI_PROTOCOL_PINCTRL, "pinctrl" },
 	{ }
 };
 MODULE_DEVICE_TABLE(scmi, scmi_id_table);