[PATCH] cpufreq: tegra186: Default divider to 35 if register read fails

[PATCH] cpufreq: tegra186: Default divider to 35 if register read fails

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

Several of the cores fail to read any registers and thus fail to
initialize cpufreq. With shared policies, this only affects the Denver
cluster, but one of the A57 cores also exhibits this behaviour. If the
value is initialized to match what is read by the downstream vendor
kernel, scaling works as expected. I have never seen this value be
anything other than 35, so it should be a relatively safe assumption.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 drivers/cpufreq/tegra186-cpufreq.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c
index cbabb726c6645d2e5f1857a47e5643c8552d1933..f017f903c6843f1881e8311753f6269637c6bc69 100644
--- a/drivers/cpufreq/tegra186-cpufreq.c
+++ b/drivers/cpufreq/tegra186-cpufreq.c
@@ -115,6 +115,11 @@ static unsigned int tegra186_cpufreq_get(unsigned int cpu)
 
 	edvd_offset = data->cpus[policy->cpu].edvd_offset;
 	ndiv = readl(data->regs + edvd_offset) & EDVD_CORE_VOLT_FREQ_F_MASK;
+	if (ndiv == 0) {
+		dev_warn_once(get_cpu_device(policy->cpu),
+			 "Scaling registers invalid, using expected values");
+		ndiv = 35;
+	}
 	cluster_id = data->cpus[policy->cpu].bpmp_cluster_id;
 	cluster = &data->clusters[cluster_id];
 	cpufreq_cpu_put(policy);

---
base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
change-id: 20250824-tegra186-cpufreq-ndiv-bc97a22814a9

Best regards,
-- 
Aaron Kling <webgeek1234@gmail.com>

Re: [PATCH] cpufreq: tegra186: Default divider to 35 if register read fails

[Aaron Kling]

On Mon, Aug 25, 2025 at 12:03 AM Aaron Kling via B4 Relay
<devnull+webgeek1234.gmail.com@kernel.org> wrote:
>
> From: Aaron Kling <webgeek1234@gmail.com>
>
> Several of the cores fail to read any registers and thus fail to
> initialize cpufreq. With shared policies, this only affects the Denver
> cluster, but one of the A57 cores also exhibits this behaviour. If the
> value is initialized to match what is read by the downstream vendor
> kernel, scaling works as expected. I have never seen this value be
> anything other than 35, so it should be a relatively safe assumption.
>
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> ---
>  drivers/cpufreq/tegra186-cpufreq.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c
> index cbabb726c6645d2e5f1857a47e5643c8552d1933..f017f903c6843f1881e8311753f6269637c6bc69 100644
> --- a/drivers/cpufreq/tegra186-cpufreq.c
> +++ b/drivers/cpufreq/tegra186-cpufreq.c
> @@ -115,6 +115,11 @@ static unsigned int tegra186_cpufreq_get(unsigned int cpu)
>
>         edvd_offset = data->cpus[policy->cpu].edvd_offset;
>         ndiv = readl(data->regs + edvd_offset) & EDVD_CORE_VOLT_FREQ_F_MASK;
> +       if (ndiv == 0) {
> +               dev_warn_once(get_cpu_device(policy->cpu),
> +                        "Scaling registers invalid, using expected values");
> +               ndiv = 35;
> +       }
>         cluster_id = data->cpus[policy->cpu].bpmp_cluster_id;
>         cluster = &data->clusters[cluster_id];
>         cpufreq_cpu_put(policy);
>
> ---
> base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
> change-id: 20250824-tegra186-cpufreq-ndiv-bc97a22814a9
>
> Best regards,
> --
> Aaron Kling <webgeek1234@gmail.com>
>
>

I tried to open a discussion about this issue on the mailing list [0],
but well over two months later, no responses have been given. I know
this isn't an ideal solution, but in lieu of better knowledge of
what's happening, this is the best I have. And this unblocks scaling
the denver cores on tegra186.

Aaron

[0] https://lore.kernel.org/linux-tegra/CALHNRZ98_gh5dKw_GoayG4ieU3TWYAFFt29=495g4+dNLzwjbg@mail.gmail.com/

Re: [PATCH] cpufreq: tegra186: Default divider to 35 if register read fails

[Mikko Perttunen]

On Monday, August 25, 2025 2:08 PM Aaron Kling wrote:
> On Mon, Aug 25, 2025 at 12:03 AM Aaron Kling via B4 Relay
> 
> <devnull+webgeek1234.gmail.com@kernel.org> wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> > 
> > Several of the cores fail to read any registers and thus fail to
> > initialize cpufreq. With shared policies, this only affects the Denver

By failing to read any registers, do you just mean that they read as 0?

I suspect the issue may be that the EDVD_COREx_VOLT_FREQ registers are just 
used to request VF transitions. If no one has requested anything, the register 
will be at its reset value, zero.

AIUI, in downstream, the driver retrieves the CPU clock rate by measuring it 
instead of calculating it from an NDIV value, hence it would not run into this 
issue. I think the conclusion would be that if the register reads as zero, we 
cannot assume any clock rate. Is it possible to tell the cpufreq framework 
that we don't know the rate and it should ask us to set the rate to something? 
Or otherwise at probe time do this by ourselves.

Thanks,
Mikko

> > cluster, but one of the A57 cores also exhibits this behaviour. If the
> > value is initialized to match what is read by the downstream vendor
> > kernel, scaling works as expected. I have never seen this value be
> > anything other than 35, so it should be a relatively safe assumption.
> > 
> > Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> > ---
> > 
> >  drivers/cpufreq/tegra186-cpufreq.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/cpufreq/tegra186-cpufreq.c
> > b/drivers/cpufreq/tegra186-cpufreq.c index
> > cbabb726c6645d2e5f1857a47e5643c8552d1933..f017f903c6843f1881e8311753f6269
> > 637c6bc69 100644 --- a/drivers/cpufreq/tegra186-cpufreq.c
> > +++ b/drivers/cpufreq/tegra186-cpufreq.c
> > @@ -115,6 +115,11 @@ static unsigned int tegra186_cpufreq_get(unsigned int
> > cpu)> 
> >         edvd_offset = data->cpus[policy->cpu].edvd_offset;
> >         ndiv = readl(data->regs + edvd_offset) &
> >         EDVD_CORE_VOLT_FREQ_F_MASK;
> > 
> > +       if (ndiv == 0) {
> > +               dev_warn_once(get_cpu_device(policy->cpu),
> > +                        "Scaling registers invalid, using expected
> > values"); +               ndiv = 35;
> > +       }
> > 
> >         cluster_id = data->cpus[policy->cpu].bpmp_cluster_id;
> >         cluster = &data->clusters[cluster_id];
> >         cpufreq_cpu_put(policy);
> > 
> > ---
> > base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
> > change-id: 20250824-tegra186-cpufreq-ndiv-bc97a22814a9
> > 
> > Best regards,
> > --
> > Aaron Kling <webgeek1234@gmail.com>
> 
> I tried to open a discussion about this issue on the mailing list [0],
> but well over two months later, no responses have been given. I know
> this isn't an ideal solution, but in lieu of better knowledge of
> what's happening, this is the best I have. And this unblocks scaling
> the denver cores on tegra186.
> 
> Aaron
> 
> [0]
> https://lore.kernel.org/linux-tegra/CALHNRZ98_gh5dKw_GoayG4ieU3TWYAFFt29=49
> 5g4+dNLzwjbg@mail.gmail.com/

Re: [PATCH] cpufreq: tegra186: Default divider to 35 if register read fails

[Aaron Kling]

On Mon, Aug 25, 2025 at 11:48 PM Mikko Perttunen <mperttunen@nvidia.com> wrote:
>
> On Monday, August 25, 2025 2:08 PM Aaron Kling wrote:
> > On Mon, Aug 25, 2025 at 12:03 AM Aaron Kling via B4 Relay
> >
> > <devnull+webgeek1234.gmail.com@kernel.org> wrote:
> > > From: Aaron Kling <webgeek1234@gmail.com>
> > >
> > > Several of the cores fail to read any registers and thus fail to
> > > initialize cpufreq. With shared policies, this only affects the Denver
>
> By failing to read any registers, do you just mean that they read as 0?

Yes, that is correct.

> I suspect the issue may be that the EDVD_COREx_VOLT_FREQ registers are just
> used to request VF transitions. If no one has requested anything, the register
> will be at its reset value, zero.
>
> AIUI, in downstream, the driver retrieves the CPU clock rate by measuring it
> instead of calculating it from an NDIV value, hence it would not run into this
> issue. I think the conclusion would be that if the register reads as zero, we
> cannot assume any clock rate. Is it possible to tell the cpufreq framework
> that we don't know the rate and it should ask us to set the rate to something?
> Or otherwise at probe time do this by ourselves.

This is a very helpful pointer. If I initialize all cores to their
respective base frequencies during probe, the subsequent get's work as
intended. I want to do a little more verification before sending in
another patch as I also found another issue with my previous shared
policy patch. I will submit a new series once that is done, so this
patch can be abandoned.

Aaron