Hello Gurav/Adam ,
> -----Original Message-----
> From: U-Boot <[email protected]> On Behalf Of Adam Ford
> Sent: Tuesday, November 2, 2021 12:19 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]>; Simon Glass <[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: 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 <[email protected]> wrote:
> >
> > Hello Adam
> >
> > > -----Original Message-----
> > > From: Adam Ford <[email protected]>
> > > Sent: Monday, November 1, 2021 6:30 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]>; Simon Glass <[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 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 <[email protected]> wrote:
> > > >
> > > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for
> > > > secure boot.
> > > >
> > > > Signed-off-by: Gaurav Jain <[email protected]>
> > > > Reviewed-by: Ye Li <[email protected]>
> > > > ---
> > > > 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
> > > >
