* Re: [PATCH v4 44/46] KVM: PPC: Book3S HV P9: implement hash guest support
From: Nicholas Piggin @ 2021-03-24 1:34 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87tup1kgtb.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of March 24, 2021 1:53 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Guest entry/exit has to restore and save/clear the SLB, plus several
>> other bits to accommodate hash guests in the P9 path.
>>
>> Radix host, hash guest support is removed from the P7/8 path.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>
> <snip>
>
>> diff --git a/arch/powerpc/kvm/book3s_hv_interrupt.c b/arch/powerpc/kvm/book3s_hv_interrupt.c
>> index cd84d2c37632..03fbfef708a8 100644
>> --- a/arch/powerpc/kvm/book3s_hv_interrupt.c
>> +++ b/arch/powerpc/kvm/book3s_hv_interrupt.c
>> @@ -55,6 +55,50 @@ static void __accumulate_time(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator
>> #define accumulate_time(vcpu, next) do {} while (0)
>> #endif
>>
>> +static inline void mfslb(unsigned int idx, u64 *slbee, u64 *slbev)
>> +{
>> + asm volatile("slbmfev %0,%1" : "=r" (*slbev) : "r" (idx));
>> + asm volatile("slbmfee %0,%1" : "=r" (*slbee) : "r" (idx));
>> +}
>> +
>> +static inline void __mtslb(u64 slbee, u64 slbev)
>> +{
>> + asm volatile("slbmte %0,%1" :: "r" (slbev), "r" (slbee));
>> +}
>> +
>> +static inline void mtslb(unsigned int idx, u64 slbee, u64 slbev)
>> +{
>> + BUG_ON((slbee & 0xfff) != idx);
>> +
>> + __mtslb(slbee, slbev);
>> +}
>> +
>> +static inline void slb_invalidate(unsigned int ih)
>> +{
>> + asm volatile("slbia %0" :: "i"(ih));
>> +}
>
> Fyi, in my environment the assembler complains:
>
> {standard input}: Assembler messages:
> {standard input}:1293: Error: junk at end of line: `6'
> {standard input}:2138: Error: junk at end of line: `6'
> make[3]: *** [../scripts/Makefile.build:271:
> arch/powerpc/kvm/book3s_hv_interrupt.o] Error 1
>
> This works:
>
> - asm volatile("slbia %0" :: "i"(ih));
> + asm volatile(PPC_SLBIA(%0) :: "i"(ih));
>
> But I don't know what is going on.
Ah yes, we still need to use PPC_SLBIA. IH parameter to slbia was only
added in binutils 2.27 and we support down to 2.23.
Thanks for the fix I'll add it.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: heying (H) @ 2021-03-24 1:58 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
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.
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Madhavan Srinivasan @ 2021-03-24 4:35 UTC (permalink / raw)
To: Athira Rajeev, linuxppc-dev, linux-kernel, linux-perf-users, mpe,
acme, jolsa
Cc: peterz, ravi.bangoria, kan.liang, kjain
In-Reply-To: <1616425047-1666-2-git-send-email-atrajeev@linux.vnet.ibm.com>
On 3/22/21 8:27 PM, Athira Rajeev wrote:
> Performance Monitoring Unit (PMU) registers in powerpc provides
> information on cycles elapsed between different stages in the
> pipeline. This can be used for application tuning. On ISA v3.1
> platform, this information is exposed by sampling registers.
> Patch adds kernel support to capture two of the cycle counters
> as part of perf sample using the sample type:
> PERF_SAMPLE_WEIGHT_STRUCT.
>
> The power PMU function 'get_mem_weight' currently uses 64 bit weight
> field of perf_sample_data to capture memory latency. But following the
> introduction of PERF_SAMPLE_WEIGHT_TYPE, weight field could contain
> 64-bit or 32-bit value depending on the architexture support for
> PERF_SAMPLE_WEIGHT_STRUCT. Patches uses WEIGHT_STRUCT to expose the
> pipeline stage cycles info. Hence update the ppmu functions to work for
> 64-bit and 32-bit weight values.
>
> If the sample type is PERF_SAMPLE_WEIGHT, use the 64-bit weight field.
> if the sample type is PERF_SAMPLE_WEIGHT_STRUCT, memory subsystem
> latency is stored in the low 32bits of perf_sample_weight structure.
> Also for CPU_FTR_ARCH_31, capture the two cycle counter information in
> two 16 bit fields of perf_sample_weight structure.
Changes looks fine to me.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/perf_event_server.h | 2 +-
> arch/powerpc/perf/core-book3s.c | 4 ++--
> arch/powerpc/perf/isa207-common.c | 29 +++++++++++++++++++++++++---
> arch/powerpc/perf/isa207-common.h | 6 +++++-
> 4 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 00e7e671bb4b..112cf092d7b3 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -43,7 +43,7 @@ struct power_pmu {
> u64 alt[]);
> void (*get_mem_data_src)(union perf_mem_data_src *dsrc,
> u32 flags, struct pt_regs *regs);
> - void (*get_mem_weight)(u64 *weight);
> + void (*get_mem_weight)(u64 *weight, u64 type);
> unsigned long group_constraint_mask;
> unsigned long group_constraint_val;
> u64 (*bhrb_filter_map)(u64 branch_sample_type);
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 766f064f00fb..6936763246bd 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2206,9 +2206,9 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
> ppmu->get_mem_data_src)
> ppmu->get_mem_data_src(&data.data_src, ppmu->flags, regs);
>
> - if (event->attr.sample_type & PERF_SAMPLE_WEIGHT &&
> + if (event->attr.sample_type & PERF_SAMPLE_WEIGHT_TYPE &&
> ppmu->get_mem_weight)
> - ppmu->get_mem_weight(&data.weight.full);
> + ppmu->get_mem_weight(&data.weight.full, event->attr.sample_type);
>
> if (perf_event_overflow(event, &data, regs))
> power_pmu_stop(event, 0);
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index e4f577da33d8..5dcbdbd54598 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -284,8 +284,10 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> }
> }
>
> -void isa207_get_mem_weight(u64 *weight)
> +void isa207_get_mem_weight(u64 *weight, u64 type)
> {
> + union perf_sample_weight *weight_fields;
> + u64 weight_lat;
> u64 mmcra = mfspr(SPRN_MMCRA);
> u64 exp = MMCRA_THR_CTR_EXP(mmcra);
> u64 mantissa = MMCRA_THR_CTR_MANT(mmcra);
> @@ -296,9 +298,30 @@ void isa207_get_mem_weight(u64 *weight)
> mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
>
> if (val == 0 || val == 7)
> - *weight = 0;
> + weight_lat = 0;
> else
> - *weight = mantissa << (2 * exp);
> + weight_lat = mantissa << (2 * exp);
> +
> + /*
> + * Use 64 bit weight field (full) if sample type is
> + * WEIGHT.
> + *
> + * if sample type is WEIGHT_STRUCT:
> + * - store memory latency in the lower 32 bits.
> + * - For ISA v3.1, use remaining two 16 bit fields of
> + * perf_sample_weight to store cycle counter values
> + * from sier2.
> + */
> + weight_fields = (union perf_sample_weight *)weight;
> + if (type & PERF_SAMPLE_WEIGHT)
> + weight_fields->full = weight_lat;
> + else {
> + weight_fields->var1_dw = (u32)weight_lat;
> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> + weight_fields->var2_w = P10_SIER2_FINISH_CYC(mfspr(SPRN_SIER2));
> + weight_fields->var3_w = P10_SIER2_DISPATCH_CYC(mfspr(SPRN_SIER2));
> + }
> + }
> }
>
> int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp, u64 event_config1)
> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> index 1af0e8c97ac7..fc30d43c4d0c 100644
> --- a/arch/powerpc/perf/isa207-common.h
> +++ b/arch/powerpc/perf/isa207-common.h
> @@ -265,6 +265,10 @@
> #define ISA207_SIER_DATA_SRC_SHIFT 53
> #define ISA207_SIER_DATA_SRC_MASK (0x7ull << ISA207_SIER_DATA_SRC_SHIFT)
>
> +/* Bits in SIER2/SIER3 for Power10 */
> +#define P10_SIER2_FINISH_CYC(sier2) (((sier2) >> (63 - 37)) & 0x7fful)
> +#define P10_SIER2_DISPATCH_CYC(sier2) (((sier2) >> (63 - 13)) & 0x7fful)
> +
> #define P(a, b) PERF_MEM_S(a, b)
> #define PH(a, b) (P(LVL, HIT) | P(a, b))
> #define PM(a, b) (P(LVL, MISS) | P(a, b))
> @@ -278,6 +282,6 @@ int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
> const unsigned int ev_alt[][MAX_ALT]);
> void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> struct pt_regs *regs);
> -void isa207_get_mem_weight(u64 *weight);
> +void isa207_get_mem_weight(u64 *weight, u64 type);
>
> #endif
^ permalink raw reply
* Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
From: Cye Borg @ 2021-03-24 5:47 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Jens Axboe, Thomas Bogendoerfer, linux-doc, linux-alpha,
linux-kernel, Christoph Hellwig, linux-ide, linux-m68k,
Geert Uytterhoeven, John Paul Adrian Glaubitz, Ivan Kokshaysky,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210323184321.GE1463@shell.armlinux.org.uk>
Sure, here it is:
snow / # lspci -vxxx -s 7.0
00:07.0 ISA bridge: Contaq Microsystems 82c693
Flags: bus master, medium devsel, latency 0
Kernel modules: pata_cypress
00: 80 10 93 c6 47 00 80 02 00 00 01 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 03 02 00 00 26 60 00 01 f0 60 00 80 80 71 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Best regards,
Barnabas
ps.: let me know, if anything else I can do.
On Tue, Mar 23, 2021 at 7:43 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Mon, Mar 22, 2021 at 06:10:01PM +0100, Cye Borg wrote:
> > PWS 500au:
> >
> > snow / # lspci -vvx -s 7.1
> > 00:07.1 IDE interface: Contaq Microsystems 82c693 (prog-if 80 [ISA
> > Compatibility mode-only controller, supports bus mastering])
> > Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop-
> > ParErr+ Stepping- SERR- FastB2B- DisINTx-
> > Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> > Latency: 0
> > Interrupt: pin A routed to IRQ 0
> > Region 0: I/O ports at 01f0 [size=8]
> > Region 1: I/O ports at 03f4
> > Region 4: I/O ports at 9080 [size=16]
> > Kernel driver in use: pata_cypress
> > Kernel modules: pata_cypress
> > 00: 80 10 93 c6 45 00 80 02 00 80 01 01 00 00 80 00
> > 10: f1 01 00 00 f5 03 00 00 00 00 00 00 00 00 00 00
> > 20: 81 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00
> >
> > snow / # lspci -vvx -s 7.2
> > 00:07.2 IDE interface: Contaq Microsystems 82c693 (prog-if 00 [ISA
> > Compatibility mode-only controller])
> > Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop-
> > ParErr+ Stepping- SERR- FastB2B- DisINTx-
> > Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> > Latency: 0
> > Interrupt: pin B routed to IRQ 0
> > Region 0: I/O ports at 0170 [size=8]
> > Region 1: I/O ports at 0374
> > Region 4: Memory at 0c240000 (32-bit, non-prefetchable)
> > [disabled] [size=64K]
> > Kernel modules: pata_cypress
> > 00: 80 10 93 c6 45 00 80 02 00 00 01 01 00 00 80 00
> > 10: 71 01 00 00 75 03 00 00 00 00 00 00 00 00 00 00
> > 20: 00 00 24 0c 00 00 00 00 00 00 00 00 00 00 00 00
> > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00
>
> Thanks very much.
>
> Could I also ask for the output of:
>
> # lspci -vxxx -s 7.0
>
> as well please - this will dump all 256 bytes for the ISA bridge, which
> contains a bunch of configuration registers. Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24 6:14 UTC (permalink / raw)
To: Alexandre Belloni, He Ying
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFp0Qc2P61V+3bm0@piout.net>
Le 24/03/2021 à 00:05, Alexandre Belloni a écrit :
> 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 <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.
>>
>> 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:
>
Looking at it once more, it looks like including linux/mc146818rtc.h is the thing to do, at least
for now. Several platforms are defining the rtc_lock exactly the same way as powerpc does, and
including mc146818rtc.h
I think that to get it clean, this change should go in a dedicated patch and do a bit more and
explain exactly what is being do and why. I'll try to draft something for it.
He Y., can you make a version v3 of your patch excluding the rtc_lock change ?
Christophe
^ permalink raw reply
* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24 6:22 UTC (permalink / raw)
To: Alexandre Belloni, He Ying
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <95cd80c5-40ff-1316-9c89-2e8e7836fb6a@csgroup.eu>
Le 24/03/2021 à 07:14, Christophe Leroy a écrit :
>
>
> Le 24/03/2021 à 00:05, Alexandre Belloni a écrit :
>> 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 <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.
>>>
>>> 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:
>>
>
> Looking at it once more, it looks like including linux/mc146818rtc.h is the thing to do, at least
> for now. Several platforms are defining the rtc_lock exactly the same way as powerpc does, and
> including mc146818rtc.h
>
> I think that to get it clean, this change should go in a dedicated patch and do a bit more and
> explain exactly what is being do and why. I'll try to draft something for it.
>
> He Y., can you make a version v3 of your patch excluding the rtc_lock change ?
>
Finally, I think there is not enough changes to justify a separate patch.
So you can send a V3 based on your V1. In addition to the changes you had in V1, please remove the
declaration of rfc_lock in arch/powerpc/platforms/chrp/chrp.h
Christophe
^ permalink raw reply
* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Geert Uytterhoeven @ 2021-03-24 8:19 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-rtc, Alessandro Zummo, Geert Uytterhoeven, Peter Zijlstra,
frederic, Linux Kernel Mailing List, Paul Mackerras,
Nicholas Piggin, kernelfans, Thomas Gleixner, Michal Suchanek,
linuxppc-dev, He Ying
In-Reply-To: <YFppJkpZRHMJFay0@piout.net>
Hi Alexandre,
On Tue, Mar 23, 2021 at 11:18 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> 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.
>
> 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.
IIRC, on CHRP LongTrail, NVRAM was inherited from CHRP's Mac ancestry,
not from CHRP's PC ancestry, and thus NVRAM is not the one in the
mc146818-compatible RTC.
http://users.telenet.be/geertu/Linux/PPC/DeviceTree.html confirms that,
showing that nvram is a different device node than rtc.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: heying (H) @ 2021-03-24 8:29 UTC (permalink / raw)
To: Christophe Leroy, Alexandre Belloni
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <a30a94af-f551-c2a4-18df-a9488ba11d53@csgroup.eu>
Dear,
在 2021/3/24 14:22, Christophe Leroy 写道:
>
>
> Le 24/03/2021 à 07:14, Christophe Leroy a écrit :
>>
>>
>> Le 24/03/2021 à 00:05, Alexandre Belloni a écrit :
>>> 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 <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.
>>>>
>>>> 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:
>>>
>>
>> Looking at it once more, it looks like including linux/mc146818rtc.h
>> is the thing to do, at least for now. Several platforms are defining
>> the rtc_lock exactly the same way as powerpc does, and including
>> mc146818rtc.h
>>
>> I think that to get it clean, this change should go in a dedicated
>> patch and do a bit more and explain exactly what is being do and why.
>> I'll try to draft something for it.
>>
>> He Y., can you make a version v3 of your patch excluding the rtc_lock
>> change ?
>>
>
> Finally, I think there is not enough changes to justify a separate patch.
>
> So you can send a V3 based on your V1. In addition to the changes you
> had in V1, please remove the declaration of rfc_lock in
> arch/powerpc/platforms/chrp/chrp.h
So glad to hear that. I'll do that and send my V3.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Alexandre Belloni @ 2021-03-24 9:09 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-rtc, Alessandro Zummo, Geert Uytterhoeven, Peter Zijlstra,
frederic, Linux Kernel Mailing List, Paul Mackerras,
Nicholas Piggin, kernelfans, Thomas Gleixner, Michal Suchanek,
linuxppc-dev, He Ying
In-Reply-To: <CAMuHMdWfFtJOQQf0b-2QJRd1EMLSW7rDsjNYzjjZhg6=JNZ0AA@mail.gmail.com>
On 24/03/2021 09:19:58+0100, Geert Uytterhoeven wrote:
> Hi Alexandre,
>
> On Tue, Mar 23, 2021 at 11:18 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > 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.
> >
> > 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.
>
> IIRC, on CHRP LongTrail, NVRAM was inherited from CHRP's Mac ancestry,
> not from CHRP's PC ancestry, and thus NVRAM is not the one in the
> mc146818-compatible RTC.
>
> http://users.telenet.be/geertu/Linux/PPC/DeviceTree.html confirms that,
> showing that nvram is a different device node than rtc.
>
Yes, what I missed was the ifdefery in drivers/char/nvram.c that makes
it a completely different driver on both platforms. I tend to forget
about that as reading this driver is not a pleasant experience. I would
really like to get rid of the x86 part which would in turn allow to
remove the global rtc_lock spinlock on all architectures.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: He Ying @ 2021-03-24 9:09 UTC (permalink / raw)
To: mpe, benh, paulus, a.zummo, alexandre.belloni, christophe.leroy,
npiggin, msuchanek, heying24, tglx, peterz, geert, geert+renesas,
kernelfans, frederic
Cc: linux-rtc, linuxppc-dev, linux-kernel
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' in powerpc asm/time.h.
Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
platforms/chrp/time.c because it has included linux/mc146818rtc.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.
V3:
- Recover to V1, that is including linux/mc146818rtc.h in powerpc
kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
platforms/chrp/time.c because it has included linux/mc146818rtc.h.
arch/powerpc/include/asm/time.h | 1 +
arch/powerpc/kernel/time.c | 9 ++++-----
arch/powerpc/platforms/chrp/time.c | 2 --
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 8dd3cdb25338..2cd2b50bedda 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -22,6 +22,7 @@ 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 void generic_calibrate_decr(void);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b67d93a609a2..ac81f043bf49 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -55,8 +55,9 @@
#include <linux/sched/cputime.h>
#include <linux/sched/clock.h>
#include <linux/processor.h>
-#include <asm/trace.h>
+#include <linux/mc146818rtc.h>
+#include <asm/trace.h>
#include <asm/interrupt.h>
#include <asm/io.h>
#include <asm/nvram.h>
@@ -150,10 +151,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 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
#include <asm/dtl.h>
+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/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c
index acde7bbe0716..b94dfd5090d8 100644
--- a/arch/powerpc/platforms/chrp/time.c
+++ b/arch/powerpc/platforms/chrp/time.c
@@ -30,8 +30,6 @@
#include <platforms/chrp/chrp.h>
-extern spinlock_t rtc_lock;
-
#define NVRAM_AS0 0x74
#define NVRAM_AS1 0x75
#define NVRAM_DATA 0x77
--
2.17.1
^ permalink raw reply related
* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: Alexandre Belloni @ 2021-03-24 9:29 UTC (permalink / raw)
To: He Ying
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, kernelfans, paulus, geert, tglx, msuchanek, linuxppc-dev
In-Reply-To: <20210324090939.143477-1-heying24@huawei.com>
On 24/03/2021 05:09:39-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' in powerpc asm/time.h.
> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.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.
> V3:
> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
> kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>
> arch/powerpc/include/asm/time.h | 1 +
> arch/powerpc/kernel/time.c | 9 ++++-----
> arch/powerpc/platforms/chrp/time.c | 2 --
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..2cd2b50bedda 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -22,6 +22,7 @@ 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 void generic_calibrate_decr(void);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b67d93a609a2..ac81f043bf49 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -55,8 +55,9 @@
> #include <linux/sched/cputime.h>
> #include <linux/sched/clock.h>
> #include <linux/processor.h>
> -#include <asm/trace.h>
> +#include <linux/mc146818rtc.h>
I'm fine with that but I really think my suggestion to make the rtc_lock
local to the platforms was better because it is only used to synchronize
between concurrent invocations of chrp_set_rtc_time or
maple_set_rtc_time. The rtc core will never do that and the only case
would be concurrent calls to rtc_ops.set_time and
update_persistent_clock64 (which should also be removed at some point).
>
> +#include <asm/trace.h>
> #include <asm/interrupt.h>
> #include <asm/io.h>
> #include <asm/nvram.h>
> @@ -150,10 +151,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 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
>
> #include <asm/dtl.h>
>
> +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/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c
> index acde7bbe0716..b94dfd5090d8 100644
> --- a/arch/powerpc/platforms/chrp/time.c
> +++ b/arch/powerpc/platforms/chrp/time.c
> @@ -30,8 +30,6 @@
>
> #include <platforms/chrp/chrp.h>
>
> -extern spinlock_t rtc_lock;
> -
> #define NVRAM_AS0 0x74
> #define NVRAM_AS1 0x75
> #define NVRAM_DATA 0x77
> --
> 2.17.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24 9:29 UTC (permalink / raw)
To: He Ying, mpe, benh, paulus, a.zummo, alexandre.belloni, npiggin,
msuchanek, tglx, peterz, geert, geert+renesas, kernelfans,
frederic
Cc: linux-rtc, linuxppc-dev, linux-kernel
In-Reply-To: <20210324090939.143477-1-heying24@huawei.com>
Le 24/03/2021 à 10:09, He Ying a écrit :
> 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' in powerpc asm/time.h.
> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.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>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> V2:
> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
> rtc_lock in powerpc asm/time.h.
> V3:
> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
> kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>
> arch/powerpc/include/asm/time.h | 1 +
> arch/powerpc/kernel/time.c | 9 ++++-----
> arch/powerpc/platforms/chrp/time.c | 2 --
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..2cd2b50bedda 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -22,6 +22,7 @@ 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 void generic_calibrate_decr(void);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b67d93a609a2..ac81f043bf49 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -55,8 +55,9 @@
> #include <linux/sched/cputime.h>
> #include <linux/sched/clock.h>
> #include <linux/processor.h>
> -#include <asm/trace.h>
> +#include <linux/mc146818rtc.h>
>
> +#include <asm/trace.h>
> #include <asm/interrupt.h>
> #include <asm/io.h>
> #include <asm/nvram.h>
> @@ -150,10 +151,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 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
>
> #include <asm/dtl.h>
>
> +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/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c
> index acde7bbe0716..b94dfd5090d8 100644
> --- a/arch/powerpc/platforms/chrp/time.c
> +++ b/arch/powerpc/platforms/chrp/time.c
> @@ -30,8 +30,6 @@
>
> #include <platforms/chrp/chrp.h>
>
> -extern spinlock_t rtc_lock;
> -
> #define NVRAM_AS0 0x74
> #define NVRAM_AS1 0x75
> #define NVRAM_DATA 0x77
>
^ permalink raw reply
* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24 9:43 UTC (permalink / raw)
To: Alexandre Belloni, He Ying
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, paulus, kernelfans, geert, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFsGYgdNH5HrlqDJ@piout.net>
Le 24/03/2021 à 10:29, Alexandre Belloni a écrit :
> On 24/03/2021 05:09:39-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' in powerpc asm/time.h.
>> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
>> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
>> platforms/chrp/time.c because it has included linux/mc146818rtc.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.
>> V3:
>> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>> kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>>
>> arch/powerpc/include/asm/time.h | 1 +
>> arch/powerpc/kernel/time.c | 9 ++++-----
>> arch/powerpc/platforms/chrp/time.c | 2 --
>> 3 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
>> index 8dd3cdb25338..2cd2b50bedda 100644
>> --- a/arch/powerpc/include/asm/time.h
>> +++ b/arch/powerpc/include/asm/time.h
>> @@ -22,6 +22,7 @@ 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 void generic_calibrate_decr(void);
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index b67d93a609a2..ac81f043bf49 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -55,8 +55,9 @@
>> #include <linux/sched/cputime.h>
>> #include <linux/sched/clock.h>
>> #include <linux/processor.h>
>> -#include <asm/trace.h>
>> +#include <linux/mc146818rtc.h>
>
> I'm fine with that but I really think my suggestion to make the rtc_lock
> local to the platforms was better because it is only used to synchronize
> between concurrent invocations of chrp_set_rtc_time or
> maple_set_rtc_time. The rtc core will never do that and the only case
> would be concurrent calls to rtc_ops.set_time and
> update_persistent_clock64 (which should also be removed at some point).
I agree but I think it must be done carefully. If the lock is local to the driver really and without
a link with the RTC core, then the lock var should probably be static. But then we'll have name
conflict with the global rtc_lock which is declared in <linux/mc146818rtc.h>
All this is not easy, and I like your idea in the other mail to really clean up the rtc core and
remove this global rtc_lock completely.
For the time being I guess the fix provided by this patch is just semantic and is just fine as is,
as there is no real bug behind the messages from sparse.
Christophe
^ permalink raw reply
* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: heying (H) @ 2021-03-24 9:46 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
npiggin, kernelfans, paulus, geert, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFsGYgdNH5HrlqDJ@piout.net>
Dear Alexandre,
在 2021/3/24 17:29, Alexandre Belloni 写道:
> On 24/03/2021 05:09:39-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' in powerpc asm/time.h.
>> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
>> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
>> platforms/chrp/time.c because it has included linux/mc146818rtc.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.
>> V3:
>> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>> kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>>
>> arch/powerpc/include/asm/time.h | 1 +
>> arch/powerpc/kernel/time.c | 9 ++++-----
>> arch/powerpc/platforms/chrp/time.c | 2 --
>> 3 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
>> index 8dd3cdb25338..2cd2b50bedda 100644
>> --- a/arch/powerpc/include/asm/time.h
>> +++ b/arch/powerpc/include/asm/time.h
>> @@ -22,6 +22,7 @@ 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 void generic_calibrate_decr(void);
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index b67d93a609a2..ac81f043bf49 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -55,8 +55,9 @@
>> #include <linux/sched/cputime.h>
>> #include <linux/sched/clock.h>
>> #include <linux/processor.h>
>> -#include <asm/trace.h>
>> +#include <linux/mc146818rtc.h>
> I'm fine with that but I really think my suggestion to make the rtc_lock
> local to the platforms was better because it is only used to synchronize
> between concurrent invocations of chrp_set_rtc_time or
> maple_set_rtc_time. The rtc core will never do that and the only case
> would be concurrent calls to rtc_ops.set_time and
> update_persistent_clock64 (which should also be removed at some point).
Many thanks for your suggestion. As you suggest, rtc_lock should be
local to platforms.
Does it mean not only powerpc but also all other platforms should adapt
this change?
It might be a big change. I have no idea if that's OK. What are other
maintainers' opinions?
Thanks.
^ permalink raw reply
* Re: [PATCH v2] powerpc/papr_scm: Implement support for H_SCM_FLUSH hcall
From: Vaibhav Jain @ 2021-03-24 9:53 UTC (permalink / raw)
To: Shivaprasad G Bhat, linuxppc-dev, kvm-ppc, linux-nvdimm,
aneesh.kumar, ellerman
Cc: sbhat, linux-doc
In-Reply-To: <161651910115.13873.14215644994307713797.stgit@6532096d84d3>
Hi Shiva,
Thanks for the patch. Few minor review comments:
Shivaprasad G Bhat <sbhat@linux.ibm.com> writes:
> Add support for ND_REGION_ASYNC capability if the device tree
> indicates 'ibm,hcall-flush-required' property in the NVDIMM node.
> Flush is done by issuing H_SCM_FLUSH hcall to the hypervisor.
>
> If the flush request failed, the hypervisor is expected to
> to reflect the problem in the subsequent dimm health request call.
s/dimm/nvdimm
s/health request call/H_SCM_HEALTH hcall/
>
> This patch prevents mmap of namespaces with MAP_SYNC flag if the
> nvdimm requires explicit flush[1].
s/explicit/an explicit/
>
> References:
> [1] https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c
>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
> v1 - https://www.spinics.net/lists/kvm-ppc/msg18272.html
> Changes from v1:
> - Hcall semantics finalized, all changes are to accomodate them.
>
> Documentation/powerpc/papr_hcalls.rst | 14 ++++++++++
> arch/powerpc/include/asm/hvcall.h | 3 +-
> arch/powerpc/platforms/pseries/papr_scm.c | 39 +++++++++++++++++++++++++++++
> 3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/powerpc/papr_hcalls.rst b/Documentation/powerpc/papr_hcalls.rst
> index 48fcf1255a33..648f278eea8f 100644
> --- a/Documentation/powerpc/papr_hcalls.rst
> +++ b/Documentation/powerpc/papr_hcalls.rst
> @@ -275,6 +275,20 @@ Health Bitmap Flags:
> Given a DRC Index collect the performance statistics for NVDIMM and copy them
> to the resultBuffer.
>
> +**H_SCM_FLUSH**
> +
> +| Input: *drcIndex, continue-token*
> +| Out: *continue-token*
> +| Return Value: *H_SUCCESS, H_Parameter, H_P2, H_BUSY*
> +
> +Given a DRC Index Flush the data to backend NVDIMM device.
> +
> +The hcall returns H_BUSY when the flush takes longer time and the hcall needs
> +to be issued multiple times in order to be completely serviced. The
> +*continue-token* from the output to be passed in the argument list of
> +subsequent hcalls to the hypervisor until the hcall is completely serviced
> +at which point H_SUCCESS or other error is returned by the hypervisor.
> +
> References
> ==========
> .. [1] "Power Architecture Platform Reference"
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index ed6086d57b22..9f7729a97ebd 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -315,7 +315,8 @@
> #define H_SCM_HEALTH 0x400
> #define H_SCM_PERFORMANCE_STATS 0x418
> #define H_RPT_INVALIDATE 0x448
> -#define MAX_HCALL_OPCODE H_RPT_INVALIDATE
> +#define H_SCM_FLUSH 0x44C
> +#define MAX_HCALL_OPCODE H_SCM_FLUSH
>
> /* Scope args for H_SCM_UNBIND_ALL */
> #define H_UNBIND_SCOPE_ALL (0x1)
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 835163f54244..f0407e135410 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -93,6 +93,7 @@ struct papr_scm_priv {
> uint64_t block_size;
> int metadata_size;
> bool is_volatile;
> + bool hcall_flush_required;
>
> uint64_t bound_addr;
>
> @@ -117,6 +118,38 @@ struct papr_scm_priv {
> size_t stat_buffer_len;
> };
>
> +static int papr_scm_pmem_flush(struct nd_region *nd_region,
> + struct bio *bio __maybe_unused)
> +{
> + struct papr_scm_priv *p = nd_region_provider_data(nd_region);
> + unsigned long ret_buf[PLPAR_HCALL_BUFSIZE];
> + uint64_t token = 0;
> + int64_t rc;
> +
Suggest adding a dev_dbg to to indicate a flush request to a drc. That
way if the loop below gets stuck the issue can be debugged with kernel
logs.
> + do {
> + rc = plpar_hcall(H_SCM_FLUSH, ret_buf, p->drc_index, token);
> + token = ret_buf[0];
> +
> + /* Check if we are stalled for some time */
> + if (H_IS_LONG_BUSY(rc)) {
> + msleep(get_longbusy_msecs(rc));
> + rc = H_BUSY;
> + } else if (rc == H_BUSY) {
> + cond_resched();
> + }
> +
> + } while (rc == H_BUSY);
> +
> + if (rc) {
> + dev_err(&p->pdev->dev, "flush error: %lld", rc);
> + rc = -EIO;
> + } else {
> + dev_dbg(&p->pdev->dev, "flush drc 0x%x complete", p->drc_index);
> + }
> +
> + return rc;
> +}
> +
> static LIST_HEAD(papr_nd_regions);
> static DEFINE_MUTEX(papr_ndr_lock);
>
> @@ -943,6 +976,11 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> ndr_desc.num_mappings = 1;
> ndr_desc.nd_set = &p->nd_set;
>
> + if (p->hcall_flush_required) {
> + set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
> + ndr_desc.flush = papr_scm_pmem_flush;
> + }
> +
> if (p->is_volatile)
> p->region = nvdimm_volatile_region_create(p->bus, &ndr_desc);
> else {
> @@ -1088,6 +1126,7 @@ static int papr_scm_probe(struct platform_device *pdev)
> p->block_size = block_size;
> p->blocks = blocks;
> p->is_volatile = !of_property_read_bool(dn, "ibm,cache-flush-required");
> + p->hcall_flush_required = of_property_read_bool(dn, "ibm,hcall-flush-required");
>
> /* We just need to ensure that set cookies are unique across */
> uuid_parse(uuid_str, (uuid_t *) uuid);
>
>
--
Cheers
~ Vaibhav
^ permalink raw reply
* Re: [PATCH 07/10] MIPS: disable CONFIG_IDE in bigsur_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 10:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-8-hch@lst.de>
On Thu, Mar 18, 2021 at 05:57:03AM +0100, Christoph Hellwig wrote:
> bigsur_defconfig enables CONFIG_IDE for the tc86c001 ide driver, which
> is a Toshiba plug in card that does not make much sense to use on bigsur
> platforms. For all other ATA cards libata support is already enabled.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/mips/configs/bigsur_defconfig | 4 ----
> 1 file changed, 4 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH 05/10] MIPS: switch workpad_defconfig from legacy IDE to libata
From: Thomas Bogendoerfer @ 2021-03-24 10:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-6-hch@lst.de>
On Thu, Mar 18, 2021 at 05:57:01AM +0100, Christoph Hellwig wrote:
> Use libata instead of the deprecated legacy ide driver in
> workpad_defconfig.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/mips/configs/workpad_defconfig | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH 06/10] MIPS: disable CONFIG_IDE in rbtx49xx_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 10:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-7-hch@lst.de>
On Thu, Mar 18, 2021 at 05:57:02AM +0100, Christoph Hellwig wrote:
> rbtx49xx_defconfig enables CONFIG_IDE for the tx4938 and tx4939 ide
> drivers, but those aren't actually used by the last known remaining user:
>
> https://lore.kernel.org/lkml/20210107.101729.1936921832901251107.anemo@mba.ocn.ne.jp/
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/mips/configs/rbtx49xx_defconfig | 3 ---
> 1 file changed, 3 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH 04/10] MIPS: disable CONFIG_IDE in sb1250_swarm_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 9:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-5-hch@lst.de>
On Thu, Mar 18, 2021 at 05:57:00AM +0100, Christoph Hellwig wrote:
> sb1250_swarm_defconfig enables CONFIG_IDE but no actual host controller
> driver, so just drop CONFIG_IDE, CONFIG_BLK_DEV_IDECD and
> CONFIG_BLK_DEV_IDETAPE as they are useless.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/mips/configs/sb1250_swarm_defconfig | 3 ---
> 1 file changed, 3 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH 08/10] MIPS: disable CONFIG_IDE in malta*_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 10:01 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-9-hch@lst.de>
On Thu, Mar 18, 2021 at 05:57:04AM +0100, Christoph Hellwig wrote:
> Various malta defconfigs enable CONFIG_IDE for the tc86c001 ide driver,
> hich is a Toshiba plug in card that does not make much sense to use on
> bigsur platforms. For all other ATA cards libata support is already
> enabled.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/mips/configs/malta_kvm_defconfig | 3 ---
> arch/mips/configs/malta_kvm_guest_defconfig | 3 ---
> arch/mips/configs/maltaup_xpa_defconfig | 3 ---
> 3 files changed, 9 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* [PATCH 0/6] ASoC: fsl: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24 9:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
When there is power domain bind with ipg clock,
The call flow:
devm_regmap_init_mmio_clk
- clk_prepare()
- clk_pm_runtime_get()
cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.
So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk.
Shengjiu Wang (6):
ASoC: fsl_esai: Don't use devm_regmap_init_mmio_clk
ASoC: fsl_spdif: Don't use devm_regmap_init_mmio_clk
ASoC: fsl_asrc: Don't use devm_regmap_init_mmio_clk
ASoC: fsl_easrc: Don't use devm_regmap_init_mmio_clk
ASoC: fsl_audmix: Don't use devm_regmap_init_mmio_clk
ASoC: fsl_micfil: Don't use devm_regmap_init_mmio_clk
sound/soc/fsl/fsl_asrc.c | 57 +++++++++++++++++++++++++++++---------
sound/soc/fsl/fsl_audmix.c | 3 +-
sound/soc/fsl/fsl_easrc.c | 3 +-
sound/soc/fsl/fsl_esai.c | 48 ++++++++++++++++++++++++--------
sound/soc/fsl/fsl_micfil.c | 25 +++++++++++++----
sound/soc/fsl/fsl_spdif.c | 3 +-
6 files changed, 103 insertions(+), 36 deletions(-)
--
2.27.0
^ permalink raw reply
* [PATCH 5/6] ASoC: fsl_audmix: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24 9:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>
When there is power domain bind with ipg clock,
The call flow:
devm_regmap_init_mmio_clk
- clk_prepare()
- clk_pm_runtime_get()
cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.
So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_audmix.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index 8dc44dec7956..f931288e256c 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -476,8 +476,7 @@ static int fsl_audmix_probe(struct platform_device *pdev)
if (IS_ERR(regs))
return PTR_ERR(regs);
- priv->regmap = devm_regmap_init_mmio_clk(dev, "ipg", regs,
- &fsl_audmix_regmap_config);
+ priv->regmap = devm_regmap_init_mmio(dev, regs, &fsl_audmix_regmap_config);
if (IS_ERR(priv->regmap)) {
dev_err(dev, "failed to init regmap\n");
return PTR_ERR(priv->regmap);
--
2.27.0
^ permalink raw reply related
* [PATCH 2/6] ASoC: fsl_spdif: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24 9:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>
When there is power domain bind with core clock,
The call flow:
devm_regmap_init_mmio_clk
- clk_prepare()
- clk_pm_runtime_get()
cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.
So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_spdif.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 1cd3441d1c03..c631de325a6e 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1294,8 +1294,7 @@ static int fsl_spdif_probe(struct platform_device *pdev)
if (IS_ERR(regs))
return PTR_ERR(regs);
- spdif_priv->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
- "core", regs, &fsl_spdif_regmap_config);
+ spdif_priv->regmap = devm_regmap_init_mmio(&pdev->dev, regs, &fsl_spdif_regmap_config);
if (IS_ERR(spdif_priv->regmap)) {
dev_err(&pdev->dev, "regmap init failed\n");
return PTR_ERR(spdif_priv->regmap);
--
2.27.0
^ permalink raw reply related
* [PATCH 4/6] ASoC: fsl_easrc: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24 9:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>
When there is power domain bind with mem clock,
The call flow:
devm_regmap_init_mmio_clk
- clk_prepare()
- clk_pm_runtime_get()
cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.
So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_easrc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 600e0d670ca6..5e33afe87c4a 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -1896,8 +1896,7 @@ static int fsl_easrc_probe(struct platform_device *pdev)
easrc->paddr = res->start;
- easrc->regmap = devm_regmap_init_mmio_clk(dev, "mem", regs,
- &fsl_easrc_regmap_config);
+ easrc->regmap = devm_regmap_init_mmio(dev, regs, &fsl_easrc_regmap_config);
if (IS_ERR(easrc->regmap)) {
dev_err(dev, "failed to init regmap");
return PTR_ERR(easrc->regmap);
--
2.27.0
^ permalink raw reply related
* [PATCH 6/6] ASoC: fsl_micfil: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24 9:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>
When there is power domain bind with ipg_clk clock,
The call flow:
devm_regmap_init_mmio_clk
- clk_prepare()
- clk_pm_runtime_get()
cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.
So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Reviewed-by: Viorel Suman <viorel.suman@nxp.com>
---
sound/soc/fsl/fsl_micfil.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 2b9edd4bb94d..3cf789ed6cbe 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -31,6 +31,7 @@ struct fsl_micfil {
struct platform_device *pdev;
struct regmap *regmap;
const struct fsl_micfil_soc_data *soc;
+ struct clk *busclk;
struct clk *mclk;
struct snd_dmaengine_dai_dma_data dma_params_rx;
unsigned int dataline;
@@ -660,16 +661,22 @@ static int fsl_micfil_probe(struct platform_device *pdev)
return PTR_ERR(micfil->mclk);
}
+ micfil->busclk = devm_clk_get(&pdev->dev, "ipg_clk");
+ if (IS_ERR(micfil->busclk)) {
+ dev_err(&pdev->dev, "failed to get ipg clock: %ld\n",
+ PTR_ERR(micfil->busclk));
+ return PTR_ERR(micfil->busclk);
+ }
+
/* init regmap */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
regs = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(regs))
return PTR_ERR(regs);
- micfil->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
- "ipg_clk",
- regs,
- &fsl_micfil_regmap_config);
+ micfil->regmap = devm_regmap_init_mmio(&pdev->dev,
+ regs,
+ &fsl_micfil_regmap_config);
if (IS_ERR(micfil->regmap)) {
dev_err(&pdev->dev, "failed to init MICFIL regmap: %ld\n",
PTR_ERR(micfil->regmap));
@@ -729,6 +736,7 @@ static int fsl_micfil_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, micfil);
pm_runtime_enable(&pdev->dev);
+ regcache_cache_only(micfil->regmap, true);
ret = devm_snd_soc_register_component(&pdev->dev, &fsl_micfil_component,
&fsl_micfil_dai, 1);
@@ -752,6 +760,7 @@ static int __maybe_unused fsl_micfil_runtime_suspend(struct device *dev)
regcache_cache_only(micfil->regmap, true);
clk_disable_unprepare(micfil->mclk);
+ clk_disable_unprepare(micfil->busclk);
return 0;
}
@@ -761,10 +770,16 @@ static int __maybe_unused fsl_micfil_runtime_resume(struct device *dev)
struct fsl_micfil *micfil = dev_get_drvdata(dev);
int ret;
- ret = clk_prepare_enable(micfil->mclk);
+ ret = clk_prepare_enable(micfil->busclk);
if (ret < 0)
return ret;
+ ret = clk_prepare_enable(micfil->mclk);
+ if (ret < 0) {
+ clk_disable_unprepare(micfil->busclk);
+ return ret;
+ }
+
regcache_cache_only(micfil->regmap, false);
regcache_mark_dirty(micfil->regmap);
regcache_sync(micfil->regmap);
--
2.27.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox