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

Mon Dec 6 06:32:41 CET 2021

Hello

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Wednesday, December 1, 2021 11:32 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> <peng.fan at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>; Silvano Di
> Ninno <silvano.dininno at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; dl-
> uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu <shengzhou.liu at nxp.com>;
> Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: Re: [EXT] Re: [PATCH v4 01/16] crypto/fsl: Add support for CAAM Job
> ring driver model
> 
> Caution: EXT Email
> 
> On Wed, 1 Dec 2021 at 05:14, Gaurav Jain <gaurav.jain at nxp.com> wrote:
> >
> > Hello Simon
> >
> > > -----Original Message-----
> > > From: Simon Glass <sjg at chromium.org>
> > > Sent: Wednesday, December 1, 2021 11:57 AM
> > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> > > <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> > > <peng.fan at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> > > <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> > > <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>;
> > > Silvano Di Ninno <silvano.dininno at nxp.com>; Sahil Malhotra
> > > <sahil.malhotra at nxp.com>; Pankaj Gupta <pankaj.gupta at nxp.com>; Varun
> > > Sethi <V.Sethi at nxp.com>; dl- uboot-imx <uboot-imx at nxp.com>;
> > > Shengzhou Liu <shengzhou.liu at nxp.com>; Mingkai Hu
> > > <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> > > Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> > > <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> > > Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> > > Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean
> > > <olteanv at gmail.com>
> > > Subject: Re: [EXT] Re: [PATCH v4 01/16] crypto/fsl: Add support for
> > > CAAM Job ring driver model
> > >
> > > Caution: EXT Email
> > >
> > > Hi Gaurav,
> > >
> > > On Mon, 29 Nov 2021 at 00:25, Gaurav Jain <gaurav.jain at nxp.com> wrote:
> > > >
> > > > Hello Simon
> > > >
> > > > > -----Original Message-----
> > > > > From: Simon Glass <sjg at chromium.org>
> > > > > Sent: Thursday, November 25, 2021 5:42 AM
> > > > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> > > > > <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> > > > > <peng.fan at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> > > > > <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> > > > > <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>;
> > > > > Silvano Di Ninno <silvano.dininno at nxp.com>; Sahil Malhotra
> > > > > <sahil.malhotra at nxp.com>; Pankaj Gupta <pankaj.gupta at nxp.com>;
> > > > > Varun Sethi <V.Sethi at nxp.com>; dl- uboot-imx
> > > > > <uboot-imx at nxp.com>; Shengzhou Liu <shengzhou.liu at nxp.com>;
> > > > > Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat
> > > > > <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal
> > > > > <meenakshi.aggarwal at nxp.com>; Wasim Khan <wasim.khan at nxp.com>;
> > > > > Alison Wang <alison.wang at nxp.com>; Pramod Kumar
> > > > > <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> Adrian
> > > > > Alonso <adrian.alonso at nxp.com>; Vladimir Oltean
> > > > > <olteanv at gmail.com>
> > > > > Subject: Re: [EXT] Re: [PATCH v4 01/16] crypto/fsl: Add support
> > > > > for CAAM Job ring driver model
> > > > >
> > > > > Caution: EXT Email
> > > > >
> > > > > Hi Gaurav,
> > > > >
> > > > > On Mon, 8 Nov 2021 at 02:30, Gaurav Jain <gaurav.jain at nxp.com>
> wrote:
> > > > > >
> > > > > > Hello Simon
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Simon Glass <sjg at chromium.org>
> > > > > > > Sent: Tuesday, November 2, 2021 8:26 PM
> > > > > > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > > > > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano
> > > > > > > Babic <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>;
> > > > > > > Peng Fan <peng.fan at nxp.com>; Priyanka Jain
> > > > > > > <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>; Horia Geanta
> > > > > > > <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck
> > > > > > > Lenormand <franck.lenormand at nxp.com>; Silvano Di Ninno
> > > > > > > <silvano.dininno at nxp.com>; Sahil Malhotra
> > > > > > > <sahil.malhotra at nxp.com>; Pankaj Gupta
> > > > > > > <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; dl-
> > > > > > > uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu
> > > > > > > <shengzhou.liu at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>;
> > > > > > > Rajesh Bhagat <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal
> > > > > > > <meenakshi.aggarwal at nxp.com>; Wasim Khan
> > > > > > > <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>;
> > > > > > > Pramod Kumar <pramod.kumar_1 at nxp.com>; Andy Tang
> > > > > > > <andy.tang at nxp.com>;
> > > Adrian
> > > > > > > Alonso <adrian.alonso at nxp.com>; Vladimir Oltean
> > > > > > > <olteanv at gmail.com>
> > > > > > > Subject: [EXT] Re: [PATCH v4 01/16] crypto/fsl: Add support
> > > > > > > for CAAM Job ring driver model
> > > > > > >
> > > > > > > Caution: EXT Email
> > > > > > >
> > > > > > > Hi Gaurav,
> > > > > > >
> > > > > > > On Tue, 26 Oct 2021 at 00:56, 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>
> > > > > > > > ---
> > > > > > > >  cmd/Kconfig                 |   1 +
> > > > > > > >  drivers/crypto/fsl/Kconfig  |   7 +
> > > > > > > >  drivers/crypto/fsl/Makefile |   4 +-
> > > > > > > >  drivers/crypto/fsl/jr.c     | 318 ++++++++++++++++++++++++---------
> ---
> > > > > > > >  drivers/crypto/fsl/jr.h     |  14 ++
> > > > > > > >  5 files changed, 234 insertions(+), 110 deletions(-)
> > > > > > >
> > > > > > > You should not have CONFIG_ARCH_IMX8 in a driver. Things
> > > > > > > like that should be handled by using a different compatible string.
> > > > > > > Also please use the
> > > > > livetree API.
> > > > > > >
> > > > > > > I asked about the use of MISC as a uclass earlier. It seems
> > > > > > > that this device provides random numbers and perhaps
> > > > > > > hashing? It is hard to know since I am not sure where the
> > > > > > > documentation is in this series. It seems odd to be modelled
> > > > > > > as a MISC device. My understanding is that there are
> > > > > > > problems with the size in SPL if a different
> > > > > UCLASS is used.
> > > > > > > Is that correct? I asked about of-platdata but I didn't see
> > > > > > > any comment on
> > > > > that.
> > > > > > >
> > > > > > > This does seem to be a significant clean-up even as it is,
> > > > > > > though, so these thoughts could be looked at after this series is
> applied.
> > > > > > >
> > > > > > Kernel dtb uses single compatible string for all imx platforms.
> > > > > > Adding a different compatible string will introduce a
> > > > > > difference with kernel dts
> > > > > nodes.
> > > > >
> > > > > That sounds like a bug?
> > > > In jr.c,  for IMX8 we need to do only Jr initialization and
> > > > skipping RNG
> > > instantiation as it is already done.
> > > > This is achieved using CONFIG_ARCH_IMX8. Are you suggesting to use
> > > different macro?
> > >
> > > Don't add #ifdefs to drivers. Use a compatible string instead.
> > defining a different compatible string for imx8 in -u-boot-dtsi file which will
> overwrite the original is ok??
> 
> Can you not fix this in the kernel? How did you come to use the same compatible
> string for two different pieces of hardware?
This will be addressed in uboot in next version.

Regards
Gaurav Jain
> 
> Regards,
> Simon