[EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device tree for supporting DM in SPL
ZHIZHIKIN Andrey
andrey.zhizhikin at leica-geosystems.com
Wed Nov 3 12:34:47 CET 2021
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
>
> 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