From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EB556C433EF for ; Sat, 25 Jun 2022 00:52:25 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9C277843FB; Sat, 25 Jun 2022 02:52:23 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id C22D684449; Sat, 25 Jun 2022 02:52:21 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 1DE2684392 for ; Sat, 25 Jun 2022 02:52:19 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6E4481042; Fri, 24 Jun 2022 17:52:18 -0700 (PDT) Received: from slackpad.lan (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EB4F43F792; Fri, 24 Jun 2022 17:52:16 -0700 (PDT) Date: Sat, 25 Jun 2022 01:51:18 +0100 From: Andre Przywara To: Chen-Yu Tsai Cc: qianfan , U-Boot Mailing List , Jagan Teki , Maxime Ripard , Peter Robinson Subject: Re: [PATCH v2] sunxi: psci: Fix sunxi_power_switch on sun8i-r40 platform Message-ID: <20220625015038.00a260bc@slackpad.lan> In-Reply-To: References: <20220514031923.974-1-qianfanguijin@163.com> Organization: Arm Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Sat, 14 May 2022 11:52:01 +0800 Chen-Yu Tsai wrote: Hi Qianfan, > On Sat, May 14, 2022 at 11:19 AM wrote: > > > > From: qianfan Zhao > > > > 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 > > 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. 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 > >