2022-07-20 07:27:46

by Bibo Mao

[permalink] [raw]
Subject: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage

On physical machine we can save power by disabling clock of hot removed cpu.
However there will be problem, since different platforms have different clock
setting methods, the code is platform relative. Also it can be in firmware/pmu
compoments or cpu regulator driver, rather than general loongarch cpu booting
flow.

Also on qemu virt machine, device clock/freq setting is not
emulated, there is no such registers.

This patch removes hard-coded register accessing in generic
loongarch cpu boot flow.

Signed-off-by: Bibo Mao <[email protected]>
---
arch/loongarch/kernel/smp.c | 29 -----------------------------
include/linux/cpuhotplug.h | 1 -
2 files changed, 30 deletions(-)

diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index 73cec62504fb..98b3e059d344 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -359,35 +359,6 @@ void play_dead(void)
play_dead_uncached(state_addr);
}

-static int loongson3_enable_clock(unsigned int cpu)
-{
- uint64_t core_id = cpu_data[cpu].core;
- uint64_t package_id = cpu_data[cpu].package;
-
- LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
-
- return 0;
-}
-
-static int loongson3_disable_clock(unsigned int cpu)
-{
- uint64_t core_id = cpu_data[cpu].core;
- uint64_t package_id = cpu_data[cpu].package;
-
- LOONGSON_FREQCTRL(package_id) &= ~(1 << (core_id * 4 + 3));
-
- return 0;
-}
-
-static int register_loongson3_notifier(void)
-{
- return cpuhp_setup_state_nocalls(CPUHP_LOONGARCH_SOC_PREPARE,
- "loongarch/loongson:prepare",
- loongson3_enable_clock,
- loongson3_disable_clock);
-}
-early_initcall(register_loongson3_notifier);
-
#endif

/*
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 19f0dbfdd7fe..b66c5f389159 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -130,7 +130,6 @@ enum cpuhp_state {
CPUHP_ZCOMP_PREPARE,
CPUHP_TIMERS_PREPARE,
CPUHP_MIPS_SOC_PREPARE,
- CPUHP_LOONGARCH_SOC_PREPARE,
CPUHP_BP_PREPARE_DYN,
CPUHP_BP_PREPARE_DYN_END = CPUHP_BP_PREPARE_DYN + 20,
CPUHP_BRINGUP_CPU,
--
2.31.1


2022-07-20 07:37:27

by Bibo Mao

[permalink] [raw]
Subject: [PATCH 2/2] LoongArch: Remove unused variable

There are some variables never used or referenced, this patch
removes these varaibles and make the code cleaner.

Signed-off-by: Bibo Mao <[email protected]>
---
arch/loongarch/include/asm/loongson.h | 12 ------------
arch/loongarch/kernel/env.c | 20 --------------------
2 files changed, 32 deletions(-)

diff --git a/arch/loongarch/include/asm/loongson.h b/arch/loongarch/include/asm/loongson.h
index 6a8038725ba7..2df649e73371 100644
--- a/arch/loongarch/include/asm/loongson.h
+++ b/arch/loongarch/include/asm/loongson.h
@@ -39,18 +39,6 @@ extern const struct plat_smp_ops loongson3_smp_ops;

#define MAX_PACKAGES 16

-/* Chip Config register of each physical cpu package */
-extern u64 loongson_chipcfg[MAX_PACKAGES];
-#define LOONGSON_CHIPCFG(id) (*(volatile u32 *)(loongson_chipcfg[id]))
-
-/* Chip Temperature register of each physical cpu package */
-extern u64 loongson_chiptemp[MAX_PACKAGES];
-#define LOONGSON_CHIPTEMP(id) (*(volatile u32 *)(loongson_chiptemp[id]))
-
-/* Freq Control register of each physical cpu package */
-extern u64 loongson_freqctrl[MAX_PACKAGES];
-#define LOONGSON_FREQCTRL(id) (*(volatile u32 *)(loongson_freqctrl[id]))
-
#define xconf_readl(addr) readl(addr)
#define xconf_readq(addr) readq(addr)

diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
index 467946ecf451..82b478a5c665 100644
--- a/arch/loongarch/kernel/env.c
+++ b/arch/loongarch/kernel/env.c
@@ -17,21 +17,6 @@ u64 efi_system_table;
struct loongson_system_configuration loongson_sysconf;
EXPORT_SYMBOL(loongson_sysconf);

-u64 loongson_chipcfg[MAX_PACKAGES];
-u64 loongson_chiptemp[MAX_PACKAGES];
-u64 loongson_freqctrl[MAX_PACKAGES];
-unsigned long long smp_group[MAX_PACKAGES];
-
-static void __init register_addrs_set(u64 *registers, const u64 addr, int num)
-{
- u64 i;
-
- for (i = 0; i < num; i++) {
- *registers = (i << 44) | addr;
- registers++;
- }
-}
-
void __init init_environ(void)
{
int efi_boot = fw_arg0;
@@ -50,11 +35,6 @@ void __init init_environ(void)
efi_memmap_init_early(&data);
memblock_reserve(data.phys_map & PAGE_MASK,
PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
-
- register_addrs_set(smp_group, TO_UNCACHE(0x1fe01000), 16);
- register_addrs_set(loongson_chipcfg, TO_UNCACHE(0x1fe00180), 16);
- register_addrs_set(loongson_chiptemp, TO_UNCACHE(0x1fe0019c), 16);
- register_addrs_set(loongson_freqctrl, TO_UNCACHE(0x1fe001d0), 16);
}

static int __init init_cpu_fullname(void)
--
2.31.1

2022-07-20 09:10:24

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage

Hi, Bibo,

This series looks good to me, but I want to wait for some time to see
if others have comments. Thanks.

Huacai

On Wed, Jul 20, 2022 at 3:22 PM Bibo Mao <[email protected]> wrote:
>
> On physical machine we can save power by disabling clock of hot removed cpu.
> However there will be problem, since different platforms have different clock
> setting methods, the code is platform relative. Also it can be in firmware/pmu
> compoments or cpu regulator driver, rather than general loongarch cpu booting
> flow.
>
> Also on qemu virt machine, device clock/freq setting is not
> emulated, there is no such registers.
>
> This patch removes hard-coded register accessing in generic
> loongarch cpu boot flow.
>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> arch/loongarch/kernel/smp.c | 29 -----------------------------
> include/linux/cpuhotplug.h | 1 -
> 2 files changed, 30 deletions(-)
>
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index 73cec62504fb..98b3e059d344 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -359,35 +359,6 @@ void play_dead(void)
> play_dead_uncached(state_addr);
> }
>
> -static int loongson3_enable_clock(unsigned int cpu)
> -{
> - uint64_t core_id = cpu_data[cpu].core;
> - uint64_t package_id = cpu_data[cpu].package;
> -
> - LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
> -
> - return 0;
> -}
> -
> -static int loongson3_disable_clock(unsigned int cpu)
> -{
> - uint64_t core_id = cpu_data[cpu].core;
> - uint64_t package_id = cpu_data[cpu].package;
> -
> - LOONGSON_FREQCTRL(package_id) &= ~(1 << (core_id * 4 + 3));
> -
> - return 0;
> -}
> -
> -static int register_loongson3_notifier(void)
> -{
> - return cpuhp_setup_state_nocalls(CPUHP_LOONGARCH_SOC_PREPARE,
> - "loongarch/loongson:prepare",
> - loongson3_enable_clock,
> - loongson3_disable_clock);
> -}
> -early_initcall(register_loongson3_notifier);
> -
> #endif
>
> /*
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 19f0dbfdd7fe..b66c5f389159 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -130,7 +130,6 @@ enum cpuhp_state {
> CPUHP_ZCOMP_PREPARE,
> CPUHP_TIMERS_PREPARE,
> CPUHP_MIPS_SOC_PREPARE,
> - CPUHP_LOONGARCH_SOC_PREPARE,
> CPUHP_BP_PREPARE_DYN,
> CPUHP_BP_PREPARE_DYN_END = CPUHP_BP_PREPARE_DYN + 20,
> CPUHP_BRINGUP_CPU,
> --
> 2.31.1
>
>

2022-07-22 08:17:35

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH 2/2] LoongArch: Remove unused variable


On 2022/7/20 15:21, Bibo Mao wrote:
> There are some variables never used or referenced, this patch
> removes these varaibles and make the code cleaner.
>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> arch/loongarch/include/asm/loongson.h | 12 ------------
> arch/loongarch/kernel/env.c | 20 --------------------
> 2 files changed, 32 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/loongson.h b/arch/loongarch/include/asm/loongson.h
> index 6a8038725ba7..2df649e73371 100644
> --- a/arch/loongarch/include/asm/loongson.h
> +++ b/arch/loongarch/include/asm/loongson.h
> @@ -39,18 +39,6 @@ extern const struct plat_smp_ops loongson3_smp_ops;
>
> #define MAX_PACKAGES 16
>
> -/* Chip Config register of each physical cpu package */
> -extern u64 loongson_chipcfg[MAX_PACKAGES];
> -#define LOONGSON_CHIPCFG(id) (*(volatile u32 *)(loongson_chipcfg[id]))
> -
> -/* Chip Temperature register of each physical cpu package */
> -extern u64 loongson_chiptemp[MAX_PACKAGES];
> -#define LOONGSON_CHIPTEMP(id) (*(volatile u32 *)(loongson_chiptemp[id]))
> -
> -/* Freq Control register of each physical cpu package */
> -extern u64 loongson_freqctrl[MAX_PACKAGES];
> -#define LOONGSON_FREQCTRL(id) (*(volatile u32 *)(loongson_freqctrl[id]))
> -
> #define xconf_readl(addr) readl(addr)
> #define xconf_readq(addr) readq(addr)
>
> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> index 467946ecf451..82b478a5c665 100644
> --- a/arch/loongarch/kernel/env.c
> +++ b/arch/loongarch/kernel/env.c
> @@ -17,21 +17,6 @@ u64 efi_system_table;
> struct loongson_system_configuration loongson_sysconf;
> EXPORT_SYMBOL(loongson_sysconf);
>
> -u64 loongson_chipcfg[MAX_PACKAGES];
> -u64 loongson_chiptemp[MAX_PACKAGES];
> -u64 loongson_freqctrl[MAX_PACKAGES];
> -unsigned long long smp_group[MAX_PACKAGES];
> -
> -static void __init register_addrs_set(u64 *registers, const u64 addr, int num)
> -{
> - u64 i;
> -
> - for (i = 0; i < num; i++) {
> - *registers = (i << 44) | addr;
> - registers++;
> - }
> -}
> -
> void __init init_environ(void)
> {
> int efi_boot = fw_arg0;
> @@ -50,11 +35,6 @@ void __init init_environ(void)
> efi_memmap_init_early(&data);
> memblock_reserve(data.phys_map & PAGE_MASK,
> PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> -
> - register_addrs_set(smp_group, TO_UNCACHE(0x1fe01000), 16);
> - register_addrs_set(loongson_chipcfg, TO_UNCACHE(0x1fe00180), 16);
> - register_addrs_set(loongson_chiptemp, TO_UNCACHE(0x1fe0019c), 16);
> - register_addrs_set(loongson_freqctrl, TO_UNCACHE(0x1fe001d0), 16);
> }
>
> static int __init init_cpu_fullname(void)

These information, although removed here, are actually available in the
official docs repo [1], so no harm in cleaning the code. Thanks.

Reviewed-by: WANG Xuerui <[email protected]>

[1]:
https://loongson.github.io/LoongArch-Documentation/Loongson-3A5000-usermanual-EN.html

2022-07-22 08:58:38

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage

On 2022/7/20 15:21, Bibo Mao wrote:
> On physical machine we can save power by disabling clock of hot removed cpu.
> However there will be problem, since different platforms have different clock
> setting methods, the code is platform relative. Also it can be in firmware/pmu
> compoments or cpu regulator driver, rather than general loongarch cpu booting
> flow.
>
> Also on qemu virt machine, device clock/freq setting is not
> emulated, there is no such registers.
>
> This patch removes hard-coded register accessing in generic
> loongarch cpu boot flow.

Improving a little on the wording (mostly fixing eyesore Chinglish):

"On physical machine we can save power by disabling clock of hot removed
cpu. However as different platforms require different methods to
configure clocks, the code is platform-specific, and probably belongs to
firmware/pmu or cpu regulator, rather than generic arch/loongarch code.

Also, there is no such register on QEMU virt machine since the
clock/frequency regulation is not emulated.

This patch removes the hard-coded clock register accesses in generic
loongarch cpu hotplug flow."

>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> arch/loongarch/kernel/smp.c | 29 -----------------------------
> include/linux/cpuhotplug.h | 1 -
> 2 files changed, 30 deletions(-)
>
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index 73cec62504fb..98b3e059d344 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -359,35 +359,6 @@ void play_dead(void)
> play_dead_uncached(state_addr);
> }
>
> -static int loongson3_enable_clock(unsigned int cpu)
> -{
> - uint64_t core_id = cpu_data[cpu].core;
> - uint64_t package_id = cpu_data[cpu].package;
> -
> - LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
> -
> - return 0;
> -}
> -
> -static int loongson3_disable_clock(unsigned int cpu)
> -{
> - uint64_t core_id = cpu_data[cpu].core;
> - uint64_t package_id = cpu_data[cpu].package;
> -
> - LOONGSON_FREQCTRL(package_id) &= ~(1 << (core_id * 4 + 3));
> -
> - return 0;
> -}
> -
> -static int register_loongson3_notifier(void)
> -{
> - return cpuhp_setup_state_nocalls(CPUHP_LOONGARCH_SOC_PREPARE,
> - "loongarch/loongson:prepare",
> - loongson3_enable_clock,
> - loongson3_disable_clock);
> -}
> -early_initcall(register_loongson3_notifier);
> -
> #endif
>
> /*
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 19f0dbfdd7fe..b66c5f389159 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -130,7 +130,6 @@ enum cpuhp_state {
> CPUHP_ZCOMP_PREPARE,
> CPUHP_TIMERS_PREPARE,
> CPUHP_MIPS_SOC_PREPARE,
> - CPUHP_LOONGARCH_SOC_PREPARE,
> CPUHP_BP_PREPARE_DYN,
> CPUHP_BP_PREPARE_DYN_END = CPUHP_BP_PREPARE_DYN + 20,
> CPUHP_BRINGUP_CPU,

Seems good. I should have noticed earlier the fact the clock control
registers are, in every regard, model-specific, thus not appropriate for
arch/loongarch. Proper drivers should be added afterwards, though I
assume you must internally have something like that already.

Reviewed-by: WANG Xuerui <[email protected]>

2022-07-25 03:30:30

by Bibo Mao

[permalink] [raw]
Subject: Re: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage



在 2022/7/22 16:10, WANG Xuerui 写道:
> On 2022/7/20 15:21, Bibo Mao wrote:
>> On physical machine we can save power by disabling clock of hot removed cpu.
>> However there will be problem, since different platforms have different clock
>> setting methods, the code is platform relative. Also it can be in firmware/pmu
>> compoments or cpu regulator driver, rather than general loongarch cpu booting
>> flow.
>>
>> Also on qemu virt machine, device clock/freq setting is not
>> emulated, there is no such registers.
>>
>> This patch removes hard-coded register accessing in generic
>> loongarch cpu boot flow.
>
> Improving a little on the wording (mostly fixing eyesore Chinglish):
>
> "On physical machine we can save power by disabling clock of hot removed cpu. However as different platforms require different methods to configure clocks, the code is platform-specific, and probably belongs to firmware/pmu or cpu regulator, rather than generic arch/loongarch code.
>
> Also, there is no such register on QEMU virt machine since the clock/frequency regulation is not emulated.
>
> This patch removes the hard-coded clock register accesses in generic loongarch cpu hotplug flow."
>
Xuerui,

Thanks for reviewing my patch, I will modify the issue in next patch.

regards
bibo, mao

>>
>> Signed-off-by: Bibo Mao <[email protected]>
>> ---
>>   arch/loongarch/kernel/smp.c | 29 -----------------------------
>>   include/linux/cpuhotplug.h  |  1 -
>>   2 files changed, 30 deletions(-)
>>
>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>> index 73cec62504fb..98b3e059d344 100644
>> --- a/arch/loongarch/kernel/smp.c
>> +++ b/arch/loongarch/kernel/smp.c
>> @@ -359,35 +359,6 @@ void play_dead(void)
>>       play_dead_uncached(state_addr);
>>   }
>>   -static int loongson3_enable_clock(unsigned int cpu)
>> -{
>> -    uint64_t core_id = cpu_data[cpu].core;
>> -    uint64_t package_id = cpu_data[cpu].package;
>> -
>> -    LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
>> -
>> -    return 0;
>> -}
>> -
>> -static int loongson3_disable_clock(unsigned int cpu)
>> -{
>> -    uint64_t core_id = cpu_data[cpu].core;
>> -    uint64_t package_id = cpu_data[cpu].package;
>> -
>> -    LOONGSON_FREQCTRL(package_id) &= ~(1 << (core_id * 4 + 3));
>> -
>> -    return 0;
>> -}
>> -
>> -static int register_loongson3_notifier(void)
>> -{
>> -    return cpuhp_setup_state_nocalls(CPUHP_LOONGARCH_SOC_PREPARE,
>> -                     "loongarch/loongson:prepare",
>> -                     loongson3_enable_clock,
>> -                     loongson3_disable_clock);
>> -}
>> -early_initcall(register_loongson3_notifier);
>> -
>>   #endif
>>     /*
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index 19f0dbfdd7fe..b66c5f389159 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -130,7 +130,6 @@ enum cpuhp_state {
>>       CPUHP_ZCOMP_PREPARE,
>>       CPUHP_TIMERS_PREPARE,
>>       CPUHP_MIPS_SOC_PREPARE,
>> -    CPUHP_LOONGARCH_SOC_PREPARE,
>>       CPUHP_BP_PREPARE_DYN,
>>       CPUHP_BP_PREPARE_DYN_END        = CPUHP_BP_PREPARE_DYN + 20,
>>       CPUHP_BRINGUP_CPU,
>
> Seems good. I should have noticed earlier the fact the clock control registers are, in every regard, model-specific, thus not appropriate for arch/loongarch. Proper drivers should be added afterwards, though I assume you must internally have something like that already.
>
> Reviewed-by: WANG Xuerui <[email protected]>