Re: [PATCH v2 5/9] firmware: arm_scmi: Add SCMIv4.0 Powercap FCs support

[Sudeep Holla]

On Wed, Feb 04, 2026 at 11:19:46AM +0000, Philip Radford wrote:
> From: Cristian Marussi <cristian.marussi@xxxxxxx>
> 
> Add support for new SCMIv4.0 Powercap Fastchannels.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>
> [Philip: removed reference to old versioning logic]
> Signed-off-by: Philip Radford <philip.radford@xxxxxxx>
> ---
> V1->V2
> - Removed creation of pi powercap_info struct due to legacy code
>   change
> ---
>  drivers/firmware/arm_scmi/powercap.c | 323 ++++++++++++++++++---------
>  1 file changed, 221 insertions(+), 102 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c
> index 3291bde78367..22bf8e480450 100644
> --- a/drivers/firmware/arm_scmi/powercap.c
> +++ b/drivers/firmware/arm_scmi/powercap.c
>  

[...]

> @@ -1019,37 +1092,77 @@ static const struct scmi_powercap_proto_ops powercap_proto_ops = {
>  };
>  
>  static void scmi_powercap_domain_init_fc(const struct scmi_protocol_handle *ph,
> -					 u32 domain, struct scmi_fc_info **p_fc)
> +					 struct scmi_powercap_info *dom_info)
>  {
> -	struct scmi_fc_info *fc;
> -
> -	fc = devm_kcalloc(ph->dev, POWERCAP_FC_MAX, sizeof(*fc), GFP_KERNEL);
> -	if (!fc)
> -		return;
> -
> -	ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> -				   POWERCAP_CAP_SET, 4, domain, NULL,
> -				   &fc[POWERCAP_FC_CAP].set_addr,
> -				   &fc[POWERCAP_FC_CAP].set_db,
> -				   &fc[POWERCAP_FC_CAP].rate_limit);
> -
> -	ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> -				   POWERCAP_CAP_GET, 4, domain, NULL,
> -				   &fc[POWERCAP_FC_CAP].get_addr, NULL,
> -				   &fc[POWERCAP_FC_CAP].rate_limit);
> -
> -	ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> -				   POWERCAP_PAI_SET, 4, domain, NULL,
> -				   &fc[POWERCAP_FC_PAI].set_addr,
> -				   &fc[POWERCAP_FC_PAI].set_db,
> -				   &fc[POWERCAP_FC_PAI].rate_limit);
> -
> -	ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> -				   POWERCAP_PAI_GET, 4, domain, NULL,
> -				   &fc[POWERCAP_FC_PAI].get_addr, NULL,
> -				   &fc[POWERCAP_FC_PAI].rate_limit);
> -
> -	*p_fc = fc;
> +	for (int id = 0; id < dom_info->num_cpli; id++) {
> +		struct scmi_fc_info *fc;
> +		u32 *cpl_id, zero_cpl_id = 0;
> +
> +		fc = devm_kcalloc(ph->dev, POWERCAP_FC_MAX, sizeof(*fc), GFP_KERNEL);
> +		if (!fc)
> +			return;
> +
> +		/* NOTE THAT when num_cpli == 1 the arg *cpl_id is 0 */
> +		cpl_id = (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) ? &id : NULL;
> +
> +		ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +					   POWERCAP_CAP_SET, 4, dom_info->id,
> +					   cpl_id,
> +					   &fc[POWERCAP_FC_CAP].set_addr,
> +					   &fc[POWERCAP_FC_CAP].set_db,
> +					   &fc[POWERCAP_FC_CAP].rate_limit);
> +
> +		ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +					   POWERCAP_CAP_GET, 4, dom_info->id,
> +					   cpl_id,
> +					   &fc[POWERCAP_FC_CAP].get_addr, NULL,
> +					   &fc[POWERCAP_FC_CAP].rate_limit);
> +
> +		if (PROTOCOL_REV_MAJOR(ph->version) < 0x3) {
> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_PAI_SET, 4,
> +						   dom_info->id, NULL,
> +						   &fc[POWERCAP_FC_XAI].set_addr,
> +						   &fc[POWERCAP_FC_XAI].set_db,
> +						   &fc[POWERCAP_FC_XAI].rate_limit);
> +
> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_PAI_GET, 4,
> +						   dom_info->id, NULL,
> +						   &fc[POWERCAP_FC_XAI].get_addr, NULL,
> +						   &fc[POWERCAP_FC_XAI].rate_limit);
> +		} else {
> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_CAI_SET, 4,
> +						   dom_info->id, &id,
> +						   &fc[POWERCAP_FC_XAI].set_addr,
> +						   &fc[POWERCAP_FC_XAI].set_db,
> +						   &fc[POWERCAP_FC_XAI].rate_limit);
> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_CAI_GET, 4,
> +						   dom_info->id, &id,
> +						   &fc[POWERCAP_FC_XAI].get_addr, NULL,
> +						   &fc[POWERCAP_FC_XAI].rate_limit);

Can the above "&id" changed to cpl_id like the first 2 instance for consistency
unless I am reading it wrong.

> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_MAI_SET, 4,
> +						   dom_info->id, &zero_cpl_id,
> +						   &fc[POWERCAP_FC_MAI].set_addr,
> +						   &fc[POWERCAP_FC_MAI].set_db,
> +						   &fc[POWERCAP_FC_MAI].rate_limit);
> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_MAI_GET, 4,
> +						   dom_info->id, &zero_cpl_id,
> +						   &fc[POWERCAP_FC_MAI].get_addr, NULL,
> +						   &fc[POWERCAP_FC_MAI].rate_limit);
> +			ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL,
> +						   POWERCAP_MEASUREMENTS_GET, 4,
> +						   dom_info->id, &zero_cpl_id,
> +						   &fc[POWERCAP_FC_MEASUREMENT].get_addr, NULL,
> +						   &fc[POWERCAP_FC_MEASUREMENT].rate_limit);

Also why should the above 3 as well as the NULL cpl_id calls be inside this
loop ?

-- 
Regards,
Sudeep