* [PATCH 1/2] cpufreq: tegra186: Fix initial frequency @ 2020-07-12 10:06 Jon Hunter [not found] ` <20200712100645.13927-1-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jon Hunter @ 2020-07-12 10:06 UTC (permalink / raw) To: Thierry Reding Cc: Rafael J . Wysocki, Viresh Kumar, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jon Hunter Commit 6cc3d0e9a097 ("cpufreq: tegra186: add CPUFREQ_NEED_INITIAL_FREQ_CHECK flag") fixed CPUFREQ support for Tegra186 but as a consequence the following warnings are now seen on boot ... cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 0 KHz cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 2035200 KHz cpufreq: cpufreq_online: CPU1: Running at unlisted freq: 0 KHz cpufreq: cpufreq_online: CPU1: Unlisted initial frequency changed to: 2035200 KHz cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 0 KHz cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 2035200 KHz cpufreq: cpufreq_online: CPU3: Running at unlisted freq: 0 KHz cpufreq: cpufreq_online: CPU3: Unlisted initial frequency changed to: 2035200 KHz cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 0 KHz cpufreq: cpufreq_online: CPU4: Unlisted initial frequency changed to: 2035200 KHz cpufreq: cpufreq_online: CPU5: Running at unlisted freq: 0 KHz cpufreq: cpufreq_online: CPU5: Unlisted initial frequency changed to: 2035200 KHz Although we could fix this by adding a 'get' operator for the Tegra186 CPUFREQ driver, there is really little point because the CPUFREQ on Tegra186 is set by writing a value stored in the frequency table to a register and we just need to set the initial frequency. So for Tegra186 the simplest way to fix this is read the register that sets the frequency for each CPU and set the initial frequency when initialising the CPUFREQ driver. Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- drivers/cpufreq/tegra186-cpufreq.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c index 3d2f143748ef..c44190ce3f03 100644 --- a/drivers/cpufreq/tegra186-cpufreq.c +++ b/drivers/cpufreq/tegra186-cpufreq.c @@ -59,6 +59,7 @@ static int tegra186_cpufreq_init(struct cpufreq_policy *policy) struct tegra186_cpufreq_cluster *cluster = &data->clusters[i]; const struct tegra186_cpufreq_cluster_info *info = cluster->info; + u32 edvd_val; int core; for (core = 0; core < ARRAY_SIZE(info->cpus); core++) { @@ -71,6 +72,13 @@ static int tegra186_cpufreq_init(struct cpufreq_policy *policy) policy->driver_data = data->regs + info->offset + EDVD_CORE_VOLT_FREQ(core); policy->freq_table = cluster->table; + + edvd_val = readl(policy->driver_data); + + for (i = 0; cluster->table[i].frequency != CPUFREQ_TABLE_END; i++) { + if (cluster->table[i].driver_data == edvd_val) + policy->cur = cluster->table[i].frequency; + } break; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20200712100645.13927-1-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* [PATCH 2/2] cpufreq: tegra186: Simplify probe return path [not found] ` <20200712100645.13927-1-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2020-07-12 10:06 ` Jon Hunter [not found] ` <20200712100645.13927-2-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-13 3:25 ` [PATCH 1/2] cpufreq: tegra186: Fix initial frequency Viresh Kumar 1 sibling, 1 reply; 10+ messages in thread From: Jon Hunter @ 2020-07-12 10:06 UTC (permalink / raw) To: Thierry Reding Cc: Rafael J . Wysocki, Viresh Kumar, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jon Hunter We always put the reference to BPMP device on exit of the Tegra186 CPUFREQ driver and so there is no need to have separate exit paths for success and failure. Therefore, simplify the probe return path in the Tegra186 CPUFREQ driver by combining the success and failure paths. Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- drivers/cpufreq/tegra186-cpufreq.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c index c44190ce3f03..bf8cab357277 100644 --- a/drivers/cpufreq/tegra186-cpufreq.c +++ b/drivers/cpufreq/tegra186-cpufreq.c @@ -231,15 +231,9 @@ static int tegra186_cpufreq_probe(struct platform_device *pdev) } } - tegra_bpmp_put(bpmp); - tegra186_cpufreq_driver.driver_data = data; err = cpufreq_register_driver(&tegra186_cpufreq_driver); - if (err) - return err; - - return 0; put_bpmp: tegra_bpmp_put(bpmp); -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20200712100645.13927-2-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 2/2] cpufreq: tegra186: Simplify probe return path [not found] ` <20200712100645.13927-2-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2020-07-13 3:28 ` Viresh Kumar 0 siblings, 0 replies; 10+ messages in thread From: Viresh Kumar @ 2020-07-13 3:28 UTC (permalink / raw) To: Jon Hunter Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 12-07-20, 11:06, Jon Hunter wrote: > We always put the reference to BPMP device on exit of the Tegra186 > CPUFREQ driver and so there is no need to have separate exit paths > for success and failure. Therefore, simplify the probe return path > in the Tegra186 CPUFREQ driver by combining the success and failure > paths. > > Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > drivers/cpufreq/tegra186-cpufreq.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c > index c44190ce3f03..bf8cab357277 100644 > --- a/drivers/cpufreq/tegra186-cpufreq.c > +++ b/drivers/cpufreq/tegra186-cpufreq.c > @@ -231,15 +231,9 @@ static int tegra186_cpufreq_probe(struct platform_device *pdev) > } > } > > - tegra_bpmp_put(bpmp); > - > tegra186_cpufreq_driver.driver_data = data; > > err = cpufreq_register_driver(&tegra186_cpufreq_driver); > - if (err) > - return err; > - > - return 0; > > put_bpmp: > tegra_bpmp_put(bpmp); Applied. Thanks. -- viresh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency [not found] ` <20200712100645.13927-1-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-12 10:06 ` [PATCH 2/2] cpufreq: tegra186: Simplify probe return path Jon Hunter @ 2020-07-13 3:25 ` Viresh Kumar 2020-07-13 16:37 ` Jon Hunter 1 sibling, 1 reply; 10+ messages in thread From: Viresh Kumar @ 2020-07-13 3:25 UTC (permalink / raw) To: Jon Hunter Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 12-07-20, 11:06, Jon Hunter wrote: > Commit 6cc3d0e9a097 ("cpufreq: tegra186: add > CPUFREQ_NEED_INITIAL_FREQ_CHECK flag") fixed CPUFREQ support for > Tegra186 but as a consequence the following warnings are now seen on > boot ... > > cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 0 KHz > cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 2035200 KHz > cpufreq: cpufreq_online: CPU1: Running at unlisted freq: 0 KHz > cpufreq: cpufreq_online: CPU1: Unlisted initial frequency changed to: 2035200 KHz > cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 0 KHz > cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 2035200 KHz > cpufreq: cpufreq_online: CPU3: Running at unlisted freq: 0 KHz > cpufreq: cpufreq_online: CPU3: Unlisted initial frequency changed to: 2035200 KHz > cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 0 KHz > cpufreq: cpufreq_online: CPU4: Unlisted initial frequency changed to: 2035200 KHz > cpufreq: cpufreq_online: CPU5: Running at unlisted freq: 0 KHz > cpufreq: cpufreq_online: CPU5: Unlisted initial frequency changed to: 2035200 KHz > > Although we could fix this by adding a 'get' operator for the Tegra186 > CPUFREQ driver, there is really little point because the CPUFREQ on > Tegra186 is set by writing a value stored in the frequency table to a > register and we just need to set the initial frequency. The hardware still runs at the frequency requested by cpufreq core here, right ? It is better to provide the get() callback as it is also used to show the current frequency in userspace. > So for Tegra186 > the simplest way to fix this is read the register that sets the > frequency for each CPU and set the initial frequency when initialising > the CPUFREQ driver. > > Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > drivers/cpufreq/tegra186-cpufreq.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c > index 3d2f143748ef..c44190ce3f03 100644 > --- a/drivers/cpufreq/tegra186-cpufreq.c > +++ b/drivers/cpufreq/tegra186-cpufreq.c > @@ -59,6 +59,7 @@ static int tegra186_cpufreq_init(struct cpufreq_policy *policy) > struct tegra186_cpufreq_cluster *cluster = &data->clusters[i]; > const struct tegra186_cpufreq_cluster_info *info = > cluster->info; > + u32 edvd_val; > int core; > > for (core = 0; core < ARRAY_SIZE(info->cpus); core++) { > @@ -71,6 +72,13 @@ static int tegra186_cpufreq_init(struct cpufreq_policy *policy) > policy->driver_data = > data->regs + info->offset + EDVD_CORE_VOLT_FREQ(core); > policy->freq_table = cluster->table; > + > + edvd_val = readl(policy->driver_data); > + > + for (i = 0; cluster->table[i].frequency != CPUFREQ_TABLE_END; i++) { > + if (cluster->table[i].driver_data == edvd_val) > + policy->cur = cluster->table[i].frequency; > + } > break; > } > > -- > 2.17.1 -- viresh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency 2020-07-13 3:25 ` [PATCH 1/2] cpufreq: tegra186: Fix initial frequency Viresh Kumar @ 2020-07-13 16:37 ` Jon Hunter [not found] ` <3d6091f2-6b04-185f-6c23-e39a34b87877-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jon Hunter @ 2020-07-13 16:37 UTC (permalink / raw) To: Viresh Kumar Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 13/07/2020 04:25, Viresh Kumar wrote: > On 12-07-20, 11:06, Jon Hunter wrote: >> Commit 6cc3d0e9a097 ("cpufreq: tegra186: add >> CPUFREQ_NEED_INITIAL_FREQ_CHECK flag") fixed CPUFREQ support for >> Tegra186 but as a consequence the following warnings are now seen on >> boot ... >> >> cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 0 KHz >> cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 2035200 KHz >> cpufreq: cpufreq_online: CPU1: Running at unlisted freq: 0 KHz >> cpufreq: cpufreq_online: CPU1: Unlisted initial frequency changed to: 2035200 KHz >> cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 0 KHz >> cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 2035200 KHz >> cpufreq: cpufreq_online: CPU3: Running at unlisted freq: 0 KHz >> cpufreq: cpufreq_online: CPU3: Unlisted initial frequency changed to: 2035200 KHz >> cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 0 KHz >> cpufreq: cpufreq_online: CPU4: Unlisted initial frequency changed to: 2035200 KHz >> cpufreq: cpufreq_online: CPU5: Running at unlisted freq: 0 KHz >> cpufreq: cpufreq_online: CPU5: Unlisted initial frequency changed to: 2035200 KHz >> >> Although we could fix this by adding a 'get' operator for the Tegra186 >> CPUFREQ driver, there is really little point because the CPUFREQ on >> Tegra186 is set by writing a value stored in the frequency table to a >> register and we just need to set the initial frequency. > > The hardware still runs at the frequency requested by cpufreq core here, right ? Yes. > It is better to provide the get() callback as it is also used to show the > current frequency in userspace. I looked at that and I saw that if the get() callback is not provided, the current frequency showed by userspace is policy->cur. For this device, policy->cur is accurate and so if we added the get() callback we essentially just going to return policy->cur. Therefore, given that we already know policy->cur, I did not see the point in adding a device specific handler to do the same thing. Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <3d6091f2-6b04-185f-6c23-e39a34b87877-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency [not found] ` <3d6091f2-6b04-185f-6c23-e39a34b87877-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2020-07-14 3:46 ` Viresh Kumar 2020-07-14 7:26 ` Jon Hunter 2020-07-31 12:14 ` Jon Hunter 0 siblings, 2 replies; 10+ messages in thread From: Viresh Kumar @ 2020-07-14 3:46 UTC (permalink / raw) To: Jon Hunter Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 13-07-20, 17:37, Jon Hunter wrote: > > On 13/07/2020 04:25, Viresh Kumar wrote: > > On 12-07-20, 11:06, Jon Hunter wrote: > >> Commit 6cc3d0e9a097 ("cpufreq: tegra186: add > >> CPUFREQ_NEED_INITIAL_FREQ_CHECK flag") fixed CPUFREQ support for > >> Tegra186 but as a consequence the following warnings are now seen on > >> boot ... > >> > >> cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 0 KHz > >> cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 2035200 KHz > >> cpufreq: cpufreq_online: CPU1: Running at unlisted freq: 0 KHz > >> cpufreq: cpufreq_online: CPU1: Unlisted initial frequency changed to: 2035200 KHz > >> cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 0 KHz > >> cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 2035200 KHz > >> cpufreq: cpufreq_online: CPU3: Running at unlisted freq: 0 KHz > >> cpufreq: cpufreq_online: CPU3: Unlisted initial frequency changed to: 2035200 KHz > >> cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 0 KHz > >> cpufreq: cpufreq_online: CPU4: Unlisted initial frequency changed to: 2035200 KHz > >> cpufreq: cpufreq_online: CPU5: Running at unlisted freq: 0 KHz > >> cpufreq: cpufreq_online: CPU5: Unlisted initial frequency changed to: 2035200 KHz > >> > >> Although we could fix this by adding a 'get' operator for the Tegra186 > >> CPUFREQ driver, there is really little point because the CPUFREQ on > >> Tegra186 is set by writing a value stored in the frequency table to a > >> register and we just need to set the initial frequency. > > > > The hardware still runs at the frequency requested by cpufreq core here, right ? > > Yes. > > > It is better to provide the get() callback as it is also used to show the > > current frequency in userspace. > > I looked at that and I saw that if the get() callback is not provided, > the current frequency showed by userspace is policy->cur. For this > device, policy->cur is accurate and so if we added the get() callback we > essentially just going to return policy->cur. Therefore, given that we > already know policy->cur, I did not see the point in adding a device > specific handler to do the same thing. The get() callback is supposed to read the frequency from hardware and return it, no cached value here. policy->cur may end up being wrong in case there is a bug. -- viresh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency 2020-07-14 3:46 ` Viresh Kumar @ 2020-07-14 7:26 ` Jon Hunter [not found] ` <aa941c67-1dec-5363-7bd7-5e9d8d324110-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-31 12:14 ` Jon Hunter 1 sibling, 1 reply; 10+ messages in thread From: Jon Hunter @ 2020-07-14 7:26 UTC (permalink / raw) To: Viresh Kumar Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 14/07/2020 04:46, Viresh Kumar wrote: > On 13-07-20, 17:37, Jon Hunter wrote: >> >> On 13/07/2020 04:25, Viresh Kumar wrote: >>> On 12-07-20, 11:06, Jon Hunter wrote: >>>> Commit 6cc3d0e9a097 ("cpufreq: tegra186: add >>>> CPUFREQ_NEED_INITIAL_FREQ_CHECK flag") fixed CPUFREQ support for >>>> Tegra186 but as a consequence the following warnings are now seen on >>>> boot ... >>>> >>>> cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 0 KHz >>>> cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 2035200 KHz >>>> cpufreq: cpufreq_online: CPU1: Running at unlisted freq: 0 KHz >>>> cpufreq: cpufreq_online: CPU1: Unlisted initial frequency changed to: 2035200 KHz >>>> cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 0 KHz >>>> cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 2035200 KHz >>>> cpufreq: cpufreq_online: CPU3: Running at unlisted freq: 0 KHz >>>> cpufreq: cpufreq_online: CPU3: Unlisted initial frequency changed to: 2035200 KHz >>>> cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 0 KHz >>>> cpufreq: cpufreq_online: CPU4: Unlisted initial frequency changed to: 2035200 KHz >>>> cpufreq: cpufreq_online: CPU5: Running at unlisted freq: 0 KHz >>>> cpufreq: cpufreq_online: CPU5: Unlisted initial frequency changed to: 2035200 KHz >>>> >>>> Although we could fix this by adding a 'get' operator for the Tegra186 >>>> CPUFREQ driver, there is really little point because the CPUFREQ on >>>> Tegra186 is set by writing a value stored in the frequency table to a >>>> register and we just need to set the initial frequency. >>> >>> The hardware still runs at the frequency requested by cpufreq core here, right ? >> >> Yes. >> >>> It is better to provide the get() callback as it is also used to show the >>> current frequency in userspace. >> >> I looked at that and I saw that if the get() callback is not provided, >> the current frequency showed by userspace is policy->cur. For this >> device, policy->cur is accurate and so if we added the get() callback we >> essentially just going to return policy->cur. Therefore, given that we >> already know policy->cur, I did not see the point in adding a device >> specific handler to do the same thing. > > The get() callback is supposed to read the frequency from hardware and > return it, no cached value here. policy->cur may end up being wrong in > case there is a bug. OK, I can add a get callback. However, there are a few other drivers that set the current frequency in the init() and don't implement a get() callback ... drivers/cpufreq/pasemi-cpufreq.c drivers/cpufreq/ppc_cbe_cpufreq.c drivers/cpufreq/intel_pstate.c Jon -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <aa941c67-1dec-5363-7bd7-5e9d8d324110-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency [not found] ` <aa941c67-1dec-5363-7bd7-5e9d8d324110-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2020-07-14 7:31 ` Viresh Kumar 0 siblings, 0 replies; 10+ messages in thread From: Viresh Kumar @ 2020-07-14 7:31 UTC (permalink / raw) To: Jon Hunter Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 14-07-20, 08:26, Jon Hunter wrote: > OK, I can add a get callback. However, there are a few other drivers > that set the current frequency in the init() and don't implement a get() > callback ... > > drivers/cpufreq/pasemi-cpufreq.c > drivers/cpufreq/ppc_cbe_cpufreq.c These are quite old and I am not sure why they didn't do it. > drivers/cpufreq/intel_pstate.c With intel-pstate driver, the firmware sets the frequency of the CPUs and it can be different from what cpufreq core has asked it to. And so the kernel normally has no idea of what the frequency a CPU is running at. -- viresh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency 2020-07-14 3:46 ` Viresh Kumar 2020-07-14 7:26 ` Jon Hunter @ 2020-07-31 12:14 ` Jon Hunter 2020-08-04 5:25 ` Viresh Kumar 1 sibling, 1 reply; 10+ messages in thread From: Jon Hunter @ 2020-07-31 12:14 UTC (permalink / raw) To: Viresh Kumar Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra, linux-kernel Hi Viresh, On 14/07/2020 04:46, Viresh Kumar wrote: ... > The get() callback is supposed to read the frequency from hardware and > return it, no cached value here. policy->cur may end up being wrong in > case there is a bug. I have been doing some more testing on Tegra, I noticed that when reading the current CPU frequency via the sysfs scaling_cur_freq entry, this always returns the cached value (at least for Tegra). Looking at the implementation of scaling_cur_freq I see ... static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) { ssize_t ret; unsigned int freq; freq = arch_freq_get_on_cpu(policy->cpu); if (freq) ret = sprintf(buf, "%u\n", freq); else if (cpufreq_driver && cpufreq_driver->setpolicy && cpufreq_driver->get) ret = sprintf(buf, "%u\n", cpufreq_driver->get(policy->cpu)); else ret = sprintf(buf, "%u\n", policy->cur); return ret; } The various Tegra CPU frequency drivers do not implement the set_policy callback and hence why we always get the cached value. I see the following commit added this and before it simply return the cached value ... commit c034b02e213d271b98c45c4a7b54af8f69aaac1e Author: Dirk Brandewie <dirk.j.brandewie@intel.com> Date: Mon Oct 13 08:37:40 2014 -0700 cpufreq: expose scaling_cur_freq sysfs file for set_policy() drivers Is this intentional? Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cpufreq: tegra186: Fix initial frequency 2020-07-31 12:14 ` Jon Hunter @ 2020-08-04 5:25 ` Viresh Kumar 0 siblings, 0 replies; 10+ messages in thread From: Viresh Kumar @ 2020-08-04 5:25 UTC (permalink / raw) To: Jon Hunter; +Cc: Thierry Reding, Rafael J . Wysocki, linux-tegra, linux-kernel On 31-07-20, 13:14, Jon Hunter wrote: > I have been doing some more testing on Tegra, I noticed that when > reading the current CPU frequency via the sysfs scaling_cur_freq entry, > this always returns the cached value (at least for Tegra). Looking at > the implementation of scaling_cur_freq I see ... > > static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) > { > ssize_t ret; > unsigned int freq; > > freq = arch_freq_get_on_cpu(policy->cpu); > if (freq) > ret = sprintf(buf, "%u\n", freq); > else if (cpufreq_driver && cpufreq_driver->setpolicy && > cpufreq_driver->get) > ret = sprintf(buf, "%u\n", cpufreq_driver->get(policy->cpu)); > else > ret = sprintf(buf, "%u\n", policy->cur); > return ret; > } > > The various Tegra CPU frequency drivers do not implement the > set_policy callback and hence why we always get the cached value. I > see the following commit added this and before it simply return the > cached value ... > > commit c034b02e213d271b98c45c4a7b54af8f69aaac1e > Author: Dirk Brandewie <dirk.j.brandewie@intel.com> > Date: Mon Oct 13 08:37:40 2014 -0700 > > cpufreq: expose scaling_cur_freq sysfs file for set_policy() drivers > > Is this intentional? Yes, it is. There are two sysfs files to read the current frequency. - scaling_cur_freq: as you noticed it returns cached value unless it is for setpolicy drivers in whose case cpufreq core doesn't control the frequency and so doesn't cache any values. - cpuinfo_cur_freq: This will return the value as read from hardware using ->get() callback. -- viresh ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-04 5:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-12 10:06 [PATCH 1/2] cpufreq: tegra186: Fix initial frequency Jon Hunter [not found] ` <20200712100645.13927-1-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-12 10:06 ` [PATCH 2/2] cpufreq: tegra186: Simplify probe return path Jon Hunter [not found] ` <20200712100645.13927-2-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-13 3:28 ` Viresh Kumar 2020-07-13 3:25 ` [PATCH 1/2] cpufreq: tegra186: Fix initial frequency Viresh Kumar 2020-07-13 16:37 ` Jon Hunter [not found] ` <3d6091f2-6b04-185f-6c23-e39a34b87877-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-14 3:46 ` Viresh Kumar 2020-07-14 7:26 ` Jon Hunter [not found] ` <aa941c67-1dec-5363-7bd7-5e9d8d324110-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-14 7:31 ` Viresh Kumar 2020-07-31 12:14 ` Jon Hunter 2020-08-04 5:25 ` Viresh Kumar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).