All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] arm: mach-k3: am62: Add core number autodetection and fix fdt
@ 2023-07-12 13:47 Francesco Dolcini
  2023-07-12 13:47 ` [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC Francesco Dolcini
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-12 13:47 UTC (permalink / raw)
  To: u-boot, Tom Rini; +Cc: Francesco Dolcini, Vignesh Raghavendra, Andrew Davis

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(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC
  2023-07-12 13:47 [PATCH v1 0/3] arm: mach-k3: am62: Add core number autodetection and fix fdt Francesco Dolcini
@ 2023-07-12 13:47 ` Francesco Dolcini
  2023-07-12 14:15   ` Andrew Davis
  2023-07-12 13:47 ` [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition Francesco Dolcini
  2023-07-12 13:47 ` [PATCH v1 3/3] arm: mach-k3: am62: Fixup CPU core counts in fdt Francesco Dolcini
  2 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-12 13:47 UTC (permalink / raw)
  To: u-boot, Tom Rini
  Cc: Emanuele Ghidoli, Vignesh Raghavendra, Andrew Davis, Francesco Dolcini

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;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition
  2023-07-12 13:47 [PATCH v1 0/3] arm: mach-k3: am62: Add core number autodetection and fix fdt Francesco Dolcini
  2023-07-12 13:47 ` [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC Francesco Dolcini
@ 2023-07-12 13:47 ` Francesco Dolcini
  2023-07-12 14:18   ` Andrew Davis
  2023-07-12 13:47 ` [PATCH v1 3/3] arm: mach-k3: am62: Fixup CPU core counts in fdt Francesco Dolcini
  2 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-12 13:47 UTC (permalink / raw)
  To: u-boot, Tom Rini
  Cc: Emanuele Ghidoli, Vignesh Raghavendra, Andrew Davis, Francesco Dolcini

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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 3/3] arm: mach-k3: am62: Fixup CPU core counts in fdt
  2023-07-12 13:47 [PATCH v1 0/3] arm: mach-k3: am62: Add core number autodetection and fix fdt Francesco Dolcini
  2023-07-12 13:47 ` [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC Francesco Dolcini
  2023-07-12 13:47 ` [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition Francesco Dolcini
@ 2023-07-12 13:47 ` Francesco Dolcini
  2 siblings, 0 replies; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-12 13:47 UTC (permalink / raw)
  To: u-boot, Tom Rini
  Cc: Emanuele Ghidoli, Vignesh Raghavendra, Andrew Davis, Francesco Dolcini

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)) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC
  2023-07-12 13:47 ` [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC Francesco Dolcini
@ 2023-07-12 14:15   ` Andrew Davis
  2023-07-12 14:32     ` Francesco Dolcini
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Davis @ 2023-07-12 14:15 UTC (permalink / raw)
  To: Francesco Dolcini, u-boot, Tom Rini
  Cc: Emanuele Ghidoli, Vignesh Raghavendra, Francesco Dolcini

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;
>   }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition
  2023-07-12 13:47 ` [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition Francesco Dolcini
@ 2023-07-12 14:18   ` Andrew Davis
  2023-07-12 14:25     ` Francesco Dolcini
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Davis @ 2023-07-12 14:18 UTC (permalink / raw)
  To: Francesco Dolcini, u-boot, Tom Rini
  Cc: Emanuele Ghidoli, Vignesh Raghavendra, Francesco Dolcini

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition
  2023-07-12 14:18   ` Andrew Davis
@ 2023-07-12 14:25     ` Francesco Dolcini
  0 siblings, 0 replies; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-12 14:25 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Francesco Dolcini, u-boot, Tom Rini, Emanuele Ghidoli,
	Vignesh Raghavendra, Francesco Dolcini

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC
  2023-07-12 14:15   ` Andrew Davis
@ 2023-07-12 14:32     ` Francesco Dolcini
  2023-07-12 14:38       ` Andrew Davis
  0 siblings, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-12 14:32 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Francesco Dolcini, u-boot, Tom Rini, Emanuele Ghidoli,
	Vignesh Raghavendra, Francesco Dolcini

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC
  2023-07-12 14:32     ` Francesco Dolcini
@ 2023-07-12 14:38       ` Andrew Davis
  2023-07-13  6:17         ` Francesco Dolcini
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Davis @ 2023-07-12 14:38 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: u-boot, Tom Rini, Emanuele Ghidoli, Vignesh Raghavendra,
	Francesco Dolcini

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC
  2023-07-12 14:38       ` Andrew Davis
@ 2023-07-13  6:17         ` Francesco Dolcini
  0 siblings, 0 replies; 10+ messages in thread
From: Francesco Dolcini @ 2023-07-13  6:17 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Francesco Dolcini, u-boot, Tom Rini, Emanuele Ghidoli,
	Vignesh Raghavendra, Francesco Dolcini

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-07-13  6:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12 13:47 [PATCH v1 0/3] arm: mach-k3: am62: Add core number autodetection and fix fdt Francesco Dolcini
2023-07-12 13:47 ` [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC Francesco Dolcini
2023-07-12 14:15   ` Andrew Davis
2023-07-12 14:32     ` Francesco Dolcini
2023-07-12 14:38       ` Andrew Davis
2023-07-13  6:17         ` Francesco Dolcini
2023-07-12 13:47 ` [PATCH v1 2/3] arm: mach-k3: am62: Add CTRLMMR_WKUP_JTAG_DEVICE_ID register definition Francesco Dolcini
2023-07-12 14:18   ` Andrew Davis
2023-07-12 14:25     ` Francesco Dolcini
2023-07-12 13:47 ` [PATCH v1 3/3] arm: mach-k3: am62: Fixup CPU core counts in fdt Francesco Dolcini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.