Re: [PATCH v3] imx8: Add ahab_commit command

[Fabio Estevam]

Hi John,

This version looks better, but error handling could be improved:

On Mon, Sep 8, 2025 at 6:18 PM John Ripple <john.rip...@keysight.com> wrote:

> +static int do_ahab_commit(struct cmd_tbl *cmdtp, int flag, int argc,
> +                         char *const argv[])
> +{
> +       u32 info;
> +
> +       if (argc < 2)
> +               return CMD_RET_USAGE;
> +
> +       info = simple_strtoul(argv[1], NULL, 16);
> +       printf("Commit index is 0x%x\n", info);
> +
> +       if (sc_seco_commit(-1, &info)) {
> +               printf("Error in AHAB commit\n");
> +               return -EIO;
> +       }

What about:

ret = sc_seco_commit(-1, &info);
if (ret) {
    printf("Error in AHAB commit: %d\n", ret);
    return ret;
}

> +int sc_seco_commit(sc_ipc_t ipc, u32 *info)
> +{
> +       struct udevice *dev = gd->arch.scu_dev;
> +       struct sc_rpc_msg_s msg;
> +       int size = sizeof(struct sc_rpc_msg_s);
> +       int ret;
> +
> +       /* Fill in header */
> +       RPC_VER(&msg) = SC_RPC_VERSION;
> +       RPC_SIZE(&msg) = 2U;
> +       RPC_SVC(&msg) = (u8)SC_RPC_SVC_SECO;
> +       RPC_FUNC(&msg) = (u8)SECO_FUNC_COMMIT;
> +
> +       /* Fill in send message */
> +       RPC_U32(&msg, 0U) = *info;
> +
> +       /* Call RPC */
> +       ret = misc_call(dev, SC_FALSE, &msg, size, &msg, size);

 It is better to check the value of 'ret' and return immediately in
case of error:

 ret = misc_call(dev, SC_FALSE, &msg, size, &msg, size);
 if (ret)
     return ret;