[EXT] Re: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model

[Gaurav Jain]

Hello Michael

> -----Original Message-----
> From: Michael Walle <michael at walle.cc>
> Sent: Tuesday, November 16, 2021 4:32 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>
> Cc: Shengzhou Liu <shengzhou.liu at nxp.com>; Varun Sethi
> <V.Sethi at nxp.com>; Adrian Alonso <adrian.alonso at nxp.com>; Alison Wang
> <alison.wang at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> festevam at gmail.com; Franck Lenormand <franck.lenormand at nxp.com>;
> Horia Geanta <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>;
> Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Mingkai Hu
> <mingkai.hu at nxp.com>; olteanv at gmail.com; Pankaj Gupta
> <pankaj.gupta at nxp.com>; Peng Fan <peng.fan at nxp.com>; Pramod Kumar
> <pramod.kumar_1 at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>;
> Rajesh Bhagat <rajesh.bhagat at nxp.com>; Sahil Malhotra
> <sahil.malhotra at nxp.com>; sbabic at denx.de; Silvano Di Ninno
> <silvano.dininno at nxp.com>; sjg at chromium.org; u-boot at lists.denx.de; dl-
> uboot-imx <uboot-imx at nxp.com>; Wasim Khan <wasim.khan at nxp.com>;
> Ye Li <ye.li at nxp.com>; Michael Walle <michael at walle.cc>
> Subject: [EXT] Re: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job
> ring driver model
> 
> Caution: EXT Email
> 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig index 5b30b13e43..2b24672505
> > 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2009,6 +2009,7 @@ config CMD_AES
> >
> >  config CMD_BLOB
> >       bool "Enable the 'blob' command"
> > +     select FSL_BLOB
> 
> this looks wrong, because CMD_BLOB sounds like a generic command but it
> will automatically select FSL_BLOB which in turn sounds freescale specific.
> Looking at the help text, this command is (at least at the moment) freescale
> specific, but the code seems to be generic and the blob_encap() and
> blob_decap() are weak functions, thus they could be implemented in a
> different way and not just by fsl_blob.c.
> 
> I don't think this should automatically select FSL_BLOB.
Ok.. will change in next version of this series.
> 
> Also, shouldn't this be an uclass with encap and decap ops?

I agree with your suggestion. but in the context of current patch series this is not required immediately.
Will test encap and decap function after converting as uclass ops and send a separate patch.

> 
> >       depends on !MX6ULL && !MX6SLL && !MX6SL
> >       select IMX_HAB if ARCH_MX6 || ARCH_MX7 || ARCH_MX7ULP ||
> ARCH_IMX8M
> >       help
> > diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
> > index 94ff540111..ab59d516f8 100644
> > --- a/drivers/crypto/fsl/Kconfig
> > +++ b/drivers/crypto/fsl/Kconfig
> > @@ -66,4 +66,11 @@ config FSL_CAAM_RNG
> >         using the prediction resistance flag which means the DRGB is
> >         reseeded from the TRNG every time random data is generated.
> >
> > +config FSL_BLOB
> > +        bool "Enable Blob Encap/Decap, Blob KEK support"
> 
> wrong indendation?
Will be addressed in next version..

> 
> > +     help
> > +       Enable support for the hardware based crytographic blob
> encap/decap
> > +       module of the CAAM. blobs can be safely placed into non-volatile
> > +       storage. blobs can only be decapsulated by the SoC that created it.
> > +       Enable support for blob key encryption key generation.
> >  endif