| Message ID | 20210304110057.22144-3-zhangqing@loongson.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 76e0c88dbd2498487044b9705641de306d8f23ab |
| Headers | show |
| Series | some cleanup code | expand |
在 2021/3/4 下午7:00, Qing Zhang 写道: > The purpose of separating loongson_system_configuration from boot_param.h > is to keep the other structure consistent with the firmware. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com> - Jiaxun > --- > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ > drivers/irqchip/irq-loongson-liointc.c | 2 +- > 3 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h > index 1c1cdf57137e..035b1a69e2d0 100644 > --- a/arch/mips/include/asm/mach-loongson64/boot_param.h > +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h > @@ -198,24 +198,6 @@ enum loongson_bridge_type { > VIRTUAL = 3 > }; > > -struct loongson_system_configuration { > - u32 nr_cpus; > - u32 nr_nodes; > - int cores_per_node; > - int cores_per_package; > - u16 boot_cpu_id; > - u16 reserved_cpus_mask; > - enum loongson_cpu_type cputype; > - enum loongson_bridge_type bridgetype; > - u64 restart_addr; > - u64 poweroff_addr; > - u64 suspend_addr; > - u64 vgabios_addr; > - u32 dma_mask_bits; > - u64 workarounds; > - void (*early_config)(void); > -}; > - > extern struct efi_memory_map_loongson *loongson_memmap; > extern struct loongson_system_configuration loongson_sysconf; > > diff --git a/arch/mips/include/asm/mach-loongson64/loongson.h b/arch/mips/include/asm/mach-loongson64/loongson.h > index ac1c20e172a2..6189deb188cf 100644 > --- a/arch/mips/include/asm/mach-loongson64/loongson.h > +++ b/arch/mips/include/asm/mach-loongson64/loongson.h > @@ -12,6 +12,24 @@ > #include <linux/irq.h> > #include <boot_param.h> > > +/* machine-specific boot configuration */ > +struct loongson_system_configuration { > + u32 nr_cpus; > + u32 nr_nodes; > + int cores_per_node; > + int cores_per_package; > + u16 boot_cpu_id; > + u16 reserved_cpus_mask; > + enum loongson_cpu_type cputype; > + enum loongson_bridge_type bridgetype; > + u64 restart_addr; > + u64 poweroff_addr; > + u64 suspend_addr; > + u64 vgabios_addr; > + u32 dma_mask_bits; > + u64 workarounds; > + void (*early_config)(void); > +}; > > /* machine-specific reboot/halt operation */ > extern void mach_prepare_reboot(void); > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c > index 09b91b81851c..249566a23cc4 100644 > --- a/drivers/irqchip/irq-loongson-liointc.c > +++ b/drivers/irqchip/irq-loongson-liointc.c > @@ -16,7 +16,7 @@ > #include <linux/smp.h> > #include <linux/irqchip/chained_irq.h> > > -#include <boot_param.h> > +#include <loongson.h> > > #define LIOINTC_CHIP_IRQ 32 > #define LIOINTC_NUM_PARENT 4 >
Hi, On Thu, Mar 4, 2021 at 5:35 PM Qing Zhang <zhangqing@loongson.cn> wrote: > > The purpose of separating loongson_system_configuration from boot_param.h > is to keep the other structure consistent with the firmware. This is supposed to be a trivial patch, but the description actually confuses me. Why is the move out of "boot_param.h" keeping it consistent with fw? > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > --- > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ > drivers/irqchip/irq-loongson-liointc.c | 2 +- > 3 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h > index 1c1cdf57137e..035b1a69e2d0 100644 > --- a/arch/mips/include/asm/mach-loongson64/boot_param.h > +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h > @@ -198,24 +198,6 @@ enum loongson_bridge_type { > VIRTUAL = 3 > }; > > -struct loongson_system_configuration { > - u32 nr_cpus; > - u32 nr_nodes; > - int cores_per_node; > - int cores_per_package; > - u16 boot_cpu_id; > - u16 reserved_cpus_mask; > - enum loongson_cpu_type cputype; > - enum loongson_bridge_type bridgetype; > - u64 restart_addr; > - u64 poweroff_addr; > - u64 suspend_addr; > - u64 vgabios_addr; > - u32 dma_mask_bits; > - u64 workarounds; > - void (*early_config)(void); > -}; > - > extern struct efi_memory_map_loongson *loongson_memmap; > extern struct loongson_system_configuration loongson_sysconf; > > diff --git a/arch/mips/include/asm/mach-loongson64/loongson.h b/arch/mips/include/asm/mach-loongson64/loongson.h > index ac1c20e172a2..6189deb188cf 100644 > --- a/arch/mips/include/asm/mach-loongson64/loongson.h > +++ b/arch/mips/include/asm/mach-loongson64/loongson.h > @@ -12,6 +12,24 @@ > #include <linux/irq.h> > #include <boot_param.h> > > +/* machine-specific boot configuration */ > +struct loongson_system_configuration { > + u32 nr_cpus; > + u32 nr_nodes; > + int cores_per_node; > + int cores_per_package; > + u16 boot_cpu_id; > + u16 reserved_cpus_mask; > + enum loongson_cpu_type cputype; > + enum loongson_bridge_type bridgetype; > + u64 restart_addr; > + u64 poweroff_addr; > + u64 suspend_addr; > + u64 vgabios_addr; > + u32 dma_mask_bits; > + u64 workarounds; > + void (*early_config)(void); > +}; > > /* machine-specific reboot/halt operation */ > extern void mach_prepare_reboot(void); > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c > index 09b91b81851c..249566a23cc4 100644 > --- a/drivers/irqchip/irq-loongson-liointc.c > +++ b/drivers/irqchip/irq-loongson-liointc.c > @@ -16,7 +16,7 @@ > #include <linux/smp.h> > #include <linux/irqchip/chained_irq.h> > > -#include <boot_param.h> > +#include <loongson.h> > > #define LIOINTC_CHIP_IRQ 32 > #define LIOINTC_NUM_PARENT 4 > -- > 2.20.1 >
On 03/05/2021 06:01 PM, Philippe Mathieu-Daudé wrote: > Hi, > > On Thu, Mar 4, 2021 at 5:35 PM Qing Zhang <zhangqing@loongson.cn> wrote: >> The purpose of separating loongson_system_configuration from boot_param.h >> is to keep the other structure consistent with the firmware. > This is supposed to be a trivial patch, but the description actually > confuses me. > > Why is the move out of "boot_param.h" keeping it consistent with fw? Hi, PhilippeMathieu-Daudé Thank you for your reply. The boot_param.h file must be consistent in the kernel and the firmware pmon/cmds/bootparam.h In env.c, the loongson_system_configuration structure member gets the value passed to the firmware. eg: struct boot_params *boot_p; loongson_sysconf.restart_addr = boot_p->reset_system.ResetWarm; loongson_sysconf.poweroff_addr = boot_p->reset_system.Shutdown; loongson_sysconf.suspend_addr = boot_p->reset_system.DoSuspend; The boot_params structure is consistent with the firmware, The loongson_system_configuration is filled in the kernel, and there is no such structure in pmon-loongson3, it is actually defined in the kernel. So, remove its definition from boot_param.h. Thanks, Qing
On Thu, Mar 04, 2021 at 07:00:57PM +0800, Qing Zhang wrote: > The purpose of separating loongson_system_configuration from boot_param.h > is to keep the other structure consistent with the firmware. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > --- > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ as you are already touching mach-loongson64 files... Is there a chance you clean up that up even further ? My goal is to have only files in mach-<platform> files, which have an mach-generic counterpart. Everything else should go to its own directory. So in case of loongson something like arch/mips/include/asm/loongson for common stuff arch/mips/include/asm/loongson/32 arch/mips/include/asm/loongson/64 Comments ? Thomas.
On 03/06/2021 04:03 PM, Thomas Bogendoerfer wrote: > as you are already touching mach-loongson64 files... > > Is there a chance you clean up that up even further ? My goal is to > have only files in mach-<platform> files, which have an mach-generic > counterpart. Everything else should go to its own directory. So in > case of loongson something > > like > > arch/mips/include/asm/loongson for common stuff > arch/mips/include/asm/loongson/32 > arch/mips/include/asm/loongson/64 > > Comments ? Hi,Thomas I am very interested in cleaning up. Can you merge these two patches first? Submitting the remaining patches after other clean-up work is completed, it seems that the impact will not be significant. Thanks Qing > > Thomas.
On Sat, Mar 6, 2021, at 4:03 PM, Thomas Bogendoerfer wrote: > On Thu, Mar 04, 2021 at 07:00:57PM +0800, Qing Zhang wrote: > > The purpose of separating loongson_system_configuration from boot_param.h > > is to keep the other structure consistent with the firmware. > > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > > --- > > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ > > as you are already touching mach-loongson64 files... > > Is there a chance you clean up that up even further ? My goal is to > have only files in mach-<platform> files, which have an mach-generic > counterpart. Everything else should go to its own directory. So in > case of loongson something > > like > > arch/mips/include/asm/loongson for common stuff > arch/mips/include/asm/loongson/32 > arch/mips/include/asm/loongson/64 Hi Thomas I'm object to this idea as loongson32/2ef/64 have nothing in common. They're different cores and different SoC designed by different team with different booting protocol. Maybe it's possible to merge loongson32 into generic platform but my LS1C borad is broken... Thanks. - Jiaxun > > Comments ? > > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ] >
On Sat, Mar 06, 2021 at 05:00:15PM +0800, Jiaxun Yang wrote: > > > On Sat, Mar 6, 2021, at 4:03 PM, Thomas Bogendoerfer wrote: > > On Thu, Mar 04, 2021 at 07:00:57PM +0800, Qing Zhang wrote: > > > The purpose of separating loongson_system_configuration from boot_param.h > > > is to keep the other structure consistent with the firmware. > > > > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > > > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > > > --- > > > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > > > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ > > > > as you are already touching mach-loongson64 files... > > > > Is there a chance you clean up that up even further ? My goal is to > > have only files in mach-<platform> files, which have an mach-generic > > counterpart. Everything else should go to its own directory. So in > > case of loongson something > > > > like > > > > arch/mips/include/asm/loongson for common stuff > > arch/mips/include/asm/loongson/32 > > arch/mips/include/asm/loongson/64 > > Hi Thomas > > I'm object to this idea as loongson32/2ef/64 have nothing in common. at least they share the name loongson, so having arch/mips/include/asm/loongson sounds like a good move. And seeing diff -u mach-loongson2ef/ mach-loongson64/loongson.h | diffstat loongson.h | 137 +++++++++++++------------------------------------------------ 1 file changed, 30 insertions(+), 107 deletions(-) wc mach-loongson2ef/loongson.h 318 963 11278 mach-loongson2ef/loongson.h so there is something to shared. To me it looks like 2ef could be merged into 64, but that's nothing I'm wanting. Just to understand you, you want arch/mips/include/asm/loongson/2ef arch/mips/include/asm/loongson/32 arch/mips/include/asm/loongson/64 ? Thomas.
On Sat, Mar 6, 2021, at 5:53 PM, Thomas Bogendoerfer wrote: > On Sat, Mar 06, 2021 at 05:00:15PM +0800, Jiaxun Yang wrote: > > > > > > On Sat, Mar 6, 2021, at 4:03 PM, Thomas Bogendoerfer wrote: > > > On Thu, Mar 04, 2021 at 07:00:57PM +0800, Qing Zhang wrote: > > > > The purpose of separating loongson_system_configuration from boot_param.h > > > > is to keep the other structure consistent with the firmware. > > > > > > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > > > > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > > > > --- > > > > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > > > > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ > > > > > > as you are already touching mach-loongson64 files... > > > > > > Is there a chance you clean up that up even further ? My goal is to > > > have only files in mach-<platform> files, which have an mach-generic > > > counterpart. Everything else should go to its own directory. So in > > > case of loongson something > > > > > > like > > > > > > arch/mips/include/asm/loongson for common stuff > > > arch/mips/include/asm/loongson/32 > > > arch/mips/include/asm/loongson/64 > > > > Hi Thomas > > > > I'm object to this idea as loongson32/2ef/64 have nothing in common. > > at least they share the name loongson, so having > > arch/mips/include/asm/loongson > > sounds like a good move. > > And seeing > > diff -u mach-loongson2ef/ mach-loongson64/loongson.h | diffstat > loongson.h | 137 +++++++++++++------------------------------------------------ > 1 file changed, 30 insertions(+), 107 deletions(-) > > wc mach-loongson2ef/loongson.h > 318 963 11278 mach-loongson2ef/loongson.h > > so there is something to shared. To me it looks like 2ef could be merged > into 64, but that's nothing I'm wanting. Hmm there are duplications in loongson.h just because we didn't clean them up when splitting loongson2ef out of loongson64. > > Just to understand you, you want > > arch/mips/include/asm/loongson/2ef > arch/mips/include/asm/loongson/32 > arch/mips/include/asm/loongson/64 Yeah it looks reasonable but from my point of view doing these movement brings no actual benefit :-( Thanks. - Jiaxun > > ? > > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ] >
On Sat, Mar 06, 2021 at 06:55:41PM +0800, Jiaxun Yang wrote: > > > On Sat, Mar 6, 2021, at 5:53 PM, Thomas Bogendoerfer wrote: > [...] > > Just to understand you, you want > > > > arch/mips/include/asm/loongson/2ef > > arch/mips/include/asm/loongson/32 > > arch/mips/include/asm/loongson/64 > > Yeah it looks reasonable but from my point of view doing these movement > brings no actual benefit :-( oh it does for sure. There will no more build errors for non loogson configs for things like #include <loongson_regs.h> because it will not work for loongson either. And it will be clear, which of the 3 loongson.h is used. Which then gives chances for even more cleanups. Thomas.
On Sat, Mar 06, 2021 at 04:57:30PM +0800, zhangqing wrote: > > > On 03/06/2021 04:03 PM, Thomas Bogendoerfer wrote: > > as you are already touching mach-loongson64 files... > > > > Is there a chance you clean up that up even further ? My goal is to > > have only files in mach-<platform> files, which have an mach-generic > > counterpart. Everything else should go to its own directory. So in > > case of loongson something > > > > like > > > > arch/mips/include/asm/loongson for common stuff > > arch/mips/include/asm/loongson/32 > > arch/mips/include/asm/loongson/64 > > > > Comments ? > > Hi,Thomas > > I am very interested in cleaning up. > Can you merge these two patches first? yes, I'll just want to get fixes for 5.12 done before restarting applying mips-next stuff. Thomas.
在 2021/3/8 17:49, Thomas Bogendoerfer 写道: > On Sat, Mar 06, 2021 at 06:55:41PM +0800, Jiaxun Yang wrote: >> >> On Sat, Mar 6, 2021, at 5:53 PM, Thomas Bogendoerfer wrote: >> [...] >>> Just to understand you, you want >>> >>> arch/mips/include/asm/loongson/2ef >>> arch/mips/include/asm/loongson/32 >>> arch/mips/include/asm/loongson/64 >> Yeah it looks reasonable but from my point of view doing these movement >> brings no actual benefit :-( > oh it does for sure. There will no more build errors for non loogson > configs for things like > > #include <loongson_regs.h> It should be loongson64_regs.h then.... > > because it will not work for loongson either. And it will be clear, > which of the 3 loongson.h is used. Which then gives chances for even > more cleanups. Ahh, got your point. Probably we should rename all these loongson.h as they should be indepedent. Thanks. - Jiaxun > > Thomas. >
On Thu, Mar 04, 2021 at 07:00:57PM +0800, Qing Zhang wrote: > The purpose of separating loongson_system_configuration from boot_param.h > is to keep the other structure consistent with the firmware. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > --- > .../include/asm/mach-loongson64/boot_param.h | 18 ------------------ > .../include/asm/mach-loongson64/loongson.h | 18 ++++++++++++++++++ > drivers/irqchip/irq-loongson-liointc.c | 2 +- > 3 files changed, 19 insertions(+), 19 deletions(-) applied to mips-next. Thomas.
diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h b/arch/mips/include/asm/mach-loongson64/boot_param.h index 1c1cdf57137e..035b1a69e2d0 100644 --- a/arch/mips/include/asm/mach-loongson64/boot_param.h +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h @@ -198,24 +198,6 @@ enum loongson_bridge_type { VIRTUAL = 3 }; -struct loongson_system_configuration { - u32 nr_cpus; - u32 nr_nodes; - int cores_per_node; - int cores_per_package; - u16 boot_cpu_id; - u16 reserved_cpus_mask; - enum loongson_cpu_type cputype; - enum loongson_bridge_type bridgetype; - u64 restart_addr; - u64 poweroff_addr; - u64 suspend_addr; - u64 vgabios_addr; - u32 dma_mask_bits; - u64 workarounds; - void (*early_config)(void); -}; - extern struct efi_memory_map_loongson *loongson_memmap; extern struct loongson_system_configuration loongson_sysconf; diff --git a/arch/mips/include/asm/mach-loongson64/loongson.h b/arch/mips/include/asm/mach-loongson64/loongson.h index ac1c20e172a2..6189deb188cf 100644 --- a/arch/mips/include/asm/mach-loongson64/loongson.h +++ b/arch/mips/include/asm/mach-loongson64/loongson.h @@ -12,6 +12,24 @@ #include <linux/irq.h> #include <boot_param.h> +/* machine-specific boot configuration */ +struct loongson_system_configuration { + u32 nr_cpus; + u32 nr_nodes; + int cores_per_node; + int cores_per_package; + u16 boot_cpu_id; + u16 reserved_cpus_mask; + enum loongson_cpu_type cputype; + enum loongson_bridge_type bridgetype; + u64 restart_addr; + u64 poweroff_addr; + u64 suspend_addr; + u64 vgabios_addr; + u32 dma_mask_bits; + u64 workarounds; + void (*early_config)(void); +}; /* machine-specific reboot/halt operation */ extern void mach_prepare_reboot(void); diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c index 09b91b81851c..249566a23cc4 100644 --- a/drivers/irqchip/irq-loongson-liointc.c +++ b/drivers/irqchip/irq-loongson-liointc.c @@ -16,7 +16,7 @@ #include <linux/smp.h> #include <linux/irqchip/chained_irq.h> -#include <boot_param.h> +#include <loongson.h> #define LIOINTC_CHIP_IRQ 32 #define LIOINTC_NUM_PARENT 4