[PATCH v7 01/15] crypto/fsl: Add support for CAAM Job ring driver model

[Simon Glass]

Hi Gaurav,

On Tue, 7 Dec 2021 at 00:42, Gaurav Jain <gaurav.jain at nxp.com> wrote:
>
> added device tree support for job ring driver.
> sec is initialized based on job ring information processed
> from device tree.
>
> Signed-off-by: Gaurav Jain <gaurav.jain at nxp.com>
> Reviewed-by: Ye Li <ye.li at nxp.com>
> ---
>  drivers/crypto/fsl/jr.c | 323 ++++++++++++++++++++++++++--------------
>  drivers/crypto/fsl/jr.h |  19 +++
>  2 files changed, 231 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index 22b649219e..441550f348 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Copyright 2008-2014 Freescale Semiconductor, Inc.
> - * Copyright 2018 NXP
> + * Copyright 2018, 2021 NXP
>   *
>   * Based on CAAM driver in drivers/crypto/caam in Linux
>   */
> @@ -11,7 +11,6 @@
>  #include <linux/kernel.h>
>  #include <log.h>
>  #include <malloc.h>
> -#include "fsl_sec.h"
>  #include "jr.h"
>  #include "jobdesc.h"
>  #include "desc_constr.h"
> @@ -21,8 +20,11 @@
>  #include <asm/cache.h>
>  #include <asm/fsl_pamu.h>
>  #endif
> +#include <dm.h>
>  #include <dm/lists.h>
>  #include <linux/delay.h>
> +#include <dm/root.h>
> +#include <dm/device-internal.h>

These should go up one line.

https://www.denx.de/wiki/U-Boot/CodingStyle

Re the uclass, this is using MISC. What operations are actually
supported? I am still not sure why this doesn't deserve its own
uclass.


[..]

> diff --git a/drivers/crypto/fsl/jr.h b/drivers/crypto/fsl/jr.h
> index 1047aa772c..c5e5ed5c26 100644
> --- a/drivers/crypto/fsl/jr.h
> +++ b/drivers/crypto/fsl/jr.h
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0+ */
>  /*
>   * Copyright 2008-2014 Freescale Semiconductor, Inc.
> + * Copyright 2021 NXP
>   *
>   */
>
> @@ -8,7 +9,9 @@
>  #define __JR_H
>
>  #include <linux/compiler.h>
> +#include "fsl_sec.h"
>  #include "type.h"
> +#include <misc.h>
>
>  #define JR_SIZE 4
>  /* Timeout currently defined as 10 sec */
> @@ -35,12 +38,21 @@
>  #define JRSLIODN_SHIFT         0
>  #define JRSLIODN_MASK          0x00000fff
>
> +#define JRDID_MS_PRIM_DID      BIT(0)
> +#define JRDID_MS_PRIM_TZ       BIT(4)
> +#define JRDID_MS_TZ_OWN                BIT(15)
> +
>  #define JQ_DEQ_ERR             -1
>  #define JQ_DEQ_TO_ERR          -2
>  #define JQ_ENQ_ERR             -3

Good idea to put brackets around #defines that are a -ve number.

>
>  #define RNG4_MAX_HANDLES       2
>
> +enum {
> +       /* Run caam jobring descriptor(in buf) */
> +       CAAM_JR_RUN_DESC,
> +};
> +
>  struct op_ring {
>         caam_dma_addr_t desc;
>         uint32_t status;
> @@ -102,6 +114,13 @@ struct result {
>         uint32_t status;
>  };
>
> +struct caam_regs {
> +       ccsr_sec_t *sec;
> +       struct jr_regs *regs;
> +       u8 jrid;
> +       struct jobring jr[CONFIG_SYS_FSL_MAX_NUM_OF_SEC];
> +};

Don't forget to comment the structs.
> +
>  void caam_jr_strstatus(u32 status);
>  int run_descriptor_jr(uint32_t *desc);

and exported functions (normally drivers shouldn't have these).

>
> --
> 2.17.1
>

Regards,
Simon

Regards,
Simon