Re: [PATCH v3 3/9] firmware: arm_scmi: Convert OF-only paths to generic fwnode in SCMI core

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Wed, 2 Sep 2026 14:39:31 +0100
Sudeep Holla <sudeep.holla@kernel.org> wrote:

> On Mon, Aug 24, 2026 at 10:39:29AM -0700, Jonathan Cameron wrote:
> > On Thu, 13 Aug 2026 12:32:58 +0100
> > Sudeep Holla <sudeep.holla@kernel.org> wrote:
> >   
> > > Switch SCMI core plumbing from struct device_node * to struct
> > > fwnode_handle * so the core can describe SCMI instances and protocols using
> > > firmware nodes rather than OF nodes directly.
> > > 
> > > This change:
> > >   - Replaces core OF property lookups with fwnode_property_*() helpers.
> > >   - Switches child enumeration to
> > >     fwnode_for_each_available_child_node_scoped().  
> > 
> > Why the scoped version? See inline for more on this.  Probably just needs
> > a comment here (or use the non _scoped variant)
> >   
> 
> Sure
> 
> > >   - Plumbs fwnode through the SCMI device creation and channel setup paths.
> > >   - Updates transport ->chan_available() callbacks to take a fwnode.
> > >   - Stores per-protocol child fwnodes in info->active_protocols so the core
> > >     can later locate the descriptor for a given protocol ID.
> > >   - Updates mailbox/optee/smc/virtio transports to accept fwnodes and map
> > >     back to OF nodes where their existing parsing remains DT-specific.
> > > 
> > > DT-only transports such as mailbox, OP-TEE and SMC still parse DT
> > > properties by mapping the fwnode back to an OF node. On non-DT systems
> > > these transports report no channel available.
> > > 
> > > This is a mechanical step towards firmware-node neutrality and prepares the
> > > SCMI core for non-DT transports, such as an ACPI/PCC transport. DT users
> > > continue to work unchanged; no non-DT transport is enabled by this patch.
> > > 
> > > Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>  
> > 
> > A few minor things - with those cleaned up.  Only the cleanup.h one really
> > matters.
> >   
> 
> Fixed now.
> 
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> >   
> > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> > > index ef29fd223287..9014723e0f7f 100644
> > > --- a/drivers/firmware/arm_scmi/driver.c
> > > +++ b/drivers/firmware/arm_scmi/driver.c  
> > 
> > ...
> >   
> > > @@ -3352,10 +3354,10 @@ static int scmi_probe(struct platform_device *pdev)
> > >  
> > >  	scmi_enable_matching_quirks(info);
> > >  
> > > -	for_each_available_child_of_node(np, child) {
> > > +	fwnode_for_each_available_child_node_scoped(dev_fwnode(dev), child) {  
> > 
> > There are no early exits yet and I can't see any added later in the series.
> > So the scoped bit is irrelevant.  I don't mind the change on basis of
> > hardening or similar but it needs a comment in the patch description to 
> > justify that.
> >   
> 
> This is mainly to release the reference taken in
> fwnode_get_next_available_child_node(). It is taken explicitly via
> fwnode_handle_get() before scmi_create_protocol_devices() as discussed
> below.

When the loop continues to normal termination the puts are all there
anyway as part of the iterators.  The _scoped case just deals with
early exits via break or return.

> 
> > >  		u32 prot_id;
> > >  
> > > -		if (of_property_read_u32(child, "reg", &prot_id))
> > > +		if (fwnode_property_read_u32(child, "reg", &prot_id))
> > >  			continue;
> > >  
> > >  		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) {