| Message ID | 20220607102126.2605773-3-etienne.carriere@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/3] drivers: tee: optee: remove unused probe local variable | expand |
Hi, 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@linaro.org> > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com> > Signed-off-by: Etienne Carriere <etienne.carriere@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(-) Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> Thanks Patrick
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@linaro.org> > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com> > Signed-off-by: Etienne Carriere <etienne.carriere@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
Hi Patrick, On Fri, 17 Jun 2022 at 14:06, Patrick DELAUNAY <patrick.delaunay@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@linaro.org> > > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com> > > Signed-off-by: Etienne Carriere <etienne.carriere@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 >
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> +#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 + /** 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,
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@linaro.org> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com> Signed-off-by: Etienne Carriere <etienne.carriere@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(-)