[PATCH v1 0/3] arm: mach-k3: am62: Add core number autodetection and fix fdt
From: Francesco Dolcini francesco.dolcini@toradex.com
AM62x SoC is available in multiple variant with a different amount of CPU cores (Cortex-A) available, AM62x1, AM62x2, AM62x4 have respectively 1, 2 or 4 cores.
Update the FDT with the actual core count as read from the SoC registers, with that change is possible to have a single dts/dtb file handling the different variant at runtime.
A similar approach is implemented for example on i.MX8 and STM32MP1 SoC.
Since we start using ft_system_setup on a SoC without msmc ram and fdt_fixup_msmc_ram function fails on those SoC (triggering a reset loop) the first commit fix it by calling that function only on specific SoC.
Emanuele Ghidoli (3): arm: k3: Fix ft_system_setup so it can be enabled on any SoC arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition arm: mach-k3: am62: Fixup CPU core counts in fdt
arch/arm/mach-k3/common.c | 67 ++++++++++++++++--- arch/arm/mach-k3/include/mach/am62_hardware.h | 18 +++++ 2 files changed, 77 insertions(+), 8 deletions(-)
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
ft_system_setup cannot be enabled on SoC without msmc sram otherwise fdt_fixup_msmc_ram function fails causing system reset.
Fix by calling fdt_fixup_msmc_ram only on these specific SoC: - J721S2 - AM654 - J721E
This change was verified to not change anything on any existing board (all the J721S2, AM654 and J721E boards requires it, none of the remaining k3 boards require it).
Fixes: 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level") Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com --- arch/arm/mach-k3/common.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index 34737a43aa08..f86ccaedc94f 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -433,14 +433,18 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name) #if defined(CONFIG_OF_SYSTEM_SETUP) int ft_system_setup(void *blob, struct bd_info *bd) { - int ret; - - ret = fdt_fixup_msmc_ram(blob, "/bus@100000", "sram@70000000"); - if (ret < 0) - ret = fdt_fixup_msmc_ram(blob, "/interconnect@100000", - "sram@70000000"); - if (ret) - printf("%s: fixing up msmc ram failed %d\n", __func__, ret); + int ret = 0; + + if (IS_ENABLED(CONFIG_SOC_K3_J721S2) || + IS_ENABLED(CONFIG_SOC_K3_AM654) || + IS_ENABLED(CONFIG_SOC_K3_J721E)) { + ret = fdt_fixup_msmc_ram(blob, "/bus@100000", "sram@70000000"); + if (ret < 0) + ret = fdt_fixup_msmc_ram(blob, "/interconnect@100000", + "sram@70000000"); + if (ret) + printf("%s: fixing up msmc ram failed %d\n", __func__, ret); + }
return ret; }
On 7/12/23 8:47 AM, Francesco Dolcini wrote:
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
ft_system_setup cannot be enabled on SoC without msmc sram otherwise fdt_fixup_msmc_ram function fails causing system reset.
Fix by calling fdt_fixup_msmc_ram only on these specific SoC:
- J721S2
- AM654
- J721E
This change was verified to not change anything on any existing board (all the J721S2, AM654 and J721E boards requires it, none of the remaining k3 boards require it).
Fixes: 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level") Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com
arch/arm/mach-k3/common.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index 34737a43aa08..f86ccaedc94f 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -433,14 +433,18 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name) #if defined(CONFIG_OF_SYSTEM_SETUP) int ft_system_setup(void *blob, struct bd_info *bd)
How about we rename this function to something like ft_msmc_sram_setup(), then only call it from board_setup for the platforms that needed it.
Andrew
{
- int ret;
- ret = fdt_fixup_msmc_ram(blob, "/bus@100000", "sram@70000000");
- if (ret < 0)
ret = fdt_fixup_msmc_ram(blob, "/interconnect@100000","sram@70000000");- if (ret)
printf("%s: fixing up msmc ram failed %d\n", __func__, ret);
int ret = 0;
if (IS_ENABLED(CONFIG_SOC_K3_J721S2) ||
IS_ENABLED(CONFIG_SOC_K3_AM654) ||IS_ENABLED(CONFIG_SOC_K3_J721E)) {ret = fdt_fixup_msmc_ram(blob, "/bus@100000", "sram@70000000");if (ret < 0)ret = fdt_fixup_msmc_ram(blob, "/interconnect@100000","sram@70000000");if (ret)printf("%s: fixing up msmc ram failed %d\n", __func__, ret);}
return ret; }
On Wed, Jul 12, 2023 at 09:15:21AM -0500, Andrew Davis wrote:
On 7/12/23 8:47 AM, Francesco Dolcini wrote:
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
ft_system_setup cannot be enabled on SoC without msmc sram otherwise fdt_fixup_msmc_ram function fails causing system reset.
Fix by calling fdt_fixup_msmc_ram only on these specific SoC:
- J721S2
- AM654
- J721E
This change was verified to not change anything on any existing board (all the J721S2, AM654 and J721E boards requires it, none of the remaining k3 boards require it).
Fixes: 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level") Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com
arch/arm/mach-k3/common.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index 34737a43aa08..f86ccaedc94f 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -433,14 +433,18 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name) #if defined(CONFIG_OF_SYSTEM_SETUP) int ft_system_setup(void *blob, struct bd_info *bd)
How about we rename this function to something like ft_msmc_sram_setup(), then only call it from board_setup for the platforms that needed it.
This is partially reverting what you did in commit 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level").
Is this fixup going to be required if tomorrow we have a new board using the AM654 SoC (for example) or not? Depending on this answer we can decide if ft_board_setup or ft_system_setup is the right function to use.
Francesco
On 7/12/23 9:32 AM, Francesco Dolcini wrote:
On Wed, Jul 12, 2023 at 09:15:21AM -0500, Andrew Davis wrote:
On 7/12/23 8:47 AM, Francesco Dolcini wrote:
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
ft_system_setup cannot be enabled on SoC without msmc sram otherwise fdt_fixup_msmc_ram function fails causing system reset.
Fix by calling fdt_fixup_msmc_ram only on these specific SoC:
- J721S2
- AM654
- J721E
This change was verified to not change anything on any existing board (all the J721S2, AM654 and J721E boards requires it, none of the remaining k3 boards require it).
Fixes: 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level") Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com
arch/arm/mach-k3/common.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index 34737a43aa08..f86ccaedc94f 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -433,14 +433,18 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name) #if defined(CONFIG_OF_SYSTEM_SETUP) int ft_system_setup(void *blob, struct bd_info *bd)
How about we rename this function to something like ft_msmc_sram_setup(), then only call it from board_setup for the platforms that needed it.
This is partially reverting what you did in commit 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level").
Is this fixup going to be required if tomorrow we have a new board using the AM654 SoC (for example) or not? Depending on this answer we can decide if ft_board_setup or ft_system_setup is the right function to use.
Maybe the issue is that we have a common ft_system_setup() function for all of K3, when these should be specific to each SoC.. They could call into common functions for common tasks like MSMC fixup, but no need to have one big ft_system_setup() for all K3.
Andrew
On Wed, Jul 12, 2023 at 09:38:33AM -0500, Andrew Davis wrote:
On 7/12/23 9:32 AM, Francesco Dolcini wrote:
On Wed, Jul 12, 2023 at 09:15:21AM -0500, Andrew Davis wrote:
On 7/12/23 8:47 AM, Francesco Dolcini wrote:
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
ft_system_setup cannot be enabled on SoC without msmc sram otherwise fdt_fixup_msmc_ram function fails causing system reset.
Fix by calling fdt_fixup_msmc_ram only on these specific SoC:
- J721S2
- AM654
- J721E
This change was verified to not change anything on any existing board (all the J721S2, AM654 and J721E boards requires it, none of the remaining k3 boards require it).
Fixes: 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level") Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com
arch/arm/mach-k3/common.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index 34737a43aa08..f86ccaedc94f 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -433,14 +433,18 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name) #if defined(CONFIG_OF_SYSTEM_SETUP) int ft_system_setup(void *blob, struct bd_info *bd)
How about we rename this function to something like ft_msmc_sram_setup(), then only call it from board_setup for the platforms that needed it.
This is partially reverting what you did in commit 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level").
Is this fixup going to be required if tomorrow we have a new board using the AM654 SoC (for example) or not? Depending on this answer we can decide if ft_board_setup or ft_system_setup is the right function to use.
Maybe the issue is that we have a common ft_system_setup() function for all of K3, when these should be specific to each SoC.. They could call into common functions for common tasks like MSMC fixup, but no need to have one big ft_system_setup() for all K3.
Looks fair, we'll send a v2 with something like
common_fdt.c - fdt_fixup_msmc_ram() goes here - compiled for every SoC
SoC specific file, compiled only for the required SoC, ft_system_setup() is defined here and eventually use shared code from common_fdt.
- am625_fdt.c - am654_fdt.c - j721e_fdt.c - j721s2_fdt.c
Francesco
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
Add register address and relevant bitmasks and shifts. Allow reading these information: - device identification - number of cores (part of device identification) - features (currently: PRU / no PRU) - security - functional safety - speed grade - temperature grade - package
Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com --- arch/arm/mach-k3/include/mach/am62_hardware.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h index 88d58947269a..d67045a697df 100644 --- a/arch/arm/mach-k3/include/mach/am62_hardware.h +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h @@ -20,6 +20,24 @@ #define MCU_CTRL_MMR0_BASE 0x04500000 #define WKUP_CTRL_MMR0_BASE 0x43000000
+#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18) +#define JTAG_DEV_ID_MASK GENMASK(31, 18) +#define JTAG_DEV_ID_SHIFT 18 +#define JTAG_DEV_CORE_NR_MASK GENMASK(21, 19) +#define JTAG_DEV_CORE_NR_SHIFT 19 +#define JTAG_DEV_FEATURES_MASK GENMASK(17, 13) +#define JTAG_DEV_FEATURES_SHIFT 13 +#define JTAG_DEV_SECURITY_MASK BIT(12) +#define JTAG_DEV_SECURITY_SHIFT 12 +#define JTAG_DEV_SAFETY_MASK BIT(11) +#define JTAG_DEV_SAFETY_SHIFT 11 +#define JTAG_DEV_SPEED_MASK GENMASK(10, 6) +#define JTAG_DEV_SPEED_SHIFT 6 +#define JTAG_DEV_TEMP_MASK GENMASK(5, 3) +#define JTAG_DEV_TEMP_SHIFT 3 +#define JTAG_DEV_PKG_MASK GENMASK(2, 0) +#define JTAG_DEV_PKG_SHIFT 0 + #define CTRLMMR_MAIN_DEVSTAT (WKUP_CTRL_MMR0_BASE + 0x30) #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK GENMASK(6, 3) #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT 3
On 7/12/23 8:47 AM, Francesco Dolcini wrote:
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
Add register address and relevant bitmasks and shifts. Allow reading these information:
- device identification
- number of cores (part of device identification)
- features (currently: PRU / no PRU)
- security
- functional safety
- speed grade
- temperature grade
- package
Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com
arch/arm/mach-k3/include/mach/am62_hardware.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h index 88d58947269a..d67045a697df 100644 --- a/arch/arm/mach-k3/include/mach/am62_hardware.h +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h @@ -20,6 +20,24 @@ #define MCU_CTRL_MMR0_BASE 0x04500000 #define WKUP_CTRL_MMR0_BASE 0x43000000
+#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18)
We have similar defines in arch/arm/mach-k3/include/mach/hardware.h, if these are common that might be a better spot.
Andrew
+#define JTAG_DEV_ID_MASK GENMASK(31, 18) +#define JTAG_DEV_ID_SHIFT 18 +#define JTAG_DEV_CORE_NR_MASK GENMASK(21, 19) +#define JTAG_DEV_CORE_NR_SHIFT 19 +#define JTAG_DEV_FEATURES_MASK GENMASK(17, 13) +#define JTAG_DEV_FEATURES_SHIFT 13 +#define JTAG_DEV_SECURITY_MASK BIT(12) +#define JTAG_DEV_SECURITY_SHIFT 12 +#define JTAG_DEV_SAFETY_MASK BIT(11) +#define JTAG_DEV_SAFETY_SHIFT 11 +#define JTAG_DEV_SPEED_MASK GENMASK(10, 6) +#define JTAG_DEV_SPEED_SHIFT 6 +#define JTAG_DEV_TEMP_MASK GENMASK(5, 3) +#define JTAG_DEV_TEMP_SHIFT 3 +#define JTAG_DEV_PKG_MASK GENMASK(2, 0) +#define JTAG_DEV_PKG_SHIFT 0
- #define CTRLMMR_MAIN_DEVSTAT (WKUP_CTRL_MMR0_BASE + 0x30) #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK GENMASK(6, 3) #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT 3
On Wed, Jul 12, 2023 at 09:18:42AM -0500, Andrew Davis wrote:
On 7/12/23 8:47 AM, Francesco Dolcini wrote:
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
Add register address and relevant bitmasks and shifts. Allow reading these information:
- device identification
- number of cores (part of device identification)
- features (currently: PRU / no PRU)
- security
- functional safety
- speed grade
- temperature grade
- package
Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com
arch/arm/mach-k3/include/mach/am62_hardware.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h index 88d58947269a..d67045a697df 100644 --- a/arch/arm/mach-k3/include/mach/am62_hardware.h +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h @@ -20,6 +20,24 @@ #define MCU_CTRL_MMR0_BASE 0x04500000 #define WKUP_CTRL_MMR0_BASE 0x43000000 +#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18)
We have similar defines in arch/arm/mach-k3/include/mach/hardware.h, if these are common that might be a better spot.
I do not have complete visibility on the whole k3 architecture and I can only test on AM62. From what we were able to understand AM65 has the exact same register, but the actual content is just different.
Should we just move CTRLMMR_WKUP_JTAG_DEVICE_ID to mach-k3/include/mach/hardware.h ?
Francesco
From: Emanuele Ghidoli emanuele.ghidoli@toradex.com
AM62x SoC is available in multiple variant with a different amount of CPU cores (Cortex-A) available, AM62x1, AM62x2, AM62x4 have respectively 1, 2 or 4 cores.
Update the FDT with the actual core count as read from the SoC registers, with that change is possible to have a single dts/dtb file handling the different variant at runtime.
A similar approach is implemented for example on i.MX8 and STM32MP1 SoC.
Signed-off-by: Emanuele Ghidoli emanuele.ghidoli@toradex.com Signed-off-by: Francesco Dolcini francesco.dolcini@toradex.com --- arch/arm/mach-k3/common.c | 48 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index f86ccaedc94f..5ee1851e3aaa 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -431,10 +431,58 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name) }
#if defined(CONFIG_OF_SYSTEM_SETUP) +static int fdt_del_node_path(void *blob, const char *path) +{ + int rc; + int nodeoff; + + nodeoff = fdt_path_offset(blob, path); + if (nodeoff < 0) + return 0; /* Not found, skip it */ + + rc = fdt_del_node(blob, nodeoff); + if (rc < 0) + printf("Unable to delete node %s, err=%s\n", path, fdt_strerror(rc)); + else + debug("Deleted node %s\n", path); + return rc; +} + +static void fdt_fixup_cores_nodes_am625(void *blob, int core_nr) +{ + char node_path[32]; + + if (core_nr < 1) + return; + + for (; core_nr < 4; core_nr++) { + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", core_nr); + fdt_del_node_path(blob, node_path); + snprintf(node_path, sizeof(node_path), "/cpus/cpu-map/cluster0/core%d", core_nr); + fdt_del_node_path(blob, node_path); + snprintf(node_path, sizeof(node_path), "/bus@f0000/watchdog@e0%d0000", core_nr); + fdt_del_node_path(blob, node_path); + } +} + +static int k3_get_core_nr(void) +{ +#if defined(CONFIG_SOC_K3_AM625) + u32 full_devid = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); + + return (full_devid & JTAG_DEV_CORE_NR_MASK) >> JTAG_DEV_CORE_NR_SHIFT; +#else + return -1; +#endif +} + int ft_system_setup(void *blob, struct bd_info *bd) { int ret = 0;
+ if (IS_ENABLED(CONFIG_SOC_K3_AM625)) + fdt_fixup_cores_nodes_am625(blob, k3_get_core_nr()); + if (IS_ENABLED(CONFIG_SOC_K3_J721S2) || IS_ENABLED(CONFIG_SOC_K3_AM654) || IS_ENABLED(CONFIG_SOC_K3_J721E)) {
participants (2)
-
Andrew Davis -
Francesco Dolcini