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??
> > > > > > > > > > Will update the driver with livetree API in next version of this series. > > > > > > > > RNG instantiation is done as part of caam initialization. > > > > hwrng_generate() > > > function is added by me to generate random number via caam rng. > > > > I am not sure where to document but cover letter was updated. > > > > > > > > using OF_PLATDATA also have memory issues in SPL. It needs 15KB > > > > more > > > memory compared to no driver model in SPL. We don't have enough > > > OCRAM on few platforms. > > > > SPL size issue is not caused by MISC UCLASS but it is due to limited > > > > OCRAM. > > > > > > That's a very large increase. Can you point me to the tree which > > > enables that so I can take a look? With of-platadata the overhead > > > should be much smaller than that, perhaps 4KB. > > > > 1. Tried imx8mm_evk_defconfig with downstream uboot, which results in 15K > increase in u-boot-spl.bin when enable OF_PLATDATA. > > Can you please push a patch or push your tree somewhere? That board already > uses SPL_OF_CONTROL so I cannot fathom why enabling SPL_OF_PLATDATA as > well would increase the size. You can take the source code from https://source.codeaurora.org/external/imx/uboot-imx/ (branch: lf_v2021.04). With downstream uboot, for imx8mm SPL_OF_CONTROL is not enabled in SPL because of size issues. > > > 2. then tried imx8mq_evk_defconfig with upstream. Lot of build errors > reported with OF_PLATDATA. Just commented and build, which resulted in 11K > increase in u-boot-spl.bin(77K to 88K). > > Well let's ignore that one for now, for this issue. Ok.. Regards Gaurav Jain > > Regards, > Simon