[PATCH v2 0/3] arm: mach-imx: tzasc: port lock id_swap_bypass bit

[PATCH v2 0/3] arm: mach-imx: tzasc: port lock id_swap_bypass bit

[Stefan Kerkmann]

This series ports the U-Boot commit 1289ff7bd7e4 ("imx8m: lock
id_swap_bypass bit in tzc380 enable") to barebox and refactors the tzasc
driver for the imx platform to use the `cpu_is_mx8xyz` macros.

Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
Changes in v2:
- Rebased onto next to pickup code relocation from
  arch/arm/mach-imx/imx8m.c to drivers/soc/imx/soc-imx8m.c
- Link to v1: https://lore.kernel.org/r/20240226-v2024-02-0-topic-imx8m-n-p-tzac-v1-0-2df2430da984@pengutronix.de

---
Stefan Kerkmann (3):
      arm: mach-imx: tzasc: lock id_swap_bypass bit
      arm: mach-imx: set cpu type in pbl
      arm: mach-imx: tzasc: convert to cpu_is_mx8xyz macros

 arch/arm/mach-imx/Makefile  |  2 +-
 arch/arm/mach-imx/atf.c     | 13 +++++++++----
 arch/arm/mach-imx/tzasc.c   | 47 ++++++++++++++++++++++++++-------------------
 drivers/soc/imx/soc-imx8m.c |  2 +-
 include/mach/imx/generic.h  |  5 +++++
 include/mach/imx/tzasc.h    |  8 ++------
 6 files changed, 45 insertions(+), 32 deletions(-)
---
base-commit: 5fcad261420f1316ee924549778a7a2be2389481
change-id: 20240226-v2024-02-0-topic-imx8m-n-p-tzac-480c113ca052

Best regards,
-- 
Stefan Kerkmann <s.kerkmann@pengutronix.de>

[PATCH v2 1/3] arm: mach-imx: tzasc: lock id_swap_bypass bit

[Stefan Kerkmann]

This commit ports U-Boot commit 1289ff7bd7e4 ("imx8m: lock
id_swap_bypass bit in tzc380 enable") to barebox. This is the original
commit message:

> According to TRM for i.MX8M Nano and Plus, GPR10 register contains lock
> bit for TZASC_ID_SWAP_BYPASS bit. This bit is required to be set in
> order to avoid AXI bus errors when GPU is enabled on the platform.
> TZASC_ID_SWAP_BYPASS bit is alread set for all imx8m applicable
> derivatives, but is missing a lock settings to be applied.
>
> Set the TZASC_ID_SWAP_BYPASS_LOCK bit for those derivatives which have
> it implemented.
>
> Since we're here, provide also names to bits from TRM instead of using
> BIT() macro in the code.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Reviewed-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
 arch/arm/mach-imx/tzasc.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-imx/tzasc.c b/arch/arm/mach-imx/tzasc.c
index 9c71108c99..1f8d7426c1 100644
--- a/arch/arm/mach-imx/tzasc.c
+++ b/arch/arm/mach-imx/tzasc.c
@@ -5,37 +5,59 @@
 #include <mach/imx/imx8m-regs.h>
 #include <io.h>
 
-#define GPR_TZASC_EN		BIT(0)
-#define GPR_TZASC_SWAP_ID	BIT(1)
-#define GPR_TZASC_EN_LOCK	BIT(16)
+#define GPR_TZASC_EN					BIT(0)
+#define GPR_TZASC_ID_SWAP_BYPASS		BIT(1)
+#define GPR_TZASC_EN_LOCK				BIT(16)
+#define GPR_TZASC_ID_SWAP_BYPASS_LOCK	BIT(17)
 
-static void enable_tzc380(bool bypass_id_swap)
+#define MX8M_TZASC_REGION_ATTRIBUTES_0		(MX8M_TZASC_BASE_ADDR + 0x108)
+#define MX8M_TZASC_REGION_ATTRIBUTES_0_SP	GENMASK(31, 28)
+
+static void enable_tzc380(bool bypass_id_swap, bool bypass_id_swap_lock)
 {
 	u32 __iomem *gpr = IOMEM(MX8M_IOMUXC_GPR_BASE_ADDR);
 
 	/* Enable TZASC and lock setting */
 	setbits_le32(&gpr[10], GPR_TZASC_EN);
 	setbits_le32(&gpr[10], GPR_TZASC_EN_LOCK);
+
+	/*
+	 * According to TRM, TZASC_ID_SWAP_BYPASS should be set in
+	 * order to avoid AXI Bus errors when GPU is in use
+	 */
 	if (bypass_id_swap)
-		setbits_le32(&gpr[10], BIT(1));
+		setbits_le32(&gpr[10], GPR_TZASC_ID_SWAP_BYPASS);
+
+	/*
+	 * imx8mn and imx8mp implements the lock bit for
+	 * TZASC_ID_SWAP_BYPASS, enable it to lock settings
+	 */
+	if (bypass_id_swap_lock)
+		setbits_le32(&gpr[10], GPR_TZASC_ID_SWAP_BYPASS_LOCK);
+
 	/*
 	 * set Region 0 attribute to allow secure and non-secure
 	 * read/write permission. Found some masters like usb dwc3
 	 * controllers can't work with secure memory.
 	 */
-	writel(0xf0000000, MX8M_TZASC_BASE_ADDR + 0x108);
+	writel(MX8M_TZASC_REGION_ATTRIBUTES_0_SP,
+		   MX8M_TZASC_REGION_ATTRIBUTES_0);
 }
 
 void imx8mq_tzc380_init(void)
 {
-	enable_tzc380(false);
+	enable_tzc380(false, false);
 }
 
-void imx8mn_tzc380_init(void) __alias(imx8mm_tzc380_init);
-void imx8mp_tzc380_init(void) __alias(imx8mm_tzc380_init);
 void imx8mm_tzc380_init(void)
 {
-	enable_tzc380(true);
+	enable_tzc380(true, false);
+}
+
+void imx8mn_tzc380_init(void) __alias(imx8mp_tzc380_init);
+void imx8mp_tzc380_init(void)
+{
+	enable_tzc380(true, true);
 }
 
 bool tzc380_is_enabled(void)

-- 
2.39.2

[PATCH v2 2/3] arm: mach-imx: set cpu type in pbl

[Stefan Kerkmann]

In order to use the `cpu_is_imxxyz` macro family in the pbl,
`__imx_cpu_type` has to be defined and initialized. As we don't have
access to the devicetree at this point, we resort to manual assignment.

Note: It is safe to build the same imx.o object file for both barebox
pbl and proper as the `imx_init` function is discarded during linking as
the whole `init_call` section is not linked into the final binary.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
 arch/arm/mach-imx/Makefile | 2 +-
 arch/arm/mach-imx/atf.c    | 5 +++++
 include/mach/imx/generic.h | 5 +++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index ce8af486ae..a2d9702bf4 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_NAND_IMX) += nand.o
 lwl-$(CONFIG_ARCH_IMX_EXTERNAL_BOOT_NAND) += external-nand-boot.o
 obj-y += devices.o imx.o
 obj-$(CONFIG_CMD_BOOTROM) += bootrom-cmd.o
-obj-pbl-y += esdctl.o boot.o
+obj-pbl-y += esdctl.o boot.o imx.o
 obj-$(CONFIG_BAREBOX_UPDATE) += imx-bbu-internal.o
 obj-$(CONFIG_BAREBOX_UPDATE_IMX_EXTERNAL_NAND) += imx-bbu-external-nand.o
 pbl-$(CONFIG_USB_GADGET_DRIVER_ARC_PBL) += imx-udc.o
diff --git a/arch/arm/mach-imx/atf.c b/arch/arm/mach-imx/atf.c
index 2d8388e8e9..e8060ebd95 100644
--- a/arch/arm/mach-imx/atf.c
+++ b/arch/arm/mach-imx/atf.c
@@ -148,6 +148,7 @@ __noreturn void __imx8mm_load_and_start_image_via_tfa(void *bl33)
 	size_t bl31_size;
 	unsigned long endmem = MX8M_DDR_CSD1_BASE_ADDR + imx8m_barebox_earlymem_size(32);
 
+	imx_set_cpu_type(IMX_CPU_IMX8MM);
 	imx8mm_init_scratch_space();
 	imx8m_save_bootrom_log();
 	imx8mm_load_bl33(bl33);
@@ -218,6 +219,7 @@ __noreturn void __imx8mp_load_and_start_image_via_tfa(void *bl33)
 	size_t bl31_size;
 	unsigned long endmem = MX8M_DDR_CSD1_BASE_ADDR + imx8m_barebox_earlymem_size(32);
 
+	imx_set_cpu_type(IMX_CPU_IMX8MP);
 	imx8mp_init_scratch_space();
 	imx8m_save_bootrom_log();
 	imx8mp_load_bl33(bl33);
@@ -289,6 +291,7 @@ __noreturn void __imx8mn_load_and_start_image_via_tfa(void *bl33)
 	size_t bl31_size;
 	unsigned long endmem = MX8M_DDR_CSD1_BASE_ADDR + imx8m_barebox_earlymem_size(16);
 
+	imx_set_cpu_type(IMX_CPU_IMX8MN);
 	imx8mn_init_scratch_space();
 	imx8m_save_bootrom_log();
 	imx8mn_load_bl33(bl33);
@@ -353,6 +356,7 @@ __noreturn void __imx8mq_load_and_start_image_via_tfa(void *bl33)
 	size_t bl31_size;
 	unsigned long endmem = MX8M_DDR_CSD1_BASE_ADDR + imx8m_barebox_earlymem_size(32);
 
+	imx_set_cpu_type(IMX_CPU_IMX8MQ);
 	imx8mq_init_scratch_space();
 	imx8m_save_bootrom_log();
 	imx8mq_load_bl33(bl33);
@@ -388,6 +392,7 @@ void __noreturn imx93_load_and_start_image_via_tfa(void)
 	void *bl33 = (void *)MX93_ATF_BL33_BASE_ADDR;
 	unsigned long endmem = MX9_DDR_CSD1_BASE_ADDR + imx9_ddrc_sdram_size();
 
+	imx_set_cpu_type(IMX_CPU_IMX93);
 	imx93_init_scratch_space(true);
 
 	/*
diff --git a/include/mach/imx/generic.h b/include/mach/imx/generic.h
index 1dca335fdd..2b26a1d45a 100644
--- a/include/mach/imx/generic.h
+++ b/include/mach/imx/generic.h
@@ -82,6 +82,11 @@ void imx93_cpu_lowlevel_init(void);
 
 extern unsigned int __imx_cpu_type;
 
+static __always_inline void imx_set_cpu_type(unsigned int cpu_type)
+{
+	__imx_cpu_type = cpu_type;
+}
+
 #ifdef CONFIG_ARCH_IMX1
 # ifdef imx_cpu_type
 #  undef imx_cpu_type

-- 
2.39.2

[PATCH v2 3/3] arm: mach-imx: tzasc: convert to cpu_is_mx8xyz macros

[Stefan Kerkmann]

Instead of passing in configuration parameters at runtime we can utilize
the `cpu_is_mx8xyz` macro family to determine which bits should be set.

As the tzasc driver is imx specific, all functions are prefixed with
`imx8m_` as well.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
 arch/arm/mach-imx/atf.c     |  8 ++++----
 arch/arm/mach-imx/tzasc.c   | 25 +++++--------------------
 drivers/soc/imx/soc-imx8m.c |  2 +-
 include/mach/imx/tzasc.h    |  8 ++------
 4 files changed, 12 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-imx/atf.c b/arch/arm/mach-imx/atf.c
index e8060ebd95..9cbc38ef11 100644
--- a/arch/arm/mach-imx/atf.c
+++ b/arch/arm/mach-imx/atf.c
@@ -158,7 +158,7 @@ __noreturn void __imx8mm_load_and_start_image_via_tfa(void *bl33)
 		size_t bl32_size;
 		void *bl32_image;
 
-		imx8mm_tzc380_init();
+		imx8m_tzc380_init();
 		get_builtin_firmware_ext(imx8mm_bl32_bin,
 				bl33, &bl32_image,
 				&bl32_size);
@@ -229,7 +229,7 @@ __noreturn void __imx8mp_load_and_start_image_via_tfa(void *bl33)
 		size_t bl32_size;
 		void *bl32_image;
 
-		imx8mp_tzc380_init();
+		imx8m_tzc380_init();
 		get_builtin_firmware_ext(imx8mp_bl32_bin,
 				bl33, &bl32_image,
 				&bl32_size);
@@ -301,7 +301,7 @@ __noreturn void __imx8mn_load_and_start_image_via_tfa(void *bl33)
 		size_t bl32_size;
 		void *bl32_image;
 
-		imx8mn_tzc380_init();
+		imx8m_tzc380_init();
 		get_builtin_firmware_ext(imx8mn_bl32_bin,
 				bl33, &bl32_image,
 				&bl32_size);
@@ -366,7 +366,7 @@ __noreturn void __imx8mq_load_and_start_image_via_tfa(void *bl33)
 		size_t bl32_size;
 		void *bl32_image;
 
-		imx8mq_tzc380_init();
+		imx8m_tzc380_init();
 		get_builtin_firmware_ext(imx8mq_bl32_bin,
 				bl33, &bl32_image,
 				&bl32_size);
diff --git a/arch/arm/mach-imx/tzasc.c b/arch/arm/mach-imx/tzasc.c
index 1f8d7426c1..4cb4d7c5cf 100644
--- a/arch/arm/mach-imx/tzasc.c
+++ b/arch/arm/mach-imx/tzasc.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include <mach/imx/generic.h>
 #include <mach/imx/tzasc.h>
 #include <linux/bitops.h>
 #include <mach/imx/imx8m-regs.h>
@@ -13,7 +14,7 @@
 #define MX8M_TZASC_REGION_ATTRIBUTES_0		(MX8M_TZASC_BASE_ADDR + 0x108)
 #define MX8M_TZASC_REGION_ATTRIBUTES_0_SP	GENMASK(31, 28)
 
-static void enable_tzc380(bool bypass_id_swap, bool bypass_id_swap_lock)
+void imx8m_tzc380_init(void)
 {
 	u32 __iomem *gpr = IOMEM(MX8M_IOMUXC_GPR_BASE_ADDR);
 
@@ -25,14 +26,14 @@ static void enable_tzc380(bool bypass_id_swap, bool bypass_id_swap_lock)
 	 * According to TRM, TZASC_ID_SWAP_BYPASS should be set in
 	 * order to avoid AXI Bus errors when GPU is in use
 	 */
-	if (bypass_id_swap)
+	if (cpu_is_mx8mm() || cpu_is_mx8mn() || cpu_is_mx8mp())
 		setbits_le32(&gpr[10], GPR_TZASC_ID_SWAP_BYPASS);
 
 	/*
 	 * imx8mn and imx8mp implements the lock bit for
 	 * TZASC_ID_SWAP_BYPASS, enable it to lock settings
 	 */
-	if (bypass_id_swap_lock)
+	if (cpu_is_mx8mn() || cpu_is_mx8mp())
 		setbits_le32(&gpr[10], GPR_TZASC_ID_SWAP_BYPASS_LOCK);
 
 	/*
@@ -44,23 +45,7 @@ static void enable_tzc380(bool bypass_id_swap, bool bypass_id_swap_lock)
 		   MX8M_TZASC_REGION_ATTRIBUTES_0);
 }
 
-void imx8mq_tzc380_init(void)
-{
-	enable_tzc380(false, false);
-}
-
-void imx8mm_tzc380_init(void)
-{
-	enable_tzc380(true, false);
-}
-
-void imx8mn_tzc380_init(void) __alias(imx8mp_tzc380_init);
-void imx8mp_tzc380_init(void)
-{
-	enable_tzc380(true, true);
-}
-
-bool tzc380_is_enabled(void)
+bool imx8m_tzc380_is_enabled(void)
 {
 	u32 __iomem *gpr = IOMEM(MX8M_IOMUXC_GPR_BASE_ADDR);
 
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index 48b42a1b17..1b47c914de 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -208,7 +208,7 @@ static int imx8_soc_imx8m_init(struct soc_device_attribute *soc_dev_attr)
 	imx_set_reset_reason(src + IMX7_SRC_SRSR, imx7_reset_reasons);
 	pr_info("%s unique ID: %s\n", cputypestr, uid);
 
-	if (IS_ENABLED(CONFIG_PBL_OPTEE) && tzc380_is_enabled()) {
+	if (IS_ENABLED(CONFIG_PBL_OPTEE) && imx8m_tzc380_is_enabled()) {
 		static struct of_optee_fixup_data optee_fixup_data = {
 			.shm_size = OPTEE_SHM_SIZE,
 			.method = "smc",
diff --git a/include/mach/imx/tzasc.h b/include/mach/imx/tzasc.h
index 724ba50ead..51c86f168e 100644
--- a/include/mach/imx/tzasc.h
+++ b/include/mach/imx/tzasc.h
@@ -6,11 +6,7 @@
 #include <linux/types.h>
 #include <asm/system.h>
 
-void imx8mq_tzc380_init(void);
-void imx8mm_tzc380_init(void);
-void imx8mn_tzc380_init(void);
-void imx8mp_tzc380_init(void);
-
-bool tzc380_is_enabled(void);
+void imx8m_tzc380_init(void);
+bool imx8m_tzc380_is_enabled(void);
 
 #endif

-- 
2.39.2

Re: [PATCH v2 0/3] arm: mach-imx: tzasc: port lock id_swap_bypass bit

[Sascha Hauer]

On Wed, 28 Feb 2024 14:14:07 +0100, Stefan Kerkmann wrote:
> This series ports the U-Boot commit 1289ff7bd7e4 ("imx8m: lock
> id_swap_bypass bit in tzc380 enable") to barebox and refactors the tzasc
> driver for the imx platform to use the `cpu_is_mx8xyz` macros.
> 
> 

Applied, thanks!

[1/3] arm: mach-imx: tzasc: lock id_swap_bypass bit
      https://git.pengutronix.de/cgit/barebox/commit/?id=f772a942c578 (link may not be stable)
[2/3] arm: mach-imx: set cpu type in pbl
      https://git.pengutronix.de/cgit/barebox/commit/?id=36fecc08fc11 (link may not be stable)
[3/3] arm: mach-imx: tzasc: convert to cpu_is_mx8xyz macros
      https://git.pengutronix.de/cgit/barebox/commit/?id=f061388ffe24 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>