Re: [PATCH 4/4] firmware: xilinx: Use TF-A feature check for TF-A specific APIs

[Prasanna Kumar T S M <ptsm@linux.microsoft.com>]

Hi Jay,

On 23-07-2026 18:18, Jay Buddhabhatti wrote:
> Currently, TF-A-specific APIs are validated using the firmware
> PM_FEATURE_CHECK API, even though TF-A provides a dedicated mechanism via
> PM_API_FEATURES API. Ideally it should use the TF-A feature check
> (PM_API_FEATURES) for TF-A specific APIs. Update the feature check logic
> for TF-A specific API calls to ensure it is validated using
> PM_API_FEATURES. If this check fails, fall back to the legacy
> PM_FEATURE_CHECK to support backward compatibility.
> 
> When do_fw_call() fails, propagate the errno from zynqmp_pm_ret_code()
> instead of always returning -EOPNOTSUPP. This applies to every module ID,
> not only TF-A, because the rewrite sat in the common failure path.
> Existing callers only test ret < 0 and are unchanged.
> 
> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@amd.com>
> ---
>   drivers/firmware/xilinx/zynqmp.c | 31 +++++++++++++++++--------------
>   1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index fc7212f554ee..0b6c20a1d8be 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -224,34 +224,37 @@ static int __do_feature_check_call(const u32 api_id, u32 *ret_payload)
>   	module_id = FIELD_GET(MODULE_ID_MASK, api_id);
>   
>   	/*
> -	 * Feature check of APIs belonging to PM, XSEM, and TF-A are handled by calling
> +	 * Feature check of APIs belonging to PM and XSEM are handled by calling
>   	 * PM_FEATURE_CHECK API. For other modules, call PM_API_FEATURES API.
>   	 */
> -	if (module_id == PM_MODULE_ID || module_id == XSEM_MODULE_ID || module_id == TF_A_MODULE_ID)
> +	if (module_id == PM_MODULE_ID || module_id == XSEM_MODULE_ID)
>   		feature_check_api_id = PM_FEATURE_CHECK;
>   	else
>   		feature_check_api_id = PM_API_FEATURES;
>   
> -	/*
> -	 * Feature check of TF-A APIs is done in the TF-A layer and it expects for
> -	 * MODULE_ID_MASK bits of SMC's arg[0] to be the same as PM_MODULE_ID.
> -	 */
> -	if (module_id == TF_A_MODULE_ID) {
> -		module_id = PM_MODULE_ID;
> +	if (module_id == TF_A_MODULE_ID)
>   		smc_arg[1] = api_id;
> -	} else {
> +	else
>   		smc_arg[1] = (api_id & API_ID_MASK);
> -	}
>   
>   	smc_arg[0] = PM_SIP_SVC | FIELD_PREP(MODULE_ID_MASK, module_id) | feature_check_api_id;
>   
>   	ret = do_fw_call(ret_payload, 2, smc_arg[0], smc_arg[1]);
> +
> +	/*
> +	 * For TF-A APIs, if the feature check with PM_API_FEATURES fails,
> +	 * retry with the legacy PM_FEATURE_CHECK for backward compatibility.
> +	 */
> +	if (module_id == TF_A_MODULE_ID && ret) {
> +		smc_arg[0] = PM_SIP_SVC | FIELD_PREP(MODULE_ID_MASK, PM_MODULE_ID) |
> +			     PM_FEATURE_CHECK;
> +		ret = do_fw_call(ret_payload, 2, smc_arg[0], smc_arg[1]);
> +	}

This looks like a general improvement unrelated to the other patches in 
this series. I think this should go as a separate patch (not as a part 
of this series).

> +
>   	if (ret)
> -		ret = -EOPNOTSUPP;
> -	else
> -		ret = ret_payload[1];
> +		return ret;
>   
> -	return ret;
> +	return ret_payload[1];
>   }
>   
>   static int do_feature_check_call(const u32 api_id)

The return value change is separate improvement unrelated to the series 
or to the PM_API_FEATURES API usage. I think this change can be carved 
out to a separate patch.

Except for the above nits, code looks good to me.

Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>

Regards,
Prasanna Kumar