Re: [PATCH v23 08/13] mfd: core: Add firmware-node support to MFD cells

[Bartosz Golaszewski]

On Tue, 14 Jul 2026 19:16:36 +0200, Shivendra Pratap
<shivendra.pratap@xxxxxxxxxxxxxxxx> said:
> MFD core has no way to register a child device using an explicit firmware
> node. This prevents drivers from registering child nodes when those nodes
> do not define a compatible string. One such example is the PSCI
> "reboot-mode" node, which omits a compatible string as it describes
> boot-states provided by the underlying firmware.
>
> Extend struct mfd_cell with a named firmware-node field to identify a
> child node under the MFD parent. The node is added to the MFD child
> device during registration when none is assigned by device tree, ACPI,
> or software matching.
>
> Suggested-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@xxxxxxxxxxxxxxxx>
> ---
>  drivers/mfd/mfd-core.c   | 142 ++++++++++++++++++++++++++++++++++++++++-------
>  include/linux/mfd/core.h |  10 ++++
>  2 files changed, 131 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 7aa32b90cf1e..ba9682ead2ca 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -10,6 +10,7 @@
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/acpi.h>
> +#include <linux/fwnode.h>
>  #include <linux/list.h>
>  #include <linux/property.h>
>  #include <linux/mfd/core.h>
> @@ -22,6 +23,7 @@
>  #include <linux/regulator/consumer.h>
>
>  static LIST_HEAD(mfd_of_node_list);
> +static LIST_HEAD(mfd_named_fwnode_list);
>  static DEFINE_MUTEX(mfd_of_node_mutex);
>
>  struct mfd_of_node_entry {
> @@ -30,10 +32,91 @@ struct mfd_of_node_entry {
>  	struct device_node *np;
>  };
>
> +struct mfd_named_fwnode_entry {
> +	struct list_head list;
> +	struct device *dev;
> +	struct fwnode_handle *fwnode;
> +};
> +
>  static const struct device_type mfd_dev_type = {
>  	.name	= "mfd_device",
>  };
>
> +static int mfd_claim_named_fwnode(struct platform_device *pdev,
> +				  struct fwnode_handle *fwnode)
> +{
> +	struct mfd_named_fwnode_entry *entry, *iter;
> +
> +	entry = kzalloc_obj(*entry, GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->dev = &pdev->dev;
> +	entry->fwnode = fwnode_handle_get(fwnode);
> +
> +	scoped_guard(mutex, &mfd_of_node_mutex) {
> +		list_for_each_entry(iter, &mfd_named_fwnode_list, list)
> +			if (iter->fwnode == fwnode) {
> +				fwnode_handle_put(entry->fwnode);
> +				kfree(entry);
> +				return -EAGAIN;
> +			}
> +
> +		list_add_tail(&entry->list, &mfd_named_fwnode_list);
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Temporary MFD-local cleanup for named non-OF child fwnodes.
> + * Remove/rework this when platform core starts owning and dropping
> + * dev->fwnode references for these devices.
> + */
> +static void mfd_release_named_fwnode(struct platform_device *pdev)
> +{
> +	struct mfd_named_fwnode_entry *entry, *tmp;
> +
> +	scoped_guard(mutex, &mfd_of_node_mutex) {
> +		list_for_each_entry_safe(entry, tmp, &mfd_named_fwnode_list, list)
> +			if (entry->dev == &pdev->dev) {
> +				if (dev_fwnode(&pdev->dev) == entry->fwnode)
> +					device_set_node(&pdev->dev, NULL);
> +				fwnode_handle_put(entry->fwnode);
> +				list_del(&entry->list);
> +				kfree(entry);
> +			}
> +	}
> +}
> +
> +static int mfd_claim_of_node_to_dev(struct platform_device *pdev,
> +				    struct device_node *np)
> +{
> +	struct mfd_of_node_entry *of_entry, *iter;
> +
> +	of_entry = kzalloc_obj(*of_entry, GFP_KERNEL);
> +	if (!of_entry)
> +		return -ENOMEM;
> +
> +	of_entry->dev = &pdev->dev;
> +	of_entry->np = of_node_get(np);
> +
> +	/* Skip if OF node has previously been allocated to a device */
> +	scoped_guard(mutex, &mfd_of_node_mutex) {
> +		list_for_each_entry(iter, &mfd_of_node_list, list)
> +			if (iter->np == np) {
> +				of_node_put(of_entry->np);
> +				kfree(of_entry);
> +				return -EAGAIN;
> +			}
> +
> +		list_add_tail(&of_entry->list, &mfd_of_node_list);
> +	}
> +
> +	device_set_node(&pdev->dev, of_fwnode_handle(np));
> +	return 0;
> +}
> +
>  #if IS_ENABLED(CONFIG_ACPI)
>  struct match_ids_walk_data {
>  	struct acpi_device_id *ids;
> @@ -111,19 +194,11 @@ static int mfd_match_of_node_to_dev(struct platform_device *pdev,
>  				    struct device_node *np,
>  				    const struct mfd_cell *cell)
>  {
> -	struct mfd_of_node_entry *of_entry;
>  	u64 of_node_addr;
>
> -	/* Skip if OF node has previously been allocated to a device */
> -	scoped_guard(mutex, &mfd_of_node_mutex) {
> -		list_for_each_entry(of_entry, &mfd_of_node_list, list)
> -			if (of_entry->np == np)
> -				return -EAGAIN;
> -	}
> -
>  	if (!cell->use_of_reg)
>  		/* No of_reg defined - allocate first free compatible match */
> -		goto allocate_of_node;
> +		return mfd_claim_of_node_to_dev(pdev, np);
>
>  	/* We only care about each node's first defined address */
>  	if (of_property_read_reg(np, 0, &of_node_addr, NULL))
> @@ -134,18 +209,7 @@ static int mfd_match_of_node_to_dev(struct platform_device *pdev,
>  		/* No match */
>  		return -EAGAIN;
>
> -allocate_of_node:
> -	of_entry = kzalloc(sizeof(*of_entry), GFP_KERNEL);
> -	if (!of_entry)
> -		return -ENOMEM;
> -
> -	of_entry->dev = &pdev->dev;
> -	of_entry->np = of_node_get(np);
> -	scoped_guard(mutex, &mfd_of_node_mutex)
> -		list_add_tail(&of_entry->list, &mfd_of_node_list);
> -
> -	device_set_node(&pdev->dev, of_fwnode_handle(np));
> -	return 0;
> +	return mfd_claim_of_node_to_dev(pdev, np);
>  }
>
>  static int mfd_add_device(struct device *parent, int id,
> @@ -156,6 +220,7 @@ static int mfd_add_device(struct device *parent, int id,
>  	struct resource *res;
>  	struct platform_device *pdev;
>  	struct mfd_of_node_entry *of_entry, *tmp;
> +	struct fwnode_handle *fwnode;
>  	bool disabled = false;
>  	int ret = -ENOMEM;
>  	int platform_id;
> @@ -224,6 +289,37 @@ static int mfd_add_device(struct device *parent, int id,
>
>  	mfd_acpi_add_device(cell, pdev);
>
> +	/* named_fwnode is a fallback only when no OF/ACPI match and no swnode */
> +	if (!pdev->dev.fwnode && !cell->swnode && cell->named_fwnode) {

Use dev_fwnode() instead of pdev->dev.fwnode.

> +		struct device_node *named_np;
> +
> +		fwnode = device_get_named_child_node(parent, cell->named_fwnode);
> +		if (!fwnode) {
> +			ret = -ENODEV;
> +			goto fail_alias;
> +		}
> +
> +		named_np = to_of_node(fwnode);
> +		if (named_np) {
> +			ret = mfd_claim_of_node_to_dev(pdev, named_np);
> +			fwnode_handle_put(fwnode);
> +			if (ret == -EAGAIN)
> +				ret = -EBUSY;
> +			if (ret)
> +				goto fail_alias;
> +		} else {
> +			ret = mfd_claim_named_fwnode(pdev, fwnode);
> +			if (ret) {
> +				fwnode_handle_put(fwnode);
> +				if (ret == -EAGAIN)
> +					ret = -EBUSY;
> +				goto fail_alias;
> +			}
> +			device_set_node(&pdev->dev, fwnode);
> +			fwnode_handle_put(fwnode);
> +		}

What is the reason to have this split into OF and fwnode functions? I don't
really see why you would need to do it twice, isn't fwnode sufficient for the
OF node case too?

> +	}
> +
>  	if (cell->pdata_size) {
>  		ret = platform_device_add_data(pdev,
>  					cell->platform_data, cell->pdata_size);
> @@ -295,6 +391,7 @@ static int mfd_add_device(struct device *parent, int id,
>  	if (cell->swnode)
>  		device_remove_software_node(&pdev->dev);
>  fail_of_entry:
> +	mfd_release_named_fwnode(pdev);
>  	scoped_guard(mutex, &mfd_of_node_mutex) {
>  		list_for_each_entry_safe(of_entry, tmp, &mfd_of_node_list, list)
>  			if (of_entry->dev == &pdev->dev) {
> @@ -382,7 +479,10 @@ static int mfd_remove_devices_fn(struct device *dev, void *data)
>  	regulator_bulk_unregister_supply_alias(dev, cell->parent_supplies,
>  					       cell->num_parent_supplies);
>
> +	get_device(&pdev->dev);
>  	platform_device_unregister(pdev);
> +	mfd_release_named_fwnode(pdev);
> +	put_device(&pdev->dev);
>  	return 0;
>  }
>
> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
> index faeea7abd688..8daa83dd31ea 100644
> --- a/include/linux/mfd/core.h
> +++ b/include/linux/mfd/core.h
> @@ -80,6 +80,16 @@ struct mfd_cell {
>
>  	/* Software node for the device. */
>  	const struct software_node *swnode;
> +	/*
> +	 * Name of a child firmware node under the MFD parent device.
> +	 *
> +	 * Used only as a fallback when no firmware node is assigned to MFD
> +	 * child and no software node is provided.
> +	 *
> +	 * For Device Tree parents, lookup is by base node name only
> +	 * (the part before '@'). Unit-addresses are not matched.
> +	 */

Side note: struct mfd_cell could really use a proper kernel doc but that's out
of scope of this series.

> +	const char *named_fwnode;
>
>  	/*
>  	 * Device Tree compatible string
>
> --
> 2.34.1
>
>

Bartosz