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 --gita/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 --gita/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 --gita/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