Linux-Tegra Archive on lore.kernel.org
 help / color / Atom feed
* [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	[flat|nested] 10+ messages in thread

* [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	[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 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
  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

* 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

* 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, back to index

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

Linux-Tegra Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-tegra/0 linux-tegra/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-tegra linux-tegra/ https://lore.kernel.org/linux-tegra \
		linux-tegra@vger.kernel.org
	public-inbox-index linux-tegra

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-tegra


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git