On Wed, 1 Dec 2021 at 05:14, Gaurav Jain <gaurav.j...@nxp.com> wrote:
>
> Hello Simon
>
> > -----Original Message-----
> > From: Simon Glass <s...@chromium.org>
> > Sent: Wednesday, December 1, 2021 11:57 AM
> > To: Gaurav Jain <gaurav.j...@nxp.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > <sba...@denx.de>; Fabio Estevam <feste...@gmail.com>; Peng Fan
> > <peng....@nxp.com>; Priyanka Jain <priyanka.j...@nxp.com>; Ye Li
> > <ye...@nxp.com>; Horia Geanta <horia.gea...@nxp.com>; Ji Luo
> > <ji....@nxp.com>; Franck Lenormand <franck.lenorm...@nxp.com>; Silvano Di
> > Ninno <silvano.dini...@nxp.com>; Sahil Malhotra <sahil.malho...@nxp.com>;
> > Pankaj Gupta <pankaj.gu...@nxp.com>; Varun Sethi <v.se...@nxp.com>; dl-
> > uboot-imx <uboot-...@nxp.com>; Shengzhou Liu <shengzhou....@nxp.com>;
> > Mingkai Hu <mingkai...@nxp.com>; Rajesh Bhagat <rajesh.bha...@nxp.com>;
> > Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; Wasim Khan
> > <wasim.k...@nxp.com>; Alison Wang <alison.w...@nxp.com>; Pramod
> > Kumar <pramod.kuma...@nxp.com>; Andy Tang <andy.t...@nxp.com>;
> > Adrian Alonso <adrian.alo...@nxp.com>; Vladimir Oltean <olte...@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.j...@nxp.com> wrote:
> > >
> > > Hello Simon
> > >
> > > > -----Original Message-----
> > > > From: Simon Glass <s...@chromium.org>
> > > > Sent: Thursday, November 25, 2021 5:42 AM
> > > > To: Gaurav Jain <gaurav.j...@nxp.com>
> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > > <sba...@denx.de>; Fabio Estevam <feste...@gmail.com>; Peng Fan
> > > > <peng....@nxp.com>; Priyanka Jain <priyanka.j...@nxp.com>; Ye Li
> > > > <ye...@nxp.com>; Horia Geanta <horia.gea...@nxp.com>; Ji Luo
> > > > <ji....@nxp.com>; Franck Lenormand <franck.lenorm...@nxp.com>;
> > > > Silvano Di Ninno <silvano.dini...@nxp.com>; Sahil Malhotra
> > > > <sahil.malho...@nxp.com>; Pankaj Gupta <pankaj.gu...@nxp.com>; Varun
> > > > Sethi <v.se...@nxp.com>; dl- uboot-imx <uboot-...@nxp.com>;
> > > > Shengzhou Liu <shengzhou....@nxp.com>; Mingkai Hu
> > > > <mingkai...@nxp.com>; Rajesh Bhagat <rajesh.bha...@nxp.com>;
> > > > Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; Wasim Khan
> > > > <wasim.k...@nxp.com>; Alison Wang <alison.w...@nxp.com>; Pramod
> > > > Kumar <pramod.kuma...@nxp.com>; Andy Tang <andy.t...@nxp.com>;
> > > > Adrian Alonso <adrian.alo...@nxp.com>; Vladimir Oltean
> > > > <olte...@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.j...@nxp.com> wrote:
> > > > >
> > > > > Hello Simon
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Simon Glass <s...@chromium.org>
> > > > > > Sent: Tuesday, November 2, 2021 8:26 PM
> > > > > > To: Gaurav Jain <gaurav.j...@nxp.com>
> > > > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > > > > <sba...@denx.de>; Fabio Estevam <feste...@gmail.com>; Peng Fan
> > > > > > <peng....@nxp.com>; Priyanka Jain <priyanka.j...@nxp.com>; Ye Li
> > > > > > <ye...@nxp.com>; Horia Geanta <horia.gea...@nxp.com>; Ji Luo
> > > > > > <ji....@nxp.com>; Franck Lenormand <franck.lenorm...@nxp.com>;
> > > > > > Silvano Di Ninno <silvano.dini...@nxp.com>; Sahil Malhotra
> > > > > > <sahil.malho...@nxp.com>; Pankaj Gupta <pankaj.gu...@nxp.com>;
> > > > > > Varun Sethi <v.se...@nxp.com>; dl- uboot-imx
> > > > > > <uboot-...@nxp.com>; Shengzhou Liu <shengzhou....@nxp.com>;
> > > > > > Mingkai Hu <mingkai...@nxp.com>; Rajesh Bhagat
> > > > > > <rajesh.bha...@nxp.com>; Meenakshi Aggarwal
> > > > > > <meenakshi.aggar...@nxp.com>; Wasim Khan <wasim.k...@nxp.com>;
> > > > > > Alison Wang <alison.w...@nxp.com>; Pramod Kumar
> > > > > > <pramod.kuma...@nxp.com>; Andy Tang <andy.t...@nxp.com>;
> > Adrian
> > > > > > Alonso <adrian.alo...@nxp.com>; Vladimir Oltean
> > > > > > <olte...@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.j...@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.j...@nxp.com>
> > > > > > > Reviewed-by: Ye Li <ye...@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?

Regards,
Simon

Reply via email to