Re: [PATCH 23/44] optee: add revision info to tee devinfo output

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi Sascha,

On 8/12/25 11:35, Sascha Hauer wrote:
> On Mon, Aug 11, 2025 at 02:28:03PM +0200, Ahmad Fatoum wrote:
>> We currently only report an implementation ID when doing devinfo on the
>> tee device. Improve upon that by translating the ID to a string and
>> printing the OP-TEE revision if possible.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
>> ---
>>  drivers/tee/optee/smc_abi.c | 16 +++++++++++-----
>>  drivers/tee/tee_core.c      | 20 +++++++++++++++++++-
>>  2 files changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
>> index aab8ebb186ed..0b8d581cf19b 100644
>> --- a/drivers/tee/optee/smc_abi.c
>> +++ b/drivers/tee/optee/smc_abi.c
>> @@ -549,8 +549,10 @@ static bool optee_msg_api_uid_is_optee_api(optee_invoke_fn *invoke_fn)
>>  	return false;
>>  }
>>  
>> -static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn)
>> +static void optee_msg_get_os_revision(struct device *dev,
>> +				      optee_invoke_fn *invoke_fn)
>>  {
>> +	struct param_d *param;
>>  	union {
>>  		struct arm_smccc_res smccc;
>>  		struct optee_smc_call_get_os_revision_result result;
>> @@ -564,10 +566,14 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn)
>>  		  &res.smccc);
>>  
>>  	if (res.result.build_id)
>> -		pr_info("revision %lu.%lu (%08lx)\n", res.result.major,
>> -			res.result.minor, res.result.build_id);
>> +		param = dev_add_param_fixed(dev, "revision", "%lu.%lu (%08lx)",
>> +					    res.result.major, res.result.minor,
>> +					    res.result.build_id);
>>  	else
>> -		pr_info("revision %lu.%lu\n", res.result.major, res.result.minor);
>> +		param = dev_add_param_fixed(dev, "revision", "%lu.%lu",
>> +				    res.result.major, res.result.minor);
>> +
>> +	pr_info("revision %s\n", param->get(dev, param));
> 
> In 20/44 you protected this with a if (!IS_ERR(param)). Don't we need this
> here as well?

Yes, good point. I will add a helper that does this check and use if for
v2 (which will hopefully be much shorter, because the rest is applied? ;-)

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |