Re: [PATCH 1/8] firmware: arm_scmi: Set fwnode for the genrated SCMI platform device

Jonathan Cameron
2025-10-20T17:07:07Z

On Fri, 17 Oct 2025 14:23:44 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

generated (in patch title).

> Add a call to device_set_node() in the SCMI probe helper to associate
> generated SCMI platform device with the firmware node of its supplier
> transport device.
>
> This complements device_set_of_node_from_dev() and ensures that
> firmware node information is propagated correctly for both Device Tree
> and non-DT (e.g. ACPI) based systems.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scmi/common.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 07b9e629276d..911941e6885d 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -473,6 +473,7 @@ static int __tag##_probe(struct platform_device *pdev) \
> return -ENOMEM; \
> \
> device_set_of_node_from_dev(&spdev->dev, dev); \
> + device_set_node(&spdev->dev, dev_fwnode(dev)); \

device_set_node() is supposed to handle both dt and ACPI. So this surprised me
and I went digging.

dev_fnode for acpi is dev->fwnode, for of it is of_fwnode_handle(dev->of_node)
Which is dev->of_node->fwnode

So for acpi this is
device_set_node(&spdev->dev, dev->fwnode);
which is:
spdev->dev->fwnode = fwnode;
spdev->dev->of_node = NULL;

For dt
device_set_node(&spdev->dev, dev->of_node->fwnode);
which is
spdev->dev->fwnode = dev->of_node->fwnode;
spdev->dev->opf-node = dev->of_node; (via some container of magic)

The device_set_of_node_from_dev(&spdev->dev, dev)
is same as:
of_node_put(spdev->dev->of_node);
spdev->dev->of_node = of_node_get(dev->of_node);
spdev->dev->of_node_reused = true;

So subject to some reference counting that I don't think you need as the
spdev->dev parent is the dev here, the first call does nothing extra.

Maybe I missed something?

Jonathan

> \
> strans.supplier = dev; \
> memcpy(&strans.desc, &(__desc), sizeof(strans.desc)); \
>