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 X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 20EC0C433C1 for ; Tue, 23 Mar 2021 23:06:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D2E43619CC for ; Tue, 23 Mar 2021 23:06:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230324AbhCWXFv (ORCPT ); Tue, 23 Mar 2021 19:05:51 -0400 Received: from relay9-d.mail.gandi.net ([217.70.183.199]:42787 "EHLO relay9-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229931AbhCWXFm (ORCPT ); Tue, 23 Mar 2021 19:05:42 -0400 X-Originating-IP: 90.65.108.55 Received: from localhost (lfbn-lyo-1-1676-55.w90-65.abo.wanadoo.fr [90.65.108.55]) (Authenticated sender: alexandre.belloni@bootlin.com) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 8A772FF805; Tue, 23 Mar 2021 23:05:37 +0000 (UTC) Date: Wed, 24 Mar 2021 00:05:37 +0100 From: Alexandre Belloni To: He Ying Cc: mpe@ellerman.id.au, benh@kernel.crashing.org, paulus@samba.org, a.zummo@towertech.it, christophe.leroy@csgroup.eu, npiggin@gmail.com, msuchanek@suse.de, tglx@linutronix.de, peterz@infradead.org, geert+renesas@glider.be, kernelfans@gmail.com, frederic@kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-rtc@vger.kernel.org Subject: Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings Message-ID: References: <20210323091257.90054-1-heying24@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org On 23/03/2021 23:18:17+0100, Alexandre Belloni wrote: > Hello, > > On 23/03/2021 05:12:57-0400, He Ying wrote: > > We found these warnings in arch/powerpc/kernel/time.c as follows: > > warning: symbol 'decrementer_max' was not declared. Should it be static? > > warning: symbol 'rtc_lock' was not declared. Should it be static? > > warning: symbol 'dtl_consumer' was not declared. Should it be static? > > > > Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h. > > Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to > > avoid the conflict with the variable in powerpc asm/time.h. > > Move 'dtl_consumer' definition behind "include " because it > > is declared there. > > > > Reported-by: Hulk Robot > > Signed-off-by: He Ying > > --- > > v2: > > - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare > > rtc_lock in powerpc asm/time.h. > > > > V1 was actually the correct thing to do. rtc_lock is there exactly > because chrp and maple are using mc146818 compatible RTCs. This is then > useful because then drivers/char/nvram.c is enabled. The proper fix > would be to scrap all of that and use rtc-cmos for those platforms as > this drives the RTC properly and exposes the NVRAM for the mc146818. > > Or at least, if there are no users for the char/nvram driver on those > two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or > more likely rename the symbol as it seems to be abused by both chrp and > powermac. > Ok so rtc_lock is not even used by the char/nvram.c driver as it is completely compiled out. I guess it is fine having it move to the individual platform as looking very quickly at the Kconfig, it is not possible to select both simultaneously. Tentative patch: 8<----- >From dfa59b6f44fdfdefafffa7666aec89e62bbd5c80 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Wed, 24 Mar 2021 00:00:03 +0100 Subject: [PATCH] powerpc: move rtc_lock to specific platforms Signed-off-by: Alexandre Belloni --- arch/powerpc/kernel/time.c | 3 --- arch/powerpc/platforms/chrp/time.c | 2 +- arch/powerpc/platforms/maple/time.c | 2 ++ 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 67feb3524460..d3bb189ea7f4 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -123,9 +123,6 @@ EXPORT_SYMBOL(tb_ticks_per_usec); unsigned long tb_ticks_per_sec; EXPORT_SYMBOL(tb_ticks_per_sec); /* for cputime_t conversions */ -DEFINE_SPINLOCK(rtc_lock); -EXPORT_SYMBOL_GPL(rtc_lock); - static u64 tb_to_ns_scale __read_mostly; static unsigned tb_to_ns_shift __read_mostly; static u64 boot_tb __read_mostly; diff --git a/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c index acde7bbe0716..ea90c15f5edd 100644 --- a/arch/powerpc/platforms/chrp/time.c +++ b/arch/powerpc/platforms/chrp/time.c @@ -30,7 +30,7 @@ #include -extern spinlock_t rtc_lock; +DEFINE_SPINLOCK(rtc_lock); #define NVRAM_AS0 0x74 #define NVRAM_AS1 0x75 diff --git a/arch/powerpc/platforms/maple/time.c b/arch/powerpc/platforms/maple/time.c index 78209bb7629c..ddda02010d86 100644 --- a/arch/powerpc/platforms/maple/time.c +++ b/arch/powerpc/platforms/maple/time.c @@ -34,6 +34,8 @@ #define DBG(x...) #endif +DEFINE_SPINLOCK(rtc_lock); + static int maple_rtc_addr; static int maple_clock_read(int addr) -- 2.25.1 > I'm not completely against the rename in vr41xxx but the fix for the > warnings can and should be contained in arch/powerpc. > > > arch/powerpc/include/asm/time.h | 3 +++ > > arch/powerpc/kernel/time.c | 6 ++---- > > drivers/rtc/rtc-vr41xx.c | 22 +++++++++++----------- > > 3 files changed, 16 insertions(+), 15 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h > > index 8dd3cdb25338..64a3ef0b4270 100644 > > --- a/arch/powerpc/include/asm/time.h > > +++ b/arch/powerpc/include/asm/time.h > > @@ -12,6 +12,7 @@ > > #ifdef __KERNEL__ > > #include > > #include > > +#include > > > > #include > > #include > > @@ -22,6 +23,8 @@ extern unsigned long tb_ticks_per_jiffy; > > extern unsigned long tb_ticks_per_usec; > > extern unsigned long tb_ticks_per_sec; > > extern struct clock_event_device decrementer_clockevent; > > +extern u64 decrementer_max; > > +extern spinlock_t rtc_lock; > > > > > > extern void generic_calibrate_decr(void); > > diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c > > index b67d93a609a2..60b6ac7d3685 100644 > > --- a/arch/powerpc/kernel/time.c > > +++ b/arch/powerpc/kernel/time.c > > @@ -150,10 +150,6 @@ bool tb_invalid; > > u64 __cputime_usec_factor; > > EXPORT_SYMBOL(__cputime_usec_factor); > > > > -#ifdef CONFIG_PPC_SPLPAR > > -void (*dtl_consumer)(struct dtl_entry *, u64); > > -#endif > > - > > static void calc_cputime_factors(void) > > { > > struct div_result res; > > @@ -179,6 +175,8 @@ static inline unsigned long read_spurr(unsigned long tb) > > > > #include > > > > +void (*dtl_consumer)(struct dtl_entry *, u64); > > + > > /* > > * Scan the dispatch trace log and count up the stolen time. > > * Should be called with interrupts disabled. > > diff --git a/drivers/rtc/rtc-vr41xx.c b/drivers/rtc/rtc-vr41xx.c > > index 5a9f9ad86d32..cc31db058197 100644 > > --- a/drivers/rtc/rtc-vr41xx.c > > +++ b/drivers/rtc/rtc-vr41xx.c > > @@ -72,7 +72,7 @@ static void __iomem *rtc2_base; > > > > static unsigned long epoch = 1970; /* Jan 1 1970 00:00:00 */ > > > > -static DEFINE_SPINLOCK(rtc_lock); > > +static DEFINE_SPINLOCK(vr41xx_rtc_lock); > > static char rtc_name[] = "RTC"; > > static unsigned long periodic_count; > > static unsigned int alarm_enabled; > > @@ -101,13 +101,13 @@ static inline time64_t read_elapsed_second(void) > > > > static inline void write_elapsed_second(time64_t sec) > > { > > - spin_lock_irq(&rtc_lock); > > + spin_lock_irq(&vr41xx_rtc_lock); > > > > rtc1_write(ETIMELREG, (uint16_t)(sec << 15)); > > rtc1_write(ETIMEMREG, (uint16_t)(sec >> 1)); > > rtc1_write(ETIMEHREG, (uint16_t)(sec >> 17)); > > > > - spin_unlock_irq(&rtc_lock); > > + spin_unlock_irq(&vr41xx_rtc_lock); > > } > > > > static int vr41xx_rtc_read_time(struct device *dev, struct rtc_time *time) > > @@ -139,14 +139,14 @@ static int vr41xx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm) > > unsigned long low, mid, high; > > struct rtc_time *time = &wkalrm->time; > > > > - spin_lock_irq(&rtc_lock); > > + spin_lock_irq(&vr41xx_rtc_lock); > > > > low = rtc1_read(ECMPLREG); > > mid = rtc1_read(ECMPMREG); > > high = rtc1_read(ECMPHREG); > > wkalrm->enabled = alarm_enabled; > > > > - spin_unlock_irq(&rtc_lock); > > + spin_unlock_irq(&vr41xx_rtc_lock); > > > > rtc_time64_to_tm((high << 17) | (mid << 1) | (low >> 15), time); > > > > @@ -159,7 +159,7 @@ static int vr41xx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm) > > > > alarm_sec = rtc_tm_to_time64(&wkalrm->time); > > > > - spin_lock_irq(&rtc_lock); > > + spin_lock_irq(&vr41xx_rtc_lock); > > > > if (alarm_enabled) > > disable_irq(aie_irq); > > @@ -173,7 +173,7 @@ static int vr41xx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm) > > > > alarm_enabled = wkalrm->enabled; > > > > - spin_unlock_irq(&rtc_lock); > > + spin_unlock_irq(&vr41xx_rtc_lock); > > > > return 0; > > } > > @@ -202,7 +202,7 @@ static int vr41xx_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long > > > > static int vr41xx_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) > > { > > - spin_lock_irq(&rtc_lock); > > + spin_lock_irq(&vr41xx_rtc_lock); > > if (enabled) { > > if (!alarm_enabled) { > > enable_irq(aie_irq); > > @@ -214,7 +214,7 @@ static int vr41xx_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) > > alarm_enabled = 0; > > } > > } > > - spin_unlock_irq(&rtc_lock); > > + spin_unlock_irq(&vr41xx_rtc_lock); > > return 0; > > } > > > > @@ -296,7 +296,7 @@ static int rtc_probe(struct platform_device *pdev) > > rtc->range_max = (1ULL << 33) - 1; > > rtc->max_user_freq = MAX_PERIODIC_RATE; > > > > - spin_lock_irq(&rtc_lock); > > + spin_lock_irq(&vr41xx_rtc_lock); > > > > rtc1_write(ECMPLREG, 0); > > rtc1_write(ECMPMREG, 0); > > @@ -304,7 +304,7 @@ static int rtc_probe(struct platform_device *pdev) > > rtc1_write(RTCL1LREG, 0); > > rtc1_write(RTCL1HREG, 0); > > > > - spin_unlock_irq(&rtc_lock); > > + spin_unlock_irq(&vr41xx_rtc_lock); > > > > aie_irq = platform_get_irq(pdev, 0); > > if (aie_irq <= 0) { > > -- > > 2.17.1 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com