All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] MIPS: add support for buggy MT7621S core detection
@ 2021-04-03  6:19 Ilya Lipnitskiy
  2021-04-06  1:22 ` Maciej W. Rozycki
  0 siblings, 1 reply; 10+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-03  6:19 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Wei Li, Tiezhu Yang, linux-mips, linux-kernel
  Cc: Ilya Lipnitskiy, Felix Fietkau

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


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

* Re: [PATCH] MIPS: add support for buggy MT7621S core detection
  2021-04-03  6:19 [PATCH] MIPS: add support for buggy MT7621S core detection Ilya Lipnitskiy
@ 2021-04-06  1:22 ` Maciej W. Rozycki
  2021-04-06  1:54   ` Ilya Lipnitskiy
  2021-04-06  4:23   ` [PATCH v2] " Ilya Lipnitskiy
  0 siblings, 2 replies; 10+ messages in thread
From: Maciej W. Rozycki @ 2021-04-06  1:22 UTC (permalink / raw)
  To: Ilya Lipnitskiy
  Cc: Thomas Bogendoerfer, Wei Li, Tiezhu Yang, linux-mips,
	linux-kernel, Felix Fietkau

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

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

* Re: [PATCH] MIPS: add support for buggy MT7621S core detection
  2021-04-06  1:22 ` Maciej W. Rozycki
@ 2021-04-06  1:54   ` Ilya Lipnitskiy
  2021-04-07 13:49     ` Maciej W. Rozycki
  2021-04-06  4:23   ` [PATCH v2] " Ilya Lipnitskiy
  1 sibling, 1 reply; 10+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-06  1:54 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, Wei Li, Tiezhu Yang, linux-mips,
	Linux Kernel Mailing List, Felix Fietkau

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

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

* [PATCH v2] MIPS: add support for buggy MT7621S core detection
  2021-04-06  1:22 ` Maciej W. Rozycki
  2021-04-06  1:54   ` Ilya Lipnitskiy
@ 2021-04-06  4:23   ` Ilya Lipnitskiy
  2021-04-07 13:52     ` Maciej W. Rozycki
  1 sibling, 1 reply; 10+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-06  4:23 UTC (permalink / raw)
  To: macro
  Cc: ilya.lipnitskiy, linux-kernel, linux-mips, liwei391, nbd,
	tsbogend, yangtiezhu

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


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

* Re: [PATCH] MIPS: add support for buggy MT7621S core detection
  2021-04-06  1:54   ` Ilya Lipnitskiy
@ 2021-04-07 13:49     ` Maciej W. Rozycki
  2021-04-07 18:49       ` Ilya Lipnitskiy
  0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2021-04-07 13:49 UTC (permalink / raw)
  To: Ilya Lipnitskiy
  Cc: Thomas Bogendoerfer, Wei Li, Tiezhu Yang, linux-mips,
	Linux Kernel Mailing List, Felix Fietkau

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

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

* Re: [PATCH v2] MIPS: add support for buggy MT7621S core detection
  2021-04-06  4:23   ` [PATCH v2] " Ilya Lipnitskiy
@ 2021-04-07 13:52     ` Maciej W. Rozycki
  2021-04-07 20:07       ` [PATCH v3] " Ilya Lipnitskiy
  0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2021-04-07 13:52 UTC (permalink / raw)
  To: Ilya Lipnitskiy
  Cc: linux-kernel, linux-mips, liwei391, nbd, Thomas Bogendoerfer, yangtiezhu

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

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

* Re: [PATCH] MIPS: add support for buggy MT7621S core detection
  2021-04-07 13:49     ` Maciej W. Rozycki
@ 2021-04-07 18:49       ` Ilya Lipnitskiy
  2021-06-25 10:54         ` Strontium
  0 siblings, 1 reply; 10+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-07 18:49 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, Wei Li, Tiezhu Yang, linux-mips,
	Linux Kernel Mailing List, Felix Fietkau

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

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

* [PATCH v3] MIPS: add support for buggy MT7621S core detection
  2021-04-07 13:52     ` Maciej W. Rozycki
@ 2021-04-07 20:07       ` Ilya Lipnitskiy
  2021-04-12 15:03         ` Thomas Bogendoerfer
  0 siblings, 1 reply; 10+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-07 20:07 UTC (permalink / raw)
  To: macro
  Cc: ilya.lipnitskiy, linux-kernel, linux-mips, liwei391, nbd,
	tsbogend, yangtiezhu

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


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

* Re: [PATCH v3] MIPS: add support for buggy MT7621S core detection
  2021-04-07 20:07       ` [PATCH v3] " Ilya Lipnitskiy
@ 2021-04-12 15:03         ` Thomas Bogendoerfer
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2021-04-12 15:03 UTC (permalink / raw)
  To: Ilya Lipnitskiy
  Cc: macro, linux-kernel, linux-mips, liwei391, nbd, yangtiezhu

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 ]

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

* Re: [PATCH] MIPS: add support for buggy MT7621S core detection
  2021-04-07 18:49       ` Ilya Lipnitskiy
@ 2021-06-25 10:54         ` Strontium
  0 siblings, 0 replies; 10+ messages in thread
From: Strontium @ 2021-06-25 10:54 UTC (permalink / raw)
  To: Ilya Lipnitskiy, Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, Wei Li, Tiezhu Yang, linux-mips,
	Linux Kernel Mailing List, Felix Fietkau

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


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

end of thread, other threads:[~2021-06-25 10:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-03  6:19 [PATCH] MIPS: add support for buggy MT7621S core detection Ilya Lipnitskiy
2021-04-06  1:22 ` Maciej W. Rozycki
2021-04-06  1:54   ` Ilya Lipnitskiy
2021-04-07 13:49     ` Maciej W. Rozycki
2021-04-07 18:49       ` Ilya Lipnitskiy
2021-06-25 10:54         ` Strontium
2021-04-06  4:23   ` [PATCH v2] " Ilya Lipnitskiy
2021-04-07 13:52     ` Maciej W. Rozycki
2021-04-07 20:07       ` [PATCH v3] " Ilya Lipnitskiy
2021-04-12 15:03         ` Thomas Bogendoerfer

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