[EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device tree for supporting DM in SPL

Gaurav Jain gaurav.jain at nxp.com
Mon Nov 8 09:20:39 CET 2021


Hello Andrey

> -----Original Message-----
> From: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> Sent: Wednesday, November 3, 2021 5:05 PM
> To: Adam Ford <aford173 at gmail.com>; Gaurav Jain <gaurav.jain at nxp.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> <peng.fan at nxp.com>; Simon Glass <sjg at chromium.org>; Priyanka Jain
> <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>; Horia Geanta
> <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck Lenormand
> <franck.lenormand at nxp.com>; Silvano Di Ninno <silvano.dininno at nxp.com>;
> Sahil Malhotra <sahil.malhotra at nxp.com>; Pankaj Gupta
> <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; dl-uboot-imx
> <uboot-imx at nxp.com>; Shengzhou Liu <shengzhou.liu at nxp.com>; Mingkai Hu
> <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: RE: [EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device tree for
> supporting DM in SPL
> 
> Caution: EXT Email
> 
> Hello Gurav/Adam ,
> 
> > -----Original Message-----
> > From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Adam Ford
> > Sent: Tuesday, November 2, 2021 12:19 PM
> > To: Gaurav Jain <gaurav.jain at nxp.com>
> > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> > <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> > <peng.fan at nxp.com>; Simon Glass <sjg at chromium.org>; Priyanka Jain
> > <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>; Horia Geanta
> > <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck Lenormand
> > <franck.lenormand at nxp.com>; Silvano Di Ninno
> > <silvano.dininno at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> > Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>;
> > dl-uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu
> > <shengzhou.liu at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>; Rajesh
> > Bhagat <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal
> > <meenakshi.aggarwal at nxp.com>; Wasim Khan <wasim.khan at nxp.com>;
> Alison
> > Wang <alison.wang at nxp.com>; Pramod Kumar
> <pramod.kumar_1 at nxp.com>;
> > Andy Tang <andy.tang at nxp.com>; Adrian Alonso <adrian.alonso at nxp.com>;
> > Vladimir Oltean <olteanv at gmail.com>
> > Subject: Re: [EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device
> > tree for supporting DM in SPL
> >
> >
> > On Tue, Nov 2, 2021 at 3:17 AM Gaurav Jain <gaurav.jain at nxp.com> wrote:
> > >
> > > Hello Adam
> > >
> > > > -----Original Message-----
> > > > From: Adam Ford <aford173 at gmail.com>
> > > > Sent: Monday, November 1, 2021 6:30 PM
> > > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> > > > <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> > > > <peng.fan at nxp.com>; Simon Glass <sjg at chromium.org>; Priyanka Jain
> > > > <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>; Horia Geanta
> > > > <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck Lenormand
> > > > <franck.lenormand at nxp.com>; Silvano Di Ninno
> > > > <silvano.dininno at nxp.com>; Sahil Malhotra
> > > > <sahil.malhotra at nxp.com>; Pankaj Gupta <pankaj.gupta at nxp.com>;
> > > > Varun Sethi <V.Sethi at nxp.com>; dl-uboot-imx <uboot-imx at nxp.com>;
> > > > Shengzhou Liu <shengzhou.liu at nxp.com>; Mingkai Hu
> > > > <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> > > > Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> > > > <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> > > > Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> > > > Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean
> > > > <olteanv at gmail.com>
> > > > Subject: [EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device
> > > > tree for supporting DM in SPL
> > > >
> > > > Caution: EXT Email
> > > >
> > > > On Tue, Oct 26, 2021 at 1:57 AM Gaurav Jain <gaurav.jain at nxp.com>
> wrote:
> > > > >
> > > > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for
> > > > > secure boot.
> > > > >
> > > > > Signed-off-by: Gaurav Jain <gaurav.jain at nxp.com>
> > > > > Reviewed-by: Ye Li <ye.li at nxp.com>
> > > > > ---
> > > > >  arch/arm/dts/imx8mm-evk-u-boot.dtsi      | 18 +++++++++++++++++-
> > > > >  arch/arm/dts/imx8mm.dtsi                 |  1 +
> > > > >  arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 18 +++++++++++++++++-
> > > > >  arch/arm/dts/imx8mn.dtsi                 |  1 +
> > > > >  arch/arm/dts/imx8mp-evk-u-boot.dtsi      | 18 +++++++++++++++++-
> > > > >  arch/arm/dts/imx8mp.dtsi                 |  1 +
> > > > >  arch/arm/dts/imx8mq.dtsi                 |  1 +
> > > > >  7 files changed, 55 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > index 3c75415e8f..40f5cfda9a 100644
> > > > > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > @@ -1,6 +1,6 @@
> > > > >  // SPDX-License-Identifier: GPL-2.0+
> > > > >  /*
> > > > > - * Copyright 2019 NXP
> > > > > + * Copyright 2019, 2021 NXP
> > > > >   */
> > > > >
> > > > >  #include "imx8mm-u-boot.dtsi"
> > > > > @@ -72,6 +72,22 @@
> > > > >         u-boot,dm-spl;
> > > > >  };
> > > > >
> > > > > +&crypto {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr0 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr1 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr2 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > >  &usdhc1 {
> > > > >         u-boot,dm-spl;
> > > > >  };
> > > > > diff --git a/arch/arm/dts/imx8mm.dtsi b/arch/arm/dts/imx8mm.dtsi
> > > > > index b142b80734..009999bf3a 100644
> > > > > --- a/arch/arm/dts/imx8mm.dtsi
> > > > > +++ b/arch/arm/dts/imx8mm.dtsi
> > > > > @@ -824,6 +824,7 @@
> > > > >                                         compatible = "fsl,sec-v4.0-job-ring";
> > > > >                                         reg = <0x1000 0x1000>;
> > > > >                                         interrupts = <GIC_SPI
> > > > > 105 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                                       status = "disabled";
> > > >
> > > > Changing the SoC DTSI files makes future re-synchronizing difficult.
> > > > If you mark these as disabled in the respective u-boot.dtsi files,
> > > > it will create less work in the future.  You're already enabling a
> > > > bunch of new features in the respective -u-boot.dtsi files, I
> > > > would suggest
> > doing the disable in the same way.
> > >
> > > JR0 status is marked as disabled, as it is reserved in ATF and
> > > cannot be accessed
> > in SPL and Uboot.
> > > For JR driver model(new feature) to work, at least one Jobring has
> > > to be
> > initialized, which is not possible with JR0.
> > > JR1 will be initialized.
> >
> > I wasn't suggesting that it should not be disabled.  I was asking that
> > it be disabled in the -u-boot.dtsi file instead of the imx8mm.dtsi
> > file because when the imx8mm.dtsi file gets resync'd with Linux, it
> > might be lost.  Having it done in the - u-boot.dtsi file instead helps
> > reduce the likelihood of being undone later, and that is the purpose of the -u-
> boot.dtsi files.
> 
> If I may add my 2c here:
> It seems to me that this patch should be split into at least 2 separate patches:
> one that disables the JR0 node, and one - for the rest.
> 
> The reason being: if the JR0 node is reserved for ATF and should not be available
> - then it should be disabled in the Kernel DTB as well as here.
> 
> This part is missing in upstream Kernel, so the boot process throws following
> errors during JR probing:
> ----
> [    1.509894] caam 30900000.crypto: job rings = 3, qi = 0
> [    1.525201] caam_jr 30901000.jr: failed to flush job ring 0
> [    1.525214] caam_jr: probe of 30901000.jr failed with error -5
> ----
> 
> Disabling the JR0 node in the kernel makes this error go away, and decreases the
> JR count to 2 which can be observed in the NXP vendor kernel.
> 
> I suggest you to extract the node disabling from this patch and send it to Kernel.
> Once accepted in upstream - this change would be picked up by the U-Boot at
> next DTB re-sync.
> 
> -- andrey
> 

Removing the JR0 disable change from this patch will cause CAAM failed to initialize on iMX8M series.
As Adam suggested, I will disable the JR0 in the - u-boot.dtsi. I will also send a patch to kernel for disabling the JRO.
When U-boot DTB will be synced with kernel, I will remove the JR0 disable change from - u-boot.dtsi

Regards
Gaurav Jain

> >
> > adam
> > >
> > > Regards
> > > Gaurav Jain
> > > >
> > > > >                                 };
> > > > >
> > > > >                                 sec_jr1: jr at 2000 { diff --git
> > > > > a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > > b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > > index 1d3844437d..b462d24eb2 100644
> > > > > --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > > +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > > @@ -1,6 +1,6 @@
> > > > >  // SPDX-License-Identifier: GPL-2.0+
> > > > >  /*
> > > > > - * Copyright 2019 NXP
> > > > > + * Copyright 2019, 2021 NXP
> > > > >   */
> > > > >
> > > > >  / {
> > > > > @@ -104,6 +104,22 @@
> > > > >         u-boot,dm-spl;
> > > > >  };
> > > > >
> > > > > +&crypto {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr0 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr1 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr2 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > >  &usdhc1 {
> > > > >         u-boot,dm-spl;
> > > > >  };
> > > > > diff --git a/arch/arm/dts/imx8mn.dtsi b/arch/arm/dts/imx8mn.dtsi
> > > > > index
> > > > > edcb415b53..1820a5af37 100644
> > > > > --- a/arch/arm/dts/imx8mn.dtsi
> > > > > +++ b/arch/arm/dts/imx8mn.dtsi
> > > > > @@ -822,6 +822,7 @@
> > > > >                                          compatible = "fsl,sec-v4.0-job-ring";
> > > > >                                          reg = <0x1000 0x1000>;
> > > > >                                          interrupts = <GIC_SPI
> > > > > 105 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                                        status = "disabled";
> > > >
> > > > ditto
> > > >
> > > > >                                 };
> > > > >
> > > > >                                 sec_jr1: jr at 2000 { diff --git
> > > > > a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > > b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > > index ab849ebaac..52f473dc52 100644
> > > > > --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > > +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > > @@ -1,6 +1,6 @@
> > > > >  // SPDX-License-Identifier: GPL-2.0+
> > > > >  /*
> > > > > - * Copyright 2019 NXP
> > > > > + * Copyright 2019, 2021 NXP
> > > > >   */
> > > > >
> > > > >  #include "imx8mp-u-boot.dtsi"
> > > > > @@ -67,6 +67,22 @@
> > > > >         u-boot,dm-spl;
> > > > >  };
> > > > >
> > > > > +&crypto {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr0 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr1 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > > +&sec_jr2 {
> > > > > +       u-boot,dm-spl;
> > > > > +};
> > > > > +
> > > > >  &i2c1 {
> > > > >         u-boot,dm-spl;
> > > > >  };
> > > > > diff --git a/arch/arm/dts/imx8mp.dtsi b/arch/arm/dts/imx8mp.dtsi
> > > > > index
> > > > > c2d51a46cb..57b01c3a57 100644
> > > > > --- a/arch/arm/dts/imx8mp.dtsi
> > > > > +++ b/arch/arm/dts/imx8mp.dtsi
> > > > > @@ -624,6 +624,7 @@
> > > > >                                         compatible = "fsl,sec-v4.0-job-ring";
> > > > >                                         reg = <0x1000 0x1000>;
> > > > >                                         interrupts = <GIC_SPI
> > > > > 105 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                                       status = "disabled";
> > > > ditto
> > > >
> > > > >                                 };
> > > > >
> > > > >                                 sec_jr1: jr at 2000 { diff --git
> > > > > a/arch/arm/dts/imx8mq.dtsi b/arch/arm/dts/imx8mq.dtsi index
> > > > > a44f729d0e..ecab44ca13 100644
> > > > > --- a/arch/arm/dts/imx8mq.dtsi
> > > > > +++ b/arch/arm/dts/imx8mq.dtsi
> > > > > @@ -955,6 +955,7 @@
> > > > >                                         compatible = "fsl,sec-v4.0-job-ring";
> > > > >                                         reg = <0x1000 0x1000>;
> > > > >                                         interrupts = <GIC_SPI
> > > > > 105 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                                       status = "disabled";
> > > > ditto
> > > > >                                 };
> > > > >
> > > > >                                 sec_jr1: jr at 2000 {
> > > > > --
> > > > > 2.17.1
> > > > >


More information about the U-Boot mailing list