Hi Gaurav, On Mon, 29 Nov 2021 at 00:25, Gaurav Jain <[email protected]> wrote: > > Hello Simon > > > -----Original Message----- > > From: Simon Glass <[email protected]> > > Sent: Thursday, November 25, 2021 5:42 AM > > To: Gaurav Jain <[email protected]> > > Cc: U-Boot Mailing List <[email protected]>; Stefano Babic > > <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan > > <[email protected]>; Priyanka Jain <[email protected]>; Ye Li > > <[email protected]>; Horia Geanta <[email protected]>; Ji Luo > > <[email protected]>; Franck Lenormand <[email protected]>; Silvano Di > > Ninno <[email protected]>; Sahil Malhotra <[email protected]>; > > Pankaj Gupta <[email protected]>; Varun Sethi <[email protected]>; dl- > > uboot-imx <[email protected]>; Shengzhou Liu <[email protected]>; > > Mingkai Hu <[email protected]>; Rajesh Bhagat <[email protected]>; > > Meenakshi Aggarwal <[email protected]>; Wasim Khan > > <[email protected]>; Alison Wang <[email protected]>; Pramod > > Kumar <[email protected]>; Andy Tang <[email protected]>; > > Adrian Alonso <[email protected]>; Vladimir Oltean <[email protected]> > > 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 <[email protected]> wrote: > > > > > > Hello Simon > > > > > > > -----Original Message----- > > > > From: Simon Glass <[email protected]> > > > > Sent: Tuesday, November 2, 2021 8:26 PM > > > > To: Gaurav Jain <[email protected]> > > > > Cc: U-Boot Mailing List <[email protected]>; Stefano Babic > > > > <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan > > > > <[email protected]>; Priyanka Jain <[email protected]>; Ye Li > > > > <[email protected]>; Horia Geanta <[email protected]>; Ji Luo > > > > <[email protected]>; Franck Lenormand <[email protected]>; > > > > Silvano Di Ninno <[email protected]>; Sahil Malhotra > > > > <[email protected]>; Pankaj Gupta <[email protected]>; Varun > > > > Sethi <[email protected]>; dl- uboot-imx <[email protected]>; > > > > Shengzhou Liu <[email protected]>; Mingkai Hu > > > > <[email protected]>; Rajesh Bhagat <[email protected]>; > > > > Meenakshi Aggarwal <[email protected]>; Wasim Khan > > > > <[email protected]>; Alison Wang <[email protected]>; Pramod > > > > Kumar <[email protected]>; Andy Tang <[email protected]>; > > > > Adrian Alonso <[email protected]>; Vladimir Oltean > > > > <[email protected]> > > > > 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 <[email protected]> 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 <[email protected]> > > > > > Reviewed-by: Ye Li <[email protected]> > > > > > --- > > > > > 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. > > > > > > 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. > 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. Regards, Simon
