On Tue, 8 Sept 2026 at 12:53, Jan Kiszka <[email protected]> wrote:
>
> 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.

This can happen in the kernel (e.g the emulated RPMB that the
supplicant presents), but I don't think it happens in u-boot. However,
this is a valid reason not to exit immediately, so I am fine leaving
it as is

Cheers
/Ilias
>
> Jan
>
> --
> Siemens AG, Foundational Technologies
> Linux Expert Center

Reply via email to