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?

> 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.

>
> I agree to your suggestion for any change/cleanup after the series is applied.

Regards,
Simon

Reply via email to