From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933249AbbCRNXw (ORCPT ); Wed, 18 Mar 2015 09:23:52 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:26778 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932855AbbCRNXq (ORCPT ); Wed, 18 Mar 2015 09:23:46 -0400 X-AuditID: cbfee61b-f79d76d0000024d6-6a-55097c60a67d From: Bartlomiej Zolnierkiewicz To: Krzysztof Kozlowski Cc: Kukjin Kim , Kukjin Kim , linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org, Daniel Lezcano , Tomasz Figa , linux-kernel@vger.kernel.org, Chanwoo Choi , Kyungmin Park , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 1/4] ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250 Date: Wed, 18 Mar 2015 14:23:35 +0100 Message-id: <3449559.PngGo6WKuq@amdc1032> User-Agent: KMail/4.8.4 (Linux/3.2.0-70-generic-pae; KDE/4.8.5; i686; ; ) In-reply-to: References: <1426683113-31209-1-git-send-email-b.zolnierkie@samsung.com> <1426683113-31209-2-git-send-email-b.zolnierkie@samsung.com> MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=UTF-8 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrCLMWRmVeSWpSXmKPExsVy+t9jAd2EGs5Qg5Vn+C2uf3nOajHvs6zF 6xeGFr0LrrJZ9D9+zWxxtukNu8Wmx9dYLS7vmsNm8bn3CKPFjPP7mCxW7frD6MDtsXPWXXaP Tas62TzuXNvD5rF5Sb1H35ZVjB6fN8kFsEVx2aSk5mSWpRbp2yVwZbzZfYyxYJVCRcORDvYG xg8SXYycHBICJhJ9p/pZIGwxiQv31rN1MXJxCAksYpS4MO0KE4TTwiRxtrOfEaSKTcBKYmL7 KjBbRMBQ4uDu7WBFzAKfmCR6bk9nAkkIC4RLzD78hhXEZhFQlWhe8JMdxOYV0JRoPv0ezBYV 8JTYOf0A2CBOgWCJ7YcOs0JsO8cocXLnU1aIBkGJH5Pvgd3HLCAvsW//VFYIW11i0rxFzBMY BWYhKZuFpGwWkrIFjMyrGEVTC5ILipPSc430ihNzi0vz0vWS83M3MYLj4pn0DsZVDRaHGAU4 GJV4eCWucoQKsSaWFVfmHmKU4GBWEuHlzuIMFeJNSaysSi3Kjy8qzUktPsQozcGiJM6rZN8W IiSQnliSmp2aWpBaBJNl4uCUamDMMS3QNK/15XpmW1+UI/ut9Pu5K63Bb5MMokR14rUuHPm5 7sX+WQ87AkrfxTWI/hYxufLv5EylV2laQfcKV8067TurYceyb+9NOM8dO+HddTX92SwDZuf7 X/5MmcTPv+/o/ssiM7y88u9yu0e4HpZRCPyVf81reuDNKRP/e5/auuLLhPzrX7iXK7EUZyQa ajEXFScCAFyrONmHAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wednesday, March 18, 2015 02:10:31 PM Krzysztof Kozlowski wrote: > 2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz : > > CPU1 hotplug may hang when AFTR is used. Fix it by: > > - setting AUTOWAKEUP_EN bit in ARM_COREx_CONFIGURATION register in > > exynos_cpu_power_up() > > - not clearing reserved bits of ARM_COREx_CONFIGURATION register in > > exynos_cpu_power_down() > > - waiting while an undocumented register 0x0908 becomes non-zero in > > exynos_core_restart() > > - using dsb_sev() instead of IPI in exynos_boot_secondary() on > > Exynos3250 > > > > Cc: Daniel Lezcano > > Signed-off-by: Chanwoo Choi > > Signed-off-by: Krzysztof Kozlowski > > Signed-off-by: Bartlomiej Zolnierkiewicz > > Acked-by: Kyungmin Park > > --- > > arch/arm/mach-exynos/platsmp.c | 23 ++++++++++++++++++++--- > > arch/arm/mach-exynos/regs-pmu.h | 2 ++ > > 2 files changed, 22 insertions(+), 3 deletions(-) > > > Looks good (except one nit below) and this also fixes hotplug issues > during resume from S2R: > $ echo mem > /sys/power/state > [ 156.517266] Disabling non-boot CPUs ... > [ 156.517781] IRQ18 no longer affine to CPU1 > [ 156.518043] CPU1: shutdown > [ 156.544718] Enabling non-boot CPUs ... > [ 156.554925] CPU1: Software reset > [ 158.552631] CPU1: failed to come online > [ 158.552753] Error taking CPU1 up: -5 > > Reviewed and tested on Rinato (Gear 2/Exynos 3250) board: > > Reviewed-by: Krzysztof Kozlowski > Tested-by: Krzysztof Kozlowski Thank you! > One comment below... > > > > > diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c > > index d2e9f12..ebd135b 100644 > > --- a/arch/arm/mach-exynos/platsmp.c > > +++ b/arch/arm/mach-exynos/platsmp.c > > @@ -126,6 +126,8 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious) > > */ > > void exynos_cpu_power_down(int cpu) > > { > > + u32 core_conf; > > + > > if (cpu == 0 && (soc_is_exynos5420() || soc_is_exynos5800())) { > > /* > > * Bypass power down for CPU0 during suspend. Check for > > @@ -137,7 +139,10 @@ void exynos_cpu_power_down(int cpu) > > if (!(val & S5P_CORE_LOCAL_PWR_EN)) > > return; > > } > > - pmu_raw_writel(0, EXYNOS_ARM_CORE_CONFIGURATION(cpu)); > > + > > + core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu)); > > + core_conf &= ~S5P_CORE_LOCAL_PWR_EN; > > + pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu)); > > } > > > > /** > > @@ -148,7 +153,12 @@ void exynos_cpu_power_down(int cpu) > > */ > > void exynos_cpu_power_up(int cpu) > > { > > - pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, > > + u32 core_conf = S5P_CORE_LOCAL_PWR_EN; > > + > > + if (soc_is_exynos3250()) > > + core_conf |= S5P_CORE_AUTOWAKEUP_EN; > > + > > + pmu_raw_writel(core_conf, > > EXYNOS_ARM_CORE_CONFIGURATION(cpu)); > > } > > > > @@ -226,6 +236,10 @@ static void exynos_core_restart(u32 core_id) > > if (!of_machine_is_compatible("samsung,exynos3250")) > > return; > > > > + while (!pmu_raw_readl(S5P_PMU_SPARE2)) > > + udelay(10); > > + udelay(10); > > We really need to start documenting this. Please add short description > why this SPARE2 check is here and who uses it. Without documenting > this behavior future generations won't be able to debug this stuff. > Imagine replacing sboot with uboot by someone... I've already planned to do this for this code and for coupled cpuidle use of SPARE2 as well. However I would really prefer to do it in an incremental patch if there are no other issues with this patchset. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics