Re: [PATCH v3 1/9] firmware: arm_scmi: Set fwnode for the generated SCMI platform device

On Wed, 2 Sep 2026 08:51:10 +0100
Sudeep Holla [off-list ref] wrote:

On Mon, Aug 24, 2026 at 09:51:33AM -0700, Jonathan Cameron wrote:

quoted

On Thu, 13 Aug 2026 12:32:56 +0100
Sudeep Holla [off-list ref] wrote:
  

quoted

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.  

Hi Sudeep,
  

Hi Jonathan,

Thanks for all the reviews. I was away for a while and started to look at
addressing these now.

quoted

When you say 'complement' I'd like to see a little more.  The guts
of both calls includes setting dev->ofnode() adn the extra bit
in device_set_of_node_from_dev() is about ensuring we drop existing
node reference (there isn't one) and increment the reference for what
is the parent of_node.
  

Yes I agree. At the time, when I hacked this up, I was supposed to take
a look at it again which I never did.

quoted

I may be wrong, but I think that is unnecessary and if I'm wrong I'd
like to see more info here on why it is needed.
  

No your concerns are valid. For reference-counted non-OF fwnodes like ACPI or
software node, platform_device_release() drops the fwnode reference,
but device_set_node() does not acquire one.

quoted

Also, if this is commonly needed why doesn't device_set_node() include
that dance?
  

I think there are new helpers added recently in the tree, thinking of using
then. See below.

quoted

Unless I fumbled the command, a grep suggests there are no instances
in tree of these both being called near each other.
  

I did that too before adding but somehow kept it for future investigation.

quoted

Jonathan
  

quoted

Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 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 fe8c22cfb9f7..cc7d11c3c1f3 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h

@@ -650,6 +650,7 @@ static int __tag##_probe(struct platform_device *pdev)			       \
 	}								       \
 									       \
 	device_set_of_node_from_dev(&spdev->dev, dev);			       \
+	device_set_node(&spdev->dev, dev_fwnode(dev));			       \  

How about something like this instead of above 2 lines:

if (is_of_node(dev_fwnode(dev)))
	platform_device_set_of_node_from_dev(spdev, dev);
else
	platform_device_set_fwnode(spdev, dev_fwnode(dev));

To me, if the reference counting is useful (and I'm a bit doubtful about that)
platform_device_set_fwnode() should be doing it. +CC those who might have
more insight on this.

Jonathan