[EXT] Re: [PATCH v20 3/7] firmware: imx: add driver for NXP EdgeLock Enclave

[Pankaj Gupta]

> 
> smatch warnings:
> drivers/firmware/imx/ele_common.c:264 se_save_imem_state() warn:
> missing error code? 'ret'
> drivers/firmware/imx/ele_common.c:302 se_restore_imem_state() warn:
> missing error code? 'ret'
> 

Accepted.
Will return -EIO;

> vim +/ret +264 drivers/firmware/imx/ele_common.c
> 
> a9e23a74789346 Pankaj Gupta 2025-12-03  251  int
> se_save_imem_state(struct se_if_priv *priv, struct se_imem_buf *imem)
> a9e23a74789346 Pankaj Gupta 2025-12-03  252  {
> a9e23a74789346 Pankaj Gupta 2025-12-03  253     struct ele_dev_info s_info =
> {0};
> a9e23a74789346 Pankaj Gupta 2025-12-03  254     int ret;
> a9e23a74789346 Pankaj Gupta 2025-12-03  255
> a9e23a74789346 Pankaj Gupta 2025-12-03  256     ret = ele_get_info(priv,
> &s_info);
> a9e23a74789346 Pankaj Gupta 2025-12-03  257     if (ret) {
> a9e23a74789346 Pankaj Gupta 2025-12-03  258             dev_err(priv->dev,
> "Failed to get info from ELE.\n");
> a9e23a74789346 Pankaj Gupta 2025-12-03  259             return ret;
> a9e23a74789346 Pankaj Gupta 2025-12-03  260     }
> a9e23a74789346 Pankaj Gupta 2025-12-03  261
> a9e23a74789346 Pankaj Gupta 2025-12-03  262     /* Do not save the IMEM
> buffer, if the current IMEM state is BAD. */
> a9e23a74789346 Pankaj Gupta 2025-12-03  263     if
> (s_info.d_addn_info.imem_state == ELE_IMEM_STATE_BAD)
> a9e23a74789346 Pankaj Gupta 2025-12-03 @264             return ret;
> 
> This returns success if the state is _BAD.
> 
> a9e23a74789346 Pankaj Gupta 2025-12-03  265
> a9e23a74789346 Pankaj Gupta 2025-12-03  266     /*
> a9e23a74789346 Pankaj Gupta 2025-12-03  267      * EXPORT command will
> save encrypted IMEM to given address,
> a9e23a74789346 Pankaj Gupta 2025-12-03  268      * so later in resume, IMEM
> can be restored from the given
> a9e23a74789346 Pankaj Gupta 2025-12-03  269      * address.
> a9e23a74789346 Pankaj Gupta 2025-12-03  270      *
> a9e23a74789346 Pankaj Gupta 2025-12-03  271      * Size must be at least 64
> kB.
> a9e23a74789346 Pankaj Gupta 2025-12-03  272      */
> a9e23a74789346 Pankaj Gupta 2025-12-03  273     ret = ele_service_swap(priv,
> imem->phyaddr, ELE_IMEM_SIZE, ELE_IMEM_EXPORT);
> a9e23a74789346 Pankaj Gupta 2025-12-03  274     if (ret < 0) {
> a9e23a74789346 Pankaj Gupta 2025-12-03  275             dev_err(priv->dev,
> "Failed to export IMEM.");
> a9e23a74789346 Pankaj Gupta 2025-12-03  276             imem->size = 0;
> a9e23a74789346 Pankaj Gupta 2025-12-03  277     } else {
> a9e23a74789346 Pankaj Gupta 2025-12-03  278             dev_dbg(priv->dev,
> a9e23a74789346 Pankaj Gupta 2025-12-03  279                     "Exported %d
> bytes of encrypted IMEM.",
> a9e23a74789346 Pankaj Gupta 2025-12-03  280                     ret);
> a9e23a74789346 Pankaj Gupta 2025-12-03  281             imem->size = ret;
> a9e23a74789346 Pankaj Gupta 2025-12-03  282     }
> a9e23a74789346 Pankaj Gupta 2025-12-03  283
> a9e23a74789346 Pankaj Gupta 2025-12-03  284     return ret > 0 ? 0 : ret;
> a9e23a74789346 Pankaj Gupta 2025-12-03  285  }
> a9e23a74789346 Pankaj Gupta 2025-12-03  286
> a9e23a74789346 Pankaj Gupta 2025-12-03  287  int
> se_restore_imem_state(struct se_if_priv *priv, struct se_imem_buf *imem)
> a9e23a74789346 Pankaj Gupta 2025-12-03  288  {
> a9e23a74789346 Pankaj Gupta 2025-12-03  289     struct ele_dev_info s_info;
> a9e23a74789346 Pankaj Gupta 2025-12-03  290     int ret;
> a9e23a74789346 Pankaj Gupta 2025-12-03  291
> a9e23a74789346 Pankaj Gupta 2025-12-03  292     /* get info from ELE */
> a9e23a74789346 Pankaj Gupta 2025-12-03  293     ret = ele_get_info(priv,
> &s_info);
> a9e23a74789346 Pankaj Gupta 2025-12-03  294     if (ret) {
> a9e23a74789346 Pankaj Gupta 2025-12-03  295             dev_err(priv->dev,
> "Failed to get info from ELE.");
> a9e23a74789346 Pankaj Gupta 2025-12-03  296             return ret;
> a9e23a74789346 Pankaj Gupta 2025-12-03  297     }
> a9e23a74789346 Pankaj Gupta 2025-12-03  298     imem->state =
> s_info.d_addn_info.imem_state;
> a9e23a74789346 Pankaj Gupta 2025-12-03  299
> a9e23a74789346 Pankaj Gupta 2025-12-03  300     /* Get IMEM state, if 0xFE
> then import IMEM if imem size is non-zero. */
> 
> I really can't understand this comment at all.  :(

Accepted.
Will update the this comment in V21, with the following:
-       /* Get IMEM state, if 0xFE then import IMEM if imem size is non-zero. */
+       /* Check for the imem-state and imem-size before continue to
+        * restore imem state.
+        */

> 
> a9e23a74789346 Pankaj Gupta 2025-12-03  301     if
> (s_info.d_addn_info.imem_state != ELE_IMEM_STATE_BAD || !imem->size)
> a9e23a74789346 Pankaj Gupta 2025-12-03 @302             return ret;
> 
> This feels like returning zero is intentional, but please return a literal zero.
> s/return ret;/return 0;/.

Accepted. As mentioned above, will return -EIO;

>