[PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Gaurav Jain]

disabled use of JR0 in SPL and uboot, as JR0 is reserved
for HAB in TF-A.

Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
---
 arch/arm/dts/imx8mm-evk-u-boot.dtsi        |  1 +
 arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi   |  1 +
 arch/arm/dts/imx8mp-evk-u-boot.dtsi        |  1 +
 arch/arm/dts/imx8mq-evk-u-boot.dtsi        |  4 ++++
 arch/arm/include/asm/arch-imx8m/imx-regs.h |  1 +
 drivers/crypto/fsl/jr.c                    | 14 +++++++++++---
 scripts/config_whitelist.txt               |  1 +
 7 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
index e9fbf7b802..8cd37b5205 100644
--- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
@@ -74,6 +74,7 @@
 
 &sec_jr0 {
 	u-boot,dm-spl;
+	status = "disabled";
 };
 
 &sec_jr1 {
diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
index 4d0ecb07d4..0c31f2737a 100644
--- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
@@ -114,6 +114,7 @@
 
 &sec_jr0 {
 	u-boot,dm-spl;
+	status = "disabled";
 };
 
 &sec_jr1 {
diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
index f43eb6238d..28dce55fb9 100644
--- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
@@ -77,6 +77,7 @@
 
 &sec_jr0 {
 	u-boot,dm-spl;
+	status = "disabled";
 };
 
 &sec_jr1 {
diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
index 67da69a2eb..37364eb6b4 100644
--- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
@@ -18,3 +18,7 @@
 &uart1 {
 	u-boot,dm-spl;
 };
+
+&sec_jr0 {
+	status = "disabled";
+};
diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h b/arch/arm/include/asm/arch-imx8m/imx-regs.h
index 1da75528d4..e6e2974df3 100644
--- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
+++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
@@ -89,6 +89,7 @@
 #define CONFIG_SYS_FSL_SEC_ADDR         (CAAM_IPS_BASE_ADDR + \
 					 CONFIG_SYS_FSL_SEC_OFFSET)
 #define CONFIG_SYS_FSL_JR0_OFFSET       (0x1000)
+#define CONFIG_SYS_FSL_JR1_OFFSET	(0x2000)
 #define CONFIG_SYS_FSL_JR0_ADDR         (CONFIG_SYS_FSL_SEC_ADDR + \
 					 CONFIG_SYS_FSL_JR0_OFFSET)
 #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC   1
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index acd29924f7..66dd9cf365 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -44,9 +44,17 @@ struct udevice *caam_dev;
 #define SEC_ADDR(idx)	\
 	(ulong)((CONFIG_SYS_FSL_SEC_ADDR + sec_offset[idx]))
 
-#define SEC_JR0_ADDR(idx)	\
+#ifndef CONFIG_IMX8M
+#define SEC_JR_ADDR(idx)	\
 	(ulong)(SEC_ADDR(idx) +	\
 	 (CONFIG_SYS_FSL_JR0_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
+#define JR_ID 0
+#else
+#define SEC_JR_ADDR(idx)	\
+	(ulong)(SEC_ADDR(idx) + \
+	(CONFIG_SYS_FSL_JR1_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
+#define JR_ID 1
+#endif
 struct caam_regs caam_st;
 #endif
 
@@ -685,8 +693,8 @@ int sec_init_idx(uint8_t sec_idx)
 	caam = dev_get_priv(caam_dev);
 #else
 	caam_st.sec = (void *)SEC_ADDR(sec_idx);
-	caam_st.regs = (struct jr_regs *)SEC_JR0_ADDR(sec_idx);
-	caam_st.jrid = 0;
+	caam_st.regs = (struct jr_regs *)SEC_JR_ADDR(sec_idx);
+	caam_st.jrid = JR_ID;
 	caam = &caam_st;
 #endif
 #if CONFIG_IS_ENABLED(OF_CONTROL)
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index cecdda6781..b99aeacbc4 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -1040,6 +1040,7 @@ CONFIG_SYS_FSL_IFC_LE
 CONFIG_SYS_FSL_ISBC_VER
 CONFIG_SYS_FSL_JR0_ADDR
 CONFIG_SYS_FSL_JR0_OFFSET
+CONFIG_SYS_FSL_JR1_OFFSET
 CONFIG_SYS_FSL_LS1_CLK_ADDR
 CONFIG_SYS_FSL_LSCH3_SERDES_ADDR
 CONFIG_SYS_FSL_MAX_NUM_OF_SEC
-- 
2.25.1

Re: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Fabio Estevam]

Hi Gaurav,

On Wed, Jun 8, 2022 at 10:34 AM Gaurav Jain <gaurav.jain@nxp.com> wrote:
>
> disabled use of JR0 in SPL and uboot, as JR0 is reserved
> for HAB in TF-A.
>
> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>

Thanks for your patch.

With your patch applied, I can use the NXP TF-A lf_v2.4 successfully:
no more CAAM sha256 errors are seen when launching a fitImage.

Tested on a imx8mm-evk.

Tested-by: Fabio Estevam <festevam@denx.de>

Re: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Fabio Estevam]

Hi Gaurav,

On Wed, Jun 8, 2022 at 10:56 AM Fabio Estevam <festevam@gmail.com> wrote:

> Thanks for your patch.
>
> With your patch applied, I can use the NXP TF-A lf_v2.4 successfully:
> no more CAAM sha256 errors are seen when launching a fitImage.
>
> Tested on a imx8mm-evk.
>
> Tested-by: Fabio Estevam <festevam@denx.de>

I noticed a different issue when using the NXP TF-A lf_v2.4: There is
a job ring probe error in the kernel (upstream 5.15.43):

# dmesg | grep jr
[    2.449204] caam_jr 30901000.jr: failed to flush job ring 0
[    2.454867] caam_jr: probe of 30901000.jr failed with error -5

This problem does not happen if I use NXP TF-A imx_5.4.47_2.2.0 version.

If I disable sec_jr0 in the Linux devicetree, then the error does not happen.

Should U-Boot disable sec_jr0 of the Linux devicetree too?

What is a proper solution for this issue?

Thanks,

Fabio Estevam

RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[ZHIZHIKIN Andrey]

Hello Gaurav,

Cc: Michael Walle here.

I guess this is a re-incarnation of the previous discussions we had
regarding the JR reservation, see [1].

> -----Original Message-----
> From: Gaurav Jain <gaurav.jain@nxp.com>
> Sent: Wednesday, June 8, 2022 3:34 PM
> To: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio Estevam
> <festevam@gmail.com>; Tommaso Merciai <tommaso.merciai@amarulasolutions.com>;
> ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Michael Trimarchi
> <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>; Simon Glass
> <sjg@chromium.org>; Patrick Delaunay <patrick.delaunay@foss.st.com>; Stefan Roese
> <sr@denx.de>; Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>
> Cc: NXP i . MX U-Boot Team <uboot-imx@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>
> Subject: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> 
> disabled use of JR0 in SPL and uboot, as JR0 is reserved
> for HAB in TF-A.
> 
> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> ---
>  arch/arm/dts/imx8mm-evk-u-boot.dtsi        |  1 +
>  arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi   |  1 +
>  arch/arm/dts/imx8mp-evk-u-boot.dtsi        |  1 +
>  arch/arm/dts/imx8mq-evk-u-boot.dtsi        |  4 ++++

Shall those DTB changes be sync'd with Kernel?

Now that the JR0 reservation is done in both upstream and
downstream TF-A - Kernel would fail to initialize the JR0.

This is what Fabio just noted and posted as a comment. :-)

I suggest that this is submitted into Kernel, and then picked
up during the next DTB re-sync.

>  arch/arm/include/asm/arch-imx8m/imx-regs.h |  1 +
>  drivers/crypto/fsl/jr.c                    | 14 +++++++++++---
>  scripts/config_whitelist.txt               |  1 +
>  7 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi b/arch/arm/dts/imx8mm-evk-u-
> boot.dtsi
> index e9fbf7b802..8cd37b5205 100644
> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> @@ -74,6 +74,7 @@
> 
>  &sec_jr0 {
>         u-boot,dm-spl;
> +       status = "disabled";
>  };
> 
>  &sec_jr1 {
> diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-ddr4-
> evk-u-boot.dtsi
> index 4d0ecb07d4..0c31f2737a 100644
> --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> @@ -114,6 +114,7 @@
> 
>  &sec_jr0 {
>         u-boot,dm-spl;
> +       status = "disabled";
>  };
> 
>  &sec_jr1 {
> diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi b/arch/arm/dts/imx8mp-evk-u-
> boot.dtsi
> index f43eb6238d..28dce55fb9 100644
> --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> @@ -77,6 +77,7 @@
> 
>  &sec_jr0 {
>         u-boot,dm-spl;
> +       status = "disabled";
>  };
> 
>  &sec_jr1 {
> diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi b/arch/arm/dts/imx8mq-evk-u-
> boot.dtsi
> index 67da69a2eb..37364eb6b4 100644
> --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> @@ -18,3 +18,7 @@
>  &uart1 {
>         u-boot,dm-spl;
>  };
> +
> +&sec_jr0 {
> +       status = "disabled";
> +};
> diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> index 1da75528d4..e6e2974df3 100644
> --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> @@ -89,6 +89,7 @@
>  #define CONFIG_SYS_FSL_SEC_ADDR         (CAAM_IPS_BASE_ADDR + \
>                                          CONFIG_SYS_FSL_SEC_OFFSET)
>  #define CONFIG_SYS_FSL_JR0_OFFSET       (0x1000)
> +#define CONFIG_SYS_FSL_JR1_OFFSET      (0x2000)
>  #define CONFIG_SYS_FSL_JR0_ADDR         (CONFIG_SYS_FSL_SEC_ADDR + \
>                                          CONFIG_SYS_FSL_JR0_OFFSET)
>  #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC   1
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index acd29924f7..66dd9cf365 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -44,9 +44,17 @@ struct udevice *caam_dev;
>  #define SEC_ADDR(idx)  \
>         (ulong)((CONFIG_SYS_FSL_SEC_ADDR + sec_offset[idx]))
> 
> -#define SEC_JR0_ADDR(idx)      \
> +#ifndef CONFIG_IMX8M
> +#define SEC_JR_ADDR(idx)       \
>         (ulong)(SEC_ADDR(idx) + \
>          (CONFIG_SYS_FSL_JR0_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> +#define JR_ID 0
> +#else
> +#define SEC_JR_ADDR(idx)       \
> +       (ulong)(SEC_ADDR(idx) + \
> +       (CONFIG_SYS_FSL_JR1_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> +#define JR_ID 1
> +#endif

I believe this whole macro can be simplified, isn't it?

>  struct caam_regs caam_st;
>  #endif
> 
> @@ -685,8 +693,8 @@ int sec_init_idx(uint8_t sec_idx)
>         caam = dev_get_priv(caam_dev);
>  #else
>         caam_st.sec = (void *)SEC_ADDR(sec_idx);
> -       caam_st.regs = (struct jr_regs *)SEC_JR0_ADDR(sec_idx);
> -       caam_st.jrid = 0;
> +       caam_st.regs = (struct jr_regs *)SEC_JR_ADDR(sec_idx);
> +       caam_st.jrid = JR_ID;
>         caam = &caam_st;
>  #endif
>  #if CONFIG_IS_ENABLED(OF_CONTROL)
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index cecdda6781..b99aeacbc4 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -1040,6 +1040,7 @@ CONFIG_SYS_FSL_IFC_LE
>  CONFIG_SYS_FSL_ISBC_VER
>  CONFIG_SYS_FSL_JR0_ADDR
>  CONFIG_SYS_FSL_JR0_OFFSET
> +CONFIG_SYS_FSL_JR1_OFFSET
>  CONFIG_SYS_FSL_LS1_CLK_ADDR
>  CONFIG_SYS_FSL_LSCH3_SERDES_ADDR
>  CONFIG_SYS_FSL_MAX_NUM_OF_SEC
> --
> 2.25.1


Link: [1]: https://lore.kernel.org/u-boot/VI1PR04MB53426E0295D8E2508BA01302E7089@VI1PR04MB5342.eurprd04.prod.outlook.com/
-- andrey

Re: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Fabio Estevam]

Hi Andrey,

On Wed, Jun 8, 2022 at 12:00 PM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:

> Shall those DTB changes be sync'd with Kernel?
>
> Now that the JR0 reservation is done in both upstream and
> downstream TF-A - Kernel would fail to initialize the JR0.
>
> This is what Fabio just noted and posted as a comment. :-)
>
> I suggest that this is submitted into Kernel, and then picked
> up during the next DTB re-sync.

Yes, agreed.

Just submitted a patch doing this:

http://lists.infradead.org/pipermail/linux-arm-kernel/2022-June/749625.html

Thanks

RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Gaurav Jain]

Hello Andrey

> -----Original Message-----
> From: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Sent: Wednesday, June 8, 2022 8:31 PM
> To: Gaurav Jain <gaurav.jain@nxp.com>; u-boot@lists.denx.de; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Tommaso Merciai
> <tommaso.merciai@amarulasolutions.com>; Michael Trimarchi
> <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>; Simon
> Glass <sjg@chromium.org>; Patrick Delaunay <patrick.delaunay@foss.st.com>;
> Stefan Roese <sr@denx.de>; Horia Geanta <horia.geanta@nxp.com>; Pankaj
> Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Ye Li
> <ye.li@nxp.com>; Michael Walle <michael@walle.cc>
> Cc: dl-uboot-imx <uboot-imx@nxp.com>
> Subject: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> 
> Caution: EXT Email
> 
> Hello Gaurav,
> 
> Cc: Michael Walle here.
> 
> I guess this is a re-incarnation of the previous discussions we had regarding the
> JR reservation, see [1].
> 
> > -----Original Message-----
> > From: Gaurav Jain <gaurav.jain@nxp.com>
> > Sent: Wednesday, June 8, 2022 3:34 PM
> > To: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio
> > Estevam <festevam@gmail.com>; Tommaso Merciai
> > <tommaso.merciai@amarulasolutions.com>;
> > ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Michael
> > Trimarchi <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>;
> > Simon Glass <sjg@chromium.org>; Patrick Delaunay
> > <patrick.delaunay@foss.st.com>; Stefan Roese <sr@denx.de>; Horia
> > Geanta <horia.geanta@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>;
> > Varun Sethi <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>
> > Cc: NXP i . MX U-Boot Team <uboot-imx@nxp.com>; Gaurav Jain
> > <gaurav.jain@nxp.com>
> > Subject: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> >
> > disabled use of JR0 in SPL and uboot, as JR0 is reserved for HAB in
> > TF-A.
> >
> > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> > ---
> >  arch/arm/dts/imx8mm-evk-u-boot.dtsi        |  1 +
> >  arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi   |  1 +
> >  arch/arm/dts/imx8mp-evk-u-boot.dtsi        |  1 +
> >  arch/arm/dts/imx8mq-evk-u-boot.dtsi        |  4 ++++
> 
> Shall those DTB changes be sync'd with Kernel?
> 
> Now that the JR0 reservation is done in both upstream and downstream TF-A -
> Kernel would fail to initialize the JR0.
> 
> This is what Fabio just noted and posted as a comment. :-)
> 
> I suggest that this is submitted into Kernel, and then picked up during the next
> DTB re-sync.

Ok. Fabio has already submitted a patch for this.
> 
> >  arch/arm/include/asm/arch-imx8m/imx-regs.h |  1 +
> >  drivers/crypto/fsl/jr.c                    | 14 +++++++++++---
> >  scripts/config_whitelist.txt               |  1 +
> >  7 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > b/arch/arm/dts/imx8mm-evk-u- boot.dtsi index e9fbf7b802..8cd37b5205
> > 100644
> > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > @@ -74,6 +74,7 @@
> >
> >  &sec_jr0 {
> >         u-boot,dm-spl;
> > +       status = "disabled";
> >  };
> >
> >  &sec_jr1 {
> > diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > b/arch/arm/dts/imx8mn-ddr4- evk-u-boot.dtsi index
> > 4d0ecb07d4..0c31f2737a 100644
> > --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > @@ -114,6 +114,7 @@
> >
> >  &sec_jr0 {
> >         u-boot,dm-spl;
> > +       status = "disabled";
> >  };
> >
> >  &sec_jr1 {
> > diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > b/arch/arm/dts/imx8mp-evk-u- boot.dtsi index f43eb6238d..28dce55fb9
> > 100644
> > --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > @@ -77,6 +77,7 @@
> >
> >  &sec_jr0 {
> >         u-boot,dm-spl;
> > +       status = "disabled";
> >  };
> >
> >  &sec_jr1 {
> > diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > b/arch/arm/dts/imx8mq-evk-u- boot.dtsi index 67da69a2eb..37364eb6b4
> > 100644
> > --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > @@ -18,3 +18,7 @@
> >  &uart1 {
> >         u-boot,dm-spl;
> >  };
> > +
> > +&sec_jr0 {
> > +       status = "disabled";
> > +};
> > diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > index 1da75528d4..e6e2974df3 100644
> > --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > @@ -89,6 +89,7 @@
> >  #define CONFIG_SYS_FSL_SEC_ADDR         (CAAM_IPS_BASE_ADDR + \
> >                                          CONFIG_SYS_FSL_SEC_OFFSET)
> >  #define CONFIG_SYS_FSL_JR0_OFFSET       (0x1000)
> > +#define CONFIG_SYS_FSL_JR1_OFFSET      (0x2000)
> >  #define CONFIG_SYS_FSL_JR0_ADDR         (CONFIG_SYS_FSL_SEC_ADDR + \
> >                                          CONFIG_SYS_FSL_JR0_OFFSET)
> >  #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC   1
> > diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index
> > acd29924f7..66dd9cf365 100644
> > --- a/drivers/crypto/fsl/jr.c
> > +++ b/drivers/crypto/fsl/jr.c
> > @@ -44,9 +44,17 @@ struct udevice *caam_dev;  #define SEC_ADDR(idx)  \
> >         (ulong)((CONFIG_SYS_FSL_SEC_ADDR + sec_offset[idx]))
> >
> > -#define SEC_JR0_ADDR(idx)      \
> > +#ifndef CONFIG_IMX8M
> > +#define SEC_JR_ADDR(idx)       \
> >         (ulong)(SEC_ADDR(idx) + \
> >          (CONFIG_SYS_FSL_JR0_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> > +#define JR_ID 0
> > +#else
> > +#define SEC_JR_ADDR(idx)       \
> > +       (ulong)(SEC_ADDR(idx) + \
> > +       (CONFIG_SYS_FSL_JR1_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> > +#define JR_ID 1 #endif
> 
> I believe this whole macro can be simplified, isn't it?

I reused the old macro to define for JR1. 
How you want to simplify?

Regards
Gaurav Jain
> 
> >  struct caam_regs caam_st;
> >  #endif
> >
> > @@ -685,8 +693,8 @@ int sec_init_idx(uint8_t sec_idx)
> >         caam = dev_get_priv(caam_dev);  #else
> >         caam_st.sec = (void *)SEC_ADDR(sec_idx);
> > -       caam_st.regs = (struct jr_regs *)SEC_JR0_ADDR(sec_idx);
> > -       caam_st.jrid = 0;
> > +       caam_st.regs = (struct jr_regs *)SEC_JR_ADDR(sec_idx);
> > +       caam_st.jrid = JR_ID;
> >         caam = &caam_st;
> >  #endif
> >  #if CONFIG_IS_ENABLED(OF_CONTROL)
> > diff --git a/scripts/config_whitelist.txt
> > b/scripts/config_whitelist.txt index cecdda6781..b99aeacbc4 100644
> > --- a/scripts/config_whitelist.txt
> > +++ b/scripts/config_whitelist.txt
> > @@ -1040,6 +1040,7 @@ CONFIG_SYS_FSL_IFC_LE
> CONFIG_SYS_FSL_ISBC_VER
> > CONFIG_SYS_FSL_JR0_ADDR  CONFIG_SYS_FSL_JR0_OFFSET
> > +CONFIG_SYS_FSL_JR1_OFFSET
> >  CONFIG_SYS_FSL_LS1_CLK_ADDR
> >  CONFIG_SYS_FSL_LSCH3_SERDES_ADDR
> >  CONFIG_SYS_FSL_MAX_NUM_OF_SEC
> > --
> > 2.25.1
> 
> 
> Link: [1]:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> el.org%2Fu-
> boot%2FVI1PR04MB53426E0295D8E2508BA01302E7089%40VI1PR04MB5342.e
> urprd04.prod.outlook.com%2F&amp;data=05%7C01%7Cgaurav.jain%40nxp.co
> m%7Cc387babc3fed40edb32f08da495fa3f4%7C686ea1d3bc2b4c6fa92cd99c5c3
> 01635%7C0%7C0%7C637902972380396116%7CUnknown%7CTWFpbGZsb3d8ey
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> 3000%7C%7C%7C&amp;sdata=WETAijFtyFGHnrimKkeT4jICk0OQSXfMbRb8vWsK
> Mp0%3D&amp;reserved=0
> -- andrey

RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[ZHIZHIKIN Andrey]

Hello Gaurav,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Gaurav Jain
> Sent: Thursday, June 9, 2022 3:13 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; u-
> boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio Estevam
> <festevam@gmail.com>; Tommaso Merciai <tommaso.merciai@amarulasolutions.com>;
> Michael Trimarchi <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>;
> Simon Glass <sjg@chromium.org>; Patrick Delaunay <patrick.delaunay@foss.st.com>;
> Stefan Roese <sr@denx.de>; Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>;
> Michael Walle <michael@walle.cc>
> Cc: dl-uboot-imx <uboot-imx@nxp.com>
> Subject: RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> 
> Hello Andrey
> 
> > -----Original Message-----
> > From: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > Sent: Wednesday, June 8, 2022 8:31 PM
> > To: Gaurav Jain <gaurav.jain@nxp.com>; u-boot@lists.denx.de; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; Tommaso Merciai
> > <tommaso.merciai@amarulasolutions.com>; Michael Trimarchi
> > <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>; Simon
> > Glass <sjg@chromium.org>; Patrick Delaunay <patrick.delaunay@foss.st.com>;
> > Stefan Roese <sr@denx.de>; Horia Geanta <horia.geanta@nxp.com>; Pankaj
> > Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Ye Li
> > <ye.li@nxp.com>; Michael Walle <michael@walle.cc>
> > Cc: dl-uboot-imx <uboot-imx@nxp.com>
> > Subject: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> >
> > Caution: EXT Email
> >
> > Hello Gaurav,
> >
> > Cc: Michael Walle here.
> >
> > I guess this is a re-incarnation of the previous discussions we had regarding
> the
> > JR reservation, see [1].
> >
> > > -----Original Message-----
> > > From: Gaurav Jain <gaurav.jain@nxp.com>
> > > Sent: Wednesday, June 8, 2022 3:34 PM
> > > To: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio
> > > Estevam <festevam@gmail.com>; Tommaso Merciai
> > > <tommaso.merciai@amarulasolutions.com>;
> > > ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Michael
> > > Trimarchi <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>;
> > > Simon Glass <sjg@chromium.org>; Patrick Delaunay
> > > <patrick.delaunay@foss.st.com>; Stefan Roese <sr@denx.de>; Horia
> > > Geanta <horia.geanta@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>;
> > > Varun Sethi <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>
> > > Cc: NXP i . MX U-Boot Team <uboot-imx@nxp.com>; Gaurav Jain
> > > <gaurav.jain@nxp.com>
> > > Subject: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> > >
> > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for HAB in
> > > TF-A.
> > >
> > > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> > > ---
> > >  arch/arm/dts/imx8mm-evk-u-boot.dtsi        |  1 +
> > >  arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi   |  1 +
> > >  arch/arm/dts/imx8mp-evk-u-boot.dtsi        |  1 +
> > >  arch/arm/dts/imx8mq-evk-u-boot.dtsi        |  4 ++++
> >
> > Shall those DTB changes be sync'd with Kernel?
> >
> > Now that the JR0 reservation is done in both upstream and downstream TF-A -
> > Kernel would fail to initialize the JR0.
> >
> > This is what Fabio just noted and posted as a comment. :-)
> >
> > I suggest that this is submitted into Kernel, and then picked up during the
> next
> > DTB re-sync.
> 
> Ok. Fabio has already submitted a patch for this.

Great! Then I think DTB part will not be required in this patch.

> >
> > >  arch/arm/include/asm/arch-imx8m/imx-regs.h |  1 +
> > >  drivers/crypto/fsl/jr.c                    | 14 +++++++++++---
> > >  scripts/config_whitelist.txt               |  1 +
> > >  7 files changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mm-evk-u- boot.dtsi index e9fbf7b802..8cd37b5205
> > > 100644
> > > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > @@ -74,6 +74,7 @@
> > >
> > >  &sec_jr0 {
> > >         u-boot,dm-spl;
> > > +       status = "disabled";
> > >  };
> > >
> > >  &sec_jr1 {
> > > diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mn-ddr4- evk-u-boot.dtsi index
> > > 4d0ecb07d4..0c31f2737a 100644
> > > --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > @@ -114,6 +114,7 @@
> > >
> > >  &sec_jr0 {
> > >         u-boot,dm-spl;
> > > +       status = "disabled";
> > >  };
> > >
> > >  &sec_jr1 {
> > > diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mp-evk-u- boot.dtsi index f43eb6238d..28dce55fb9
> > > 100644
> > > --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > @@ -77,6 +77,7 @@
> > >
> > >  &sec_jr0 {
> > >         u-boot,dm-spl;
> > > +       status = "disabled";
> > >  };
> > >
> > >  &sec_jr1 {
> > > diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mq-evk-u- boot.dtsi index 67da69a2eb..37364eb6b4
> > > 100644
> > > --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > > @@ -18,3 +18,7 @@
> > >  &uart1 {
> > >         u-boot,dm-spl;
> > >  };
> > > +
> > > +&sec_jr0 {
> > > +       status = "disabled";
> > > +};
> > > diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > index 1da75528d4..e6e2974df3 100644
> > > --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > @@ -89,6 +89,7 @@
> > >  #define CONFIG_SYS_FSL_SEC_ADDR         (CAAM_IPS_BASE_ADDR + \
> > >                                          CONFIG_SYS_FSL_SEC_OFFSET)
> > >  #define CONFIG_SYS_FSL_JR0_OFFSET       (0x1000)
> > > +#define CONFIG_SYS_FSL_JR1_OFFSET      (0x2000)

This shall be converted to config option, as each SoC family defines this
now differently.

Ideally, only JR ID shall be provided by config option, ADDR and OFFSET
can then be deducted based on the index.

> > >  #define CONFIG_SYS_FSL_JR0_ADDR         (CONFIG_SYS_FSL_SEC_ADDR + \
> > >                                          CONFIG_SYS_FSL_JR0_OFFSET)
> > >  #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC   1
> > > diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index
> > > acd29924f7..66dd9cf365 100644
> > > --- a/drivers/crypto/fsl/jr.c
> > > +++ b/drivers/crypto/fsl/jr.c
> > > @@ -44,9 +44,17 @@ struct udevice *caam_dev;  #define SEC_ADDR(idx)  \
> > >         (ulong)((CONFIG_SYS_FSL_SEC_ADDR + sec_offset[idx]))
> > >
> > > -#define SEC_JR0_ADDR(idx)      \
> > > +#ifndef CONFIG_IMX8M
> > > +#define SEC_JR_ADDR(idx)       \
> > >         (ulong)(SEC_ADDR(idx) + \
> > >          (CONFIG_SYS_FSL_JR0_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> > > +#define JR_ID 0
> > > +#else
> > > +#define SEC_JR_ADDR(idx)       \
> > > +       (ulong)(SEC_ADDR(idx) + \
> > > +       (CONFIG_SYS_FSL_JR1_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> > > +#define JR_ID 1 #endif
> >

Once config option is introduced, it can be used here instead of macro.
Reserved JR ID can be used to deduct both ADDR and OFFSET parameters.
Then this re-use would not be necessary.

> > I believe this whole macro can be simplified, isn't it?
> 
> I reused the old macro to define for JR1.
> How you want to simplify?

See suggestions above.

> 
> Regards
> Gaurav Jain
> >
> > >  struct caam_regs caam_st;
> > >  #endif
> > >
> > > @@ -685,8 +693,8 @@ int sec_init_idx(uint8_t sec_idx)
> > >         caam = dev_get_priv(caam_dev);  #else
> > >         caam_st.sec = (void *)SEC_ADDR(sec_idx);
> > > -       caam_st.regs = (struct jr_regs *)SEC_JR0_ADDR(sec_idx);
> > > -       caam_st.jrid = 0;
> > > +       caam_st.regs = (struct jr_regs *)SEC_JR_ADDR(sec_idx);
> > > +       caam_st.jrid = JR_ID;
> > >         caam = &caam_st;
> > >  #endif
> > >  #if CONFIG_IS_ENABLED(OF_CONTROL)
> > > diff --git a/scripts/config_whitelist.txt
> > > b/scripts/config_whitelist.txt index cecdda6781..b99aeacbc4 100644
> > > --- a/scripts/config_whitelist.txt
> > > +++ b/scripts/config_whitelist.txt
> > > @@ -1040,6 +1040,7 @@ CONFIG_SYS_FSL_IFC_LE
> > CONFIG_SYS_FSL_ISBC_VER
> > > CONFIG_SYS_FSL_JR0_ADDR  CONFIG_SYS_FSL_JR0_OFFSET
> > > +CONFIG_SYS_FSL_JR1_OFFSET

This will not be necessary, and shall rather be replaced by JR ID config option.

> > >  CONFIG_SYS_FSL_LS1_CLK_ADDR
> > >  CONFIG_SYS_FSL_LSCH3_SERDES_ADDR
> > >  CONFIG_SYS_FSL_MAX_NUM_OF_SEC
> > > --
> > > 2.25.1
> >
> >
> > Link: [1]:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> > el.org%2Fu-
> > boot%2FVI1PR04MB53426E0295D8E2508BA01302E7089%40VI1PR04MB5342.e
> > urprd04.prod.outlook.com%2F&amp;data=05%7C01%7Cgaurav.jain%40nxp.co
> > m%7Cc387babc3fed40edb32f08da495fa3f4%7C686ea1d3bc2b4c6fa92cd99c5c3
> > 01635%7C0%7C0%7C637902972380396116%7CUnknown%7CTWFpbGZsb3d8ey
> > JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> > 3000%7C%7C%7C&amp;sdata=WETAijFtyFGHnrimKkeT4jICk0OQSXfMbRb8vWsK
> > Mp0%3D&amp;reserved=0
> > -- andrey

-- andrey

Re: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Fabio Estevam]

Hi Gaurav,

On Thu, Jun 9, 2022 at 10:12 AM Gaurav Jain <gaurav.jain@nxp.com> wrote:

> > I suggest that this is submitted into Kernel, and then picked up during the next
> > DTB re-sync.
>
> Ok. Fabio has already submitted a patch for this.

Yes, if you have a chance please send your Tested-by or Reviewed-by to
that patch.

Thanks

RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Gaurav Jain]

Hi Fabio

I can not see the patch in my mails. 
Can you include me using my mail id.

Regards
Gaurav Jain

> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Friday, June 10, 2022 1:36 AM
> To: Gaurav Jain <gaurav.jain@nxp.com>
> Cc: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; u-
> boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Tommaso Merciai
> <tommaso.merciai@amarulasolutions.com>; Michael Trimarchi
> <michael@amarulasolutions.com>; Marek Vasut <marex@denx.de>; Simon
> Glass <sjg@chromium.org>; Patrick Delaunay <patrick.delaunay@foss.st.com>;
> Stefan Roese <sr@denx.de>; Horia Geanta <horia.geanta@nxp.com>; Pankaj
> Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Ye Li
> <ye.li@nxp.com>; Michael Walle <michael@walle.cc>; dl-uboot-imx <uboot-
> imx@nxp.com>
> Subject: Re: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> 
> Caution: EXT Email
> 
> Hi Gaurav,
> 
> On Thu, Jun 9, 2022 at 10:12 AM Gaurav Jain <gaurav.jain@nxp.com> wrote:
> 
> > > I suggest that this is submitted into Kernel, and then picked up
> > > during the next DTB re-sync.
> >
> > Ok. Fabio has already submitted a patch for this.
> 
> Yes, if you have a chance please send your Tested-by or Reviewed-by to that
> patch.
> 
> Thanks

Re: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

[Fabio Estevam]

Hi Gaurav,

On Fri, Jun 10, 2022 at 8:51 AM Gaurav Jain <gaurav.jain@nxp.com> wrote:
>
> Hi Fabio
>
> I can not see the patch in my mails.
> Can you include me using my mail id.

I added your <gaurav.jain@nxp.com> email, when submitting:
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-June/749625.html

Regards,

Fabio Estevam