Hi,

On 11/11/22 18:18, Srinivas Kandagatla wrote:

On 10/11/2022 15:45, Patrick Delaunay wrote:

quoted

For boot with OP-TEE on STM32MP13, the communication with the secure
world no more use STMicroelectronics SMC but communication with the
BSEC TA, for data access (read/write) or lock operation:
- all the request are sent to OP-TEE trusted application,
- for upper OTP with ECC protection and with word programming only
   each OTP are permanently locked when programmed to avoid ECC error
   on the second write operation

Signed-off-by: Patrick Delaunay <redacted>
---

Changes in v2:
- rebase series on linux-next/master
- minor update after V1 revue
- add missing sentinel in stm32_romem_of_match()
- reorder function and remove prototypes for stm32_bsec_pta... functions
- change stm32_bsec_pta_find to static
- add return value in dev_err()
- cleanups some comments, which can be on one line
- remove test on priv->ctx in stm32_bsec_pta_remove
- add missing tee_shm_free(shm) in stm32_bsec_pta_write() when
   tee_shm_get_va failed
- return error in stm32_bsec_pta_find when devm_add_action_or_reset 
failed
- handle driver_register error in stm32_romem_init

  drivers/nvmem/stm32-romem.c | 445 +++++++++++++++++++++++++++++++++++-
  1 file changed, 441 insertions(+), 4 deletions(-)

diff --git a/drivers/nvmem/stm32-romem.c b/drivers/nvmem/stm32-romem.c
index d1d03c2ad081..0a0e29d09b67 100644
--- a/drivers/nvmem/stm32-romem.c
+++ b/drivers/nvmem/stm32-romem.c

@@ -11,6 +11,7 @@

  #include <linux/module.h>
  #include <linux/nvmem-provider.h>
  #include <linux/of_device.h>
+#include <linux/tee_drv.h>
    /* BSEC secure service access from non-secure */
  #define STM32_SMC_BSEC            0x82001003

@@ -25,14 +26,401 @@

  struct stm32_romem_cfg {
      int size;
      u8 lower;
+    bool ta;
  };
    struct stm32_romem_priv {
      void __iomem *base;
      struct nvmem_config cfg;
      u8 lower;
+    struct device *ta;
  };
  +#if IS_ENABLED(CONFIG_OPTEE)
+/*

...

quoted

+
+static const struct tee_client_device_id stm32_bsec_id_table[] = {
+    {
+        UUID_INIT(0x94cf71ad, 0x80e6, 0x40b5,
+              0xa7, 0xc6, 0x3d, 0xc5, 0x01, 0xeb, 0x28, 0x03)
+    },
+    { }
+};
+
+MODULE_DEVICE_TABLE(tee, stm32_bsec_id_table);
+
+static struct tee_client_driver stm32_bsec_pta_driver = {
+    .id_table    = stm32_bsec_id_table,
+    .driver        = {
+        .name = "stm32-bsec-pta",
+        .bus = &tee_bus_type,
+        .probe = stm32_bsec_pta_probe,
+        .remove = stm32_bsec_pta_remove,
+    },
+};
+
+static void stm32_bsec_put_device(void *data)
+{
+    put_device(data);
+}
+
+static struct device *stm32_bsec_pta_find(struct device *dev)
+{
+    struct device *pta_dev;
+    int ret;
+
+    pta_dev = driver_find_next_device(&stm32_bsec_pta_driver.driver, 
NULL);

This is clearly not representing the dependencies in a proper device 
model.


If the nvmem provider is a TEE client driver lets model it that way.. 
brining in a additional device and somehow trying to link it with TEE 
driver is  a hack.

TEE is a firmware which allow access to secure ressource... including 
BSEC ressources


I think it is also the case on a other driver = mson_sm.c

=> econfig->priv = fw;

       fw is a handle to the firmware (secure monitor) which provide 
access to secure ressource



BSEC is a hardware device on the bus,

it it describe in the device tree, with a compatible,

the same description should be used for any SW, not only Linux kernel.

and the nvmem cell description are sub-node of BSEC node, used as 
phandle by other device.


I need to have a link between the NVMEM driver and the OP-TEE session;

But I use the tee bus discovery here it is a error,

because that create a second uneeded driver "stm32_bsec_pta_driver"...


I will remove this part, and only kept the PTA request with new lib 
functions "stm32_bsec_pta_XXX()".

quoted

+
+    if (pta_dev) {
+        ret = devm_add_action_or_reset(dev, stm32_bsec_put_device, 
pta_dev);
+        if (ret)
+            dev_err(dev, "devm_add_action_or_reset() failed (%d)\n", 
ret);
+
+        return ERR_PTR(ret);
+    }
+
+    return pta_dev;
+}
+
+#else
+static int stm32_bsec_pta_read(void *context, unsigned int offset, 
void *buf,
+                   size_t bytes)
+{
+    pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
+
+    return -ENXIO;
+}
+
+static int stm32_bsec_pta_write(void *context, unsigned int offset, 
void *buf,
+                size_t bytes)
+{
+    pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
+
+    return -ENXIO;
+}
+
+static struct device *stm32_bsec_pta_find(struct device *dev)
+{
+    pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
+
+    return NULL;
+}
+#endif

ifdefing inside the drvier is really ugly, please move this libary 
functions to a seperate file and add dependecy properly in Kconfig.

Ok

regards


Patrick


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel