[EXT] Re: [PATCH v8 01/15] crypto/fsl: Add support for CAAM Job ring driver model

Tue Mar 8 12:10:07 CET 2022

Hi Tom

> -----Original Message-----
> From: Tom Rini <trini at konsulko.com>
> Sent: Monday, March 7, 2022 8:46 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>
> Cc: Michael Walle <michael at walle.cc>; sbabic at denx.de; Varun Sethi
> <V.Sethi at nxp.com>; Adrian Alonso <adrian.alonso at nxp.com>; Alison Wang
> <alison.wang at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> festevam at gmail.com; Franck Lenormand <franck.lenormand at nxp.com>; Horia
> Geanta <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; marex at denx.de;
> Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Mingkai Hu
> <mingkai.hu at nxp.com>; olteanv at gmail.com; Pankaj Gupta
> <pankaj.gupta at nxp.com>; Peng Fan <peng.fan at nxp.com>; Pramod Kumar
> <pramod.kumar_1 at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>; Rajesh
> Bhagat <rajesh.bhagat at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> Shengzhou Liu <shengzhou.liu at nxp.com>; Silvano Di Ninno
> <silvano.dininno at nxp.com>; sjg at chromium.org; u-boot at lists.denx.de; dl-
> uboot-imx <uboot-imx at nxp.com>; Wasim Khan <wasim.khan at nxp.com>; Ye Li
> <ye.li at nxp.com>
> Subject: Re: [EXT] Re: [PATCH v8 01/15] crypto/fsl: Add support for CAAM Job
> ring driver model
> 
> On Mon, Mar 07, 2022 at 12:03:42PM +0000, Gaurav Jain wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michael Walle <michael at walle.cc>
> > > Sent: Monday, March 7, 2022 5:12 PM
> > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > Cc: sbabic at denx.de; Varun Sethi <V.Sethi at nxp.com>; Adrian Alonso
> > > <adrian.alonso at nxp.com>; Alison Wang <alison.wang at nxp.com>; Andy
> > > Tang <andy.tang at nxp.com>; festevam at gmail.com; Franck Lenormand
> > > <franck.lenormand at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji
> > > Luo <ji.luo at nxp.com>; marex at denx.de; Meenakshi Aggarwal
> > > <meenakshi.aggarwal at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>;
> > > olteanv at gmail.com; Pankaj Gupta <pankaj.gupta at nxp.com>; Peng Fan
> > > <peng.fan at nxp.com>; Pramod Kumar <pramod.kumar_1 at nxp.com>;
> Priyanka
> > > Jain <priyanka.jain at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> > > Sahil Malhotra <sahil.malhotra at nxp.com>; Shengzhou Liu
> > > <shengzhou.liu at nxp.com>; Silvano Di Ninno <silvano.dininno at nxp.com>;
> > > sjg at chromium.org; u- boot at lists.denx.de; dl-uboot-imx
> > > <uboot-imx at nxp.com>; Wasim Khan <wasim.khan at nxp.com>; Ye Li
> > > <ye.li at nxp.com>
> > > Subject: Re: [EXT] Re: [PATCH v8 01/15] crypto/fsl: Add support for
> > > CAAM Job ring driver model
> > >
> > > Caution: EXT Email
> > >
> > > Am 2022-03-07 12:33, schrieb Gaurav Jain:
> > > >> -----Original Message-----
> > > >> From: Michael Walle <michael at walle.cc>
> > > >> Sent: Monday, March 7, 2022 4:39 PM
> > > >> To: Gaurav Jain <gaurav.jain at nxp.com>
> > > >> Cc: sbabic at denx.de; Varun Sethi <V.Sethi at nxp.com>; Adrian Alonso
> > > >> <adrian.alonso at nxp.com>; Alison Wang <alison.wang at nxp.com>; Andy
> > > Tang
> > > >> <andy.tang at nxp.com>; festevam at gmail.com; Franck Lenormand
> > > >> <franck.lenormand at nxp.com>; Horia Geanta <horia.geanta at nxp.com>;
> > > >> Ji Luo <ji.luo at nxp.com>; marex at denx.de; Meenakshi Aggarwal
> > > >> <meenakshi.aggarwal at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>;
> > > >> olteanv at gmail.com; Pankaj Gupta <pankaj.gupta at nxp.com>; Peng Fan
> > > >> <peng.fan at nxp.com>; Pramod Kumar <pramod.kumar_1 at nxp.com>;
> > > Priyanka
> > > >> Jain <priyanka.jain at nxp.com>; Rajesh Bhagat
> > > >> <rajesh.bhagat at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> > > >> Shengzhou Liu <shengzhou.liu at nxp.com>; Silvano Di Ninno
> > > >> <silvano.dininno at nxp.com>; sjg at chromium.org; u-
> > > >> boot at lists.denx.de; dl-uboot-imx <uboot-imx at nxp.com>; Wasim Khan
> > > >> <wasim.khan at nxp.com>; Ye Li <ye.li at nxp.com>
> > > >> Subject: Re: [EXT] Re: [PATCH v8 01/15] crypto/fsl: Add support
> > > >> for CAAM Job ring driver model
> > > >>
> > > >> Caution: EXT Email
> > > >>
> > > >> Am 2022-03-07 11:56, schrieb Gaurav Jain:
> > > >> >> -----Original Message-----
> > > >> >> From: Michael Walle <michael at walle.cc>
> > > >> >> Sent: Monday, March 7, 2022 3:28 PM
> > > >> >> To: sbabic at denx.de
> > > >> >> Cc: Varun Sethi <V.Sethi at nxp.com>; Adrian Alonso
> > > >> >> <adrian.alonso at nxp.com>; Alison Wang <alison.wang at nxp.com>;
> > > >> >> Andy
> > > >> Tang
> > > >> >> <andy.tang at nxp.com>; festevam at gmail.com; Franck Lenormand
> > > >> >> <franck.lenormand at nxp.com>; Gaurav Jain <gaurav.jain at nxp.com>;
> > > >> >> Horia Geanta <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>;
> > > >> >> marex at denx.de; Meenakshi Aggarwal
> > > >> >> <meenakshi.aggarwal at nxp.com>; Mingkai Hu
> <mingkai.hu at nxp.com>;
> > > >> >> olteanv at gmail.com; Pankaj Gupta <pankaj.gupta at nxp.com>; Peng
> > > >> >> Fan <peng.fan at nxp.com>; Pramod
> > > Kumar
> > > >> >> <pramod.kumar_1 at nxp.com>; Priyanka Jain
> > > >> >> <priyanka.jain at nxp.com>; Rajesh Bhagat
> > > >> >> <rajesh.bhagat at nxp.com>; Sahil Malhotra
> > > >> >> <sahil.malhotra at nxp.com>; Shengzhou Liu
> > > >> >> <shengzhou.liu at nxp.com>; Silvano Di Ninno
> > > >> >> <silvano.dininno at nxp.com>; sjg at chromium.org; u-
> > > >> >> boot at lists.denx.de; dl-uboot-imx <uboot-imx at nxp.com>; Wasim
> > > >> >> Khan <wasim.khan at nxp.com>; Ye Li <ye.li at nxp.com>; Michael
> > > >> >> Walle <michael at walle.cc>
> > > >> >> Subject: Re: [EXT] Re: [PATCH v8 01/15] crypto/fsl: Add
> > > >> >> support for CAAM Job ring driver model
> > > >> >>
> > > >> >> Caution: EXT Email
> > > >> >>
> > > >> >> > On 03.03.22 14:41, Gaurav Jain wrote:
> > > >> >> >> As we have not received any response from imx6dl_mamoj
> > > >> >> >> board
> > > >> maintainer.
> > > >> >> >> I propose the below solution
> > > >> >> >>
> > > >> >> >> --- a/arch/arm/mach-imx/Kconfig
> > > >> >> >> +++ b/arch/arm/mach-imx/Kconfig
> > > >> >> >> @@ -49,8 +49,8 @@ config USE_IMXIMG_PLUGIN  config
> IMX_HAB
> > > >> >> >> -       select FSL_CAAM if HAS_CAAM
> > > >> >> >> -       imply CMD_DEKBLOB if HAS_CAAM
> > > >> >> >> +       imply FSL_CAAM if HAS_CAAM
> > > >> >> >> +       imply CMD_DEKBLOB if FSL_CAAM
> > > >> >> >>           Help
> > > >> >> >>
> > > >> >> >
> > > >> >> > IMO this is ok, I was also wrong, Marek is not the
> > > >> >> > maintainer of this board. This was the only board with
> > > >> >> > broken build - let's say, I will still wait a couple of
> > > >> >> > days, and if there is no comments, I will apply your series
> > > >> >> > (but then V10). I can apply this fix myself, no need to post
> > > >> >> > the series again (I have not seen any other comment or
> > > >> >> > request to
> > > >> >> change).
> > > >> >>
> > > >> >> I don't understand why the solution isn't the same one as for
> > > >> >> the layerscape part in this series[1]: enable the config per
> > > >> >> board (that is your boards) and leave all others the same as before?
> > > >> >
> > > >> > imx6dl_mamoj caam driver is not enabled by any of my changes.
> > > >> > This board is enabling  IMX_HAB which select FSL_CAAM.
> > > >> > Proposed changes making it imply so that FSL_CAAM can be
> > > >> > disabled in board defconfig.
> > > >>
> > > >> Ahh it was already selected before. But mhh, does IMX_HAB even
> > > >> makes sense without FSL_CAAM? Why was is a hard dependency before?
> > > >
> > > > With imply, this will still enable FSL_CAAM unless it is
> > > > explicitly disabled in defconfig. With select I do not have choice
> > > > of disabling FSL_CAAM in defconfig. Now I have disabled FSL_CAAM
> > > > only for imx6dl_mamoj_defconfig, as it is reporting spl size
> > > > issues with caam driver model approach.
> > >
> > > I can see *what* you are doing, but that doesn't answer *why* it was
> > > a hard dependency before and why now of a sudden can be a soft
> dependency.
> >
> > I am not sure of any other dependency, but from the code ./arch/arm/mach-
> imx/cmd_dek.c  needs caam for blob_encap_dek operation for mx6, mx7,
> mx7ulp.
> > Making it soft dependency allows me to disable caam for imx6dl_mamoj.
> 
> Yes, but can you reasonably, functionally, do that?  Or are you just making things
> link but now the platform is non functional?  Nothing that's an actual shell cmd
> should be linked in / included in the SPL binary (it should get discarded if built),
> but my recollection from migrating the HAB/CAAM symbols to Kconfig is that
> functionally you can't do what you're trying to do.
> 

I further checked on your concern and propose the below change to stop building caam driver in SPL for imx6dl_mamoj.

--- a/configs/imx6dl_mamoj_defconfig
+++ b/configs/imx6dl_mamoj_defconfig
@@ -61,3 +61,4 @@ CONFIG_USB_GADGET_MANUFACTURER="FSL"
 CONFIG_USB_GADGET_VENDOR_NUM=0x0525
 CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
 CONFIG_CI_UDC=y
+CONFIG_SPL_CRYPTO=n

Gaurav
> --
> Tom
