[PATCH] MIPS: add support for buggy MT7621S core detection

[PATCH] MIPS: add support for buggy MT7621S core detection

[Ilya Lipnitskiy]

Most MT7621 SoCs have 2 cores, which is detected and supported properly
by CPS.

Unfortunately, MT7621 SoC has a less common S variant with only one core.
On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
starting SMP. CPULAUNCH registers can be used in that case to detect the
absence of the second core and override the GCR_CONFIG PCORES field.

Rework a long-standing OpenWrt patch to override the value of
mips_cps_numcores on single-core MT7621 systems.

Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
MT7621 device (Netgear R6220).

Original 4.14 OpenWrt patch:
Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
Current 5.10 OpenWrt patch:
Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904

Suggested-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
---
 arch/mips/include/asm/bugs.h | 18 ++++++++++++++++++
 arch/mips/kernel/smp-cps.c   |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/arch/mips/include/asm/bugs.h b/arch/mips/include/asm/bugs.h
index d72dc6e1cf3c..d32f0c4e61f7 100644
--- a/arch/mips/include/asm/bugs.h
+++ b/arch/mips/include/asm/bugs.h
@@ -16,6 +16,7 @@
 
 #include <asm/cpu.h>
 #include <asm/cpu-info.h>
+#include <asm/mips-boards/launch.h>
 
 extern int daddiu_bug;
 
@@ -50,4 +51,21 @@ static inline int r4k_daddiu_bug(void)
 	return daddiu_bug != 0;
 }
 
+static inline void cm_gcr_pcores_bug(unsigned int *ncores)
+{
+	struct cpulaunch *launch;
+
+	if (!IS_ENABLED(CONFIG_SOC_MT7621) || !ncores)
+		return;
+
+	/*
+	 * Ralink MT7621S SoC is single core, but GCR_CONFIG always reports 2 cores.
+	 * Use legacy amon method to detect if the second core is missing.
+	 */
+	launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
+	launch += 2; /* MT7621 has 2 VPEs per core */
+	if (!(launch->flags & LAUNCH_FREADY))
+		*ncores = 1;
+}
+
 #endif /* _ASM_BUGS_H */
diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index bcd6a944b839..e1e9c11e8a7c 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -15,6 +15,7 @@
 #include <linux/irq.h>
 
 #include <asm/bcache.h>
+#include <asm/bugs.h>
 #include <asm/mips-cps.h>
 #include <asm/mips_mt.h>
 #include <asm/mipsregs.h>
@@ -60,6 +61,7 @@ static void __init cps_smp_setup(void)
 		pr_cont("{");
 
 		ncores = mips_cps_numcores(cl);
+		cm_gcr_pcores_bug(&ncores);
 		for (c = 0; c < ncores; c++) {
 			core_vpes = core_vpe_count(cl, c);
 
@@ -170,6 +172,7 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
 
 	/* Allocate core boot configuration structs */
 	ncores = mips_cps_numcores(0);
+	cm_gcr_pcores_bug(&ncores);
 	mips_cps_core_bootcfg = kcalloc(ncores, sizeof(*mips_cps_core_bootcfg),
 					GFP_KERNEL);
 	if (!mips_cps_core_bootcfg) {
-- 
2.31.1

Re: [PATCH] MIPS: add support for buggy MT7621S core detection

[Maciej W. Rozycki]

On Fri, 2 Apr 2021, Ilya Lipnitskiy wrote:

> diff --git a/arch/mips/include/asm/bugs.h b/arch/mips/include/asm/bugs.h
> index d72dc6e1cf3c..d32f0c4e61f7 100644
> --- a/arch/mips/include/asm/bugs.h
> +++ b/arch/mips/include/asm/bugs.h
> @@ -50,4 +51,21 @@ static inline int r4k_daddiu_bug(void)
>  	return daddiu_bug != 0;
>  }
>  
> +static inline void cm_gcr_pcores_bug(unsigned int *ncores)
> +{
> +	struct cpulaunch *launch;
> +
> +	if (!IS_ENABLED(CONFIG_SOC_MT7621) || !ncores)
> +		return;
> +
> +	/*
> +	 * Ralink MT7621S SoC is single core, but GCR_CONFIG always reports 2 cores.

 Overlong line.

> diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> index bcd6a944b839..e1e9c11e8a7c 100644
> --- a/arch/mips/kernel/smp-cps.c
> +++ b/arch/mips/kernel/smp-cps.c
> @@ -60,6 +61,7 @@ static void __init cps_smp_setup(void)
>  		pr_cont("{");
>  
>  		ncores = mips_cps_numcores(cl);
> +		cm_gcr_pcores_bug(&ncores);
>  		for (c = 0; c < ncores; c++) {
>  			core_vpes = core_vpe_count(cl, c);
>  
> @@ -170,6 +172,7 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
>  
>  	/* Allocate core boot configuration structs */
>  	ncores = mips_cps_numcores(0);
> +	cm_gcr_pcores_bug(&ncores);

 Why called at each `mips_cps_numcores' call site rather than within the 
callee?  Also weird inefficient interface: why isn't `ncores' passed by 
value for a new value to be returned?

  Maciej

Re: [PATCH] MIPS: add support for buggy MT7621S core detection

[Ilya Lipnitskiy]

On Mon, Apr 5, 2021 at 6:22 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Fri, 2 Apr 2021, Ilya Lipnitskiy wrote:
>
> > diff --git a/arch/mips/include/asm/bugs.h b/arch/mips/include/asm/bugs.h
> > index d72dc6e1cf3c..d32f0c4e61f7 100644
> > --- a/arch/mips/include/asm/bugs.h
> > +++ b/arch/mips/include/asm/bugs.h
> > @@ -50,4 +51,21 @@ static inline int r4k_daddiu_bug(void)
> >       return daddiu_bug != 0;
> >  }
> >
> > +static inline void cm_gcr_pcores_bug(unsigned int *ncores)
> > +{
> > +     struct cpulaunch *launch;
> > +
> > +     if (!IS_ENABLED(CONFIG_SOC_MT7621) || !ncores)
> > +             return;
> > +
> > +     /*
> > +      * Ralink MT7621S SoC is single core, but GCR_CONFIG always reports 2 cores.
>
>  Overlong line.
>
> > diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> > index bcd6a944b839..e1e9c11e8a7c 100644
> > --- a/arch/mips/kernel/smp-cps.c
> > +++ b/arch/mips/kernel/smp-cps.c
> > @@ -60,6 +61,7 @@ static void __init cps_smp_setup(void)
> >               pr_cont("{");
> >
> >               ncores = mips_cps_numcores(cl);
> > +             cm_gcr_pcores_bug(&ncores);
> >               for (c = 0; c < ncores; c++) {
> >                       core_vpes = core_vpe_count(cl, c);
> >
> > @@ -170,6 +172,7 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
> >
> >       /* Allocate core boot configuration structs */
> >       ncores = mips_cps_numcores(0);
> > +     cm_gcr_pcores_bug(&ncores);
>
>  Why called at each `mips_cps_numcores' call site rather than within the
> callee?  Also weird inefficient interface: why isn't `ncores' passed by
> value for a new value to be returned?
Thanks for the comments. Including asm/bugs.h in asm/mips-cps.h led to
some circular dependencies when I tried it, but I will try again based
on your feedback - indeed it would be much cleaner to have this logic
in mips_cps_numcores. The only wrinkle is that mips_cps_numcores may
return a different value on MT7621 after the cores have started due to
CPULAUNCH flags changing, but nobody calls mips_cps_numcores later
anyway, so it's a moot point today. I will clean up the change and
resend.

Ilya

Re: [PATCH] MIPS: add support for buggy MT7621S core detection

[Maciej W. Rozycki]

On Mon, 5 Apr 2021, Ilya Lipnitskiy wrote:

> Thanks for the comments. Including asm/bugs.h in asm/mips-cps.h led to
> some circular dependencies when I tried it, but I will try again based
> on your feedback - indeed it would be much cleaner to have this logic
> in mips_cps_numcores. The only wrinkle is that mips_cps_numcores may
> return a different value on MT7621 after the cores have started due to
> CPULAUNCH flags changing, but nobody calls mips_cps_numcores later
> anyway, so it's a moot point today. I will clean up the change and
> resend.

 Hmm, I don't know this system, but by the look of the code it queries 
launch[2], which I gather refers to the VPE #0 of an inexistent core #1, 
so why would the structure change given that there is no corresponding 
silicon?

  Maciej

Re: [PATCH] MIPS: add support for buggy MT7621S core detection

[Ilya Lipnitskiy]

On Wed, Apr 7, 2021 at 6:49 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Mon, 5 Apr 2021, Ilya Lipnitskiy wrote:
>
> > Thanks for the comments. Including asm/bugs.h in asm/mips-cps.h led to
> > some circular dependencies when I tried it, but I will try again based
> > on your feedback - indeed it would be much cleaner to have this logic
> > in mips_cps_numcores. The only wrinkle is that mips_cps_numcores may
> > return a different value on MT7621 after the cores have started due to
> > CPULAUNCH flags changing, but nobody calls mips_cps_numcores later
> > anyway, so it's a moot point today. I will clean up the change and
> > resend.
>
>  Hmm, I don't know this system, but by the look of the code it queries
> launch[2], which I gather refers to the VPE #0 of an inexistent core #1,
> so why would the structure change given that there is no corresponding
> silicon?
The structure would change only on the dual-core flavor of MT7621, the
single-core would always report 1 core, but on the dual-core, if
somebody were to call mips_cps_numcores after the second core had
already started, mips_cps_numcores would return 1 instead of 2. I
think it may be feasible to fix it by checking other flags, but there
is no use case for that today, so I'd rather keep this hacky logic to
a minimum.

Ilya

Re: [PATCH] MIPS: add support for buggy MT7621S core detection

[Strontium]

On 8/4/21 1:49 am, Ilya Lipnitskiy wrote:
> On Wed, Apr 7, 2021 at 6:49 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>> On Mon, 5 Apr 2021, Ilya Lipnitskiy wrote:
>>
>>> Thanks for the comments. Including asm/bugs.h in asm/mips-cps.h led to
>>> some circular dependencies when I tried it, but I will try again based
>>> on your feedback - indeed it would be much cleaner to have this logic
>>> in mips_cps_numcores. The only wrinkle is that mips_cps_numcores may
>>> return a different value on MT7621 after the cores have started due to
>>> CPULAUNCH flags changing, but nobody calls mips_cps_numcores later
>>> anyway, so it's a moot point today. I will clean up the change and
>>> resend.
>>  Hmm, I don't know this system, but by the look of the code it queries
>> launch[2], which I gather refers to the VPE #0 of an inexistent core #1,
>> so why would the structure change given that there is no corresponding
>> silicon?
> The structure would change only on the dual-core flavor of MT7621, the
> single-core would always report 1 core, but on the dual-core, if
> somebody were to call mips_cps_numcores after the second core had
> already started, mips_cps_numcores would return 1 instead of 2. I
> think it may be feasible to fix it by checking other flags, but there
> is no use case for that today, so I'd rather keep this hacky logic to
> a minimum.
>
> Ilya
>
>
Actually,  I am currently struggling with a side effect of this approach
in the original OpenWrt version of this method, although i think this
version will suffer from the same effect. 

When you kexec the kernel from a previously running kernel, it only
detects a single core.  I am about to disable it entirely, as i really
need to be able to run kexec on a MT7621 platform.

I have instrumented the code with some debug to prove it is the case:

Boot from u-boot:

[    0.000000] nclusters = 1
[    0.000000] VPE topology
[    0.000000] cl = 0
[    0.000000] {
[    0.000000] ncores = 2
[    0.000000] cpulaunch.pc = 000000ff
[    0.000000] cpulaunch.gp = 0000ff00
[    0.000000] cpulaunch.sp = 0000ffff
[    0.000000] cpulaunch.a0 = 08000800
[    0.000000] cpulaunch.flags = 00000020
[    0.000000] plat_cpu_core_present(0) = true
[    0.000000] core_vpes = 2
[    0.000000] 2
[    0.000000] cpulaunch.pc = 000000ff
[    0.000000] cpulaunch.gp = 0000ff00
[    0.000000] cpulaunch.sp = 0000ffff
[    0.000000] cpulaunch.a0 = 08000800
[    0.000000] cpulaunch.flags = 00000020
[    0.000000] plat_cpu_core_present(1) = true
[    0.000000] core_vpes = 2
[    0.000000] ,2} total 4

Boot from kexec:

[    0.000000] nclusters = 1
[    0.000000] VPE topology
[    0.000000] cl = 0
[    0.000000] {
[    0.000000] ncores = 2
[    0.000000] cpulaunch.pc = 00000000
[    0.000000] cpulaunch.gp = 00000000
[    0.000000] cpulaunch.sp = 00000000
[    0.000000] cpulaunch.a0 = 00000000
[    0.000000] cpulaunch.flags = 00000000
[    0.000000] plat_cpu_core_present(0) = true
[    0.000000] core_vpes = 2
[    0.000000] 2
[    0.000000] cpulaunch.pc = 00000000
[    0.000000] cpulaunch.gp = 00000000
[    0.000000] cpulaunch.sp = 00000000
[    0.000000] cpulaunch.a0 = 00000000
[    0.000000] cpulaunch.flags = 00000000} total 2

Steven

[PATCH v2] MIPS: add support for buggy MT7621S core detection

[Ilya Lipnitskiy]

Most MT7621 SoCs have 2 cores, which is detected and supported properly
by CPS.

Unfortunately, MT7621 SoC has a less common S variant with only one core.
On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
starting SMP. CPULAUNCH registers can be used in that case to detect the
absence of the second core and override the GCR_CONFIG PCORES field.

Rework a long-standing OpenWrt patch to override the value of
mips_cps_numcores on single-core MT7621 systems.

Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
MT7621 device (Netgear R6220).

Original 4.14 OpenWrt patch:
Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
Current 5.10 OpenWrt patch:
Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904

Suggested-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
---
 arch/mips/include/asm/mips-cps.h | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
index fd43d876892e..9f495ffef2b7 100644
--- a/arch/mips/include/asm/mips-cps.h
+++ b/arch/mips/include/asm/mips-cps.h
@@ -10,6 +10,8 @@
 #include <linux/io.h>
 #include <linux/types.h>
 
+#include <asm/mips-boards/launch.h>
+
 extern unsigned long __cps_access_bad_size(void)
 	__compiletime_error("Bad size for CPS accessor");
 
@@ -165,11 +167,29 @@ static inline uint64_t mips_cps_cluster_config(unsigned int cluster)
  */
 static inline unsigned int mips_cps_numcores(unsigned int cluster)
 {
+	struct cpulaunch *launch;
+	unsigned int ncores;
+
 	if (!mips_cm_present())
 		return 0;
 
 	/* Add one before masking to handle 0xff indicating no cores */
-	return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+	ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+
+	if (IS_ENABLED(CONFIG_SOC_MT7621)) {
+		/*
+		 * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
+		 * always reports 2 cores. Check the second core's LAUNCH_FREADY
+		 * flag to detect if the second core is missing. This method
+		 * only works before the core has been started.
+		 */
+		launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
+		launch += 2; /* MT7621 has 2 VPEs per core */
+		if (!(launch->flags & LAUNCH_FREADY))
+			ncores = 1;
+	}
+
+	return ncores;
 }
 
 /**
-- 
2.31.1

Re: [PATCH v2] MIPS: add support for buggy MT7621S core detection

[Maciej W. Rozycki]

On Mon, 5 Apr 2021, Ilya Lipnitskiy wrote:

> diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
> index fd43d876892e..9f495ffef2b7 100644
> --- a/arch/mips/include/asm/mips-cps.h
> +++ b/arch/mips/include/asm/mips-cps.h
> @@ -165,11 +167,29 @@ static inline uint64_t mips_cps_cluster_config(unsigned int cluster)
>   */
>  static inline unsigned int mips_cps_numcores(unsigned int cluster)
>  {
> +	struct cpulaunch *launch;
> +	unsigned int ncores;
> +
>  	if (!mips_cm_present())
>  		return 0;
>  
>  	/* Add one before masking to handle 0xff indicating no cores */
> -	return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
> +	ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
> +
> +	if (IS_ENABLED(CONFIG_SOC_MT7621)) {
> +		/*
> +		 * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
> +		 * always reports 2 cores. Check the second core's LAUNCH_FREADY
> +		 * flag to detect if the second core is missing. This method
> +		 * only works before the core has been started.
> +		 */
> +		launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
> +		launch += 2; /* MT7621 has 2 VPEs per core */
> +		if (!(launch->flags & LAUNCH_FREADY))
> +			ncores = 1;
> +	}
> +
> +	return ncores;
>  }
>  
>  /**

 Much better to me, but please move the declaration of `launch' into the 
conditional block, which is the only place that uses it.

  Maciej

[PATCH v3] MIPS: add support for buggy MT7621S core detection

[Ilya Lipnitskiy]

Most MT7621 SoCs have 2 cores, which is detected and supported properly
by CPS.

Unfortunately, MT7621 SoC has a less common S variant with only one core.
On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
starting SMP. CPULAUNCH registers can be used in that case to detect the
absence of the second core and override the GCR_CONFIG PCORES field.

Rework a long-standing OpenWrt patch to override the value of
mips_cps_numcores on single-core MT7621 systems.

Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
MT7621 device (Netgear R6220).

Original 4.14 OpenWrt patch:
Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
Current 5.10 OpenWrt patch:
Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904

Suggested-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
---
 arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
index fd43d876892e..35fb8ee6dd33 100644
--- a/arch/mips/include/asm/mips-cps.h
+++ b/arch/mips/include/asm/mips-cps.h
@@ -10,6 +10,8 @@
 #include <linux/io.h>
 #include <linux/types.h>
 
+#include <asm/mips-boards/launch.h>
+
 extern unsigned long __cps_access_bad_size(void)
 	__compiletime_error("Bad size for CPS accessor");
 
@@ -165,11 +167,30 @@ static inline uint64_t mips_cps_cluster_config(unsigned int cluster)
  */
 static inline unsigned int mips_cps_numcores(unsigned int cluster)
 {
+	unsigned int ncores;
+
 	if (!mips_cm_present())
 		return 0;
 
 	/* Add one before masking to handle 0xff indicating no cores */
-	return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+	ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+
+	if (IS_ENABLED(CONFIG_SOC_MT7621)) {
+		struct cpulaunch *launch;
+
+		/*
+		 * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
+		 * always reports 2 cores. Check the second core's LAUNCH_FREADY
+		 * flag to detect if the second core is missing. This method
+		 * only works before the core has been started.
+		 */
+		launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
+		launch += 2; /* MT7621 has 2 VPEs per core */
+		if (!(launch->flags & LAUNCH_FREADY))
+			ncores = 1;
+	}
+
+	return ncores;
 }
 
 /**
-- 
2.31.1

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Thomas Bogendoerfer]

On Wed, Apr 07, 2021 at 01:07:38PM -0700, Ilya Lipnitskiy wrote:
> Most MT7621 SoCs have 2 cores, which is detected and supported properly
> by CPS.
> 
> Unfortunately, MT7621 SoC has a less common S variant with only one core.
> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
> starting SMP. CPULAUNCH registers can be used in that case to detect the
> absence of the second core and override the GCR_CONFIG PCORES field.
> 
> Rework a long-standing OpenWrt patch to override the value of
> mips_cps_numcores on single-core MT7621 systems.
> 
> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
> MT7621 device (Netgear R6220).
> 
> Original 4.14 OpenWrt patch:
> Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
> Current 5.10 OpenWrt patch:
> Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
> 
> Suggested-by: Felix Fietkau <nbd@nbd.name>
> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
> ---
>  arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Greg Ungerer]

Hi Ilya,

> Most MT7621 SoCs have 2 cores, which is detected and supported properly
> by CPS.
> 
> Unfortunately, MT7621 SoC has a less common S variant with only one core.
> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
> starting SMP. CPULAUNCH registers can be used in that case to detect the
> absence of the second core and override the GCR_CONFIG PCORES field.
> 
> Rework a long-standing OpenWrt patch to override the value of
> mips_cps_numcores on single-core MT7621 systems.
> 
> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
> MT7621 device (Netgear R6220).

This is breaking core detection on my MT7621 based platforms.
I only get 2 cores detected now running 5.13. Reverting this change gives
me 4 cores back.

 From a fully working (pre-change) system I get:

# cat /proc/cpuinfo
system type		: MediaTek MT7621 ver:1 eco:3
machine			: Digi EX15
processor		: 0
cpu model		: MIPS 1004Kc V2.15
BogoMIPS		: 586.13
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp mt
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 0
VPE			: 0
VCED exceptions		: not available
VCEI exceptions		: not available

processor		: 1
cpu model		: MIPS 1004Kc V2.15
BogoMIPS		: 586.13
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp mt
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 0
VPE			: 1
VCED exceptions		: not available
VCEI exceptions		: not available

processor		: 2
cpu model		: MIPS 1004Kc V2.15
BogoMIPS		: 586.13
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp mt
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 1
VPE			: 0
VCED exceptions		: not available
VCEI exceptions		: not available

processor		: 3
cpu model		: MIPS 1004Kc V2.15
BogoMIPS		: 586.13
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp mt
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 1
VPE			: 1
VCED exceptions		: not available
VCEI exceptions		: not available


After this patch is applied:

# cat /proc/cpuinfo
system type		: MediaTek MT7621 ver:1 eco:3
machine			: Digi EX15
processor		: 0
cpu model		: MIPS 1004Kc V2.15
BogoMIPS		: 586.13
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp mt
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 0
VPE			: 0
VCED exceptions		: not available
VCEI exceptions		: not available

processor		: 1
cpu model		: MIPS 1004Kc V2.15
BogoMIPS		: 586.13
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp mt
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 0
VPE			: 1
VCED exceptions		: not available
VCEI exceptions		: not available

Any ideas?

Regards
Greg


> Original 4.14 OpenWrt patch:
> Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
> Current 5.10 OpenWrt patch:
> Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
> 
> Suggested-by: Felix Fietkau <nbd@nbd.name>
> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
> ---
>  arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
> index fd43d876892e..35fb8ee6dd33 100644
> --- a/arch/mips/include/asm/mips-cps.h
> +++ b/arch/mips/include/asm/mips-cps.h
> @@ -10,6 +10,8 @@
>  #include <linux/io.h>
>  #include <linux/types.h>
>  
> +#include <asm/mips-boards/launch.h>
> +
>  extern unsigned long __cps_access_bad_size(void)
>  	__compiletime_error("Bad size for CPS accessor");
>  
> @@ -165,11 +167,30 @@ static inline uint64_t mips_cps_cluster_config(unsigned int cluster)
>   */
>  static inline unsigned int mips_cps_numcores(unsigned int cluster)
>  {
> +	unsigned int ncores;
> +
>  	if (!mips_cm_present())
>  		return 0;
>  
>  	/* Add one before masking to handle 0xff indicating no cores */
> -	return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
> +	ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
> +
> +	if (IS_ENABLED(CONFIG_SOC_MT7621)) {
> +		struct cpulaunch *launch;
> +
> +		/*
> +		 * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
> +		 * always reports 2 cores. Check the second core's LAUNCH_FREADY
> +		 * flag to detect if the second core is missing. This method
> +		 * only works before the core has been started.
> +		 */
> +		launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
> +		launch += 2; /* MT7621 has 2 VPEs per core */
> +		if (!(launch->flags & LAUNCH_FREADY))
> +			ncores = 1;
> +	}
> +
> +	return ncores;
>  }
>  
>  /**
> -- 
> 2.31.1

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Sergio Paracuellos]

Hi Greg,

On Thu, Sep 16, 2021 at 4:57 AM Greg Ungerer <gerg@kernel.org> wrote:
>
> Hi Ilya,
>
> > Most MT7621 SoCs have 2 cores, which is detected and supported properly
> > by CPS.
> >
> > Unfortunately, MT7621 SoC has a less common S variant with only one core.
> > On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
> > starting SMP. CPULAUNCH registers can be used in that case to detect the
> > absence of the second core and override the GCR_CONFIG PCORES field.
> >
> > Rework a long-standing OpenWrt patch to override the value of
> > mips_cps_numcores on single-core MT7621 systems.
> >
> > Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
> > MT7621 device (Netgear R6220).
>
> This is breaking core detection on my MT7621 based platforms.
> I only get 2 cores detected now running 5.13. Reverting this change gives
> me 4 cores back.

I also have a 4 core mt7621 based board and this patch change does not
have problems for me. Let me know if you need me to check something
from my board.

I noticed that on boot the following message appears and might be
related with the way used here to make the core number detection:

[    0.000000] VPE topology {2,2} total 4

Is this the same for your board?

Best regards,
     Sergio Paracuellos

>
>  From a fully working (pre-change) system I get:
>
> # cat /proc/cpuinfo
> system type             : MediaTek MT7621 ver:1 eco:3
> machine                 : Digi EX15
> processor               : 0
> cpu model               : MIPS 1004Kc V2.15
> BogoMIPS                : 586.13
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 0
> VPE                     : 0
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
> processor               : 1
> cpu model               : MIPS 1004Kc V2.15
> BogoMIPS                : 586.13
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 0
> VPE                     : 1
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
> processor               : 2
> cpu model               : MIPS 1004Kc V2.15
> BogoMIPS                : 586.13
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 1
> VPE                     : 0
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
> processor               : 3
> cpu model               : MIPS 1004Kc V2.15
> BogoMIPS                : 586.13
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 1
> VPE                     : 1
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
>
> After this patch is applied:
>
> # cat /proc/cpuinfo
> system type             : MediaTek MT7621 ver:1 eco:3
> machine                 : Digi EX15
> processor               : 0
> cpu model               : MIPS 1004Kc V2.15
> BogoMIPS                : 586.13
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 0
> VPE                     : 0
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
> processor               : 1
> cpu model               : MIPS 1004Kc V2.15
> BogoMIPS                : 586.13
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 0
> VPE                     : 1
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
> Any ideas?
>
> Regards
> Greg
>
>
> > Original 4.14 OpenWrt patch:
> > Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
> > Current 5.10 OpenWrt patch:
> > Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
> >
> > Suggested-by: Felix Fietkau <nbd@nbd.name>
> > Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
> > ---
> >  arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
> > index fd43d876892e..35fb8ee6dd33 100644
> > --- a/arch/mips/include/asm/mips-cps.h
> > +++ b/arch/mips/include/asm/mips-cps.h
> > @@ -10,6 +10,8 @@
> >  #include <linux/io.h>
> >  #include <linux/types.h>
> >
> > +#include <asm/mips-boards/launch.h>
> > +
> >  extern unsigned long __cps_access_bad_size(void)
> >       __compiletime_error("Bad size for CPS accessor");
> >
> > @@ -165,11 +167,30 @@ static inline uint64_t mips_cps_cluster_config(unsigned int cluster)
> >   */
> >  static inline unsigned int mips_cps_numcores(unsigned int cluster)
> >  {
> > +     unsigned int ncores;
> > +
> >       if (!mips_cm_present())
> >               return 0;
> >
> >       /* Add one before masking to handle 0xff indicating no cores */
> > -     return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
> > +     ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
> > +
> > +     if (IS_ENABLED(CONFIG_SOC_MT7621)) {
> > +             struct cpulaunch *launch;
> > +
> > +             /*
> > +              * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
> > +              * always reports 2 cores. Check the second core's LAUNCH_FREADY
> > +              * flag to detect if the second core is missing. This method
> > +              * only works before the core has been started.
> > +              */
> > +             launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
> > +             launch += 2; /* MT7621 has 2 VPEs per core */
> > +             if (!(launch->flags & LAUNCH_FREADY))
> > +                     ncores = 1;
> > +     }
> > +
> > +     return ncores;
> >  }
> >
> >  /**
> > --
> > 2.31.1
>

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Greg Ungerer]

Hi Sergio,

On 16/9/21 4:33 pm, Sergio Paracuellos wrote:
> On Thu, Sep 16, 2021 at 4:57 AM Greg Ungerer <gerg@kernel.org> wrote:
>>
>> Hi Ilya,
>>
>>> Most MT7621 SoCs have 2 cores, which is detected and supported properly
>>> by CPS.
>>>
>>> Unfortunately, MT7621 SoC has a less common S variant with only one core.
>>> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
>>> starting SMP. CPULAUNCH registers can be used in that case to detect the
>>> absence of the second core and override the GCR_CONFIG PCORES field.
>>>
>>> Rework a long-standing OpenWrt patch to override the value of
>>> mips_cps_numcores on single-core MT7621 systems.
>>>
>>> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
>>> MT7621 device (Netgear R6220).
>>
>> This is breaking core detection on my MT7621 based platforms.
>> I only get 2 cores detected now running 5.13. Reverting this change gives
>> me 4 cores back.
> 
> I also have a 4 core mt7621 based board and this patch change does not
> have problems for me. Let me know if you need me to check something
> from my board.
> 
> I noticed that on boot the following message appears and might be
> related with the way used here to make the core number detection:
> 
> [    0.000000] VPE topology {2,2} total 4
> 
> Is this the same for your board?

When booting with this patch applied I get:

     VPE topology {2} total 2

with patch reverted I get:

     VPE topology {2,2} total 4

Regards
Greg


> Best regards,
>       Sergio Paracuellos
> 
>>
>>   From a fully working (pre-change) system I get:
>>
>> # cat /proc/cpuinfo
>> system type             : MediaTek MT7621 ver:1 eco:3
>> machine                 : Digi EX15
>> processor               : 0
>> cpu model               : MIPS 1004Kc V2.15
>> BogoMIPS                : 586.13
>> wait instruction        : yes
>> microsecond timers      : yes
>> tlb_entries             : 32
>> extra interrupt vector  : yes
>> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
>> isa                     : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented        : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers      : 0
>> package                 : 0
>> core                    : 0
>> VPE                     : 0
>> VCED exceptions         : not available
>> VCEI exceptions         : not available
>>
>> processor               : 1
>> cpu model               : MIPS 1004Kc V2.15
>> BogoMIPS                : 586.13
>> wait instruction        : yes
>> microsecond timers      : yes
>> tlb_entries             : 32
>> extra interrupt vector  : yes
>> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
>> isa                     : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented        : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers      : 0
>> package                 : 0
>> core                    : 0
>> VPE                     : 1
>> VCED exceptions         : not available
>> VCEI exceptions         : not available
>>
>> processor               : 2
>> cpu model               : MIPS 1004Kc V2.15
>> BogoMIPS                : 586.13
>> wait instruction        : yes
>> microsecond timers      : yes
>> tlb_entries             : 32
>> extra interrupt vector  : yes
>> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
>> isa                     : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented        : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers      : 0
>> package                 : 0
>> core                    : 1
>> VPE                     : 0
>> VCED exceptions         : not available
>> VCEI exceptions         : not available
>>
>> processor               : 3
>> cpu model               : MIPS 1004Kc V2.15
>> BogoMIPS                : 586.13
>> wait instruction        : yes
>> microsecond timers      : yes
>> tlb_entries             : 32
>> extra interrupt vector  : yes
>> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
>> isa                     : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented        : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers      : 0
>> package                 : 0
>> core                    : 1
>> VPE                     : 1
>> VCED exceptions         : not available
>> VCEI exceptions         : not available
>>
>>
>> After this patch is applied:
>>
>> # cat /proc/cpuinfo
>> system type             : MediaTek MT7621 ver:1 eco:3
>> machine                 : Digi EX15
>> processor               : 0
>> cpu model               : MIPS 1004Kc V2.15
>> BogoMIPS                : 586.13
>> wait instruction        : yes
>> microsecond timers      : yes
>> tlb_entries             : 32
>> extra interrupt vector  : yes
>> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
>> isa                     : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented        : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers      : 0
>> package                 : 0
>> core                    : 0
>> VPE                     : 0
>> VCED exceptions         : not available
>> VCEI exceptions         : not available
>>
>> processor               : 1
>> cpu model               : MIPS 1004Kc V2.15
>> BogoMIPS                : 586.13
>> wait instruction        : yes
>> microsecond timers      : yes
>> tlb_entries             : 32
>> extra interrupt vector  : yes
>> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
>> isa                     : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented        : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers      : 0
>> package                 : 0
>> core                    : 0
>> VPE                     : 1
>> VCED exceptions         : not available
>> VCEI exceptions         : not available
>>
>> Any ideas?
>>
>> Regards
>> Greg
>>
>>
>>> Original 4.14 OpenWrt patch:
>>> Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
>>> Current 5.10 OpenWrt patch:
>>> Link: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
>>>
>>> Suggested-by: Felix Fietkau <nbd@nbd.name>
>>> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
>>> ---
>>>   arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
>>>   1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
>>> index fd43d876892e..35fb8ee6dd33 100644
>>> --- a/arch/mips/include/asm/mips-cps.h
>>> +++ b/arch/mips/include/asm/mips-cps.h
>>> @@ -10,6 +10,8 @@
>>>   #include <linux/io.h>
>>>   #include <linux/types.h>
>>>
>>> +#include <asm/mips-boards/launch.h>
>>> +
>>>   extern unsigned long __cps_access_bad_size(void)
>>>        __compiletime_error("Bad size for CPS accessor");
>>>
>>> @@ -165,11 +167,30 @@ static inline uint64_t mips_cps_cluster_config(unsigned int cluster)
>>>    */
>>>   static inline unsigned int mips_cps_numcores(unsigned int cluster)
>>>   {
>>> +     unsigned int ncores;
>>> +
>>>        if (!mips_cm_present())
>>>                return 0;
>>>
>>>        /* Add one before masking to handle 0xff indicating no cores */
>>> -     return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
>>> +     ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
>>> +
>>> +     if (IS_ENABLED(CONFIG_SOC_MT7621)) {
>>> +             struct cpulaunch *launch;
>>> +
>>> +             /*
>>> +              * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
>>> +              * always reports 2 cores. Check the second core's LAUNCH_FREADY
>>> +              * flag to detect if the second core is missing. This method
>>> +              * only works before the core has been started.
>>> +              */
>>> +             launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
>>> +             launch += 2; /* MT7621 has 2 VPEs per core */
>>> +             if (!(launch->flags & LAUNCH_FREADY))
>>> +                     ncores = 1;
>>> +     }
>>> +
>>> +     return ncores;
>>>   }
>>>
>>>   /**
>>> --
>>> 2.31.1
>>

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Strontium]

Hi Greg,

I had trouble with this as well.  This line from the patch:

> if (!(launch->flags & LAUNCH_FREADY))

is checking ram which I believe is supposed to be set by the bootloader.
On my platform it looked like the preloaded uboot wasn't setting that as
expected.
If you have control over your bootloader you can force this ram address
to be what the kernel wants, or you can do what i did, because i didn't
have that targets uboot src, and wedge before the kernel starts to force
the ram to the required state, like so:

#define CORE0_INITIAL_CPU_STATE (0xf00)
#define CORE_FL_OFFSET (0x1C)
#define FLAG_LAUNCH_FREADY (1)

#define WRITEREG(r, v) *(volatile uint32_t *)(r) = v
#define KSEG1ADDR(_x) (((_x)&0x1fffffff) | 0xa0000000)

void set_core(uint32_t core)
{
    uint32_t start = CORE0_INITIAL_CPU_STATE + (0x40 * core);
    WRITEREG(KSEG1ADDR(start + CORE_FL_OFFSET), FLAG_LAUNCH_FREADY);
}

void fix_cores(void) {
    // Fixes the flags for each core, just before running the kernel.
    // Means we don't have to patch the kernel check for valid CPU's.
    for (int i = 0; i < 4; i++) {
        set_core(i);
    }
}

It seems that memory section is supposed to set all the cores registers
before the kernel runs, but i never found it did anything except that 1
flag.

Obviously a better way would be to properly detect the number of cores
and not rely on the boot loader to set a flag in ram, I don't know if
that's even possible.

Steven Johnson

On 16/9/21 09:56, Greg Ungerer wrote:
> Hi Ilya,
>
>> Most MT7621 SoCs have 2 cores, which is detected and supported properly
>> by CPS.
>>
>> Unfortunately, MT7621 SoC has a less common S variant with only one
>> core.
>> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
>> starting SMP. CPULAUNCH registers can be used in that case to detect the
>> absence of the second core and override the GCR_CONFIG PCORES field.
>>
>> Rework a long-standing OpenWrt patch to override the value of
>> mips_cps_numcores on single-core MT7621 systems.
>>
>> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
>> MT7621 device (Netgear R6220).
>
> This is breaking core detection on my MT7621 based platforms.
> I only get 2 cores detected now running 5.13. Reverting this change gives
> me 4 cores back.
>
> From a fully working (pre-change) system I get:
>
> # cat /proc/cpuinfo
> system type        : MediaTek MT7621 ver:1 eco:3
> machine            : Digi EX15
> processor        : 0
> cpu model        : MIPS 1004Kc V2.15
> BogoMIPS        : 586.13
> wait instruction    : yes
> microsecond timers    : yes
> tlb_entries        : 32
> extra interrupt vector    : yes
> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> 0x0ffc, 0x0ffb, 0x0ffb]
> isa            : mips1 mips2 mips32r1 mips32r2
> ASEs implemented    : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers    : 0
> package            : 0
> core            : 0
> VPE            : 0
> VCED exceptions        : not available
> VCEI exceptions        : not available
>
> processor        : 1
> cpu model        : MIPS 1004Kc V2.15
> BogoMIPS        : 586.13
> wait instruction    : yes
> microsecond timers    : yes
> tlb_entries        : 32
> extra interrupt vector    : yes
> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> 0x0ffc, 0x0ffb, 0x0ffb]
> isa            : mips1 mips2 mips32r1 mips32r2
> ASEs implemented    : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers    : 0
> package            : 0
> core            : 0
> VPE            : 1
> VCED exceptions        : not available
> VCEI exceptions        : not available
>
> processor        : 2
> cpu model        : MIPS 1004Kc V2.15
> BogoMIPS        : 586.13
> wait instruction    : yes
> microsecond timers    : yes
> tlb_entries        : 32
> extra interrupt vector    : yes
> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> 0x0ffc, 0x0ffb, 0x0ffb]
> isa            : mips1 mips2 mips32r1 mips32r2
> ASEs implemented    : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers    : 0
> package            : 0
> core            : 1
> VPE            : 0
> VCED exceptions        : not available
> VCEI exceptions        : not available
>
> processor        : 3
> cpu model        : MIPS 1004Kc V2.15
> BogoMIPS        : 586.13
> wait instruction    : yes
> microsecond timers    : yes
> tlb_entries        : 32
> extra interrupt vector    : yes
> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> 0x0ffc, 0x0ffb, 0x0ffb]
> isa            : mips1 mips2 mips32r1 mips32r2
> ASEs implemented    : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers    : 0
> package            : 0
> core            : 1
> VPE            : 1
> VCED exceptions        : not available
> VCEI exceptions        : not available
>
>
> After this patch is applied:
>
> # cat /proc/cpuinfo
> system type        : MediaTek MT7621 ver:1 eco:3
> machine            : Digi EX15
> processor        : 0
> cpu model        : MIPS 1004Kc V2.15
> BogoMIPS        : 586.13
> wait instruction    : yes
> microsecond timers    : yes
> tlb_entries        : 32
> extra interrupt vector    : yes
> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> 0x0ffc, 0x0ffb, 0x0ffb]
> isa            : mips1 mips2 mips32r1 mips32r2
> ASEs implemented    : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers    : 0
> package            : 0
> core            : 0
> VPE            : 0
> VCED exceptions        : not available
> VCEI exceptions        : not available
>
> processor        : 1
> cpu model        : MIPS 1004Kc V2.15
> BogoMIPS        : 586.13
> wait instruction    : yes
> microsecond timers    : yes
> tlb_entries        : 32
> extra interrupt vector    : yes
> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> 0x0ffc, 0x0ffb, 0x0ffb]
> isa            : mips1 mips2 mips32r1 mips32r2
> ASEs implemented    : mips16 dsp mt
> shadow register sets    : 1
> kscratch registers    : 0
> package            : 0
> core            : 0
> VPE            : 1
> VCED exceptions        : not available
> VCEI exceptions        : not available
>
> Any ideas?
>
> Regards
> Greg
>
>
>> Original 4.14 OpenWrt patch:
>> Link:
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
>> Current 5.10 OpenWrt patch:
>> Link:
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
>>
>> Suggested-by: Felix Fietkau <nbd@nbd.name>
>> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
>> ---
>>  arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/include/asm/mips-cps.h
>> b/arch/mips/include/asm/mips-cps.h
>> index fd43d876892e..35fb8ee6dd33 100644
>> --- a/arch/mips/include/asm/mips-cps.h
>> +++ b/arch/mips/include/asm/mips-cps.h
>> @@ -10,6 +10,8 @@
>>  #include <linux/io.h>
>>  #include <linux/types.h>
>>  
>> +#include <asm/mips-boards/launch.h>
>> +
>>  extern unsigned long __cps_access_bad_size(void)
>>      __compiletime_error("Bad size for CPS accessor");
>>  
>> @@ -165,11 +167,30 @@ static inline uint64_t
>> mips_cps_cluster_config(unsigned int cluster)
>>   */
>>  static inline unsigned int mips_cps_numcores(unsigned int cluster)
>>  {
>> +    unsigned int ncores;
>> +
>>      if (!mips_cm_present())
>>          return 0;
>>  
>>      /* Add one before masking to handle 0xff indicating no cores */
>> -    return (mips_cps_cluster_config(cluster) + 1) &
>> CM_GCR_CONFIG_PCORES;
>> +    ncores = (mips_cps_cluster_config(cluster) + 1) &
>> CM_GCR_CONFIG_PCORES;
>> +
>> +    if (IS_ENABLED(CONFIG_SOC_MT7621)) {
>> +        struct cpulaunch *launch;
>> +
>> +        /*
>> +         * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
>> +         * always reports 2 cores. Check the second core's
>> LAUNCH_FREADY
>> +         * flag to detect if the second core is missing. This method
>> +         * only works before the core has been started.
>> +         */
>> +        launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
>> +        launch += 2; /* MT7621 has 2 VPEs per core */
>> +        if (!(launch->flags & LAUNCH_FREADY))
>> +            ncores = 1;
>> +    }
>> +
>> +    return ncores;
>>  }
>>  
>>  /**
>> -- 
>> 2.31.1
>

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Greg Ungerer]

Hi Steven,

On 16/9/21 6:54 pm, Strontium wrote:
> Hi Greg,
> 
> I had trouble with this as well.  This line from the patch:
> 
>> if (!(launch->flags & LAUNCH_FREADY))
> 
> is checking ram which I believe is supposed to be set by the bootloader.
> On my platform it looked like the preloaded uboot wasn't setting that as
> expected.
> If you have control over your bootloader you can force this ram address
> to be what the kernel wants, or you can do what i did, because i didn't
> have that targets uboot src, and wedge before the kernel starts to force
> the ram to the required state, like so:

Well, my solution was to revert that patch locally :-)

But many people will not have control of or the desire to change
their u-boot loader. I would have figured this ends up being a
real regression for many (most?) users of this SoC.


> #define CORE0_INITIAL_CPU_STATE (0xf00)
> #define CORE_FL_OFFSET (0x1C)
> #define FLAG_LAUNCH_FREADY (1)
> 
> #define WRITEREG(r, v) *(volatile uint32_t *)(r) = v
> #define KSEG1ADDR(_x) (((_x)&0x1fffffff) | 0xa0000000)
> 
> void set_core(uint32_t core)
> {
>      uint32_t start = CORE0_INITIAL_CPU_STATE + (0x40 * core);
>      WRITEREG(KSEG1ADDR(start + CORE_FL_OFFSET), FLAG_LAUNCH_FREADY);
> }
> 
> void fix_cores(void) {
>      // Fixes the flags for each core, just before running the kernel.
>      // Means we don't have to patch the kernel check for valid CPU's.
>      for (int i = 0; i < 4; i++) {
>          set_core(i);
>      }
> }
> 
> It seems that memory section is supposed to set all the cores registers
> before the kernel runs, but i never found it did anything except that 1
> flag.
> 
> Obviously a better way would be to properly detect the number of cores
> and not rely on the boot loader to set a flag in ram, I don't know if
> that's even possible.

I can't help but think this commit is not a proper fix for this problem.

Regards
Greg


> Steven Johnson
> 
> On 16/9/21 09:56, Greg Ungerer wrote:
>> Hi Ilya,
>>
>>> Most MT7621 SoCs have 2 cores, which is detected and supported properly
>>> by CPS.
>>>
>>> Unfortunately, MT7621 SoC has a less common S variant with only one
>>> core.
>>> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
>>> starting SMP. CPULAUNCH registers can be used in that case to detect the
>>> absence of the second core and override the GCR_CONFIG PCORES field.
>>>
>>> Rework a long-standing OpenWrt patch to override the value of
>>> mips_cps_numcores on single-core MT7621 systems.
>>>
>>> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
>>> MT7621 device (Netgear R6220).
>>
>> This is breaking core detection on my MT7621 based platforms.
>> I only get 2 cores detected now running 5.13. Reverting this change gives
>> me 4 cores back.
>>
>>  From a fully working (pre-change) system I get:
>>
>> # cat /proc/cpuinfo
>> system type        : MediaTek MT7621 ver:1 eco:3
>> machine            : Digi EX15
>> processor        : 0
>> cpu model        : MIPS 1004Kc V2.15
>> BogoMIPS        : 586.13
>> wait instruction    : yes
>> microsecond timers    : yes
>> tlb_entries        : 32
>> extra interrupt vector    : yes
>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>> 0x0ffc, 0x0ffb, 0x0ffb]
>> isa            : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented    : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers    : 0
>> package            : 0
>> core            : 0
>> VPE            : 0
>> VCED exceptions        : not available
>> VCEI exceptions        : not available
>>
>> processor        : 1
>> cpu model        : MIPS 1004Kc V2.15
>> BogoMIPS        : 586.13
>> wait instruction    : yes
>> microsecond timers    : yes
>> tlb_entries        : 32
>> extra interrupt vector    : yes
>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>> 0x0ffc, 0x0ffb, 0x0ffb]
>> isa            : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented    : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers    : 0
>> package            : 0
>> core            : 0
>> VPE            : 1
>> VCED exceptions        : not available
>> VCEI exceptions        : not available
>>
>> processor        : 2
>> cpu model        : MIPS 1004Kc V2.15
>> BogoMIPS        : 586.13
>> wait instruction    : yes
>> microsecond timers    : yes
>> tlb_entries        : 32
>> extra interrupt vector    : yes
>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>> 0x0ffc, 0x0ffb, 0x0ffb]
>> isa            : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented    : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers    : 0
>> package            : 0
>> core            : 1
>> VPE            : 0
>> VCED exceptions        : not available
>> VCEI exceptions        : not available
>>
>> processor        : 3
>> cpu model        : MIPS 1004Kc V2.15
>> BogoMIPS        : 586.13
>> wait instruction    : yes
>> microsecond timers    : yes
>> tlb_entries        : 32
>> extra interrupt vector    : yes
>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>> 0x0ffc, 0x0ffb, 0x0ffb]
>> isa            : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented    : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers    : 0
>> package            : 0
>> core            : 1
>> VPE            : 1
>> VCED exceptions        : not available
>> VCEI exceptions        : not available
>>
>>
>> After this patch is applied:
>>
>> # cat /proc/cpuinfo
>> system type        : MediaTek MT7621 ver:1 eco:3
>> machine            : Digi EX15
>> processor        : 0
>> cpu model        : MIPS 1004Kc V2.15
>> BogoMIPS        : 586.13
>> wait instruction    : yes
>> microsecond timers    : yes
>> tlb_entries        : 32
>> extra interrupt vector    : yes
>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>> 0x0ffc, 0x0ffb, 0x0ffb]
>> isa            : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented    : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers    : 0
>> package            : 0
>> core            : 0
>> VPE            : 0
>> VCED exceptions        : not available
>> VCEI exceptions        : not available
>>
>> processor        : 1
>> cpu model        : MIPS 1004Kc V2.15
>> BogoMIPS        : 586.13
>> wait instruction    : yes
>> microsecond timers    : yes
>> tlb_entries        : 32
>> extra interrupt vector    : yes
>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>> 0x0ffc, 0x0ffb, 0x0ffb]
>> isa            : mips1 mips2 mips32r1 mips32r2
>> ASEs implemented    : mips16 dsp mt
>> shadow register sets    : 1
>> kscratch registers    : 0
>> package            : 0
>> core            : 0
>> VPE            : 1
>> VCED exceptions        : not available
>> VCEI exceptions        : not available
>>
>> Any ideas?
>>
>> Regards
>> Greg
>>
>>
>>> Original 4.14 OpenWrt patch:
>>> Link:
>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
>>> Current 5.10 OpenWrt patch:
>>> Link:
>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
>>>
>>> Suggested-by: Felix Fietkau <nbd@nbd.name>
>>> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
>>> ---
>>>   arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
>>>   1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/include/asm/mips-cps.h
>>> b/arch/mips/include/asm/mips-cps.h
>>> index fd43d876892e..35fb8ee6dd33 100644
>>> --- a/arch/mips/include/asm/mips-cps.h
>>> +++ b/arch/mips/include/asm/mips-cps.h
>>> @@ -10,6 +10,8 @@
>>>   #include <linux/io.h>
>>>   #include <linux/types.h>
>>>   
>>> +#include <asm/mips-boards/launch.h>
>>> +
>>>   extern unsigned long __cps_access_bad_size(void)
>>>       __compiletime_error("Bad size for CPS accessor");
>>>   
>>> @@ -165,11 +167,30 @@ static inline uint64_t
>>> mips_cps_cluster_config(unsigned int cluster)
>>>    */
>>>   static inline unsigned int mips_cps_numcores(unsigned int cluster)
>>>   {
>>> +    unsigned int ncores;
>>> +
>>>       if (!mips_cm_present())
>>>           return 0;
>>>   
>>>       /* Add one before masking to handle 0xff indicating no cores */
>>> -    return (mips_cps_cluster_config(cluster) + 1) &
>>> CM_GCR_CONFIG_PCORES;
>>> +    ncores = (mips_cps_cluster_config(cluster) + 1) &
>>> CM_GCR_CONFIG_PCORES;
>>> +
>>> +    if (IS_ENABLED(CONFIG_SOC_MT7621)) {
>>> +        struct cpulaunch *launch;
>>> +
>>> +        /*
>>> +         * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
>>> +         * always reports 2 cores. Check the second core's
>>> LAUNCH_FREADY
>>> +         * flag to detect if the second core is missing. This method
>>> +         * only works before the core has been started.
>>> +         */
>>> +        launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
>>> +        launch += 2; /* MT7621 has 2 VPEs per core */
>>> +        if (!(launch->flags & LAUNCH_FREADY))
>>> +            ncores = 1;
>>> +    }
>>> +
>>> +    return ncores;
>>>   }
>>>   
>>>   /**
>>> -- 
>>> 2.31.1
>>
> 

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Sergio Paracuellos]

Hi Greg,

On Thu, Sep 30, 2021 at 3:13 PM Greg Ungerer <gerg@kernel.org> wrote:
>
> Hi Steven,
>
> On 16/9/21 6:54 pm, Strontium wrote:
> > Hi Greg,
> >
> > I had trouble with this as well.  This line from the patch:
> >
> >> if (!(launch->flags & LAUNCH_FREADY))
> >
> > is checking ram which I believe is supposed to be set by the bootloader.
> > On my platform it looked like the preloaded uboot wasn't setting that as
> > expected.
> > If you have control over your bootloader you can force this ram address
> > to be what the kernel wants, or you can do what i did, because i didn't
> > have that targets uboot src, and wedge before the kernel starts to force
> > the ram to the required state, like so:
>
> Well, my solution was to revert that patch locally :-)
>
> But many people will not have control of or the desire to change
> their u-boot loader. I would have figured this ends up being a
> real regression for many (most?) users of this SoC.

Agree.

>
>
> > #define CORE0_INITIAL_CPU_STATE (0xf00)
> > #define CORE_FL_OFFSET (0x1C)
> > #define FLAG_LAUNCH_FREADY (1)
> >
> > #define WRITEREG(r, v) *(volatile uint32_t *)(r) = v
> > #define KSEG1ADDR(_x) (((_x)&0x1fffffff) | 0xa0000000)
> >
> > void set_core(uint32_t core)
> > {
> >      uint32_t start = CORE0_INITIAL_CPU_STATE + (0x40 * core);
> >      WRITEREG(KSEG1ADDR(start + CORE_FL_OFFSET), FLAG_LAUNCH_FREADY);
> > }
> >
> > void fix_cores(void) {
> >      // Fixes the flags for each core, just before running the kernel.
> >      // Means we don't have to patch the kernel check for valid CPU's.
> >      for (int i = 0; i < 4; i++) {
> >          set_core(i);
> >      }
> > }
> >
> > It seems that memory section is supposed to set all the cores registers
> > before the kernel runs, but i never found it did anything except that 1
> > flag.
> >
> > Obviously a better way would be to properly detect the number of cores
> > and not rely on the boot loader to set a flag in ram, I don't know if
> > that's even possible.
>
> I can't help but think this commit is not a proper fix for this problem.

I also do think this commit should be reverted. Ilya, do you have a
strong opinion to maintain it instead?

>
> Regards
> Greg

Best regards,
    Sergio Paracuellos
>
>
> > Steven Johnson
> >
> > On 16/9/21 09:56, Greg Ungerer wrote:
> >> Hi Ilya,
> >>
> >>> Most MT7621 SoCs have 2 cores, which is detected and supported properly
> >>> by CPS.
> >>>
> >>> Unfortunately, MT7621 SoC has a less common S variant with only one
> >>> core.
> >>> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
> >>> starting SMP. CPULAUNCH registers can be used in that case to detect the
> >>> absence of the second core and override the GCR_CONFIG PCORES field.
> >>>
> >>> Rework a long-standing OpenWrt patch to override the value of
> >>> mips_cps_numcores on single-core MT7621 systems.
> >>>
> >>> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
> >>> MT7621 device (Netgear R6220).
> >>
> >> This is breaking core detection on my MT7621 based platforms.
> >> I only get 2 cores detected now running 5.13. Reverting this change gives
> >> me 4 cores back.
> >>
> >>  From a fully working (pre-change) system I get:
> >>
> >> # cat /proc/cpuinfo
> >> system type        : MediaTek MT7621 ver:1 eco:3
> >> machine            : Digi EX15
> >> processor        : 0
> >> cpu model        : MIPS 1004Kc V2.15
> >> BogoMIPS        : 586.13
> >> wait instruction    : yes
> >> microsecond timers    : yes
> >> tlb_entries        : 32
> >> extra interrupt vector    : yes
> >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> >> 0x0ffc, 0x0ffb, 0x0ffb]
> >> isa            : mips1 mips2 mips32r1 mips32r2
> >> ASEs implemented    : mips16 dsp mt
> >> shadow register sets    : 1
> >> kscratch registers    : 0
> >> package            : 0
> >> core            : 0
> >> VPE            : 0
> >> VCED exceptions        : not available
> >> VCEI exceptions        : not available
> >>
> >> processor        : 1
> >> cpu model        : MIPS 1004Kc V2.15
> >> BogoMIPS        : 586.13
> >> wait instruction    : yes
> >> microsecond timers    : yes
> >> tlb_entries        : 32
> >> extra interrupt vector    : yes
> >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> >> 0x0ffc, 0x0ffb, 0x0ffb]
> >> isa            : mips1 mips2 mips32r1 mips32r2
> >> ASEs implemented    : mips16 dsp mt
> >> shadow register sets    : 1
> >> kscratch registers    : 0
> >> package            : 0
> >> core            : 0
> >> VPE            : 1
> >> VCED exceptions        : not available
> >> VCEI exceptions        : not available
> >>
> >> processor        : 2
> >> cpu model        : MIPS 1004Kc V2.15
> >> BogoMIPS        : 586.13
> >> wait instruction    : yes
> >> microsecond timers    : yes
> >> tlb_entries        : 32
> >> extra interrupt vector    : yes
> >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> >> 0x0ffc, 0x0ffb, 0x0ffb]
> >> isa            : mips1 mips2 mips32r1 mips32r2
> >> ASEs implemented    : mips16 dsp mt
> >> shadow register sets    : 1
> >> kscratch registers    : 0
> >> package            : 0
> >> core            : 1
> >> VPE            : 0
> >> VCED exceptions        : not available
> >> VCEI exceptions        : not available
> >>
> >> processor        : 3
> >> cpu model        : MIPS 1004Kc V2.15
> >> BogoMIPS        : 586.13
> >> wait instruction    : yes
> >> microsecond timers    : yes
> >> tlb_entries        : 32
> >> extra interrupt vector    : yes
> >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> >> 0x0ffc, 0x0ffb, 0x0ffb]
> >> isa            : mips1 mips2 mips32r1 mips32r2
> >> ASEs implemented    : mips16 dsp mt
> >> shadow register sets    : 1
> >> kscratch registers    : 0
> >> package            : 0
> >> core            : 1
> >> VPE            : 1
> >> VCED exceptions        : not available
> >> VCEI exceptions        : not available
> >>
> >>
> >> After this patch is applied:
> >>
> >> # cat /proc/cpuinfo
> >> system type        : MediaTek MT7621 ver:1 eco:3
> >> machine            : Digi EX15
> >> processor        : 0
> >> cpu model        : MIPS 1004Kc V2.15
> >> BogoMIPS        : 586.13
> >> wait instruction    : yes
> >> microsecond timers    : yes
> >> tlb_entries        : 32
> >> extra interrupt vector    : yes
> >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> >> 0x0ffc, 0x0ffb, 0x0ffb]
> >> isa            : mips1 mips2 mips32r1 mips32r2
> >> ASEs implemented    : mips16 dsp mt
> >> shadow register sets    : 1
> >> kscratch registers    : 0
> >> package            : 0
> >> core            : 0
> >> VPE            : 0
> >> VCED exceptions        : not available
> >> VCEI exceptions        : not available
> >>
> >> processor        : 1
> >> cpu model        : MIPS 1004Kc V2.15
> >> BogoMIPS        : 586.13
> >> wait instruction    : yes
> >> microsecond timers    : yes
> >> tlb_entries        : 32
> >> extra interrupt vector    : yes
> >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> >> 0x0ffc, 0x0ffb, 0x0ffb]
> >> isa            : mips1 mips2 mips32r1 mips32r2
> >> ASEs implemented    : mips16 dsp mt
> >> shadow register sets    : 1
> >> kscratch registers    : 0
> >> package            : 0
> >> core            : 0
> >> VPE            : 1
> >> VCED exceptions        : not available
> >> VCEI exceptions        : not available
> >>
> >> Any ideas?
> >>
> >> Regards
> >> Greg
> >>
> >>
> >>> Original 4.14 OpenWrt patch:
> >>> Link:
> >>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
> >>> Current 5.10 OpenWrt patch:
> >>> Link:
> >>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
> >>>
> >>> Suggested-by: Felix Fietkau <nbd@nbd.name>
> >>> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
> >>> ---
> >>>   arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
> >>>   1 file changed, 22 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/mips/include/asm/mips-cps.h
> >>> b/arch/mips/include/asm/mips-cps.h
> >>> index fd43d876892e..35fb8ee6dd33 100644
> >>> --- a/arch/mips/include/asm/mips-cps.h
> >>> +++ b/arch/mips/include/asm/mips-cps.h
> >>> @@ -10,6 +10,8 @@
> >>>   #include <linux/io.h>
> >>>   #include <linux/types.h>
> >>>
> >>> +#include <asm/mips-boards/launch.h>
> >>> +
> >>>   extern unsigned long __cps_access_bad_size(void)
> >>>       __compiletime_error("Bad size for CPS accessor");
> >>>
> >>> @@ -165,11 +167,30 @@ static inline uint64_t
> >>> mips_cps_cluster_config(unsigned int cluster)
> >>>    */
> >>>   static inline unsigned int mips_cps_numcores(unsigned int cluster)
> >>>   {
> >>> +    unsigned int ncores;
> >>> +
> >>>       if (!mips_cm_present())
> >>>           return 0;
> >>>
> >>>       /* Add one before masking to handle 0xff indicating no cores */
> >>> -    return (mips_cps_cluster_config(cluster) + 1) &
> >>> CM_GCR_CONFIG_PCORES;
> >>> +    ncores = (mips_cps_cluster_config(cluster) + 1) &
> >>> CM_GCR_CONFIG_PCORES;
> >>> +
> >>> +    if (IS_ENABLED(CONFIG_SOC_MT7621)) {
> >>> +        struct cpulaunch *launch;
> >>> +
> >>> +        /*
> >>> +         * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
> >>> +         * always reports 2 cores. Check the second core's
> >>> LAUNCH_FREADY
> >>> +         * flag to detect if the second core is missing. This method
> >>> +         * only works before the core has been started.
> >>> +         */
> >>> +        launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
> >>> +        launch += 2; /* MT7621 has 2 VPEs per core */
> >>> +        if (!(launch->flags & LAUNCH_FREADY))
> >>> +            ncores = 1;
> >>> +    }
> >>> +
> >>> +    return ncores;
> >>>   }
> >>>
> >>>   /**
> >>> --
> >>> 2.31.1
> >>
> >

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Ilya Lipnitskiy]

Hi Sergio, Greg, Steven,

On Thu, Sep 30, 2021 at 6:35 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Greg,
>
> On Thu, Sep 30, 2021 at 3:13 PM Greg Ungerer <gerg@kernel.org> wrote:
> >
> > Hi Steven,
> >
> > On 16/9/21 6:54 pm, Strontium wrote:
> > > Hi Greg,
> > >
> > > I had trouble with this as well.  This line from the patch:
> > >
> > >> if (!(launch->flags & LAUNCH_FREADY))
> > >
> > > is checking ram which I believe is supposed to be set by the bootloader.
> > > On my platform it looked like the preloaded uboot wasn't setting that as
> > > expected.
> > > If you have control over your bootloader you can force this ram address
> > > to be what the kernel wants, or you can do what i did, because i didn't
> > > have that targets uboot src, and wedge before the kernel starts to force
> > > the ram to the required state, like so:
> >
> > Well, my solution was to revert that patch locally :-)
> >
> > But many people will not have control of or the desire to change
> > their u-boot loader. I would have figured this ends up being a
> > real regression for many (most?) users of this SoC.
>
> Agree.
>
> >
> >
> > > #define CORE0_INITIAL_CPU_STATE (0xf00)
> > > #define CORE_FL_OFFSET (0x1C)
> > > #define FLAG_LAUNCH_FREADY (1)
> > >
> > > #define WRITEREG(r, v) *(volatile uint32_t *)(r) = v
> > > #define KSEG1ADDR(_x) (((_x)&0x1fffffff) | 0xa0000000)
> > >
> > > void set_core(uint32_t core)
> > > {
> > >      uint32_t start = CORE0_INITIAL_CPU_STATE + (0x40 * core);
> > >      WRITEREG(KSEG1ADDR(start + CORE_FL_OFFSET), FLAG_LAUNCH_FREADY);
> > > }
> > >
> > > void fix_cores(void) {
> > >      // Fixes the flags for each core, just before running the kernel.
> > >      // Means we don't have to patch the kernel check for valid CPU's.
> > >      for (int i = 0; i < 4; i++) {
> > >          set_core(i);
> > >      }
> > > }
> > >
> > > It seems that memory section is supposed to set all the cores registers
> > > before the kernel runs, but i never found it did anything except that 1
> > > flag.
> > >
> > > Obviously a better way would be to properly detect the number of cores
> > > and not rely on the boot loader to set a flag in ram, I don't know if
> > > that's even possible.
> >
> > I can't help but think this commit is not a proper fix for this problem.
>
> I also do think this commit should be reverted. Ilya, do you have a
> strong opinion to maintain it instead?
Not a strong opinion - I think we need a better fix that would work on
the platform I tested with as well as Greg's. I'm okay with reverting
it while trying to come up with said fix. Downstream projects, such as
OpenWrt can keep this patch or apply it only when building for MT7621S
targets until the detection logic is made more robust.

Greg - if and when a proper fix is made, a test against your platform
would help so we don't regress again in the future.

Ilya
>
> >
> > Regards
> > Greg
>
> Best regards,
>     Sergio Paracuellos
> >
> >
> > > Steven Johnson
> > >
> > > On 16/9/21 09:56, Greg Ungerer wrote:
> > >> Hi Ilya,
> > >>
> > >>> Most MT7621 SoCs have 2 cores, which is detected and supported properly
> > >>> by CPS.
> > >>>
> > >>> Unfortunately, MT7621 SoC has a less common S variant with only one
> > >>> core.
> > >>> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
> > >>> starting SMP. CPULAUNCH registers can be used in that case to detect the
> > >>> absence of the second core and override the GCR_CONFIG PCORES field.
> > >>>
> > >>> Rework a long-standing OpenWrt patch to override the value of
> > >>> mips_cps_numcores on single-core MT7621 systems.
> > >>>
> > >>> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
> > >>> MT7621 device (Netgear R6220).
> > >>
> > >> This is breaking core detection on my MT7621 based platforms.
> > >> I only get 2 cores detected now running 5.13. Reverting this change gives
> > >> me 4 cores back.
> > >>
> > >>  From a fully working (pre-change) system I get:
> > >>
> > >> # cat /proc/cpuinfo
> > >> system type        : MediaTek MT7621 ver:1 eco:3
> > >> machine            : Digi EX15
> > >> processor        : 0
> > >> cpu model        : MIPS 1004Kc V2.15
> > >> BogoMIPS        : 586.13
> > >> wait instruction    : yes
> > >> microsecond timers    : yes
> > >> tlb_entries        : 32
> > >> extra interrupt vector    : yes
> > >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> > >> 0x0ffc, 0x0ffb, 0x0ffb]
> > >> isa            : mips1 mips2 mips32r1 mips32r2
> > >> ASEs implemented    : mips16 dsp mt
> > >> shadow register sets    : 1
> > >> kscratch registers    : 0
> > >> package            : 0
> > >> core            : 0
> > >> VPE            : 0
> > >> VCED exceptions        : not available
> > >> VCEI exceptions        : not available
> > >>
> > >> processor        : 1
> > >> cpu model        : MIPS 1004Kc V2.15
> > >> BogoMIPS        : 586.13
> > >> wait instruction    : yes
> > >> microsecond timers    : yes
> > >> tlb_entries        : 32
> > >> extra interrupt vector    : yes
> > >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> > >> 0x0ffc, 0x0ffb, 0x0ffb]
> > >> isa            : mips1 mips2 mips32r1 mips32r2
> > >> ASEs implemented    : mips16 dsp mt
> > >> shadow register sets    : 1
> > >> kscratch registers    : 0
> > >> package            : 0
> > >> core            : 0
> > >> VPE            : 1
> > >> VCED exceptions        : not available
> > >> VCEI exceptions        : not available
> > >>
> > >> processor        : 2
> > >> cpu model        : MIPS 1004Kc V2.15
> > >> BogoMIPS        : 586.13
> > >> wait instruction    : yes
> > >> microsecond timers    : yes
> > >> tlb_entries        : 32
> > >> extra interrupt vector    : yes
> > >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> > >> 0x0ffc, 0x0ffb, 0x0ffb]
> > >> isa            : mips1 mips2 mips32r1 mips32r2
> > >> ASEs implemented    : mips16 dsp mt
> > >> shadow register sets    : 1
> > >> kscratch registers    : 0
> > >> package            : 0
> > >> core            : 1
> > >> VPE            : 0
> > >> VCED exceptions        : not available
> > >> VCEI exceptions        : not available
> > >>
> > >> processor        : 3
> > >> cpu model        : MIPS 1004Kc V2.15
> > >> BogoMIPS        : 586.13
> > >> wait instruction    : yes
> > >> microsecond timers    : yes
> > >> tlb_entries        : 32
> > >> extra interrupt vector    : yes
> > >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> > >> 0x0ffc, 0x0ffb, 0x0ffb]
> > >> isa            : mips1 mips2 mips32r1 mips32r2
> > >> ASEs implemented    : mips16 dsp mt
> > >> shadow register sets    : 1
> > >> kscratch registers    : 0
> > >> package            : 0
> > >> core            : 1
> > >> VPE            : 1
> > >> VCED exceptions        : not available
> > >> VCEI exceptions        : not available
> > >>
> > >>
> > >> After this patch is applied:
> > >>
> > >> # cat /proc/cpuinfo
> > >> system type        : MediaTek MT7621 ver:1 eco:3
> > >> machine            : Digi EX15
> > >> processor        : 0
> > >> cpu model        : MIPS 1004Kc V2.15
> > >> BogoMIPS        : 586.13
> > >> wait instruction    : yes
> > >> microsecond timers    : yes
> > >> tlb_entries        : 32
> > >> extra interrupt vector    : yes
> > >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> > >> 0x0ffc, 0x0ffb, 0x0ffb]
> > >> isa            : mips1 mips2 mips32r1 mips32r2
> > >> ASEs implemented    : mips16 dsp mt
> > >> shadow register sets    : 1
> > >> kscratch registers    : 0
> > >> package            : 0
> > >> core            : 0
> > >> VPE            : 0
> > >> VCED exceptions        : not available
> > >> VCEI exceptions        : not available
> > >>
> > >> processor        : 1
> > >> cpu model        : MIPS 1004Kc V2.15
> > >> BogoMIPS        : 586.13
> > >> wait instruction    : yes
> > >> microsecond timers    : yes
> > >> tlb_entries        : 32
> > >> extra interrupt vector    : yes
> > >> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
> > >> 0x0ffc, 0x0ffb, 0x0ffb]
> > >> isa            : mips1 mips2 mips32r1 mips32r2
> > >> ASEs implemented    : mips16 dsp mt
> > >> shadow register sets    : 1
> > >> kscratch registers    : 0
> > >> package            : 0
> > >> core            : 0
> > >> VPE            : 1
> > >> VCED exceptions        : not available
> > >> VCEI exceptions        : not available
> > >>
> > >> Any ideas?
> > >>
> > >> Regards
> > >> Greg
> > >>
> > >>
> > >>> Original 4.14 OpenWrt patch:
> > >>> Link:
> > >>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
> > >>> Current 5.10 OpenWrt patch:
> > >>> Link:
> > >>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
> > >>>
> > >>> Suggested-by: Felix Fietkau <nbd@nbd.name>
> > >>> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
> > >>> ---
> > >>>   arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
> > >>>   1 file changed, 22 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/arch/mips/include/asm/mips-cps.h
> > >>> b/arch/mips/include/asm/mips-cps.h
> > >>> index fd43d876892e..35fb8ee6dd33 100644
> > >>> --- a/arch/mips/include/asm/mips-cps.h
> > >>> +++ b/arch/mips/include/asm/mips-cps.h
> > >>> @@ -10,6 +10,8 @@
> > >>>   #include <linux/io.h>
> > >>>   #include <linux/types.h>
> > >>>
> > >>> +#include <asm/mips-boards/launch.h>
> > >>> +
> > >>>   extern unsigned long __cps_access_bad_size(void)
> > >>>       __compiletime_error("Bad size for CPS accessor");
> > >>>
> > >>> @@ -165,11 +167,30 @@ static inline uint64_t
> > >>> mips_cps_cluster_config(unsigned int cluster)
> > >>>    */
> > >>>   static inline unsigned int mips_cps_numcores(unsigned int cluster)
> > >>>   {
> > >>> +    unsigned int ncores;
> > >>> +
> > >>>       if (!mips_cm_present())
> > >>>           return 0;
> > >>>
> > >>>       /* Add one before masking to handle 0xff indicating no cores */
> > >>> -    return (mips_cps_cluster_config(cluster) + 1) &
> > >>> CM_GCR_CONFIG_PCORES;
> > >>> +    ncores = (mips_cps_cluster_config(cluster) + 1) &
> > >>> CM_GCR_CONFIG_PCORES;
> > >>> +
> > >>> +    if (IS_ENABLED(CONFIG_SOC_MT7621)) {
> > >>> +        struct cpulaunch *launch;
> > >>> +
> > >>> +        /*
> > >>> +         * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
> > >>> +         * always reports 2 cores. Check the second core's
> > >>> LAUNCH_FREADY
> > >>> +         * flag to detect if the second core is missing. This method
> > >>> +         * only works before the core has been started.
> > >>> +         */
> > >>> +        launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
> > >>> +        launch += 2; /* MT7621 has 2 VPEs per core */
> > >>> +        if (!(launch->flags & LAUNCH_FREADY))
> > >>> +            ncores = 1;
> > >>> +    }
> > >>> +
> > >>> +    return ncores;
> > >>>   }
> > >>>
> > >>>   /**
> > >>> --
> > >>> 2.31.1
> > >>
> > >

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Greg Ungerer]

Hi Ilya,

On 1/10/21 2:41 am, Ilya Lipnitskiy wrote:
> Hi Sergio, Greg, Steven,
> 
> On Thu, Sep 30, 2021 at 6:35 AM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>>
>> Hi Greg,
>>
>> On Thu, Sep 30, 2021 at 3:13 PM Greg Ungerer <gerg@kernel.org> wrote:
>>>
>>> Hi Steven,
>>>
>>> On 16/9/21 6:54 pm, Strontium wrote:
>>>> Hi Greg,
>>>>
>>>> I had trouble with this as well.  This line from the patch:
>>>>
>>>>> if (!(launch->flags & LAUNCH_FREADY))
>>>>
>>>> is checking ram which I believe is supposed to be set by the bootloader.
>>>> On my platform it looked like the preloaded uboot wasn't setting that as
>>>> expected.
>>>> If you have control over your bootloader you can force this ram address
>>>> to be what the kernel wants, or you can do what i did, because i didn't
>>>> have that targets uboot src, and wedge before the kernel starts to force
>>>> the ram to the required state, like so:
>>>
>>> Well, my solution was to revert that patch locally :-)
>>>
>>> But many people will not have control of or the desire to change
>>> their u-boot loader. I would have figured this ends up being a
>>> real regression for many (most?) users of this SoC.
>>
>> Agree.
>>
>>>
>>>
>>>> #define CORE0_INITIAL_CPU_STATE (0xf00)
>>>> #define CORE_FL_OFFSET (0x1C)
>>>> #define FLAG_LAUNCH_FREADY (1)
>>>>
>>>> #define WRITEREG(r, v) *(volatile uint32_t *)(r) = v
>>>> #define KSEG1ADDR(_x) (((_x)&0x1fffffff) | 0xa0000000)
>>>>
>>>> void set_core(uint32_t core)
>>>> {
>>>>       uint32_t start = CORE0_INITIAL_CPU_STATE + (0x40 * core);
>>>>       WRITEREG(KSEG1ADDR(start + CORE_FL_OFFSET), FLAG_LAUNCH_FREADY);
>>>> }
>>>>
>>>> void fix_cores(void) {
>>>>       // Fixes the flags for each core, just before running the kernel.
>>>>       // Means we don't have to patch the kernel check for valid CPU's.
>>>>       for (int i = 0; i < 4; i++) {
>>>>           set_core(i);
>>>>       }
>>>> }
>>>>
>>>> It seems that memory section is supposed to set all the cores registers
>>>> before the kernel runs, but i never found it did anything except that 1
>>>> flag.
>>>>
>>>> Obviously a better way would be to properly detect the number of cores
>>>> and not rely on the boot loader to set a flag in ram, I don't know if
>>>> that's even possible.
>>>
>>> I can't help but think this commit is not a proper fix for this problem.
>>
>> I also do think this commit should be reverted. Ilya, do you have a
>> strong opinion to maintain it instead?
> Not a strong opinion - I think we need a better fix that would work on
> the platform I tested with as well as Greg's. I'm okay with reverting
> it while trying to come up with said fix. Downstream projects, such as
> OpenWrt can keep this patch or apply it only when building for MT7621S
> targets until the detection logic is made more robust.
> 
> Greg - if and when a proper fix is made, a test against your platform
> would help so we don't regress again in the future.

Yep, no problem. Please CC me on any prospective patch and I can test.

Regards
Greg


> Ilya
>>
>>>
>>> Regards
>>> Greg
>>
>> Best regards,
>>      Sergio Paracuellos
>>>
>>>
>>>> Steven Johnson
>>>>
>>>> On 16/9/21 09:56, Greg Ungerer wrote:
>>>>> Hi Ilya,
>>>>>
>>>>>> Most MT7621 SoCs have 2 cores, which is detected and supported properly
>>>>>> by CPS.
>>>>>>
>>>>>> Unfortunately, MT7621 SoC has a less common S variant with only one
>>>>>> core.
>>>>>> On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
>>>>>> starting SMP. CPULAUNCH registers can be used in that case to detect the
>>>>>> absence of the second core and override the GCR_CONFIG PCORES field.
>>>>>>
>>>>>> Rework a long-standing OpenWrt patch to override the value of
>>>>>> mips_cps_numcores on single-core MT7621 systems.
>>>>>>
>>>>>> Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
>>>>>> MT7621 device (Netgear R6220).
>>>>>
>>>>> This is breaking core detection on my MT7621 based platforms.
>>>>> I only get 2 cores detected now running 5.13. Reverting this change gives
>>>>> me 4 cores back.
>>>>>
>>>>>   From a fully working (pre-change) system I get:
>>>>>
>>>>> # cat /proc/cpuinfo
>>>>> system type        : MediaTek MT7621 ver:1 eco:3
>>>>> machine            : Digi EX15
>>>>> processor        : 0
>>>>> cpu model        : MIPS 1004Kc V2.15
>>>>> BogoMIPS        : 586.13
>>>>> wait instruction    : yes
>>>>> microsecond timers    : yes
>>>>> tlb_entries        : 32
>>>>> extra interrupt vector    : yes
>>>>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>>>>> 0x0ffc, 0x0ffb, 0x0ffb]
>>>>> isa            : mips1 mips2 mips32r1 mips32r2
>>>>> ASEs implemented    : mips16 dsp mt
>>>>> shadow register sets    : 1
>>>>> kscratch registers    : 0
>>>>> package            : 0
>>>>> core            : 0
>>>>> VPE            : 0
>>>>> VCED exceptions        : not available
>>>>> VCEI exceptions        : not available
>>>>>
>>>>> processor        : 1
>>>>> cpu model        : MIPS 1004Kc V2.15
>>>>> BogoMIPS        : 586.13
>>>>> wait instruction    : yes
>>>>> microsecond timers    : yes
>>>>> tlb_entries        : 32
>>>>> extra interrupt vector    : yes
>>>>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>>>>> 0x0ffc, 0x0ffb, 0x0ffb]
>>>>> isa            : mips1 mips2 mips32r1 mips32r2
>>>>> ASEs implemented    : mips16 dsp mt
>>>>> shadow register sets    : 1
>>>>> kscratch registers    : 0
>>>>> package            : 0
>>>>> core            : 0
>>>>> VPE            : 1
>>>>> VCED exceptions        : not available
>>>>> VCEI exceptions        : not available
>>>>>
>>>>> processor        : 2
>>>>> cpu model        : MIPS 1004Kc V2.15
>>>>> BogoMIPS        : 586.13
>>>>> wait instruction    : yes
>>>>> microsecond timers    : yes
>>>>> tlb_entries        : 32
>>>>> extra interrupt vector    : yes
>>>>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>>>>> 0x0ffc, 0x0ffb, 0x0ffb]
>>>>> isa            : mips1 mips2 mips32r1 mips32r2
>>>>> ASEs implemented    : mips16 dsp mt
>>>>> shadow register sets    : 1
>>>>> kscratch registers    : 0
>>>>> package            : 0
>>>>> core            : 1
>>>>> VPE            : 0
>>>>> VCED exceptions        : not available
>>>>> VCEI exceptions        : not available
>>>>>
>>>>> processor        : 3
>>>>> cpu model        : MIPS 1004Kc V2.15
>>>>> BogoMIPS        : 586.13
>>>>> wait instruction    : yes
>>>>> microsecond timers    : yes
>>>>> tlb_entries        : 32
>>>>> extra interrupt vector    : yes
>>>>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>>>>> 0x0ffc, 0x0ffb, 0x0ffb]
>>>>> isa            : mips1 mips2 mips32r1 mips32r2
>>>>> ASEs implemented    : mips16 dsp mt
>>>>> shadow register sets    : 1
>>>>> kscratch registers    : 0
>>>>> package            : 0
>>>>> core            : 1
>>>>> VPE            : 1
>>>>> VCED exceptions        : not available
>>>>> VCEI exceptions        : not available
>>>>>
>>>>>
>>>>> After this patch is applied:
>>>>>
>>>>> # cat /proc/cpuinfo
>>>>> system type        : MediaTek MT7621 ver:1 eco:3
>>>>> machine            : Digi EX15
>>>>> processor        : 0
>>>>> cpu model        : MIPS 1004Kc V2.15
>>>>> BogoMIPS        : 586.13
>>>>> wait instruction    : yes
>>>>> microsecond timers    : yes
>>>>> tlb_entries        : 32
>>>>> extra interrupt vector    : yes
>>>>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>>>>> 0x0ffc, 0x0ffb, 0x0ffb]
>>>>> isa            : mips1 mips2 mips32r1 mips32r2
>>>>> ASEs implemented    : mips16 dsp mt
>>>>> shadow register sets    : 1
>>>>> kscratch registers    : 0
>>>>> package            : 0
>>>>> core            : 0
>>>>> VPE            : 0
>>>>> VCED exceptions        : not available
>>>>> VCEI exceptions        : not available
>>>>>
>>>>> processor        : 1
>>>>> cpu model        : MIPS 1004Kc V2.15
>>>>> BogoMIPS        : 586.13
>>>>> wait instruction    : yes
>>>>> microsecond timers    : yes
>>>>> tlb_entries        : 32
>>>>> extra interrupt vector    : yes
>>>>> hardware watchpoint    : yes, count: 4, address/irw mask: [0x0ffc,
>>>>> 0x0ffc, 0x0ffb, 0x0ffb]
>>>>> isa            : mips1 mips2 mips32r1 mips32r2
>>>>> ASEs implemented    : mips16 dsp mt
>>>>> shadow register sets    : 1
>>>>> kscratch registers    : 0
>>>>> package            : 0
>>>>> core            : 0
>>>>> VPE            : 1
>>>>> VCED exceptions        : not available
>>>>> VCEI exceptions        : not available
>>>>>
>>>>> Any ideas?
>>>>>
>>>>> Regards
>>>>> Greg
>>>>>
>>>>>
>>>>>> Original 4.14 OpenWrt patch:
>>>>>> Link:
>>>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
>>>>>> Current 5.10 OpenWrt patch:
>>>>>> Link:
>>>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904
>>>>>>
>>>>>> Suggested-by: Felix Fietkau <nbd@nbd.name>
>>>>>> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
>>>>>> ---
>>>>>>    arch/mips/include/asm/mips-cps.h | 23 ++++++++++++++++++++++-
>>>>>>    1 file changed, 22 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/mips/include/asm/mips-cps.h
>>>>>> b/arch/mips/include/asm/mips-cps.h
>>>>>> index fd43d876892e..35fb8ee6dd33 100644
>>>>>> --- a/arch/mips/include/asm/mips-cps.h
>>>>>> +++ b/arch/mips/include/asm/mips-cps.h
>>>>>> @@ -10,6 +10,8 @@
>>>>>>    #include <linux/io.h>
>>>>>>    #include <linux/types.h>
>>>>>>
>>>>>> +#include <asm/mips-boards/launch.h>
>>>>>> +
>>>>>>    extern unsigned long __cps_access_bad_size(void)
>>>>>>        __compiletime_error("Bad size for CPS accessor");
>>>>>>
>>>>>> @@ -165,11 +167,30 @@ static inline uint64_t
>>>>>> mips_cps_cluster_config(unsigned int cluster)
>>>>>>     */
>>>>>>    static inline unsigned int mips_cps_numcores(unsigned int cluster)
>>>>>>    {
>>>>>> +    unsigned int ncores;
>>>>>> +
>>>>>>        if (!mips_cm_present())
>>>>>>            return 0;
>>>>>>
>>>>>>        /* Add one before masking to handle 0xff indicating no cores */
>>>>>> -    return (mips_cps_cluster_config(cluster) + 1) &
>>>>>> CM_GCR_CONFIG_PCORES;
>>>>>> +    ncores = (mips_cps_cluster_config(cluster) + 1) &
>>>>>> CM_GCR_CONFIG_PCORES;
>>>>>> +
>>>>>> +    if (IS_ENABLED(CONFIG_SOC_MT7621)) {
>>>>>> +        struct cpulaunch *launch;
>>>>>> +
>>>>>> +        /*
>>>>>> +         * Ralink MT7621S SoC is single core, but the GCR_CONFIG method
>>>>>> +         * always reports 2 cores. Check the second core's
>>>>>> LAUNCH_FREADY
>>>>>> +         * flag to detect if the second core is missing. This method
>>>>>> +         * only works before the core has been started.
>>>>>> +         */
>>>>>> +        launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
>>>>>> +        launch += 2; /* MT7621 has 2 VPEs per core */
>>>>>> +        if (!(launch->flags & LAUNCH_FREADY))
>>>>>> +            ncores = 1;
>>>>>> +    }
>>>>>> +
>>>>>> +    return ncores;
>>>>>>    }
>>>>>>
>>>>>>    /**
>>>>>> --
>>>>>> 2.31.1
>>>>>
>>>>

Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection

[Sergio Paracuellos]

Hi Steven,

On Fri, Oct 1, 2021 at 4:02 AM Strontium <strntydog@gmail.com> wrote:
>
> On 30/9/21 23:41, Ilya Lipnitskiy wrote:
> > Hi Sergio, Greg, Steven,
> >
> > On Thu, Sep 30, 2021 at 6:35 AM Sergio Paracuellos
> > <sergio.paracuellos@gmail.com> wrote:
> >> Hi Greg,
> >>
> >> On Thu, Sep 30, 2021 at 3:13 PM Greg Ungerer <gerg@kernel.org> wrote:
> >>> Hi Steven,
> >>>
> >>> On 16/9/21 6:54 pm, Strontium wrote:
> >>>> Hi Greg,
> >>>>
> >>>> I had trouble with this as well.  This line from the patch:
> >>>>
> >>>>> if (!(launch->flags & LAUNCH_FREADY))
> >>>> is checking ram which I believe is supposed to be set by the bootloader.
> >>>> On my platform it looked like the preloaded uboot wasn't setting that as
> >>>> expected.
> >>>> If you have control over your bootloader you can force this ram address
> >>>> to be what the kernel wants, or you can do what i did, because i didn't
> >>>> have that targets uboot src, and wedge before the kernel starts to force
> >>>> the ram to the required state, like so:
> >>> Well, my solution was to revert that patch locally :-)
> >>>
> >>> But many people will not have control of or the desire to change
> >>> their u-boot loader. I would have figured this ends up being a
> >>> real regression for many (most?) users of this SoC.
> >> Agree.
> >>
> >>>
> >>>> #define CORE0_INITIAL_CPU_STATE (0xf00)
> >>>> #define CORE_FL_OFFSET (0x1C)
> >>>> #define FLAG_LAUNCH_FREADY (1)
> >>>>
> >>>> #define WRITEREG(r, v) *(volatile uint32_t *)(r) = v
> >>>> #define KSEG1ADDR(_x) (((_x)&0x1fffffff) | 0xa0000000)
> >>>>
> >>>> void set_core(uint32_t core)
> >>>> {
> >>>>      uint32_t start = CORE0_INITIAL_CPU_STATE + (0x40 * core);
> >>>>      WRITEREG(KSEG1ADDR(start + CORE_FL_OFFSET), FLAG_LAUNCH_FREADY);
> >>>> }
> >>>>
> >>>> void fix_cores(void) {
> >>>>      // Fixes the flags for each core, just before running the kernel.
> >>>>      // Means we don't have to patch the kernel check for valid CPU's.
> >>>>      for (int i = 0; i < 4; i++) {
> >>>>          set_core(i);
> >>>>      }
> >>>> }
> >>>>
> >>>> It seems that memory section is supposed to set all the cores registers
> >>>> before the kernel runs, but i never found it did anything except that 1
> >>>> flag.
> >>>>
> >>>> Obviously a better way would be to properly detect the number of cores
> >>>> and not rely on the boot loader to set a flag in ram, I don't know if
> >>>> that's even possible.
> >>> I can't help but think this commit is not a proper fix for this problem.
> >> I also do think this commit should be reverted. Ilya, do you have a
> >> strong opinion to maintain it instead?
> > Not a strong opinion - I think we need a better fix that would work on
> > the platform I tested with as well as Greg's. I'm okay with reverting
> > it while trying to come up with said fix. Downstream projects, such as
> > OpenWrt can keep this patch or apply it only when building for MT7621S
> > targets until the detection logic is made more robust.
> >
> > Greg - if and when a proper fix is made, a test against your platform
> > would help so we don't regress again in the future.
> >
> > Ilya
>
> Hi Ilya, Sergio and Greg,
>
> Could we, instead of checking data passed from the bootloader check
> something set in the device tree?
>
> For example currently `linux/drivers/staging/mt7621-dts/mt7621.dtsi`
> defines:
>     cpus {
>         cpu@0 {
>             compatible = "mips,mips1004Kc";
>         };
>
>         cpu@1 {
>             compatible = "mips,mips1004Kc";
>         };
>     };
>
>
> But that's not true for an mt7621s.  For this device, it should be defined:
>
>     cpus {
>         cpu@0 {
>             compatible = "mips,mips1004Kc";
>         };
>     };
>
>
> And if it was, the code that detects the cpu cores could check this and
> enable either the number of cores it probes, or the number of cpu's
> defined by the device tree, whichever is the lesser.
>
> Then Downstream just needs to properly set up the cpu in the device tree
> for the effected targets and it should work.
>
> If something like this is acceptable, I would be happy to propose a
> patch along these lines for testing.

I guess this would become a new mt7621s.dts that only include the
other original mt7621.dtsi and just overlay cpus. But I think we can
check the register related with chip name and so on [0] and see if
something is different in order to set 'soc' related code and
attributes to checkable values. Check [1] and [2]. Ilya, maybe you can
check whaever value is in there to see if the "S" stuff is different
with other normal mt7621 chips?

Best regards,
     Sergio Paracuellos

[0]: https://elixir.bootlin.com/linux/latest/source/arch/mips/include/asm/mach-ralink/mt7621.h#L15
[1]: https://elixir.bootlin.com/linux/latest/source/arch/mips/ralink/mt7621.c#L86
[2]: https://elixir.bootlin.com/linux/latest/source/arch/mips/ralink/mt7621.c#L59

>
> Steven