在 2022/5/14 11:52, Chen-Yu Tsai 写道:

Hi,

On Sat, May 14, 2022 at 11:19 AM qianfanguijin@163.com wrote:

...

From: qianfan Zhao qianfanguijin@163.com

linux system will die if we offline one of the cpu on R40 based board: eg: echo 0 > /sys/devices/system/cpu/cpu3/online

Fixed sunxi_power_switch based on allwinner lichee 3.10 kernel driver.

Signed-off-by: qianfan Zhao qianfanguijin@163.com

Please add a Fixes tag.

...

---

v2 changes: Fix the commit message, the source code doesn't change.

arch/arm/cpu/armv7/sunxi/psci.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c index 1ac50f558a..63186a9388 100644 --- a/arch/arm/cpu/armv7/sunxi/psci.c +++ b/arch/arm/cpu/armv7/sunxi/psci.c @@ -79,8 +79,7 @@ static void __secure __mdelay(u32 ms) static void __secure clamp_release(u32 __maybe_unused *clamp) { #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \

•   defined(CONFIG_MACH_SUN8I_H3) || \
  

•   defined(CONFIG_MACH_SUN8I_R40)
  

•   defined(CONFIG_MACH_SUN8I_H3)
     u32 tmp = 0x1ff;
     do {
             tmp >>= 1;
  

@@ -88,15 +87,30 @@ static void __secure clamp_release(u32 __maybe_unused *clamp) } while (tmp);

     __mdelay(10);

+#elif defined(CONFIG_MACH_SUN8I_R40)

•   u8 i, tmp = 0xfe;
  

•   for (i = 0; i < 5; i++) { /* 0xfe, 0xf8, 0xe0, 0x80, 0x00 */
  

•           writel(tmp, clamp);
  

•           tmp <<= 2;
  

•   }
  

•   while (0x00 != readl(clamp)) {
  

•           ;
  

•   }
  

  #endif }

  static void __secure clamp_set(u32 __maybe_unused *clamp) { #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \

•   defined(CONFIG_MACH_SUN8I_H3) || \
  

•   defined(CONFIG_MACH_SUN8I_R40)
  

•   defined(CONFIG_MACH_SUN8I_H3)
     writel(0xff, clamp);
  

+#elif defined(CONFIG_MACH_SUN8I_R40)

•   writel(0xff, clamp);
  

•   while (0xff != readl(clamp)) {
  

•           ;
  

•   }
  

  #endif }

@@ -153,7 +167,7 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)

     sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
                        (void *)cpucfg + SUN8I_R40_PWROFF,

•                      on, 0);
  

•                      on, cpu);
  

I think this is the only change that is needed. Looking again at the R40 user manual, the original code turned off core 0 regardless of which core was being brought down.

Could you give that a try? The power clamp stuff shouldn't change much, as the end result is the same. The readback might make a difference, but if it does, it should be applied to all SoCs.

Just change the last params of sunxi_power_switch can also work fine.

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c index 1ac50f558a..ae21879e69 100644 --- a/arch/arm/cpu/armv7/sunxi/psci.c +++ b/arch/arm/cpu/armv7/sunxi/psci.c @@ -153,7 +153,7 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)

sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),                            (void *)cpucfg + SUN8I_R40_PWROFF, -                          on, 0); +                          on, cpu);  }  #else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */  static void __secure sunxi_cpu_set_power(int cpu, bool on)

The readback might as same as __mdelay.

But I can't make sure the 0xfe, 0x80 ... 0x00 sequence number when clame_release.

Regards ChenYu

...

} #else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */ static void __secure sunxi_cpu_set_power(int cpu, bool on) -- 2.25.1

在 2022/6/25 8:51, Andre Przywara 写道:

On Sat, 14 May 2022 11:52:01 +0800 Chen-Yu Tsai wens@kernel.org wrote:

Hi Qianfan,

...

On Sat, May 14, 2022 at 11:19 AM qianfanguijin@163.com wrote:

...

From: qianfan Zhao qianfanguijin@163.com

linux system will die if we offline one of the cpu on R40 based board: eg: echo 0 > /sys/devices/system/cpu/cpu3/online

Thanks for bringing this up: indeed CPU offlining does not work on the R40 at the moment. More below ...

...

...

Fixed sunxi_power_switch based on allwinner lichee 3.10 kernel driver.

Signed-off-by: qianfan Zhao qianfanguijin@163.com

Please add a Fixes tag.

...

---

v2 changes: Fix the commit message, the source code doesn't change.

arch/arm/cpu/armv7/sunxi/psci.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c index 1ac50f558a..63186a9388 100644 --- a/arch/arm/cpu/armv7/sunxi/psci.c +++ b/arch/arm/cpu/armv7/sunxi/psci.c @@ -79,8 +79,7 @@ static void __secure __mdelay(u32 ms) static void __secure clamp_release(u32 __maybe_unused *clamp) { #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \

•   defined(CONFIG_MACH_SUN8I_H3) || \
  

•   defined(CONFIG_MACH_SUN8I_R40)
  

•   defined(CONFIG_MACH_SUN8I_H3)
     u32 tmp = 0x1ff;
     do {
             tmp >>= 1;
  

@@ -88,15 +87,30 @@ static void __secure clamp_release(u32 __maybe_unused *clamp) } while (tmp);

     __mdelay(10);

+#elif defined(CONFIG_MACH_SUN8I_R40)

•   u8 i, tmp = 0xfe;
  

•   for (i = 0; i < 5; i++) { /* 0xfe, 0xf8, 0xe0, 0x80, 0x00 */
  

•           writel(tmp, clamp);
  

•           tmp <<= 2;
  

•   }
  

•   while (0x00 != readl(clamp)) {
  

•           ;
  

•   }
  

  #endif }

  static void __secure clamp_set(u32 __maybe_unused *clamp) { #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \

•   defined(CONFIG_MACH_SUN8I_H3) || \
  

•   defined(CONFIG_MACH_SUN8I_R40)
  

•   defined(CONFIG_MACH_SUN8I_H3)
     writel(0xff, clamp);
  

+#elif defined(CONFIG_MACH_SUN8I_R40)

•   writel(0xff, clamp);
  

•   while (0xff != readl(clamp)) {
  

•           ;
  

•   }
  

  #endif }

@@ -153,7 +167,7 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)

     sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
                        (void *)cpucfg + SUN8I_R40_PWROFF,

•                      on, 0);
  

•                      on, cpu);
  

I think this is the only change that is needed. Looking again at the R40 user manual, the original code turned off core 0 regardless of which core was being brought down.

Yes, I agree here, that just always turns off core 0 :-( I wrote to the PWROFF registers from U-Boot, all four registers +0x120, +0x124, +0x128, +0x12c exist and store only the lowest 8 bits, the next four words are not implemented on the R40. So this makes it very likely that those are the indeed the PWROFF registers for the four cores, even though the manual only mentions two.

...

Could you give that a try? The power clamp stuff shouldn't change much, as the end result is the same. The readback might make a difference, but if it does, it should be applied to all SoCs.

So indeed just the change above *seems* to work, although it still didn't survive my CPU on/offline test: # for i in $(seq 1 100); do echo $((RANDOM%2)) > /sys/devices/system/cpu/cpu$((RANDOM%4))/online; done This hangs halfway through.

Next is the scripts I used:

unfortunately it also failed.

#!/bin/sh i=1

while true ; do     # cpu0 can't offline, skip it     cpu=$((RANDOM%3))     let cpu++

# togger online status     online=$(cat /sys/devices/system/cpu/cpu${cpu}/online)     if [ ${online} -eq 0 ] ; then         online=1     else         online=0     fi

echo -n -e "${i}\tcpu${cpu} ${online}\t"     echo ${online} > /sys/devices/system/cpu/cpu${cpu}/online     echo "cpu onlines: $(cat /sys/devices/system/cpu/online)"

let i++ done

Applying the other changes didn't make a difference: I tried just the read-back, read-back + delay, the "0xfe, 0xf8, 0xe0, 0x80, 0x00" sequence, and combinations. Also mdelay(100) didn't help. Not sure if this is power sequence related, or a separate bug, maybe in U-Boot's PSCI implementation? I will try to debug this further, also on other SoCs.

But for now I am tempted to take this one-line change, as this looks like an obvious bug and the fix definitely improves the situation.

Thanks! Andre

...

...

} #else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */ static void __secure sunxi_cpu_set_power(int cpu, bool on) -- 2.25.1