Hello Simon

> -----Original Message-----
> From: Simon Glass <[email protected]>
> Sent: Wednesday, December 1, 2021 11:57 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, 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.
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

Reply via email to