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

[Buddhabhatti, Jay]

AMD General

Hi Prasanna,

Thanks for the review and the Reviewed-by tag.

>-----Original Message-----
>From: Prasanna Kumar T S M <ptsm@xxxxxxxxxxxxxxxxxxx>
>Sent: Friday, July 24, 2026 12:01 PM
>To: Buddhabhatti, Jay <jay.buddhabhatti@xxxxxxx>; Simek, Michal
><michal.simek@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx>
>Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH 4/4] firmware: xilinx: Use TF-A feature check for TF-A specific
>APIs
>
>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@xxxxxxx>
>> ---
>>   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.
Good point. I've split the original patch 4/4 into two standalone
patches sent as a separate series:

  [PATCH 1/2] firmware: xilinx: Use TF-A feature check for TF-A-specific APIs
  [PATCH 2/2] firmware: xilinx: Propagate actual error from feature check

The kexec series (v2) now contains only the three kexec-related patches
(1-3) without any dependency on the feature check improvements.

Thanks,
Jay
>
>Reviewed-by: Prasanna Kumar T S M <ptsm@xxxxxxxxxxxxxxxxxxx>
>
>Regards,
>Prasanna Kumar