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

[Sudeep Holla]

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 at 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 at 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 at 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.

> >  		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)) {
> > @@ -3370,8 +3372,8 @@ static int scmi_probe(struct platform_device *pdev)
> >  		}
> >  
> >  		/*
> > -		 * Save this valid DT protocol descriptor amongst
> > -		 * @active_protocols for this SCMI instance/
> > +		 * Save this valid fwnode protocol descriptor amongst
> > +		 * @active_protocols for this SCMI instance.
> >  		 */
> >  		ret = idr_alloc(&info->active_protocols, child,
> >  				prot_id, prot_id + 1, GFP_KERNEL);
> > @@ -3381,7 +3383,7 @@ static int scmi_probe(struct platform_device *pdev)
> >  			continue;
> >  		}
> >  
> > -		of_node_get(child);
> > +		fwnode_handle_get(child);
> >  		scmi_create_protocol_devices(child, info, prot_id, NULL);
> 
> Give this is handing over ownership I'd make that explicit and do
> 
> 		scmi_create_protcol_devices(fwnode_handle_get(child), info,
> 					    prot_id, NULL);
> but that is a slightly unrelated change - so I don't mind if you don't change
> this one.

Agreed, I can change this if it help readability.

[...]

> 
> > diff --git a/drivers/firmware/arm_scmi/transports/smc.c b/drivers/firmware/arm_scmi/transports/smc.c
> > index 1fce3ccdeb7f..1079cd01190b 100644
> > --- a/drivers/firmware/arm_scmi/transports/smc.c
> > +++ b/drivers/firmware/arm_scmi/transports/smc.c
> > @@ -84,10 +84,15 @@ static irqreturn_t smc_msg_done_isr(int irq, void *data)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static bool smc_chan_available(struct device_node *of_node, int idx)
> > +static bool smc_chan_available(struct fwnode_handle *fwnode, int idx)
> >  {
> > -	struct device_node *np __free(device_node) =
> > -					of_parse_phandle(of_node, "shmem", 0);
> > +	struct device_node *of_node = to_of_node(fwnode);
> > +	struct device_node *np __free(device_node) = NULL;
> 
> See guidance in cleanup.h.  Basically says constructor and destructor
> must be together (that was after some grumpy replies from Linus to early
> cleanup.h users!)
> 

Thanks, fixed locally now.

-- 
Regards,
Sudeep