Re: [PATCH v2 1/1] arm: topology: parse the topology from the dt

[Ruifeng Zhang <ruifeng.zhang0110@gmail.com>]

Dear Dietmar,

In the last, update_cpu_capacity(cpuid) must be called in
store_cpu_topology because the cpuid_topo->package_id is always be the
initialization value.

Because of added the DT parsing logic, the cpuid_topo->package_id will
be parse in driver/base/arch_topology and the update_cpu_capacity
can't be called if DT has cpu-map.

Update cpu capacity isn't related with cpu topology, so move it to the
beginning of this function.

Please help to review and test the new patch, thanks.

Ruifeng Zhang <ruifeng.zhang0110@gmail.com> 于2021年4月14日周三 下午8:24写道：
>
> From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
>
> The arm topology still parse from the MPIDR, but it is incomplete.  When
> the armv8.2 or above cpu runs kernel in EL1 with aarch32 mode, it will
> parse out the wrong topology.
>
> Changed:
> 1) Arm using the same parse_dt_topology function as arm64.
> 2) For compatibility to keep the function of get capacity from default
>    cputype, renamed arm parse_dt_topology to get_efficiency_capacity and
>    delete related logic of parse from dt.
> 3) The update_cpu_capacity function should not depend on the topology, so
>    it is moved to the beginning of store_cpu_topology.
>
> The arm device boot step is to look for the efficiency_capacity firstly.
> Then parse the topology and capacity from dt to replace efficiency value.
>
> Signed-off-by: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
> Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com>
> ---
>  arch/arm/kernel/topology.c    | 22 ++++++----------------
>  drivers/base/arch_topology.c  |  4 ++--
>  include/linux/arch_topology.h |  1 +
>  3 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index ef0058de432b..93d875320cc4 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -72,7 +72,6 @@ static unsigned long *__cpu_capacity;
>  #define cpu_capacity(cpu)      __cpu_capacity[cpu]
>
>  static unsigned long middle_capacity = 1;
> -static bool cap_from_dt = true;
>
>  /*
>   * Iterate all CPUs' descriptor in DT and compute the efficiency
> @@ -82,7 +81,7 @@ static bool cap_from_dt = true;
>   * 'average' CPU is of middle capacity. Also see the comments near
>   * table_efficiency[] and update_cpu_capacity().
>   */
> -static void __init parse_dt_topology(void)
> +static void __init get_coretype_capacity(void)
>  {
>         const struct cpu_efficiency *cpu_eff;
>         struct device_node *cn = NULL;
> @@ -105,13 +104,6 @@ static void __init parse_dt_topology(void)
>                         continue;
>                 }
>
> -               if (topology_parse_cpu_capacity(cn, cpu)) {
> -                       of_node_put(cn);
> -                       continue;
> -               }
> -
> -               cap_from_dt = false;
> -
>                 for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
>                         if (of_device_is_compatible(cn, cpu_eff->compatible))
>                                 break;
> @@ -151,9 +143,6 @@ static void __init parse_dt_topology(void)
>         else
>                 middle_capacity = ((max_capacity / 3)
>                                 >> (SCHED_CAPACITY_SHIFT-1)) + 1;
> -
> -       if (cap_from_dt)
> -               topology_normalize_cpu_scale();
>  }
>
>  /*
> @@ -163,7 +152,7 @@ static void __init parse_dt_topology(void)
>   */
>  static void update_cpu_capacity(unsigned int cpu)
>  {
> -       if (!cpu_capacity(cpu) || cap_from_dt)
> +       if (!cpu_capacity(cpu))
>                 return;
>
>         topology_set_cpu_scale(cpu, cpu_capacity(cpu) / middle_capacity);
> @@ -173,7 +162,7 @@ static void update_cpu_capacity(unsigned int cpu)
>  }
>
>  #else
> -static inline void parse_dt_topology(void) {}
> +static inline void get_cputype_capacity(void) {}
>  static inline void update_cpu_capacity(unsigned int cpuid) {}
>  #endif
>
> @@ -187,6 +176,8 @@ void store_cpu_topology(unsigned int cpuid)
>         struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
>         unsigned int mpidr;
>
> +       update_cpu_capacity(cpuid);
> +
>         if (cpuid_topo->package_id != -1)
>                 goto topology_populated;
>
> @@ -221,8 +212,6 @@ void store_cpu_topology(unsigned int cpuid)
>                 cpuid_topo->package_id = -1;
>         }
>
> -       update_cpu_capacity(cpuid);
> -
>         pr_info("CPU%u: thread %d, cpu %d, socket %d, mpidr %x\n",
>                 cpuid, cpu_topology[cpuid].thread_id,
>                 cpu_topology[cpuid].core_id,
> @@ -241,5 +230,6 @@ void __init init_cpu_topology(void)
>         reset_cpu_topology();
>         smp_wmb();
>
> +       get_coretype_capacity();
>         parse_dt_topology();
>  }
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index de8587cc119e..a45aec356ec4 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -295,7 +295,7 @@ static void parsing_done_workfn(struct work_struct *work)
>  core_initcall(free_raw_capacity);
>  #endif
>
> -#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
>  /*
>   * This function returns the logic cpu number of the node.
>   * There are basically three kinds of return values:
> @@ -441,7 +441,7 @@ static int __init parse_cluster(struct device_node *cluster, int depth)
>         return 0;
>  }
>
> -static int __init parse_dt_topology(void)
> +int __init parse_dt_topology(void)
>  {
>         struct device_node *cn, *map;
>         int ret = 0;
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index 0f6cd6b73a61..cfa5a5072aa0 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -66,6 +66,7 @@ extern struct cpu_topology cpu_topology[NR_CPUS];
>  #define topology_llc_cpumask(cpu)      (&cpu_topology[cpu].llc_sibling)
>  void init_cpu_topology(void);
>  void store_cpu_topology(unsigned int cpuid);
> +int __init parse_dt_topology(void);
>  const struct cpumask *cpu_coregroup_mask(int cpu);
>  void update_siblings_masks(unsigned int cpu);
>  void remove_cpu_topology(unsigned int cpuid);
> --
> 2.17.1
>