diff mbox series

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

Message ID 20211026065554.29009-4-gaurav.jain@nxp.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series Add CAAM driver model support | expand

Commit Message

Gaurav Jain Oct. 26, 2021, 6:55 a.m. UTC
disabled use of JR0 in SPL and uboot, as JR0 is reserved
for secure boot.

Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
Reviewed-by: Ye Li <ye.li@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(-)

Comments

Adam Ford Nov. 1, 2021, 1 p.m. UTC | #1
On Tue, Oct 26, 2021 at 1:57 AM Gaurav Jain <gaurav.jain@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@nxp.com>
> Reviewed-by: Ye Li <ye.li@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.

>                                 };
>
>                                 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
>
Gaurav Jain Nov. 2, 2021, 8:17 a.m. UTC | #2
Hello Adam

> -----Original Message-----
> From: Adam Ford <aford173@gmail.com>
> Sent: Monday, November 1, 2021 6:30 PM
> To: Gaurav Jain <gaurav.jain@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> <franck.lenormand@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>;
> Sahil Malhotra <sahil.malhotra@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx
> <uboot-imx@nxp.com>; Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod
> Kumar <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>;
> Adrian Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@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@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@nxp.com>
> > Reviewed-by: Ye Li <ye.li@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.

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
> >
Adam Ford Nov. 2, 2021, 11:19 a.m. UTC | #3
On Tue, Nov 2, 2021 at 3:17 AM Gaurav Jain <gaurav.jain@nxp.com> wrote:
>
> Hello Adam
>
> > -----Original Message-----
> > From: Adam Ford <aford173@gmail.com>
> > Sent: Monday, November 1, 2021 6:30 PM
> > To: Gaurav Jain <gaurav.jain@nxp.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> > <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> > <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> > <franck.lenormand@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>;
> > Sahil Malhotra <sahil.malhotra@nxp.com>; Pankaj Gupta
> > <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx
> > <uboot-imx@nxp.com>; Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> > <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>; Meenakshi
> > Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> > <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod
> > Kumar <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>;
> > Adrian Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@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@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@nxp.com>
> > > Reviewed-by: Ye Li <ye.li@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.

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
> > >
ZHIZHIKIN Andrey Nov. 3, 2021, 11:34 a.m. UTC | #4
Hello Gurav/Adam ,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Adam Ford
> Sent: Tuesday, November 2, 2021 12:19 PM
> To: Gaurav Jain <gaurav.jain@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> <franck.lenormand@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>;
> Sahil Malhotra <sahil.malhotra@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx
> <uboot-imx@nxp.com>; Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod Kumar
> <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>; Adrian
> Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@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@nxp.com> wrote:
> >
> > Hello Adam
> >
> > > -----Original Message-----
> > > From: Adam Ford <aford173@gmail.com>
> > > Sent: Monday, November 1, 2021 6:30 PM
> > > To: Gaurav Jain <gaurav.jain@nxp.com>
> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> > > <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> > > <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> > > <franck.lenormand@nxp.com>; Silvano Di Ninno
> > > <silvano.dininno@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>;
> > > Pankaj Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > dl-uboot-imx <uboot-imx@nxp.com>; Shengzhou Liu
> > > <shengzhou.liu@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Rajesh
> > > Bhagat <rajesh.bhagat@nxp.com>; Meenakshi Aggarwal
> > > <meenakshi.aggarwal@nxp.com>; Wasim Khan <wasim.khan@nxp.com>;
> > > Alison Wang <alison.wang@nxp.com>; Pramod Kumar
> > > <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>; Adrian
> > > Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@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@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@nxp.com>
> > > > Reviewed-by: Ye Li <ye.li@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
> > > >
Gaurav Jain Nov. 8, 2021, 8:20 a.m. UTC | #5
Hello Andrey

> -----Original Message-----
> From: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Sent: Wednesday, November 3, 2021 5:05 PM
> To: Adam Ford <aford173@gmail.com>; Gaurav Jain <gaurav.jain@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> <franck.lenormand@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>;
> Sahil Malhotra <sahil.malhotra@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx
> <uboot-imx@nxp.com>; Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod
> Kumar <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>;
> Adrian Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@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@lists.denx.de> On Behalf Of Adam Ford
> > Sent: Tuesday, November 2, 2021 12:19 PM
> > To: Gaurav Jain <gaurav.jain@nxp.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> > <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> > <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> > <franck.lenormand@nxp.com>; Silvano Di Ninno
> > <silvano.dininno@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>;
> > Pankaj Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> > dl-uboot-imx <uboot-imx@nxp.com>; Shengzhou Liu
> > <shengzhou.liu@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Rajesh
> > Bhagat <rajesh.bhagat@nxp.com>; Meenakshi Aggarwal
> > <meenakshi.aggarwal@nxp.com>; Wasim Khan <wasim.khan@nxp.com>;
> Alison
> > Wang <alison.wang@nxp.com>; Pramod Kumar
> <pramod.kumar_1@nxp.com>;
> > Andy Tang <andy.tang@nxp.com>; Adrian Alonso <adrian.alonso@nxp.com>;
> > Vladimir Oltean <olteanv@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@nxp.com> wrote:
> > >
> > > Hello Adam
> > >
> > > > -----Original Message-----
> > > > From: Adam Ford <aford173@gmail.com>
> > > > Sent: Monday, November 1, 2021 6:30 PM
> > > > To: Gaurav Jain <gaurav.jain@nxp.com>
> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > > > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> > > > <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> > > > <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> > > > <franck.lenormand@nxp.com>; Silvano Di Ninno
> > > > <silvano.dininno@nxp.com>; Sahil Malhotra
> > > > <sahil.malhotra@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>;
> > > > Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx <uboot-imx@nxp.com>;
> > > > Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> > > > <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> > > > Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> > > > <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod
> > > > Kumar <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>;
> > > > Adrian Alonso <adrian.alonso@nxp.com>; Vladimir Oltean
> > > > <olteanv@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@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@nxp.com>
> > > > > Reviewed-by: Ye Li <ye.li@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@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
> > > > >
ZHIZHIKIN Andrey Nov. 8, 2021, 8:48 a.m. UTC | #6
Hello Gurav,

I'll Cc: Michael Walle here for the reason described below.

> -----Original Message-----
> From: Gaurav Jain <gaurav.jain@nxp.com>
> Sent: Monday, November 8, 2021 9:21 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Adam Ford
> <aford173@gmail.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> <franck.lenormand@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>;
> Sahil Malhotra <sahil.malhotra@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx
> <uboot-imx@nxp.com>; Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>; Meenakshi
> Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod Kumar
> <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>; Adrian
> Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@gmail.com>
> Subject: RE: [EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device tree for
> supporting DM in SPL
>  
> 
> Hello Andrey
> 
> > -----Original Message-----
> > From: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > Sent: Wednesday, November 3, 2021 5:05 PM
> > To: Adam Ford <aford173@gmail.com>; Gaurav Jain <gaurav.jain@nxp.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> > <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> > <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> > <franck.lenormand@nxp.com>; Silvano Di Ninno
> > <silvano.dininno@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>;
> > Pankaj Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> > dl-uboot-imx <uboot-imx@nxp.com>; Shengzhou Liu
> > <shengzhou.liu@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Rajesh
> > Bhagat <rajesh.bhagat@nxp.com>; Meenakshi Aggarwal
> > <meenakshi.aggarwal@nxp.com>; Wasim Khan <wasim.khan@nxp.com>;
> Alison
> > Wang <alison.wang@nxp.com>; Pramod Kumar <pramod.kumar_1@nxp.com>;
> > Andy Tang <andy.tang@nxp.com>; Adrian Alonso <adrian.alonso@nxp.com>;
> > Vladimir Oltean <olteanv@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@lists.denx.de> On Behalf Of Adam Ford
> > > Sent: Tuesday, November 2, 2021 12:19 PM
> > > To: Gaurav Jain <gaurav.jain@nxp.com>
> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka Jain
> > > <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia Geanta
> > > <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck Lenormand
> > > <franck.lenormand@nxp.com>; Silvano Di Ninno
> > > <silvano.dininno@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>;
> > > Pankaj Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > dl-uboot-imx <uboot-imx@nxp.com>; Shengzhou Liu
> > > <shengzhou.liu@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Rajesh
> > > Bhagat <rajesh.bhagat@nxp.com>; Meenakshi Aggarwal
> > > <meenakshi.aggarwal@nxp.com>; Wasim Khan <wasim.khan@nxp.com>;
> > Alison
> > > Wang <alison.wang@nxp.com>; Pramod Kumar
> > <pramod.kumar_1@nxp.com>;
> > > Andy Tang <andy.tang@nxp.com>; Adrian Alonso
> > > <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@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@nxp.com> wrote:
> > > >
> > > > Hello Adam
> > > >
> > > > > -----Original Message-----
> > > > > From: Adam Ford <aford173@gmail.com>
> > > > > Sent: Monday, November 1, 2021 6:30 PM
> > > > > To: Gaurav Jain <gaurav.jain@nxp.com>
> > > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic
> > > > > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > > > > <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Priyanka
> > > > > Jain <priyanka.jain@nxp.com>; Ye Li <ye.li@nxp.com>; Horia
> > > > > Geanta <horia.geanta@nxp.com>; Ji Luo <ji.luo@nxp.com>; Franck
> > > > > Lenormand <franck.lenormand@nxp.com>; Silvano Di Ninno
> > > > > <silvano.dininno@nxp.com>; Sahil Malhotra
> > > > > <sahil.malhotra@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>;
> > > > > Varun Sethi <V.Sethi@nxp.com>; dl-uboot-imx <uboot-imx@nxp.com>;
> > > > > Shengzhou Liu <shengzhou.liu@nxp.com>; Mingkai Hu
> > > > > <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> > > > > Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> > > > > <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod
> > > > > Kumar <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>;
> > > > > Adrian Alonso <adrian.alonso@nxp.com>; Vladimir Oltean
> > > > > <olteanv@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@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@nxp.com>
> > > > > > Reviewed-by: Ye Li <ye.li@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

I'm currently working on dynamic recognition of Job Rings which are reserved for TZ
at the early boot, there is a patch proposed to address this [1]. I was planning to re-work
this to also include a small clean-up of the CAAM driver in a way how JRs are probed.
The patch does not require nodes to be permanently disabled in DTB, rather they would
need to be omitted from the probing instead.

I was wondering perhaps similar changes can be introduced in the U-Boot, since it would
allow to account for cases where JRs reserved for specific purposes (e.g. HAB) can be
dynamically changed and would not require modifications of DTB to align. 

In generic case, any JR can be "taken away" into TZ. Those modifications should be
sync'ed with DTB changes, while having this dynamically recognized would have a
benefit of not touching the DTB at all.

> 
> Regards
> Gaurav Jain
> 

Link: [1]: https://lore.kernel.org/lkml/20211104162114.2019509-1-andrey.zhizhikin@leica-geosystems.com/

-- andrey
Michael Walle Nov. 8, 2021, 8:58 a.m. UTC | #7
Am 2021-11-08 09:48, schrieb ZHIZHIKIN Andrey:
..

>> > 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.

No. Please don't disable the node in the kernel device tree. IMHO TF-A
(or whoever is using the job ring) is optional. So either u-boot
should disable it, either statically by -u-boot.dtsi (but please _per_
board) or at runtime by fixing up the device tree. Another way would
be for linux to skip probing if its not available. Though I don't
know whether this is "the ususal way".

But as Andrey mentioned, any job ring could be taken away. So the
-u-boot.dtsi method doesn't seem to be that good.

-michael
Gaurav Jain Nov. 15, 2021, 7:09 a.m. UTC | #8
Hello Michael

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: Monday, November 8, 2021 2:29 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Cc: Gaurav Jain <gaurav.jain@nxp.com>; Adam Ford <aford173@gmail.com>;
> U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>;
> Fabio Estevam <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; Simon
> Glass <sjg@chromium.org>; Priyanka Jain <priyanka.jain@nxp.com>; Ye Li
> <ye.li@nxp.com>; Horia Geanta <horia.geanta@nxp.com>; Ji Luo
> <ji.luo@nxp.com>; Franck Lenormand <franck.lenormand@nxp.com>; Silvano Di
> Ninno <silvano.dininno@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>;
> Pankaj Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; dl-
> uboot-imx <uboot-imx@nxp.com>; Shengzhou Liu <shengzhou.liu@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Wasim Khan
> <wasim.khan@nxp.com>; Alison Wang <alison.wang@nxp.com>; Pramod
> Kumar <pramod.kumar_1@nxp.com>; Andy Tang <andy.tang@nxp.com>;
> Adrian Alonso <adrian.alonso@nxp.com>; Vladimir Oltean <olteanv@gmail.com>
> Subject: Re: [EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device tree for
> supporting DM in SPL
> 
> Caution: EXT Email
> 
> Am 2021-11-08 09:48, schrieb ZHIZHIKIN Andrey:
> ..
> 
> >> > 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.
> 
> No. Please don't disable the node in the kernel device tree. IMHO TF-A (or
> whoever is using the job ring) is optional. So either u-boot should disable it,
> either statically by -u-boot.dtsi (but please _per_
> board) or at runtime by fixing up the device tree. Another way would be for linux
> to skip probing if its not available. Though I don't know whether this is "the
> ususal way".
> 
> But as Andrey mentioned, any job ring could be taken away. So the -u-boot.dtsi
> method doesn't seem to be that good.
> 

Moved the JR0 disabled code to *-u-boot.dtsi files in version 5 of this patch series.

Regards
Gaurav Jain
> -michael
diff mbox series

Patch

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";
 				};
 
 				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";
 				};
 
 				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";
 				};
 
 				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";
 				};
 
 				sec_jr1: jr@2000 {