linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
@ 2015-09-01 14:17 Chanho Park
  2015-09-01 14:47 ` Anand Moon
  2015-09-02  0:15 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 8+ messages in thread
From: Chanho Park @ 2015-09-01 14:17 UTC (permalink / raw)
  To: linux-arm-kernel

The cpu booting of exynos5422 has been still broken since we discussed
it in last year[1]. This patch is inspired from Odroid XU3
code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
reset code was founded exynos5420 octa cores series SoCs and only
required for the first boot core is the Little core (Cortex A7).
Some of the exynos5420 boards and all of the exynos5422 boards will require
this code.

There is two ways to check the little core is the first cpu. One is
checking GPG2CON[1] GPIO value and the other is checking the cluster
number of the first cpu. I selected the latter because it's more easier
than the former.

[1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
[2]:https://patchwork.kernel.org/patch/6782891/

Cc: Kevin Hilman <khilman@kernel.org>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Kevin Hilman <khilman@linaro.org>
Signed-off-by: Chanho Park <parkch98@gmail.com>
---
Changes from v1:
 .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
 .append comments about waiting SPARE2 register

Changes since RFC:
 .drop checking soc_is_exynos5800 to extend this codes to
exynos5420/5422 boards.
 .kfc cores will be reset only if the cpu0 is kfc core.
 .Rebase top of the kukjin's for-next branch

 arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
 arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index 9bdf547..8926621 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -20,6 +20,7 @@
 #include <asm/cputype.h>
 #include <asm/cp15.h>
 #include <asm/mcpm.h>
+#include <asm/smp_plat.h>
 
 #include "regs-pmu.h"
 #include "common.h"
@@ -70,7 +71,29 @@ static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
 		cluster >= EXYNOS5420_NR_CLUSTERS)
 		return -EINVAL;
 
-	exynos_cpu_power_up(cpunr);
+	if (!exynos_cpu_power_state(cpunr)) {
+		exynos_cpu_power_up(cpunr);
+
+		/* This assumes the cluster number of the big cores(Cortex A15)
+		 * is 0 and the Little cores(Cortex A7) is 1.
+		 * When the system was booted from the Little core,
+		 * they should be reset during power up cpu.
+		 */
+		if (cluster &&
+		    cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1)) {
+			/* Before we reset the Little cores, we should wait
+			 * the SPARE2 register is set to 1 because the init
+			 * codes of the iROM will set the register after
+			 * initialization.
+			 */
+			while (!pmu_raw_readl(S5P_PMU_SPARE2))
+				udelay(10);
+
+			pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
+					EXYNOS_SWRESET);
+		}
+	}
+
 	return 0;
 }
 
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index b761433..fba9068 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -513,6 +513,12 @@ static inline unsigned int exynos_pmu_cpunr(unsigned int mpidr)
 #define SPREAD_ENABLE						0xF
 #define SPREAD_USE_STANDWFI					0xF
 
+#define EXYNOS5420_KFC_CORE_RESET0				BIT(8)
+#define EXYNOS5420_KFC_ETM_RESET0				BIT(20)
+
+#define EXYNOS5420_KFC_CORE_RESET(_nr)				\
+	((EXYNOS5420_KFC_CORE_RESET0 | EXYNOS5420_KFC_ETM_RESET0) << (_nr))
+
 #define EXYNOS5420_BB_CON1					0x0784
 #define EXYNOS5420_BB_SEL_EN					BIT(31)
 #define EXYNOS5420_BB_PMOS_EN					BIT(7)
-- 
2.1.0

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-01 14:17 [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up Chanho Park
@ 2015-09-01 14:47 ` Anand Moon
  2015-09-02  0:15 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Anand Moon @ 2015-09-01 14:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hi All,

On 1 September 2015 at 19:47, Chanho Park <parkch98@gmail.com> wrote:
>
> The cpu booting of exynos5422 has been still broken since we discussed
> it in last year[1]. This patch is inspired from Odroid XU3
> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
> reset code was founded exynos5420 octa cores series SoCs and only
> required for the first boot core is the Little core (Cortex A7).
> Some of the exynos5420 boards and all of the exynos5422 boards will require
> this code.
>
> There is two ways to check the little core is the first cpu. One is
> checking GPG2CON[1] GPIO value and the other is checking the cluster
> number of the first cpu. I selected the latter because it's more easier
> than the former.
>
> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
> [2]:https://patchwork.kernel.org/patch/6782891/
>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Kevin Hilman <khilman@linaro.org>
> Signed-off-by: Chanho Park <parkch98@gmail.com>
> ---
> Changes from v1:
>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>  .append comments about waiting SPARE2 register
>
> Changes since RFC:
>  .drop checking soc_is_exynos5800 to extend this codes to
> exynos5420/5422 boards.
>  .kfc cores will be reset only if the cpu0 is kfc core.
>  .Rebase top of the kukjin's for-next branch
>
>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index 9bdf547..8926621 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -20,6 +20,7 @@
>  #include <asm/cputype.h>
>  #include <asm/cp15.h>
>  #include <asm/mcpm.h>
> +#include <asm/smp_plat.h>
>
>  #include "regs-pmu.h"
>  #include "common.h"
> @@ -70,7 +71,29 @@ static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
>                 cluster >= EXYNOS5420_NR_CLUSTERS)
>                 return -EINVAL;
>
> -       exynos_cpu_power_up(cpunr);
> +       if (!exynos_cpu_power_state(cpunr)) {
> +               exynos_cpu_power_up(cpunr);
> +
> +               /* This assumes the cluster number of the big cores(Cortex A15)
> +                * is 0 and the Little cores(Cortex A7) is 1.
> +                * When the system was booted from the Little core,
> +                * they should be reset during power up cpu.
> +                */
> +               if (cluster &&
> +                   cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1)) {
> +                       /* Before we reset the Little cores, we should wait
> +                        * the SPARE2 register is set to 1 because the init
> +                        * codes of the iROM will set the register after
> +                        * initialization.
> +                        */
> +                       while (!pmu_raw_readl(S5P_PMU_SPARE2))
> +                               udelay(10);
> +
> +                       pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> +                                       EXYNOS_SWRESET);
> +               }
> +       }
> +
>         return 0;
>  }
>
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index b761433..fba9068 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -513,6 +513,12 @@ static inline unsigned int exynos_pmu_cpunr(unsigned int mpidr)
>  #define SPREAD_ENABLE                                          0xF
>  #define SPREAD_USE_STANDWFI                                    0xF
>
> +#define EXYNOS5420_KFC_CORE_RESET0                             BIT(8)
> +#define EXYNOS5420_KFC_ETM_RESET0                              BIT(20)
> +
> +#define EXYNOS5420_KFC_CORE_RESET(_nr)                         \
> +       ((EXYNOS5420_KFC_CORE_RESET0 | EXYNOS5420_KFC_ETM_RESET0) << (_nr))
> +
>  #define EXYNOS5420_BB_CON1                                     0x0784
>  #define EXYNOS5420_BB_SEL_EN                                   BIT(31)
>  #define EXYNOS5420_BB_PMOS_EN                                  BIT(7)
> --
> 2.1.0
>

Looking at the following patch

https://github.com/tobiasjakobi/linux-odroid/commit/c25aae8a02c0e3132df581d1d12be1d6738a08d6

You should also consider some similar logic for platform_do_lowpower
and exynos_boot_secondary.
In that same way to address CPUIdle issue.

I am just trying to put my observation. I may be wrong.

-Anand Moon

> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-01 14:17 [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up Chanho Park
  2015-09-01 14:47 ` Anand Moon
@ 2015-09-02  0:15 ` Krzysztof Kozlowski
  2015-09-02  0:32   ` Javier Martinez Canillas
  1 sibling, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-02  0:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 01.09.2015 23:17, Chanho Park wrote:
> The cpu booting of exynos5422 has been still broken since we discussed
> it in last year[1]. This patch is inspired from Odroid XU3
> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
> reset code was founded exynos5420 octa cores series SoCs and only
> required for the first boot core is the Little core (Cortex A7).
> Some of the exynos5420 boards and all of the exynos5422 boards will require
> this code.
> 
> There is two ways to check the little core is the first cpu. One is
> checking GPG2CON[1] GPIO value and the other is checking the cluster
> number of the first cpu. I selected the latter because it's more easier
> than the former.
> 
> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
> [2]:https://patchwork.kernel.org/patch/6782891/
> 
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Kevin Hilman <khilman@linaro.org>
> Signed-off-by: Chanho Park <parkch98@gmail.com>
> ---
> Changes from v1:
>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>  .append comments about waiting SPARE2 register
> 
> Changes since RFC:
>  .drop checking soc_is_exynos5800 to extend this codes to
> exynos5420/5422 boards.
>  .kfc cores will be reset only if the cpu0 is kfc core.
>  .Rebase top of the kukjin's for-next branch
> 
>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)

Thanks for updating the patch. Remaining minor nit about comment style
(/* on first line) can be fixed while applying.

The patch works good, after disabling bL switcher I have 8 cores running:

Tested on Odroid XU4:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

It's 4.3 merge window so the patch will go probably to v4.4.

Best regards,
Krzysztof


> 
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index 9bdf547..8926621 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -20,6 +20,7 @@
>  #include <asm/cputype.h>
>  #include <asm/cp15.h>
>  #include <asm/mcpm.h>
> +#include <asm/smp_plat.h>
>  
>  #include "regs-pmu.h"
>  #include "common.h"
> @@ -70,7 +71,29 @@ static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
>  		cluster >= EXYNOS5420_NR_CLUSTERS)
>  		return -EINVAL;
>  
> -	exynos_cpu_power_up(cpunr);
> +	if (!exynos_cpu_power_state(cpunr)) {
> +		exynos_cpu_power_up(cpunr);
> +
> +		/* This assumes the cluster number of the big cores(Cortex A15)
> +		 * is 0 and the Little cores(Cortex A7) is 1.
> +		 * When the system was booted from the Little core,
> +		 * they should be reset during power up cpu.
> +		 */
> +		if (cluster &&
> +		    cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1)) {
> +			/* Before we reset the Little cores, we should wait
> +			 * the SPARE2 register is set to 1 because the init
> +			 * codes of the iROM will set the register after
> +			 * initialization.
> +			 */
> +			while (!pmu_raw_readl(S5P_PMU_SPARE2))
> +				udelay(10);
> +
> +			pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> +					EXYNOS_SWRESET);
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index b761433..fba9068 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -513,6 +513,12 @@ static inline unsigned int exynos_pmu_cpunr(unsigned int mpidr)
>  #define SPREAD_ENABLE						0xF
>  #define SPREAD_USE_STANDWFI					0xF
>  
> +#define EXYNOS5420_KFC_CORE_RESET0				BIT(8)
> +#define EXYNOS5420_KFC_ETM_RESET0				BIT(20)
> +
> +#define EXYNOS5420_KFC_CORE_RESET(_nr)				\
> +	((EXYNOS5420_KFC_CORE_RESET0 | EXYNOS5420_KFC_ETM_RESET0) << (_nr))
> +
>  #define EXYNOS5420_BB_CON1					0x0784
>  #define EXYNOS5420_BB_SEL_EN					BIT(31)
>  #define EXYNOS5420_BB_PMOS_EN					BIT(7)
> 

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-02  0:15 ` Krzysztof Kozlowski
@ 2015-09-02  0:32   ` Javier Martinez Canillas
  2015-09-02  0:39     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Javier Martinez Canillas @ 2015-09-02  0:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Krzysztof,

On 09/02/2015 02:15 AM, Krzysztof Kozlowski wrote:
> On 01.09.2015 23:17, Chanho Park wrote:
>> The cpu booting of exynos5422 has been still broken since we discussed
>> it in last year[1]. This patch is inspired from Odroid XU3
>> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
>> reset code was founded exynos5420 octa cores series SoCs and only
>> required for the first boot core is the Little core (Cortex A7).
>> Some of the exynos5420 boards and all of the exynos5422 boards will require
>> this code.
>>
>> There is two ways to check the little core is the first cpu. One is
>> checking GPG2CON[1] GPIO value and the other is checking the cluster
>> number of the first cpu. I selected the latter because it's more easier
>> than the former.
>>
>> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
>> [2]:https://patchwork.kernel.org/patch/6782891/
>>
>> Cc: Kevin Hilman <khilman@kernel.org>
>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Tested-by: Kevin Hilman <khilman@linaro.org>
>> Signed-off-by: Chanho Park <parkch98@gmail.com>
>> ---
>> Changes from v1:
>>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>>  .append comments about waiting SPARE2 register
>>
>> Changes since RFC:
>>  .drop checking soc_is_exynos5800 to extend this codes to
>> exynos5420/5422 boards.
>>  .kfc cores will be reset only if the cpu0 is kfc core.
>>  .Rebase top of the kukjin's for-next branch
>>
>>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> Thanks for updating the patch. Remaining minor nit about comment style
> (/* on first line) can be fixed while applying.
> 
> The patch works good, after disabling bL switcher I have 8 cores running:
> 
> Tested on Odroid XU4:
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> It's 4.3 merge window so the patch will go probably to v4.4.
> 

Isn't this material for the v4.3 -rc cycle since it's fixing a bug
(CPUs not booting)? So I don't think that's necessary to wait for v4.4.

But I was expecting another re-spin as from Abhilash's latest comments [0],
it seems the same workaround is needed in arch/arm/mach-exynos/platsmp.c
to avoid the CPU's not booting issue when CCI is disabled in DT and the
MCPM backend is bypassed.

Although I guess that could be also made as a follow up patch.

> Best regards,
> Krzysztof
> 
> 

[0]: http://www.spinics.net/lists/arm-kernel/msg442399.html

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-02  0:32   ` Javier Martinez Canillas
@ 2015-09-02  0:39     ` Krzysztof Kozlowski
  2015-09-02  7:44       ` Javier Martinez Canillas
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-02  0:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 02.09.2015 09:32, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 09/02/2015 02:15 AM, Krzysztof Kozlowski wrote:
>> On 01.09.2015 23:17, Chanho Park wrote:
>>> The cpu booting of exynos5422 has been still broken since we discussed
>>> it in last year[1]. This patch is inspired from Odroid XU3
>>> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
>>> reset code was founded exynos5420 octa cores series SoCs and only
>>> required for the first boot core is the Little core (Cortex A7).
>>> Some of the exynos5420 boards and all of the exynos5422 boards will require
>>> this code.
>>>
>>> There is two ways to check the little core is the first cpu. One is
>>> checking GPG2CON[1] GPIO value and the other is checking the cluster
>>> number of the first cpu. I selected the latter because it's more easier
>>> than the former.
>>>
>>> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
>>> [2]:https://patchwork.kernel.org/patch/6782891/
>>>
>>> Cc: Kevin Hilman <khilman@kernel.org>
>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> Tested-by: Kevin Hilman <khilman@linaro.org>
>>> Signed-off-by: Chanho Park <parkch98@gmail.com>
>>> ---
>>> Changes from v1:
>>>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>>>  .append comments about waiting SPARE2 register
>>>
>>> Changes since RFC:
>>>  .drop checking soc_is_exynos5800 to extend this codes to
>>> exynos5420/5422 boards.
>>>  .kfc cores will be reset only if the cpu0 is kfc core.
>>>  .Rebase top of the kukjin's for-next branch
>>>
>>>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>>>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> Thanks for updating the patch. Remaining minor nit about comment style
>> (/* on first line) can be fixed while applying.
>>
>> The patch works good, after disabling bL switcher I have 8 cores running:
>>
>> Tested on Odroid XU4:
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>> It's 4.3 merge window so the patch will go probably to v4.4.
>>
> 
> Isn't this material for the v4.3 -rc cycle since it's fixing a bug
> (CPUs not booting)? So I don't think that's necessary to wait for v4.4.

It is a bug fix but:
1. Not a fix for regression introduced in current merge window,
2. There may be more subtle issues like the one mentioned below, not
found yet (probably no one tested it with all possible configurations),

so I don't think rushing with the patch to mainline is good idea.

However your comment reminds me about stable. This actually looks like a
candidate for stable.

> 
> But I was expecting another re-spin as from Abhilash's latest comments [0],
> it seems the same workaround is needed in arch/arm/mach-exynos/platsmp.c
> to avoid the CPU's not booting issue when CCI is disabled in DT and the
> MCPM backend is bypassed.
> 
> Although I guess that could be also made as a follow up patch.

Chanho patch fixes only MCPM path. Regular path (platsmp, firmware) is
not affected so I would expect a following patch.

Best regards,
Krzysztof

> 
>> Best regards,
>> Krzysztof
>>
>>
> 
> [0]: http://www.spinics.net/lists/arm-kernel/msg442399.html
> 
> Best regards,
> 

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-02  0:39     ` Krzysztof Kozlowski
@ 2015-09-02  7:44       ` Javier Martinez Canillas
  2015-09-02  7:59         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Javier Martinez Canillas @ 2015-09-02  7:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Krzysztof,

On 09/02/2015 02:39 AM, Krzysztof Kozlowski wrote:
> On 02.09.2015 09:32, Javier Martinez Canillas wrote:
>> Hello Krzysztof,
>>
>> On 09/02/2015 02:15 AM, Krzysztof Kozlowski wrote:
>>> On 01.09.2015 23:17, Chanho Park wrote:
>>>> The cpu booting of exynos5422 has been still broken since we discussed
>>>> it in last year[1]. This patch is inspired from Odroid XU3
>>>> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
>>>> reset code was founded exynos5420 octa cores series SoCs and only
>>>> required for the first boot core is the Little core (Cortex A7).
>>>> Some of the exynos5420 boards and all of the exynos5422 boards will require
>>>> this code.
>>>>
>>>> There is two ways to check the little core is the first cpu. One is
>>>> checking GPG2CON[1] GPIO value and the other is checking the cluster
>>>> number of the first cpu. I selected the latter because it's more easier
>>>> than the former.
>>>>
>>>> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
>>>> [2]:https://patchwork.kernel.org/patch/6782891/
>>>>
>>>> Cc: Kevin Hilman <khilman@kernel.org>
>>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> Tested-by: Kevin Hilman <khilman@linaro.org>
>>>> Signed-off-by: Chanho Park <parkch98@gmail.com>
>>>> ---
>>>> Changes from v1:
>>>>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>>>>  .append comments about waiting SPARE2 register
>>>>
>>>> Changes since RFC:
>>>>  .drop checking soc_is_exynos5800 to extend this codes to
>>>> exynos5420/5422 boards.
>>>>  .kfc cores will be reset only if the cpu0 is kfc core.
>>>>  .Rebase top of the kukjin's for-next branch
>>>>
>>>>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>>>>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>
>>> Thanks for updating the patch. Remaining minor nit about comment style
>>> (/* on first line) can be fixed while applying.
>>>
>>> The patch works good, after disabling bL switcher I have 8 cores running:
>>>
>>> Tested on Odroid XU4:
>>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>
>>> It's 4.3 merge window so the patch will go probably to v4.4.
>>>
>>
>> Isn't this material for the v4.3 -rc cycle since it's fixing a bug
>> (CPUs not booting)? So I don't think that's necessary to wait for v4.4.
> 
> It is a bug fix but:
> 1. Not a fix for regression introduced in current merge window,

I thought the -rc cycle was for stabilization and fix not only regressions
introduced during the merge window but also long standing issues.

> 2. There may be more subtle issues like the one mentioned below, not
> found yet (probably no one tested it with all possible configurations),
>

Right, this is indeed a better reason to wait for v4.4.
 
> so I don't think rushing with the patch to mainline is good idea.
>

Yes, agreed.
 
> However your comment reminds me about stable. This actually looks like a
> candidate for stable.
> 
>>
>> But I was expecting another re-spin as from Abhilash's latest comments [0],
>> it seems the same workaround is needed in arch/arm/mach-exynos/platsmp.c
>> to avoid the CPU's not booting issue when CCI is disabled in DT and the
>> MCPM backend is bypassed.
>>
>> Although I guess that could be also made as a follow up patch.
> 
> Chanho patch fixes only MCPM path. Regular path (platsmp, firmware) is
> not affected so I would expect a following patch.
>

Ok, hopefully the non MCPM path can also be fixed before v4.4.
 
> Best regards,
> Krzysztof
> 
>>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-02  7:44       ` Javier Martinez Canillas
@ 2015-09-02  7:59         ` Krzysztof Kozlowski
  2015-09-14  1:24           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-02  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 02.09.2015 16:44, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 09/02/2015 02:39 AM, Krzysztof Kozlowski wrote:
>> On 02.09.2015 09:32, Javier Martinez Canillas wrote:
>>> Hello Krzysztof,
>>>
>>> On 09/02/2015 02:15 AM, Krzysztof Kozlowski wrote:
>>>> On 01.09.2015 23:17, Chanho Park wrote:
>>>>> The cpu booting of exynos5422 has been still broken since we discussed
>>>>> it in last year[1]. This patch is inspired from Odroid XU3
>>>>> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
>>>>> reset code was founded exynos5420 octa cores series SoCs and only
>>>>> required for the first boot core is the Little core (Cortex A7).
>>>>> Some of the exynos5420 boards and all of the exynos5422 boards will require
>>>>> this code.
>>>>>
>>>>> There is two ways to check the little core is the first cpu. One is
>>>>> checking GPG2CON[1] GPIO value and the other is checking the cluster
>>>>> number of the first cpu. I selected the latter because it's more easier
>>>>> than the former.
>>>>>
>>>>> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
>>>>> [2]:https://patchwork.kernel.org/patch/6782891/
>>>>>
>>>>> Cc: Kevin Hilman <khilman@kernel.org>
>>>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>>> Tested-by: Kevin Hilman <khilman@linaro.org>
>>>>> Signed-off-by: Chanho Park <parkch98@gmail.com>
>>>>> ---
>>>>> Changes from v1:
>>>>>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>>>>>  .append comments about waiting SPARE2 register
>>>>>
>>>>> Changes since RFC:
>>>>>  .drop checking soc_is_exynos5800 to extend this codes to
>>>>> exynos5420/5422 boards.
>>>>>  .kfc cores will be reset only if the cpu0 is kfc core.
>>>>>  .Rebase top of the kukjin's for-next branch
>>>>>
>>>>>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>>>>>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>
>>>> Thanks for updating the patch. Remaining minor nit about comment style
>>>> (/* on first line) can be fixed while applying.
>>>>
>>>> The patch works good, after disabling bL switcher I have 8 cores running:
>>>>
>>>> Tested on Odroid XU4:
>>>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>>
>>>> It's 4.3 merge window so the patch will go probably to v4.4.
>>>>
>>>
>>> Isn't this material for the v4.3 -rc cycle since it's fixing a bug
>>> (CPUs not booting)? So I don't think that's necessary to wait for v4.4.
>>
>> It is a bug fix but:
>> 1. Not a fix for regression introduced in current merge window,
> 
> I thought the -rc cycle was for stabilization and fix not only regressions
> introduced during the merge window but also long standing issues.

It's all subtle. Sometimes it also depends on the mood of maintainer...
A lot of fixes for different issues are merged in -rc and in the same
time they are rejected because they are too late and they don't fix
regression from merge window. Anyway I used the argument #1 in
combination with #2.

Best regards,
Krzysztof

> 
>> 2. There may be more subtle issues like the one mentioned below, not
>> found yet (probably no one tested it with all possible configurations),
>>
> 
> Right, this is indeed a better reason to wait for v4.4.
>  
>> so I don't think rushing with the patch to mainline is good idea.
>>
> 
> Yes, agreed.
>  
>> However your comment reminds me about stable. This actually looks like a
>> candidate for stable.
>>
>>>
>>> But I was expecting another re-spin as from Abhilash's latest comments [0],
>>> it seems the same workaround is needed in arch/arm/mach-exynos/platsmp.c
>>> to avoid the CPU's not booting issue when CCI is disabled in DT and the
>>> MCPM backend is bypassed.
>>>
>>> Although I guess that could be also made as a follow up patch.
>>
>> Chanho patch fixes only MCPM path. Regular path (platsmp, firmware) is
>> not affected so I would expect a following patch.
>>
> 
> Ok, hopefully the non MCPM path can also be fixed before v4.4.
>  
>> Best regards,
>> Krzysztof
>>
>>>
> 
> Best regards,
> 

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

* [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up
  2015-09-02  7:59         ` Krzysztof Kozlowski
@ 2015-09-14  1:24           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-14  1:24 UTC (permalink / raw)
  To: linux-arm-kernel

On 02.09.2015 16:59, Krzysztof Kozlowski wrote:
> On 02.09.2015 16:44, Javier Martinez Canillas wrote:
>> Hello Krzysztof,
>>
>> On 09/02/2015 02:39 AM, Krzysztof Kozlowski wrote:
>>> On 02.09.2015 09:32, Javier Martinez Canillas wrote:
>>>> Hello Krzysztof,
>>>>
>>>> On 09/02/2015 02:15 AM, Krzysztof Kozlowski wrote:
>>>>> On 01.09.2015 23:17, Chanho Park wrote:
>>>>>> The cpu booting of exynos5422 has been still broken since we discussed
>>>>>> it in last year[1]. This patch is inspired from Odroid XU3
>>>>>> code (Actually, it was from samsung exynos vendor kernel)[2]. This weird
>>>>>> reset code was founded exynos5420 octa cores series SoCs and only
>>>>>> required for the first boot core is the Little core (Cortex A7).
>>>>>> Some of the exynos5420 boards and all of the exynos5422 boards will require
>>>>>> this code.
>>>>>>
>>>>>> There is two ways to check the little core is the first cpu. One is
>>>>>> checking GPG2CON[1] GPIO value and the other is checking the cluster
>>>>>> number of the first cpu. I selected the latter because it's more easier
>>>>>> than the former.
>>>>>>
>>>>>> [1]:http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/350632.html
>>>>>> [2]:https://patchwork.kernel.org/patch/6782891/
>>>>>>
>>>>>> Cc: Kevin Hilman <khilman@kernel.org>
>>>>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>>>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>>>> Tested-by: Kevin Hilman <khilman@linaro.org>
>>>>>> Signed-off-by: Chanho Park <parkch98@gmail.com>
>>>>>> ---
>>>>>> Changes from v1:
>>>>>>  .kfc to Little (Cortex A7) and eagle to big (Cortex A15)
>>>>>>  .append comments about waiting SPARE2 register
>>>>>>
>>>>>> Changes since RFC:
>>>>>>  .drop checking soc_is_exynos5800 to extend this codes to
>>>>>> exynos5420/5422 boards.
>>>>>>  .kfc cores will be reset only if the cpu0 is kfc core.
>>>>>>  .Rebase top of the kukjin's for-next branch
>>>>>>
>>>>>>  arch/arm/mach-exynos/mcpm-exynos.c | 25 ++++++++++++++++++++++++-
>>>>>>  arch/arm/mach-exynos/regs-pmu.h    |  6 ++++++
>>>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>>
>>>>> Thanks for updating the patch. Remaining minor nit about comment style
>>>>> (/* on first line) can be fixed while applying.
>>>>>
>>>>> The patch works good, after disabling bL switcher I have 8 cores running:
>>>>>
>>>>> Tested on Odroid XU4:
>>>>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>>> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>>>
>>>>> It's 4.3 merge window so the patch will go probably to v4.4.
>>>>>
>>>>
>>>> Isn't this material for the v4.3 -rc cycle since it's fixing a bug
>>>> (CPUs not booting)? So I don't think that's necessary to wait for v4.4.
>>>
>>> It is a bug fix but:
>>> 1. Not a fix for regression introduced in current merge window,
>>
>> I thought the -rc cycle was for stabilization and fix not only regressions
>> introduced during the merge window but also long standing issues.
> 
> It's all subtle. Sometimes it also depends on the mood of maintainer...
> A lot of fixes for different issues are merged in -rc and in the same
> time they are rejected because they are too late and they don't fix
> regression from merge window. Anyway I used the argument #1 in
> combination with #2.
> 
> Best regards,
> Krzysztof
> 
>>
>>> 2. There may be more subtle issues like the one mentioned below, not
>>> found yet (probably no one tested it with all possible configurations),
>>>
>>
>> Right, this is indeed a better reason to wait for v4.4.
>>  
>>> so I don't think rushing with the patch to mainline is good idea.
>>>
>>
>> Yes, agreed.
>>  
>>> However your comment reminds me about stable. This actually looks like a
>>> candidate for stable.

I applied the patch to my repo marking it as stable backport for v4.1+.
Earlier kernels would require manual backporting/rebasing.

It should be in linux-next soon. Let's see how it turns out. If no one
complains than I actually agree with Javier's opinion of sending this to
v4.3.

Best regards,
Krzysztof

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

end of thread, other threads:[~2015-09-14  1:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-01 14:17 [PATCHv2] ARM: EXYNOS: reset Little cores when cpu is up Chanho Park
2015-09-01 14:47 ` Anand Moon
2015-09-02  0:15 ` Krzysztof Kozlowski
2015-09-02  0:32   ` Javier Martinez Canillas
2015-09-02  0:39     ` Krzysztof Kozlowski
2015-09-02  7:44       ` Javier Martinez Canillas
2015-09-02  7:59         ` Krzysztof Kozlowski
2015-09-14  1:24           ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).