Message ID | 20190524000653.13005-1-atish.patra@wdc.com |
---|---|
Headers | show |
Series | Unify CPU topology across ARM & RISC-V | expand |
On Thu, May 23, 2019 at 05:06:50PM -0700, Atish Patra wrote: > Both RISC-V & ARM64 are using cpu-map device tree to describe > their cpu topology. It's better to move the relevant code to > a common place instead of duplicate code. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > --- > arch/arm64/include/asm/topology.h | 23 --- > arch/arm64/kernel/topology.c | 303 +----------------------------- > drivers/base/arch_topology.c | 296 +++++++++++++++++++++++++++++ > include/linux/arch_topology.h | 28 +++ > include/linux/topology.h | 1 + > 5 files changed, 329 insertions(+), 322 deletions(-) What, now _I_ have to maintain drivers/base/arch_topology.c? That's nice for everyone else, but not me :( Ugh. Anyway, what are you wanting to happen to this series? I think we need some ARM people to sign off on it before I can take the whole thing, right? thanks, greg k-h
On Fri, May 24, 2019 at 10:13:33AM +0200, Greg Kroah-Hartman wrote: > On Thu, May 23, 2019 at 05:06:50PM -0700, Atish Patra wrote: > > Both RISC-V & ARM64 are using cpu-map device tree to describe > > their cpu topology. It's better to move the relevant code to > > a common place instead of duplicate code. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > > --- > > arch/arm64/include/asm/topology.h | 23 --- > > arch/arm64/kernel/topology.c | 303 +----------------------------- > > drivers/base/arch_topology.c | 296 +++++++++++++++++++++++++++++ > > include/linux/arch_topology.h | 28 +++ > > include/linux/topology.h | 1 + > > 5 files changed, 329 insertions(+), 322 deletions(-) > > What, now _I_ have to maintain drivers/base/arch_topology.c? That's > nice for everyone else, but not me :( > > Ugh. > > Anyway, what are you wanting to happen to this series? I think we need > some ARM people to sign off on it before I can take the whole thing, > right? > Greg, I am ready to take ownership. Juri the original author of this file agreed and I have been reviewing this file since Juri first wrote it. I am happy to submit a patch assuming maintainership for this file, was just waiting to hear from you when I asked explicitly you and Juri in last version of the patch when Will wanted someone from ARM to be reviewer of this file at-least. I am happy to take over as reviewer or maintainer which ever you prefer. Sorry if I was not so clear in my earlier mail. -- Regards, Sudeep
On Fri, May 24, 2019 at 09:57:40AM +0100, Sudeep Holla wrote: > On Fri, May 24, 2019 at 10:13:33AM +0200, Greg Kroah-Hartman wrote: > > On Thu, May 23, 2019 at 05:06:50PM -0700, Atish Patra wrote: > > > Both RISC-V & ARM64 are using cpu-map device tree to describe > > > their cpu topology. It's better to move the relevant code to > > > a common place instead of duplicate code. > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > > > --- > > > arch/arm64/include/asm/topology.h | 23 --- > > > arch/arm64/kernel/topology.c | 303 +----------------------------- > > > drivers/base/arch_topology.c | 296 +++++++++++++++++++++++++++++ > > > include/linux/arch_topology.h | 28 +++ > > > include/linux/topology.h | 1 + > > > 5 files changed, 329 insertions(+), 322 deletions(-) > > > > What, now _I_ have to maintain drivers/base/arch_topology.c? That's > > nice for everyone else, but not me :( > > > > Ugh. > > > > Anyway, what are you wanting to happen to this series? I think we need > > some ARM people to sign off on it before I can take the whole thing, > > right? > > > > Greg, I am ready to take ownership. Juri the original author of this file > agreed and I have been reviewing this file since Juri first wrote it. > I am happy to submit a patch assuming maintainership for this file, was > just waiting to hear from you when I asked explicitly you and Juri in > last version of the patch when Will wanted someone from ARM to be reviewer > of this file at-least. I am happy to take over as reviewer or maintainer > which ever you prefer. Yes, please just include this update to MAINTAINERS as part of the series. I'll ack it. Will
On Thu, May 23, 2019 at 05:06:50PM -0700, Atish Patra wrote: > Both RISC-V & ARM64 are using cpu-map device tree to describe > their cpu topology. It's better to move the relevant code to > a common place instead of duplicate code. > I couldn't test this on any ARM64 server platforms, tested on Juno and other embedded platforms. Tested-by: Sudeep Holla <sudeep.holla@arm.com> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > Signed-off-by: Atish Patra <atish.patra@wdc.com> > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > --- > arch/arm64/include/asm/topology.h | 23 --- > arch/arm64/kernel/topology.c | 303 +----------------------------- > drivers/base/arch_topology.c | 296 +++++++++++++++++++++++++++++ > include/linux/arch_topology.h | 28 +++ > include/linux/topology.h | 1 + > 5 files changed, 329 insertions(+), 322 deletions(-) > [...] > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c > index 0825c4a856e3..6b95c91e7d67 100644 > --- a/arch/arm64/kernel/topology.c > +++ b/arch/arm64/kernel/topology.c > [...] > -static int __init parse_cluster(struct device_node *cluster, int depth) > -{ > - char name[10]; > - bool leaf = true; > - bool has_cores = false; > - struct device_node *c; > - static int package_id __initdata; > - int core_id = 0; [Ultra minor nit]: you seem to have reordered the above declaration when you moved, just noticed as it showed up when comparing. > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index 1739d7e1952a..20a960131bee 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c [...] > + > +static int __init parse_cluster(struct device_node *cluster, int depth) > +{ > + char name[10]; > + bool leaf = true; > + bool has_cores = false; > + int core_id = 0; > + static int package_id __initdata; > + struct device_node *c; > + int i, ret; > + [...] > +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > +void update_siblings_masks(unsigned int cpu); > +#endif > +void remove_cpu_topology(unsigned int cpuid); > + Another thing(not a block and we can do it once this is merged) is to remove these #ifdefs -- Regards, Sudeep
On Thu, May 23, 2019 at 05:06:51PM -0700, Atish Patra wrote: > Currently, ARM32 and ARM64 uses different data structures to represent > their cpu topologies. Since, we are moving the ARM64 topology to common > code to be used by other architectures, we can reuse that for ARM32 as > well. > > Take this opprtunity to remove the redundant functions from ARM32 and > reuse the common code instead. > Tested-by: Sudeep Holla <sudeep.holla@arm.com> (on TC2) Reviewed-by : Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep
On 5/29/19 3:48 AM, Sudeep Holla wrote: > On Thu, May 23, 2019 at 05:06:50PM -0700, Atish Patra wrote: >> Both RISC-V & ARM64 are using cpu-map device tree to describe >> their cpu topology. It's better to move the relevant code to >> a common place instead of duplicate code. >> > > I couldn't test this on any ARM64 server platforms, tested on Juno > and other embedded platforms. > Jeff had tested earlier patch series on ARM64 server platform. Since then, the series has changed. Even though, I don't expect it break ARM64, if it can be verified again that would be great. @Jeff: Can you give it a shot if you have some time ? > Tested-by: Sudeep Holla <sudeep.holla@arm.com> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > Thanks! >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> >> --- >> arch/arm64/include/asm/topology.h | 23 --- >> arch/arm64/kernel/topology.c | 303 +----------------------------- >> drivers/base/arch_topology.c | 296 +++++++++++++++++++++++++++++ >> include/linux/arch_topology.h | 28 +++ >> include/linux/topology.h | 1 + >> 5 files changed, 329 insertions(+), 322 deletions(-) >> > > [...] > >> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c >> index 0825c4a856e3..6b95c91e7d67 100644 >> --- a/arch/arm64/kernel/topology.c >> +++ b/arch/arm64/kernel/topology.c >> > > [...] > >> -static int __init parse_cluster(struct device_node *cluster, int depth) >> -{ >> - char name[10]; >> - bool leaf = true; >> - bool has_cores = false; >> - struct device_node *c; >> - static int package_id __initdata; >> - int core_id = 0; > > [Ultra minor nit]: you seem to have reordered the above declaration when > you moved, just noticed as it showed up when comparing. > Arrgh. Sorry! I think I was trying to fix a checkpatch or something and forgot to revert. I will update it. >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> index 1739d7e1952a..20a960131bee 100644 >> --- a/drivers/base/arch_topology.c >> +++ b/drivers/base/arch_topology.c > > [...] > >> + >> +static int __init parse_cluster(struct device_node *cluster, int depth) >> +{ >> + char name[10]; >> + bool leaf = true; >> + bool has_cores = false; >> + int core_id = 0; >> + static int package_id __initdata; >> + struct device_node *c; >> + int i, ret; >> + > > [...] > >> +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) >> +void update_siblings_masks(unsigned int cpu); >> +#endif >> +void remove_cpu_topology(unsigned int cpuid); >> + > > Another thing(not a block and we can do it once this is merged) is to > remove these #ifdefs > This #ifdef is removed in patch 4. But we should remove the other ones around init_cpu_topology, parse_dt_topology and friends in a follow up patch once this is merged. > -- > Regards, > Sudeep >