All of lore.kernel.org
 help / color / mirror / Atom feed
From: "heying (H)" <heying24@huawei.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
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
Date: Wed, 24 Mar 2021 09:58:35 +0800	[thread overview]
Message-ID: <ceeb191e-7aaf-6f02-5d5b-d6b2e8dc4948@huawei.com> (raw)
In-Reply-To: <YFppJkpZRHMJFay0@piout.net>

Dear,


在 2021/3/24 6:18, Alexandre Belloni 写道:
> 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 <asm/dtl.h>" because it
>> is declared there.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: He Ying <heying24@huawei.com>
>> ---
>> 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.

Do you mean that 'rtc_lock' declared in linux/mc146818rtc.h points to

same thing as that defined in powerpc kernel/time.c? And you think V1

was correct? Oh, I should have added you to my patch V1 senders:)

>
> 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.
>
> I'm not completely against the rename in vr41xxx but the fix for the
> warnings can and should be contained in arch/powerpc.

Yes, I agree with you. But I have no choice because there is a compiling 
error.

Maybe there's a better way.

So, what about my patch V1? Should I resend it and add you to senders?


Thanks.


WARNING: multiple messages have this Message-ID (diff)
From: "heying (H)" <heying24@huawei.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org, a.zummo@towertech.it,
	geert+renesas@glider.be, peterz@infradead.org,
	frederic@kernel.org, linux-kernel@vger.kernel.org,
	npiggin@gmail.com, paulus@samba.org, kernelfans@gmail.com,
	tglx@linutronix.de, msuchanek@suse.de,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
Date: Wed, 24 Mar 2021 09:58:35 +0800	[thread overview]
Message-ID: <ceeb191e-7aaf-6f02-5d5b-d6b2e8dc4948@huawei.com> (raw)
In-Reply-To: <YFppJkpZRHMJFay0@piout.net>

Dear,


在 2021/3/24 6:18, Alexandre Belloni 写道:
> 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 <asm/dtl.h>" because it
>> is declared there.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: He Ying <heying24@huawei.com>
>> ---
>> 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.

Do you mean that 'rtc_lock' declared in linux/mc146818rtc.h points to

same thing as that defined in powerpc kernel/time.c? And you think V1

was correct? Oh, I should have added you to my patch V1 senders:)

>
> 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.
>
> I'm not completely against the rename in vr41xxx but the fix for the
> warnings can and should be contained in arch/powerpc.

Yes, I agree with you. But I have no choice because there is a compiling 
error.

Maybe there's a better way.

So, what about my patch V1? Should I resend it and add you to senders?


Thanks.


  parent reply	other threads:[~2021-03-24  1:59 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23  9:12 [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings He Ying
2021-03-23  9:12 ` He Ying
2021-03-23 22:18 ` Alexandre Belloni
2021-03-23 22:18   ` Alexandre Belloni
2021-03-23 23:05   ` Alexandre Belloni
2021-03-23 23:05     ` Alexandre Belloni
2021-03-24  6:14     ` Christophe Leroy
2021-03-24  6:14       ` Christophe Leroy
2021-03-24  6:22       ` Christophe Leroy
2021-03-24  6:22         ` Christophe Leroy
2021-03-24  8:29         ` heying (H)
2021-03-24  8:29           ` heying (H)
2021-03-24  1:58   ` heying (H) [this message]
2021-03-24  1:58     ` heying (H)
2021-03-24  8:19   ` Geert Uytterhoeven
2021-03-24  8:19     ` Geert Uytterhoeven
2021-03-24  9:09     ` Alexandre Belloni
2021-03-24  9:09       ` Alexandre Belloni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ceeb191e-7aaf-6f02-5d5b-d6b2e8dc4948@huawei.com \
    --to=heying24@huawei.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=frederic@kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=kernelfans@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=msuchanek@suse.de \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.