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

[Patrick DELAUNAY]

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


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