On 08.09.26 11:47, Jan Kiszka wrote:
> On 08.09.26 09:08, Ilias Apalodimas wrote:
>> Hi Jan
>>
>>
>> On Sun Aug 23, 2026 at 10:47 PM EEST, Jan Kiszka wrote:
>>> From: Jan Kiszka <[email protected]>
>>>
>>> This already avoids code duplication between the different enumeration
>>> commands. It will furthermore allow to implement a third one for
>>> services with RPMB needs.
>>>
>>> Signed-off-by: Jan Kiszka <[email protected]>
>>> ---
>>>  drivers/tee/optee/core.c          | 33 +++++++++++++++++--------------
>>>  drivers/tee/optee/optee_private.h |  3 +++
>>>  2 files changed, 21 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
>>> index 4d67c948ec1..2871760a7b7 100644
>>> --- a/drivers/tee/optee/core.c
>>> +++ b/drivers/tee/optee/core.c
>>> @@ -156,7 +156,7 @@ static int enum_services(struct udevice *dev, struct 
>>> tee_shm **shm, size_t *coun
>>>     return ret;
>>>  }
>>>
>>> -static int open_enum_session(struct udevice *dev, u32 *tee_sess)
>>> +int optee_open_enum_session(struct udevice *dev, u32 *tee_sess)
>>>  {
>>>     const struct tee_optee_ta_uuid pta_uuid = PTA_DEVICE_ENUM;
>>>     struct tee_open_session_arg arg = { };
>>> @@ -176,31 +176,34 @@ static int open_enum_session(struct udevice *dev, u32 
>>> *tee_sess)
>>>     return 0;
>>>  }
>>>
>>> -static int bind_service_drivers(struct udevice *dev)
>>> +int optee_bind_services(struct udevice *dev, u32 tee_sess,
>>> +                   unsigned int pta_cmd)
>>>  {
>>>     struct tee_shm *service_list = NULL;
>>>     size_t service_count;
>>> -   u32 tee_sess;
>>> -   int ret, ret2;
>>> -
>>> -   ret = open_enum_session(dev, &tee_sess);
>>> -   if (ret)
>>> -           return ret;
>>> +   int ret;
>>>
>>>     ret = enum_services(dev, &service_list, &service_count, tee_sess,
>>> -                       PTA_CMD_GET_DEVICES);
>>> +                       pta_cmd);
>>>     if (!ret && service_count)
>>>             ret = bind_service_list(dev, service_list, service_count);
>>>
>>>     tee_shm_free(service_list);
>>> -   service_list = NULL;
>>>
>>> -   ret2 = enum_services(dev, &service_list, &service_count, tee_sess,
>>> -                        PTA_CMD_GET_DEVICES_SUPP);
>>> -   if (!ret2 && service_count)
>>> -           ret2 = bind_service_list(dev, service_list, service_count);
>>> +   return ret;
>>> +}
>>> +
>>> +static int bind_service_drivers(struct udevice *dev)
>>> +{
>>> +   u32 tee_sess;
>>> +   int ret, ret2;
>>>
>>> -   tee_shm_free(service_list);
>>> +   ret = optee_open_enum_session(dev, &tee_sess);
>>> +   if (ret)
>>> +           return ret;
>>> +
>>> +   ret = optee_bind_services(dev, tee_sess, PTA_CMD_GET_DEVICES);
>>
>> Any reason we aren't exiting immediately here?
>>
> 
> Just maintained the pre-existing behavior. But I could do this as well:
> 
>       ret = optee_bind_services(dev, tee_sess, PTA_CMD_GET_DEVICES);
>       if (ret == 0)
>               ret = optee_bind_services(dev, tee_sess,
>                                         PTA_CMD_GET_DEVICES_SUPP);
> 
> Simpler, in fact...
> 

... but logically different: We only warn if any binding attempt fails,
now we would not even attempt the other.

Key question is whether such an error scenario is possible at all, thus
only PTA_CMD_GET_DEVICES fails but not PTA_CMD_GET_DEVICES_SUPP.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Reply via email to