[U-Boot] [PATCH 1/2] armv8: fsl-layerscape: add missing sec jr base address defines
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
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);
On 3/20/2019 4:31 PM, laurentiu.tudor@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
Hi Horia,
On 21.03.2019 12:36, Horia Geanta wrote:
On 3/20/2019 4:31 PM, laurentiu.tudor@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
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@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
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@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
Hi Horia,
On 21.03.2019 12:36, Horia Geanta wrote:
On 3/20/2019 4:31 PM, laurentiu.tudor@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
Hi Horia,
On 21.03.2019 12:36, Horia Geanta wrote:
On 3/20/2019 4:31 PM, laurentiu.tudor@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-laye...
--- Best Regards, Laurentiu
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
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@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
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)
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, \
On 3/20/2019 4:31 PM, laurentiu.tudor@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
participants (3)
-
Horia Geanta -
Laurentiu Tudor -
laurentiu.tudor＠nxp.com