Re: [PATCH v4 3/3] nvmem: stm32: detect bsec pta presence for STM32MP15x

[Etienne Carriere]

Hi Patrick,

On Tue, 3 Jan 2023 at 15:08, Patrick Delaunay
<patrick.delaunay@xxxxxxxxxxx> wrote:
>
> On STM32MP15x SoC, the SMC backend is optional when OP-TEE is used;
> the PTA BSEC should be used as it is done on STM32MP13x platform,
> but the BSEC SMC can be also used: it is a legacy mode in OP-TEE,
> not recommended but used in previous OP-TEE firmware.
>
> The presence of OP-TEE is dynamically detected in STM32MP15x device tree
> and the supported NVMEM backend is dynamically detected:
> - PTA with stm32_bsec_pta_find
> - SMC with stm32_bsec_check
>
> With OP-TEE but without PTA and SMC detection, the probe is deferred for
> STM32MP15x devices.
>
> On STM32MP13x platform, only the PTA is supported with cfg->ta = true
> and this detection is skipped.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@xxxxxxxxxxx>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - use of_find_compatible_node in optee_presence_check function
>   instead of of_find_node_by_path("/firmware/optee")
>
> Changes in v2:
> - Added patch in the serie for BSEC PTA support on STM32MP15x
>   with dynamic detection of OP-TEE presence and SMC support (legacy mode)
>
>  drivers/nvmem/stm32-romem.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvmem/stm32-romem.c b/drivers/nvmem/stm32-romem.c
> index 2edc61925e52..1b90c78301fa 100644
> --- a/drivers/nvmem/stm32-romem.c
> +++ b/drivers/nvmem/stm32-romem.c
> @@ -159,6 +159,31 @@ static int stm32_bsec_pta_write(void *context, unsigned int offset, void *buf,
>         return stm32_bsec_optee_ta_write(priv->ctx, priv->lower, offset, buf, bytes);
>  }
>
> +static bool stm32_bsec_smc_check(void)
> +{
> +       u32 val;
> +       int ret;
> +
> +       /* check that the OP-TEE support the BSEC SMC (legacy mode) */
> +       ret = stm32_bsec_smc(STM32_SMC_READ_SHADOW, 0, 0, &val);
> +
> +       return !ret;
> +}
> +
> +static bool optee_presence_check(void)
> +{
> +       struct device_node *np;
> +       bool tee_detected = false;
> +
> +       /* check that the OP-TEE node is present and available. */
> +       np = of_find_compatible_node(NULL, NULL, "linaro,optee-tz");
> +       if (np && of_device_is_available(np))
> +               tee_detected = true;
> +       of_node_put(np);
> +
> +       return tee_detected;
> +}
> +
>  static int stm32_romem_probe(struct platform_device *pdev)
>  {
>         const struct stm32_romem_cfg *cfg;
> @@ -195,10 +220,14 @@ static int stm32_romem_probe(struct platform_device *pdev)
>         } else {
>                 priv->cfg.size = cfg->size;
>                 priv->lower = cfg->lower;
> -               if (cfg->ta) {
> +               if (cfg->ta || optee_presence_check()) {
>                         rc = stm32_bsec_optee_ta_open(&priv->ctx);
>                         /* wait for OP-TEE client driver to be up and ready */
> -                       if (rc)
> +                       if (rc == -EPROBE_DEFER) {
> +                               /* BSEC PTA is required or SMC not ready */
> +                               if (cfg->ta || !stm32_bsec_smc_check())
> +                                       return -EPROBE_DEFER;
> +                       } else if (rc)

Could you fix the logic? The sequence here fails to fallback to BSEC
SMC service if optee does not embed BSEC PTA service and optee driver
is probed before stm32_romem.

Br,
etienne

>                                 return rc;
>                         rc = devm_add_action_or_reset(dev, stm32_bsec_optee_ta_close, priv->ctx);
>                         if (rc) {
> --
> 2.25.1
>