* [PATCH 2/2] LoongArch: Remove unused variable
2022-07-20 7:21 [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage Bibo Mao
@ 2022-07-20 7:21 ` Bibo Mao
2022-07-22 8:12 ` WANG Xuerui
2022-07-20 9:00 ` [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage Huacai Chen
2022-07-22 8:10 ` WANG Xuerui
2 siblings, 1 reply; 6+ messages in thread
From: Bibo Mao @ 2022-07-20 7:21 UTC (permalink / raw)
To: Huacai Chen; +Cc: WANG Xuerui, Jiaxun Yang, loongarch, linux-kernel
There are some variables never used or referenced, this patch
removes these varaibles and make the code cleaner.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] LoongArch: Remove unused variable
2022-07-20 7:21 ` [PATCH 2/2] LoongArch: Remove unused variable Bibo Mao
@ 2022-07-22 8:12 ` WANG Xuerui
0 siblings, 0 replies; 6+ messages in thread
From: WANG Xuerui @ 2022-07-22 8:12 UTC (permalink / raw)
To: Bibo Mao, Huacai Chen; +Cc: WANG Xuerui, Jiaxun Yang, loongarch, linux-kernel
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 <maobibo@loongson.cn>
> ---
> 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 <git@xen0n.name>
[1]:
https://loongson.github.io/LoongArch-Documentation/Loongson-3A5000-usermanual-EN.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage
2022-07-20 7:21 [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage Bibo Mao
2022-07-20 7:21 ` [PATCH 2/2] LoongArch: Remove unused variable Bibo Mao
@ 2022-07-20 9:00 ` Huacai Chen
2022-07-22 8:10 ` WANG Xuerui
2 siblings, 0 replies; 6+ messages in thread
From: Huacai Chen @ 2022-07-20 9:00 UTC (permalink / raw)
To: Bibo Mao; +Cc: WANG Xuerui, Jiaxun Yang, loongarch, LKML
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 <maobibo@loongson.cn> 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 <maobibo@loongson.cn>
> ---
> 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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage
2022-07-20 7:21 [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage Bibo Mao
2022-07-20 7:21 ` [PATCH 2/2] LoongArch: Remove unused variable Bibo Mao
2022-07-20 9:00 ` [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage Huacai Chen
@ 2022-07-22 8:10 ` WANG Xuerui
2022-07-25 3:16 ` maobibo
2 siblings, 1 reply; 6+ messages in thread
From: WANG Xuerui @ 2022-07-22 8:10 UTC (permalink / raw)
To: Bibo Mao, Huacai Chen; +Cc: WANG Xuerui, Jiaxun Yang, loongarch, linux-kernel
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 <maobibo@loongson.cn>
> ---
> 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 <git@xen0n.name>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] LoongArch: Remove clock setting during cpu hotplug stage
2022-07-22 8:10 ` WANG Xuerui
@ 2022-07-25 3:16 ` maobibo
0 siblings, 0 replies; 6+ messages in thread
From: maobibo @ 2022-07-25 3:16 UTC (permalink / raw)
To: WANG Xuerui, Huacai Chen; +Cc: Jiaxun Yang, loongarch, linux-kernel
在 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 <maobibo@loongson.cn>
>> ---
>> 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 <git@xen0n.name>
^ permalink raw reply [flat|nested] 6+ messages in thread