Re: [RFT PATCH v5 3/5] cpu-topology: Move cpu topology code to common code.

[Atish Patra]

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@xxxxxxx>
> Reviewed-by: Sudeep Holla <sudeep.holla@xxxxxxx>

Thanks!

> > Signed-off-by: Atish Patra <atish.patra@xxxxxxx>
> > Tested-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
> > ---
> >   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


--
Regards,
Atish