[U-Boot] [PATCH 1/2] armv8: fsl-layerscape: add missing sec jr base address defines

[U-Boot] [PATCH 1/2] armv8: fsl-layerscape: add missing sec jr base address defines

[laurentiu.tudor at nxp.com]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

Add defines for all the SEC job rings base addresses.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index 9fab88ab2f..fc14fb6fe0 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -89,10 +89,18 @@
 /* SEC */
 #define CONFIG_SYS_FSL_SEC_OFFSET		0x07000000ull
 #define CONFIG_SYS_FSL_JR0_OFFSET		0x07010000ull
+#define FSL_SEC_JR0_OFFSET			CONFIG_SYS_FSL_JR0_OFFSET
+#define FSL_SEC_JR1_OFFSET			0x07020000ull
+#define FSL_SEC_JR2_OFFSET			0x07030000ull
+#define FSL_SEC_JR3_OFFSET			0x07040000ull
 #define CONFIG_SYS_FSL_SEC_ADDR \
 	(CONFIG_SYS_IMMR + CONFIG_SYS_FSL_SEC_OFFSET)
 #define CONFIG_SYS_FSL_JR0_ADDR \
 	(CONFIG_SYS_IMMR + CONFIG_SYS_FSL_JR0_OFFSET)
+#define FSL_SEC_JR0_BASE_ADDR (CONFIG_SYS_IMMR + FSL_SEC_JR0_OFFSET)
+#define FSL_SEC_JR1_BASE_ADDR (CONFIG_SYS_IMMR + FSL_SEC_JR1_OFFSET)
+#define FSL_SEC_JR2_BASE_ADDR (CONFIG_SYS_IMMR + FSL_SEC_JR2_OFFSET)
+#define FSL_SEC_JR3_BASE_ADDR (CONFIG_SYS_IMMR + FSL_SEC_JR3_OFFSET)
 
 #ifdef CONFIG_TFABOOT
 #ifdef CONFIG_NXP_LSCH3_2
-- 
2.17.1

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[laurentiu.tudor at nxp.com]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

Add ICID setup for the platform devices contained on this chip: usb,
sata, sdhc, sec. The ICID macros for SEC needed to be adapted because
the format of the registers is different. Also, the initial static ICID
allocation left SEC out so update it by grabbing an ICID from the range
allocated to PCI and use it for SEC.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 arch/arm/cpu/armv8/fsl-layerscape/Makefile    |  1 +
 .../arm/cpu/armv8/fsl-layerscape/ls1088_ids.c | 31 +++++++++++++
 arch/arm/cpu/armv8/fsl-layerscape/soc.c       |  4 ++
 .../asm/arch-fsl-layerscape/fsl_icid.h        | 43 ++++++++++++++++---
 .../asm/arch-fsl-layerscape/stream_id_lsch3.h |  4 +-
 board/freescale/ls1088a/ls1088a.c             |  4 ++
 6 files changed, 80 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ls1088_ids.c

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
index e9bc987a9c..86d572ea28 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
@@ -47,4 +47,5 @@ endif
 
 ifneq ($(CONFIG_ARCH_LS1088A),)
 obj-$(CONFIG_SYS_HAS_SERDES) += ls1088a_serdes.o
+obj-y += icid.o ls1088_ids.o
 endif
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1088_ids.c b/arch/arm/cpu/armv8/fsl-layerscape/ls1088_ids.c
new file mode 100644
index 0000000000..8f12a664d0
--- /dev/null
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1088_ids.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 NXP
+ */
+
+#include <common.h>
+#include <asm/arch-fsl-layerscape/immap_lsch3.h>
+#include <asm/arch-fsl-layerscape/fsl_icid.h>
+#include <asm/arch-fsl-layerscape/fsl_portals.h>
+
+struct icid_id_table icid_tbl[] = {
+	SET_SDHC_ICID(FSL_SDMMC_STREAM_ID),
+	SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID),
+	SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID),
+	SET_SATA_ICID(1, "fsl,ls1088a-ahci", FSL_SATA1_STREAM_ID),
+	SET_SEC_JR_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
+	SET_SEC_JR_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
+	SET_SEC_JR_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
+	SET_SEC_JR_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
+	SET_SEC_RTIC_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
+	SET_SEC_RTIC_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
+	SET_SEC_RTIC_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
+	SET_SEC_RTIC_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
+	SET_SEC_DECO_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
+	SET_SEC_DECO_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
+	SET_SEC_DECO_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
+	SET_SEC_DECO_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
+};
+
+int icid_tbl_sz = ARRAY_SIZE(icid_tbl);
+
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 06f3edb302..cd91329edc 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -332,6 +332,10 @@ void fsl_lsch3_early_init_f(void)
 	if (fsl_check_boot_mode_secure() == 1)
 		bypass_smmu();
 #endif
+
+#ifdef CONFIG_ARCH_LS1088A
+	set_icids();
+#endif
 }
 
 /* Get VDD in the unit mV from voltage ID */
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
index f971af8d26..d1ad171bde 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
@@ -38,6 +38,7 @@ void fdt_fixup_icid(void *blob);
 	  .reg_addr = addr, \
 	}
 
+#ifdef CONFIG_FSL_LSCH2
 #define SET_SCFG_ICID(compat, streamid, name, compataddr) \
 	SET_ICID_ENTRY(compat, streamid, (((streamid) << 24) | (1 << 23)), \
 		offsetof(struct ccsr_scfg, name) + CONFIG_SYS_FSL_SCFG_ADDR, \
@@ -92,12 +93,44 @@ void fdt_fixup_icid(void *blob);
 #define SET_FMAN_ICID_ENTRY(_port_id, streamid) \
 	{ .port_id = (_port_id), .icid = (streamid) }
 
+#define SEC_ICID_REG_VAL(streamid) (((streamid) << 16) | (streamid))
+
 #define SET_SEC_QI_ICID(streamid) \
-	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
+	SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \
 		0, offsetof(ccsr_sec_t, qilcr_ls) + \
 		CONFIG_SYS_FSL_SEC_ADDR, \
 		CONFIG_SYS_FSL_SEC_ADDR)
 
+extern struct fman_icid_id_table fman_icid_tbl[];
+extern int fman_icid_tbl_sz;
+
+#else /* CONFIG_FSL_LSCH2 */
+
+#define SET_GUR_ICID(compat, streamid, name, compataddr) \
+	SET_ICID_ENTRY(compat, streamid, (streamid << 24), \
+		offsetof(struct ccsr_gur, name) + CONFIG_SYS_FSL_GUTS_ADDR, \
+		compataddr)
+
+#define SET_USB_ICID(usb_num, compat, streamid) \
+	SET_GUR_ICID(compat, streamid, usb##usb_num##_amqr,\
+		CONFIG_SYS_XHCI_USB##usb_num##_ADDR)
+
+#define SET_SATA_ICID(sata_num, compat, streamid) \
+	SET_GUR_ICID(compat, streamid, sata##sata_num##_amqr, \
+	AHCI_BASE_ADDR##sata_num)
+
+#define SET_SDHC_ICID(streamid) \
+	SET_GUR_ICID("fsl,esdhc", streamid, sdmm1_amqr,\
+		CONFIG_SYS_FSL_ESDHC_ADDR)
+
+#define SET_QE_ICID(streamid) \
+	SET_GUR_ICID("fsl,qe", streamid, misc3_amqr,\
+		QE_BASE_ADDR)
+
+#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24)
+
+#endif /* !CONFIG_FSL_LSCH2 */
+
 #define SET_SEC_JR_ICID_ENTRY(jr_num, streamid) \
 	SET_ICID_ENTRY( \
 		(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT && \
@@ -106,24 +139,22 @@ void fdt_fixup_icid(void *blob);
 			? NULL \
 			: "fsl,sec-v4.0-job-ring"), \
 		streamid, \
-		(((streamid) << 16) | (streamid)), \
+		SEC_ICID_REG_VAL(streamid), \
 		offsetof(ccsr_sec_t, jrliodnr[jr_num].ls) + \
 		CONFIG_SYS_FSL_SEC_ADDR, \
 		FSL_SEC_JR##jr_num##_BASE_ADDR)
 
 #define SET_SEC_DECO_ICID_ENTRY(deco_num, streamid) \
-	SET_ICID_ENTRY(NULL, streamid, (((streamid) << 16) | (streamid)), \
+	SET_ICID_ENTRY(NULL, streamid, SEC_ICID_REG_VAL(streamid), \
 		offsetof(ccsr_sec_t, decoliodnr[deco_num].ls) + \
 		CONFIG_SYS_FSL_SEC_ADDR, 0)
 
 #define SET_SEC_RTIC_ICID_ENTRY(rtic_num, streamid) \
-	SET_ICID_ENTRY(NULL, streamid, (((streamid) << 16) | (streamid)), \
+	SET_ICID_ENTRY(NULL, streamid, SEC_ICID_REG_VAL(streamid), \
 		offsetof(ccsr_sec_t, rticliodnr[rtic_num].ls) + \
 		CONFIG_SYS_FSL_SEC_ADDR, 0)
 
 extern struct icid_id_table icid_tbl[];
-extern struct fman_icid_id_table fman_icid_tbl[];
 extern int icid_tbl_sz;
-extern int fman_icid_tbl_sz;
 
 #endif
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
index e017d8b558..508f697523 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
@@ -80,8 +80,10 @@
 #define FSL_DMA_STREAM_ID		5
 #endif
 
+#define FSL_SEC_STREAM_ID		7
+
 /* PCI - programmed in PEXn_LUT */
-#define FSL_PEX_STREAM_ID_START		7
+#define FSL_PEX_STREAM_ID_START		8
 
 #ifdef CONFIG_ARCH_LX2160A
 #define FSL_PEX_STREAM_ID_NUM		(0x100)
diff --git a/board/freescale/ls1088a/ls1088a.c b/board/freescale/ls1088a/ls1088a.c
index 6d11a134dc..7fde358325 100644
--- a/board/freescale/ls1088a/ls1088a.c
+++ b/board/freescale/ls1088a/ls1088a.c
@@ -20,6 +20,7 @@
 #include <hwconfig.h>
 #include <asm/arch/fsl_serdes.h>
 #include <asm/arch/soc.h>
+#include <asm/arch-fsl-layerscape/fsl_icid.h>
 
 #include "../common/qixis.h"
 #include "ls1088a_qixis.h"
@@ -768,6 +769,9 @@ int ft_board_setup(void *blob, bd_t *bd)
 #ifdef CONFIG_FSL_MC_ENET
 	fdt_fixup_board_enet(blob);
 #endif
+
+	fdt_fixup_icid(blob);
+
 	if (is_pb_board())
 		fixup_ls1088ardb_pb_banner(blob);
 
-- 
2.17.1

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[Horia Geanta]

On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
> +struct icid_id_table icid_tbl[] = {
> +	SET_SDHC_ICID(FSL_SDMMC_STREAM_ID),
> +	SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID),
> +	SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID),
> +	SET_SATA_ICID(1, "fsl,ls1088a-ahci", FSL_SATA1_STREAM_ID),
> +	SET_SEC_JR_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
> +	SET_SEC_JR_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
> +	SET_SEC_JR_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
> +	SET_SEC_JR_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
> +	SET_SEC_RTIC_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
> +	SET_SEC_RTIC_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
> +	SET_SEC_RTIC_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
> +	SET_SEC_RTIC_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
> +	SET_SEC_DECO_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
> +	SET_SEC_DECO_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
> +	SET_SEC_DECO_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
> +	SET_SEC_DECO_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
> +};
A single ICID is allocated to all SEC sub-blocks able to initiate transactions.
I think at least the job rings should have different ICIDs, while the rest could
share another ICID.

>  #define SET_SEC_QI_ICID(streamid) \
> -	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
> +	SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \
Is this a fix for LS104x? Then it should be a separate patch.

> +#else /* CONFIG_FSL_LSCH2 */
[...]
> +#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24)
ICID is in lower 6:0 bits, not in 31:24.

> +#endif /* !CONFIG_FSL_LSCH2 */
Nitpick: comment for #endif should match #if condition, which is CONFIG_FSL_LSCH2.

Horia

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[Laurentiu Tudor]

Hi Horia,

On 21.03.2019 12:36, Horia Geanta wrote:
> On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
>> +struct icid_id_table icid_tbl[] = {
>> +	SET_SDHC_ICID(FSL_SDMMC_STREAM_ID),
>> +	SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID),
>> +	SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID),
>> +	SET_SATA_ICID(1, "fsl,ls1088a-ahci", FSL_SATA1_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +};
> A single ICID is allocated to all SEC sub-blocks able to initiate transactions.
> I think at least the job rings should have different ICIDs, while the rest could
> share another ICID.

I agree here, problem is that on these chips ICIDs are scarce resource 
because DPAA2 gets a big chunk of them. I'd suggest to leave them like 
that and, if a user needs distinct, per JR ICIDs (s)he can easily tune them.

>>   #define SET_SEC_QI_ICID(streamid) \
>> -	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
>> +	SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \
> Is this a fix for LS104x? Then it should be a separate patch.

Not really. I added an intermediate macro, SEC_ICID_REG_VAL(streamid) 
that forms the correct register value starting from ICID, depending on 
the chip version (the register layouts are different between the two).

>> +#else /* CONFIG_FSL_LSCH2 */
> [...]
>> +#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24)
> ICID is in lower 6:0 bits, not in 31:24.

That was also my initial impression but it didn't work (smmu global 
faults with icid 0). Probably there's an ambiguity related to endianness 
in the documentation.

>> +#endif /* !CONFIG_FSL_LSCH2 */
> Nitpick: comment for #endif should match #if condition, which is CONFIG_FSL_LSCH2.
> 

Ok.

Thanks for the review!

---
Best Regards, Laurentiu

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[Horia Geanta]

On 3/21/2019 2:42 PM, Tudor Laurentiu-B10716 wrote:
> Hi Horia,
> 
> On 21.03.2019 12:36, Horia Geanta wrote:
>> On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
>>>   #define SET_SEC_QI_ICID(streamid) \
>>> -	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
>>> +	SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \
>> Is this a fix for LS104x? Then it should be a separate patch.
> 
> Not really. I added an intermediate macro, SEC_ICID_REG_VAL(streamid) 
> that forms the correct register value starting from ICID, depending on 
> the chip version (the register layouts are different between the two).
> 
This is the define
#define SEC_ICID_REG_VAL(streamid) (((streamid) << 16) | (streamid))

Thus the QI ICID for LS104x changes.

>>> +#else /* CONFIG_FSL_LSCH2 */
>> [...]
>>> +#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24)
>> ICID is in lower 6:0 bits, not in 31:24.
> 
> That was also my initial impression but it didn't work (smmu global 
> faults with icid 0). Probably there's an ambiguity related to endianness 
> in the documentation.
> 
Note that on DPAA 2.x both the core and device (CAAM) are little endian.
Probably the problem is with the I/O accessors writing the CAAM registers.

Instead of out_be32() use either sec_out32() (but this means splitting
CAAM-specific code) or out_le32().

CAAM endianness can be detected using CONFIG_SYS_FSL_SEC_LE / CONFIG_SYS_FSL_SEC_BE.

Horia

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[Laurentiu Tudor]

On 21.03.2019 17:10, Horia Geanta wrote:
> On 3/21/2019 2:42 PM, Tudor Laurentiu-B10716 wrote:
>> Hi Horia,
>>
>> On 21.03.2019 12:36, Horia Geanta wrote:
>>> On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
>>>>    #define SET_SEC_QI_ICID(streamid) \
>>>> -	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
>>>> +	SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \
>>> Is this a fix for LS104x? Then it should be a separate patch.
>>
>> Not really. I added an intermediate macro, SEC_ICID_REG_VAL(streamid)
>> that forms the correct register value starting from ICID, depending on
>> the chip version (the register layouts are different between the two).
>>
> This is the define
> #define SEC_ICID_REG_VAL(streamid) (((streamid) << 16) | (streamid))
> 
> Thus the QI ICID for LS104x changes.

Ah, alright. Good point. I checked in the manuals and this unintended 
change does indeed fix this register setup. I'm leaning towards 
mentioning this collateral fix in the commit message.

>>>> +#else /* CONFIG_FSL_LSCH2 */
>>> [...]
>>>> +#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24)
>>> ICID is in lower 6:0 bits, not in 31:24.
>>
>> That was also my initial impression but it didn't work (smmu global
>> faults with icid 0). Probably there's an ambiguity related to endianness
>> in the documentation.
>>
> Note that on DPAA 2.x both the core and device (CAAM) are little endian.
> Probably the problem is with the I/O accessors writing the CAAM registers.
> 
> Instead of out_be32() use either sec_out32() (but this means splitting
> CAAM-specific code) or out_le32().
> 
> CAAM endianness can be detected using CONFIG_SYS_FSL_SEC_LE / CONFIG_SYS_FSL_SEC_BE.
> 

Thanks for the explanation. I think I can change the macros to take into 
consideration the CONFIG_*s you pointed.

---
Best Regards, Laurentiu

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[Laurentiu Tudor]

Hi Horia,

On 21.03.2019 12:36, Horia Geanta wrote:
> On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
>> +struct icid_id_table icid_tbl[] = {
>> +	SET_SDHC_ICID(FSL_SDMMC_STREAM_ID),
>> +	SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID),
>> +	SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID),
>> +	SET_SATA_ICID(1, "fsl,ls1088a-ahci", FSL_SATA1_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +};
> A single ICID is allocated to all SEC sub-blocks able to initiate transactions.
> I think at least the job rings should have different ICIDs, while the rest could
> share another ICID.

I agree here, problem is that on these chips ICIDs are scarce resource 
because DPAA2 gets a big chunk of them. I'd suggest to leave them like 
that and, if a user needs distinct, per JR ICIDs (s)he can easily tune them.

>>   #define SET_SEC_QI_ICID(streamid) \
>> -	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
>> +	SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \
> Is this a fix for LS104x? Then it should be a separate patch.

Not really. I added an intermediate macro, SEC_ICID_REG_VAL(streamid) 
that forms the correct register value starting from ICID, depending on 
the chip version (the register layouts are different between the two).

>> +#else /* CONFIG_FSL_LSCH2 */
> [...]
>> +#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24)
> ICID is in lower 6:0 bits, not in 31:24.

That was also my initial impression but it didn't work (smmu global 
faults with icid 0). Probably there's an ambiguity related to endianness 
in the documentation.

>> +#endif /* !CONFIG_FSL_LSCH2 */
> Nitpick: comment for #endif should match #if condition, which is CONFIG_FSL_LSCH2.
> 

Ok.

Thanks for the review!

---
Best Regards, Laurentiu

[U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices

[Laurentiu Tudor]

Hi Horia,

On 21.03.2019 12:36, Horia Geanta wrote:
> On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
>> +struct icid_id_table icid_tbl[] = {
>> +	SET_SDHC_ICID(FSL_SDMMC_STREAM_ID),
>> +	SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID),
>> +	SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID),
>> +	SET_SATA_ICID(1, "fsl,ls1088a-ahci", FSL_SATA1_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_JR_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_RTIC_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(0, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(1, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(2, FSL_SEC_STREAM_ID),
>> +	SET_SEC_DECO_ICID_ENTRY(3, FSL_SEC_STREAM_ID),
>> +};
> A single ICID is allocated to all SEC sub-blocks able to initiate transactions.
> I think at least the job rings should have different ICIDs, while the rest could
> share another ICID.

Actually let me take back my statement on the scarcity of the ICIDs. I 
just noticed that in the allocation of the ICIDs we actually use just 64 
[1] out of the total 128 available. I don't know why we do that, perhaps 
it a legacy left over but anyway I've tested SEC with ICIDs > 63 and 
tests passed.
In conclusion I'll update the ICID allocation for SEC in the following spin.

[1] 
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h;h=e017d8b55895ba79328dd9281458e35a99b5900a;hb=HEAD#l99

---
Best Regards, Laurentiu

[U-Boot] [PATCH v2 1/3] fsl_sec: fix register layout on Layerscape architectures

[laurentiu.tudor at nxp.com]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

On Layerscape architectures the SEC memory map is 1MB and the
register blocks contained in it are 64KB aligned, not 4KB as
the ccsr_sec structure currently assumes. Fix the layout of
the structure for these architectures.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Reviewed-by: Horia Geanta <horia.geanta@nxp.com>
Reviewed-by: Bharat Bhushan <bharat.bhushan@nxp.com>
---
v2:
 - added Reviewed-by tags

 include/fsl_sec.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index 16e3fcb5a1..be08a2b88b 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -121,10 +121,18 @@ typedef struct ccsr_sec {
 	u32	chanum_ls;	/* CHA Number Register, LS */
 	u32	secvid_ms;	/* SEC Version ID Register, MS */
 	u32	secvid_ls;	/* SEC Version ID Register, LS */
+#if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
+	u8	res9[0x6f020];
+#else
 	u8	res9[0x6020];
+#endif
 	u32	qilcr_ms;	/* Queue Interface LIODN CFG Register, MS */
 	u32	qilcr_ls;	/* Queue Interface LIODN CFG Register, LS */
+#if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
+	u8	res10[0x8ffd8];
+#else
 	u8	res10[0x8fd8];
+#endif
 } ccsr_sec_t;
 
 #define SEC_CTPR_MS_AXI_LIODN		0x08000000
-- 
2.17.1

[U-Boot] [PATCH v2 1/3] fsl_sec: fix register layout on Layerscape architectures

[Laurentiu Tudor]

Hello,

Please disregards these 3 patches as I've resubmitted them by mistake.
I've updated their state in patchworks accordingly.

---
Best regards, Laurentiu

On 20.03.2019 16:31, laurentiu.tudor at nxp.com wrote:
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> 
> On Layerscape architectures the SEC memory map is 1MB and the
> register blocks contained in it are 64KB aligned, not 4KB as
> the ccsr_sec structure currently assumes. Fix the layout of
> the structure for these architectures.
> 
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Reviewed-by: Horia Geanta <horia.geanta@nxp.com>
> Reviewed-by: Bharat Bhushan <bharat.bhushan@nxp.com>
> ---
> v2:
>   - added Reviewed-by tags
> 
>   include/fsl_sec.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/include/fsl_sec.h b/include/fsl_sec.h
> index 16e3fcb5a1..be08a2b88b 100644
> --- a/include/fsl_sec.h
> +++ b/include/fsl_sec.h
> @@ -121,10 +121,18 @@ typedef struct ccsr_sec {
>   	u32	chanum_ls;	/* CHA Number Register, LS */
>   	u32	secvid_ms;	/* SEC Version ID Register, MS */
>   	u32	secvid_ls;	/* SEC Version ID Register, LS */
> +#if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
> +	u8	res9[0x6f020];
> +#else
>   	u8	res9[0x6020];
> +#endif
>   	u32	qilcr_ms;	/* Queue Interface LIODN CFG Register, MS */
>   	u32	qilcr_ls;	/* Queue Interface LIODN CFG Register, LS */
> +#if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
> +	u8	res10[0x8ffd8];
> +#else
>   	u8	res10[0x8fd8];
> +#endif
>   } ccsr_sec_t;
>   
>   #define SEC_CTPR_MS_AXI_LIODN		0x08000000
> 

[U-Boot] [PATCH v2 2/3] armv8: fsl-layerscape: fix SEC QI ICID setup

[laurentiu.tudor at nxp.com]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

The SEC QI ICID setup in the QIIC_LS register is actually an offset
that is being added to the ICID coming from the qman portal. Setting
it with a non-zero value breaks SMMU setup as the resulting ICID is
not known. On top of that, the SEC QI ICID must match the qman portal
ICIDs in order to share the isolation context.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Reviewed-by: Horia Geanta <horia.geanta@nxp.com>
Reviewed-by: Bharat Bhushan <bharat.bhushan@nxp.com>
---
v2:
 - added Reviewed-by tags

 arch/arm/cpu/armv8/fsl-layerscape/ls1043_ids.c      | 2 +-
 arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c      | 2 +-
 arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h | 3 +--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1043_ids.c b/arch/arm/cpu/armv8/fsl-layerscape/ls1043_ids.c
index 0e8649427e..3bd993bebf 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/ls1043_ids.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1043_ids.c
@@ -43,7 +43,7 @@ struct icid_id_table icid_tbl[] = {
 	SET_DEBUG_ICID(FSL_DEBUG_STREAM_ID),
 	SET_QE_ICID(FSL_QE_STREAM_ID),
 #ifdef CONFIG_FSL_CAAM
-	SET_SEC_QI_ICID(FSL_DPAA1_STREAM_ID_START + 2),
+	SET_SEC_QI_ICID(FSL_DPAA1_STREAM_ID_END),
 	SET_SEC_JR_ICID_ENTRY(0, FSL_DPAA1_STREAM_ID_START + 3),
 	SET_SEC_JR_ICID_ENTRY(1, FSL_DPAA1_STREAM_ID_START + 4),
 	SET_SEC_JR_ICID_ENTRY(2, FSL_DPAA1_STREAM_ID_START + 5),
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
index 2da9adab5b..abd847b5be 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
@@ -41,7 +41,7 @@ struct icid_id_table icid_tbl[] = {
 	SET_ETR_ICID(FSL_ETR_STREAM_ID),
 	SET_DEBUG_ICID(FSL_DEBUG_STREAM_ID),
 #ifdef CONFIG_FSL_CAAM
-	SET_SEC_QI_ICID(FSL_DPAA1_STREAM_ID_START + 2),
+	SET_SEC_QI_ICID(FSL_DPAA1_STREAM_ID_END),
 	SET_SEC_JR_ICID_ENTRY(0, FSL_DPAA1_STREAM_ID_START + 3),
 	SET_SEC_JR_ICID_ENTRY(1, FSL_DPAA1_STREAM_ID_START + 4),
 	SET_SEC_JR_ICID_ENTRY(2, FSL_DPAA1_STREAM_ID_START + 5),
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
index f375fe7115..e7a8801262 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
@@ -93,8 +93,7 @@ void fdt_fixup_icid(void *blob);
 
 #define SET_SEC_QI_ICID(streamid) \
 	SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \
-		(((streamid) << 16) | (streamid)), \
-		offsetof(ccsr_sec_t, qilcr_ls) + \
+		0, offsetof(ccsr_sec_t, qilcr_ls) + \
 		CONFIG_SYS_FSL_SEC_ADDR, \
 		CONFIG_SYS_FSL_SEC_ADDR)
 
-- 
2.17.1

[U-Boot] [PATCH v2 3/3] armv8: fsl-layerscape: avoid DT fixup warning

[laurentiu.tudor at nxp.com]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

sec_firmware reserves JR3 for it's own usage and deletes the JR3 node
from the device tree. This causes this warning to be issued when doing
the device tree fixup:

WARNING could not find node fsl,sec-v4.0-job-ring: FDT_ERR_NOTFOUND.

Fix it by excluding the device tree fixup for the JR reserved by
sec_firmware.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Reviewed-by: Horia Geanta <horia.geanta@nxp.com>
Reviewed-by: Bharat Bhushan <bharat.bhushan@nxp.com>
---
v2:
 - added Reviewed-by tags

 arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
index e7a8801262..f971af8d26 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
@@ -9,6 +9,7 @@
 #include <asm/types.h>
 #include <fsl_qbman.h>
 #include <fsl_sec.h>
+#include <asm/armv8/sec_firmware.h>
 
 struct icid_id_table {
 	const char *compat;
@@ -98,7 +99,13 @@ void fdt_fixup_icid(void *blob);
 		CONFIG_SYS_FSL_SEC_ADDR)
 
 #define SET_SEC_JR_ICID_ENTRY(jr_num, streamid) \
-	SET_ICID_ENTRY("fsl,sec-v4.0-job-ring", streamid, \
+	SET_ICID_ENTRY( \
+		(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT && \
+		(FSL_SEC_JR##jr_num##_OFFSET ==  \
+			SEC_JR3_OFFSET + CONFIG_SYS_FSL_SEC_OFFSET) \
+			? NULL \
+			: "fsl,sec-v4.0-job-ring"), \
+		streamid, \
 		(((streamid) << 16) | (streamid)), \
 		offsetof(ccsr_sec_t, jrliodnr[jr_num].ls) + \
 		CONFIG_SYS_FSL_SEC_ADDR, \
-- 
2.17.1

[U-Boot] [PATCH 1/2] armv8: fsl-layerscape: add missing sec jr base address defines

[Horia Geanta]

On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote:
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> 
> Add defines for all the SEC job rings base addresses.
> 
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Thanks,
Horia