Re: [PATCH 1/9] cpufreq: tegra194: move MPIDR read to CPU hotplug callback

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

On 11/5/2026 2:15 AM, Sumit Gupta wrote:
> At probe the driver reads the MPIDR of every possible CPU with
> smp_call_function_single(). For a CPU that is offline at boot, the call
> fails and the error is ignored, so the CPU's core and cluster IDs come
> from an uninitialised variable.
> 
> Read MPIDR from a CPU-hotplug online callback instead, which runs on the
> target CPU and caches its core and cluster IDs and MMIO scratch address.
> This covers CPUs online at probe and CPUs onlined later.
> 
> The check that the SoC data covers all CPUs moves to probe, against
> nr_cpu_ids. A CPU whose MPIDR does not fit the SoC data gets no scratch
> address, and its NDIV access is skipped.
> 
> Fixes: 6b121b4cf7e1 ("cpufreq: tegra194: save CPU data to avoid repeated SMP calls")
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>   drivers/cpufreq/tegra194-cpufreq.c | 114 ++++++++++++++++++++---------
>   1 file changed, 79 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
> index 1eb96e9f3310..be81ffd23edd 100644
> --- a/drivers/cpufreq/tegra194-cpufreq.c
> +++ b/drivers/cpufreq/tegra194-cpufreq.c
> @@ -5,6 +5,7 @@
>   
>   #include <linux/cpu.h>
>   #include <linux/cpufreq.h>
> +#include <linux/cpuhotplug.h>
>   #include <linux/dma-mapping.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> @@ -13,7 +14,7 @@
>   #include <linux/slab.h>
>   #include <linux/units.h>
>   
> -#include <asm/smp_plat.h>
> +#include <asm/cputype.h>
>   
>   #include <soc/tegra/bpmp.h>
>   #include <soc/tegra/bpmp-abi.h>
> @@ -77,6 +78,8 @@ struct tegra194_cpufreq_data {
>   	const struct tegra_cpufreq_soc *soc;
>   	bool icc_dram_bw_scaling;
>   	struct tegra_cpu_data *cpu_data;
> +	enum cpuhp_state cpuhp_state;
> +	struct hlist_node cpuhp_node;
>   };
>   
>   static struct workqueue_struct *read_counters_wq;
> @@ -113,7 +116,7 @@ static void tegra234_get_cpu_cluster_id(u32 cpu, u32 *cpuid, u32 *clusterid)
>   {
>   	u64 mpidr;
>   
> -	smp_call_function_single(cpu, tegra_get_cpu_mpidr, &mpidr, true);
> +	tegra_get_cpu_mpidr(&mpidr);
>   
>   	if (cpuid)
>   		*cpuid = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> @@ -121,11 +124,44 @@ static void tegra234_get_cpu_cluster_id(u32 cpu, u32 *cpuid, u32 *clusterid)
>   		*clusterid = MPIDR_AFFINITY_LEVEL(mpidr, 2);
>   }
>   
> +static int tegra_cpufreq_cpu_online(unsigned int cpu, struct hlist_node *node)
> +{
> +	struct tegra194_cpufreq_data *data;
> +	u32 cpuid, clusterid;
> +
> +	data = hlist_entry(node, struct tegra194_cpufreq_data, cpuhp_node);
> +
> +	data->soc->ops->get_cpu_cluster_id(cpu, &cpuid, &clusterid);
> +
> +	data->cpu_data[cpu].cpuid = cpuid;
> +	data->cpu_data[cpu].clusterid = clusterid;
> +
> +	/* Derive per-core register address only when ccplex MMIO is mapped. */
> +	if (data->regs) {
> +		if (clusterid >= data->soc->num_clusters ||
> +		    cpuid >= data->soc->maxcpus_per_cluster) {
> +			pr_warn_once("cpufreq: cpu%u: bad MPIDR (cluster %u, core %u)\n",
> +				     cpu, clusterid, cpuid);
> +		} else {
> +			u32 index = clusterid * data->soc->maxcpus_per_cluster + cpuid;
> +
> +			data->cpu_data[cpu].freq_core_reg =
> +				SCRATCH_FREQ_CORE_REG(data, index);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static int tegra234_get_cpu_ndiv(u32 cpu, u32 cpuid, u32 clusterid, u64 *ndiv)
>   {
>   	struct tegra194_cpufreq_data *data = cpufreq_get_driver_data();
> +	void __iomem *reg = data->cpu_data[cpu].freq_core_reg;
> +
> +	if (!reg)
> +		return -EINVAL;
>   
> -	*ndiv = readl(data->cpu_data[cpu].freq_core_reg) & NDIV_MASK;
> +	*ndiv = readl(reg) & NDIV_MASK;
>   
>   	return 0;
>   }
> @@ -135,8 +171,12 @@ static void tegra234_set_cpu_ndiv(struct cpufreq_policy *policy, u64 ndiv)
>   	struct tegra194_cpufreq_data *data = cpufreq_get_driver_data();
>   	u32 cpu;
>   
> -	for_each_cpu(cpu, policy->cpus)
> -		writel(ndiv, data->cpu_data[cpu].freq_core_reg);
> +	for_each_cpu(cpu, policy->cpus) {
> +		void __iomem *reg = data->cpu_data[cpu].freq_core_reg;
> +
> +		if (reg)
> +			writel(ndiv, reg);
> +	}
>   }
>   
>   /*
> @@ -208,7 +248,7 @@ static void tegra194_get_cpu_cluster_id(u32 cpu, u32 *cpuid, u32 *clusterid)
>   {
>   	u64 mpidr;
>   
> -	smp_call_function_single(cpu, tegra_get_cpu_mpidr, &mpidr, true);
> +	tegra_get_cpu_mpidr(&mpidr);

.get_cpu_cluster_id() now reads the local MPIDR, so the cpu argument is
ignored. Could it be dropped?



-- 
Thx and BRs,
Zhongqiu Han