[PATCH v3 3/3] drivers: rng: optee_rng: register to CONFIG_OPTEE_SERVICE_DISCOVERY

[Etienne Carriere]

Hi Patrick,

On Fri, 17 Jun 2022 at 14:06, Patrick DELAUNAY
<patrick.delaunay at foss.st.com> wrote:
>
> Hi,
>
> a minor remark
>
> On 6/7/22 12:21, Etienne Carriere wrote:
> > Changes optee_rng driver to register itself has a OP-TEE service so
> > that a device is bound for the driver when OP-TEE enumerates the
> > PTA RNG service.
> >
> > Cc: Sughosh Ganu <sughosh.ganu at linaro.org>
> > Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> > Signed-off-by: Etienne Carriere <etienne.carriere at linaro.org>
> > ---
> > No change since v2.
> >
> > No change since v1.
> > ---
> >   drivers/rng/Kconfig     |  1 +
> >   drivers/rng/optee_rng.c | 12 +++++++++++-
> >   2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> > index c10f7d345b..14e95a6213 100644
> > --- a/drivers/rng/Kconfig
> > +++ b/drivers/rng/Kconfig
> > @@ -34,6 +34,7 @@ config RNG_MSM
> >   config RNG_OPTEE
> >       bool "OP-TEE based Random Number Generator support"
> >       depends on DM_RNG && OPTEE
> > +     default y if OPTEE_SERVICE_DISCOVERY
> >       help
> >         This driver provides support for the OP-TEE based Random Number
> >         Generator on ARM SoCs where hardware entropy sources are not
> > diff --git a/drivers/rng/optee_rng.c b/drivers/rng/optee_rng.c
> > index aa8ce864d3..90d9434395 100644
> > --- a/drivers/rng/optee_rng.c
> > +++ b/drivers/rng/optee_rng.c
> > @@ -11,6 +11,9 @@
> >   #include <dm/device.h>
> >   #include <dm/device_compat.h>
> >   #include <linux/sizes.h>
> >
> >
> > a minor remark
> >
> > +#include <tee/optee_service.h>
> > +
> > +#define DRIVER_NAME  "optee-rng"
> >
> >   #define TEE_ERROR_HEALTH_TEST_FAIL  0x00000001
> >
> > @@ -35,6 +38,13 @@
> >   #define TA_HWRNG_UUID { 0xab7a617c, 0xb8e7, 0x4d8f, \
> >                       { 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64 } }
> >
> > +#ifdef CONFIG_OPTEE_SERVICE_DISCOVERY
> > +OPTEE_SERVICE_DRIVER(optee_rng) = {
> > +     .uuid = TA_HWRNG_UUID,
> > +     .driver_name = DRIVER_NAME,
> > +};
> > +#endif
> > +
>
> a minor remark
>
> I think '#ifdef CONFIG_OPTEE_SERVICE_DISCOVERY'
>
> can avoid here with the macro call:
>
> + OPTEE_SERVICE_DRIVER(optee_rng, TA_HWRNG_UUID, DRIVER_NAME);
>
> and a modified macro in patch 2:
>
> +#ifdef CONFIG_OPTEE_SERVICE_DISCOVERY
> +#define OPTEE_SERVICE_DRIVER(__name, __uuid, __drv_name) \
> +    ll_entry_declare(struct optee_service, __name, optee_service) = {\
> +        .uuid = __uuid,\
> +        .driver_name = __drv_name}
> +#else
> +#define OPTEE_SERVICE_DRIVER(__name, __uuid, __drv_name)
> +#endif
>

Makes sense and the driver code would look better without this #ifdef.
I'll send a v4.

Best regards,
Etienne

>
> This code is inspired by
>
> include/image.h:1575
>
> #defineU_BOOT_FIT_LOADABLE_HANDLER(_type, _handler) \
> ll_entry_declare(struct fit_loadable_tbl, _function, fit_loadable) ={ \
> .type =_type, \
> .handler =_handler, \
> }
>
> or also include/env.h:55
>
> /*
>   * Define a callback that can be associated with variables.
>   * when associated through the ".callbacks" environment variable, the
> callback
>   * will be executed any time the variable is inserted, overwritten, or
> deleted.
>   *
>   * For SPL these are silently dropped to reduce code size, since
> environment
>   * callbacks are not supported with SPL.
>   */
> #ifdef CONFIG_SPL_BUILD
> #define U_BOOT_ENV_CALLBACK(name, callback) \
>      static inline __maybe_unused void _u_boot_env_noop_##name(void) \
>      { \
>          (void)callback; \
>      }
> #else
> #define U_BOOT_ENV_CALLBACK(name, callback) \
>      ll_entry_declare(struct env_clbk_tbl, name, env_clbk) = \
>      {#name, callback}
> #endif
>
>
> >   /** open_session_ta_hwrng() - Open session with hwrng Trusted App
> >    *
> >    * @dev:            device
> > @@ -177,7 +187,7 @@ static const struct dm_rng_ops optee_rng_ops = {
> >   };
> >
> >   U_BOOT_DRIVER(optee_rng) = {
> > -     .name = "optee-rng",
> > +     .name = DRIVER_NAME,
> >       .id = UCLASS_RNG,
> >       .ops = &optee_rng_ops,
> >       .probe = optee_rng_probe,
>
>
> Regards
>