Hello Gurav/Adam ,
> -----Original Message-----
> From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Adam Ford
> Sent: Tuesday, November 2, 2021 12:19 PM
> To: Gaurav Jain <gaurav.j...@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> <sba...@denx.de>; Fabio Estevam <feste...@gmail.com>; Peng Fan
> <peng....@nxp.com>; Simon Glass <s...@chromium.org>; Priyanka Jain
> <priyanka.j...@nxp.com>; Ye Li <ye...@nxp.com>; Horia Geanta
> <horia.gea...@nxp.com>; Ji Luo <ji....@nxp.com>; Franck Lenormand
> <franck.lenorm...@nxp.com>; Silvano Di Ninno <silvano.dini...@nxp.com>;
> Sahil Malhotra <sahil.malho...@nxp.com>; Pankaj Gupta
> <pankaj.gu...@nxp.com>; Varun Sethi <v.se...@nxp.com>; dl-uboot-imx
> <uboot-...@nxp.com>; Shengzhou Liu <shengzhou....@nxp.com>; Mingkai Hu
> <mingkai...@nxp.com>; Rajesh Bhagat <rajesh.bha...@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggar...@nxp.com>; Wasim Khan
> <wasim.k...@nxp.com>; Alison Wang <alison.w...@nxp.com>; Pramod Kumar
> <pramod.kuma...@nxp.com>; Andy Tang <andy.t...@nxp.com>; Adrian
> Alonso <adrian.alo...@nxp.com>; Vladimir Oltean <olte...@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.j...@nxp.com> wrote:
> >
> > Hello Adam
> >
> > > -----Original Message-----
> > > From: Adam Ford <aford...@gmail.com>
> > > Sent: Monday, November 1, 2021 6:30 PM
> > > To: Gaurav Jain <gaurav.j...@nxp.com>
> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > <sba...@denx.de>; Fabio Estevam <feste...@gmail.com>; Peng Fan
> > > <peng....@nxp.com>; Simon Glass <s...@chromium.org>; Priyanka Jain
> > > <priyanka.j...@nxp.com>; Ye Li <ye...@nxp.com>; Horia Geanta
> > > <horia.gea...@nxp.com>; Ji Luo <ji....@nxp.com>; Franck Lenormand
> > > <franck.lenorm...@nxp.com>; Silvano Di Ninno
> > > <silvano.dini...@nxp.com>; Sahil Malhotra <sahil.malho...@nxp.com>;
> > > Pankaj Gupta <pankaj.gu...@nxp.com>; Varun Sethi <v.se...@nxp.com>;
> > > dl-uboot-imx <uboot-...@nxp.com>; Shengzhou Liu
> > > <shengzhou....@nxp.com>; Mingkai Hu <mingkai...@nxp.com>; Rajesh
> > > Bhagat <rajesh.bha...@nxp.com>; Meenakshi Aggarwal
> > > <meenakshi.aggar...@nxp.com>; Wasim Khan <wasim.k...@nxp.com>;
> > > Alison Wang <alison.w...@nxp.com>; Pramod Kumar
> > > <pramod.kuma...@nxp.com>; Andy Tang <andy.t...@nxp.com>; Adrian
> > > Alonso <adrian.alo...@nxp.com>; Vladimir Oltean <olte...@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.j...@nxp.com> wrote:
> > > >
> > > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for
> > > > secure boot.
> > > >
> > > > Signed-off-by: Gaurav Jain <gaurav.j...@nxp.com>
> > > > Reviewed-by: Ye Li <ye...@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@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@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@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@2000 {
> > > > --
> > > > 2.17.1
> > > >
