[PATCH v3 6/9] firmware: arm_scmi: Refactor protocol device creation logic

[Sudeep Holla]

On Mon, Aug 24, 2026 at 10:49:48AM -0700, Jonathan Cameron wrote:
> On Thu, 13 Aug 2026 12:33:01 +0100
> Sudeep Holla <sudeep.holla at kernel.org> wrote:
> 
> > Move the protocol validation and device creation logic in scmi_probe()
> > into a reusable scmi_device_check_create() helper.
> > 
> > The helper centralizes checks for the protocol ID range, implementation
> > availability and duplicate activation before invoking
> > scmi_create_protocol_devices(). This preserves the existing behavior
> > while allowing the logic to be reused by the ACPI path, where protocol
> > child fwnodes are absent.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla at kernel.org>
> A trivial thing and one follow on from earlier.  For that just be consistent
> with what you choose to do for the earlier comment.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron at oss.qualcomm.com>
> 

Thanks!

> > ---
> >  drivers/firmware/arm_scmi/driver.c | 60 ++++++++++++++++++++++----------------
> >  1 file changed, 35 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> > index aad678db0f6e..951ba3df6ba6 100644
> > --- a/drivers/firmware/arm_scmi/driver.c
> > +++ b/drivers/firmware/arm_scmi/driver.c
> > @@ -3244,6 +3244,40 @@ static void scmi_enable_matching_quirks(struct scmi_info *info)
> >  			   rev->sub_vendor_id, rev->impl_ver);
> >  }
> >  
> > +static void scmi_device_check_create(struct fwnode_handle *fwnode, int prot_id,
> > +				     struct scmi_info *info)
> > +{
> > +	int ret;
> > +	struct device *dev = info->dev;
> > +	struct scmi_handle *handle = &info->handle;
> > +
> > +	if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) {
> > +		dev_err(dev, "Out of range protocol %d\n", prot_id);
> > +		return;
> > +	}
> > +
> > +	if (!scmi_is_protocol_implemented(handle, prot_id)) {
> > +		dev_err(dev, "SCMI protocol %d not implemented\n",
> > +			prot_id);
> 
> Fits on one line. I didn't check if you modify this later in the series
> though so if this is churn reduction, fine to leave it as it stands.
> 

Fixed locally, must be side-effect of refactoring.

> > +		return;
> > +	}
> > +
> > +	/*
> > +	 * Save this valid fwnode protocol descriptor amongst
> > +	 * @active_protocols for this SCMI instance.
> > +	 */
> > +	ret = idr_alloc(&info->active_protocols, fwnode,
> > +			prot_id, prot_id + 1, GFP_KERNEL);
> > +	if (ret != prot_id) {
> > +		dev_err(dev, "SCMI protocol %d already activated. Skip\n",
> > +			prot_id);
> > +		return;
> > +	}
> > +
> > +	fwnode_handle_get(fwnode);
> > +	scmi_create_protocol_devices(fwnode, info, prot_id, NULL);
> 
> If you take the suggest I made earlier to make the hand over of ownership
> slightly more obvious by using the return value of fwmonde_handle_get()
> then applies here too.
> 

I will update 3/9, it should be there by 6/9, so no change here other
that the move itself.

-- 
Regards,
Sudeep