Re: [PATCH] cpufreq: Covert to exit callback returning void

[Viresh Kumar]

On 10-04-24, 06:22, Lizhe wrote:
> For the exit() callback function returning an int type value.
> this leads many driver authors mistakenly believing that error
> handling can be performed by returning an error code. However.
> the returned value is ignore, and to improve this situation.
> it is proposed to modify the return type of the exit() callback
> function to void
> 
> Signed-off-by: Lizhe 
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 4 +---
>  drivers/cpufreq/amd-pstate.c   | 7 ++-
>  drivers/cpufreq/apple-soc-cpufreq.c| 4 +---
>  drivers/cpufreq/bmips-cpufreq.c| 4 +---
>  drivers/cpufreq/cppc_cpufreq.c | 3 +--
>  drivers/cpufreq/cpufreq-dt.c   | 3 +--
>  drivers/cpufreq/e_powersaver.c | 3 +--
>  drivers/cpufreq/intel_pstate.c | 4 +---
>  drivers/cpufreq/mediatek-cpufreq-hw.c  | 4 +---
>  drivers/cpufreq/mediatek-cpufreq.c | 4 +---
>  drivers/cpufreq/omap-cpufreq.c | 3 +--
>  drivers/cpufreq/pasemi-cpufreq.c   | 6 ++
>  drivers/cpufreq/powernow-k6.c  | 3 +--
>  drivers/cpufreq/powernow-k7.c  | 3 +--
>  drivers/cpufreq/powernow-k8.c  | 4 +---
>  drivers/cpufreq/powernv-cpufreq.c  | 4 +---
>  drivers/cpufreq/ppc_cbe_cpufreq.c  | 3 +--
>  drivers/cpufreq/qcom-cpufreq-hw.c  | 4 +---
>  drivers/cpufreq/qoriq-cpufreq.c| 4 +---
>  drivers/cpufreq/scmi-cpufreq.c | 4 +---
>  drivers/cpufreq/scpi-cpufreq.c | 4 +---
>  drivers/cpufreq/sh-cpufreq.c   | 4 +---
>  drivers/cpufreq/sparc-us2e-cpufreq.c   | 3 +--
>  drivers/cpufreq/sparc-us3-cpufreq.c| 3 +--
>  drivers/cpufreq/speedstep-centrino.c   | 4 +---
>  drivers/cpufreq/tegra194-cpufreq.c | 4 +---
>  drivers/cpufreq/vexpress-spc-cpufreq.c | 3 +--
>  27 files changed, 29 insertions(+), 74 deletions(-)

I have discarded all emails with following subject line:

"cpufreq: Convert to exit callback returning void".

While you have sent decent patches for removing the empty exit callbacks, the
way you have handled these changes is not correct.

Don't send any patches for now and please wait and understand what's being asked
from you.

This change you are trying to make is okay and sensible, but you can not send
random patches to the list just like that. You are wasting everyone's time here
including yourself.

Now what we expect here is a single commit (with version history), which changes
all the users of the exit() function (each and every cpufreq driver) and
cpufreq.h and cpufreq.c. That change should compile fine and break none of the
platforms compilation.

Please don't send more of these patches unless this is done.

-- 
viresh

Re: [PATCH v2] cpufreq: pmac32: Use of_property_read_reg() to parse "reg"

[Viresh Kumar]

On 25-09-23, 14:02, Rob Herring wrote:
> Use the recently added of_property_read_reg() helper to get the
> untranslated "reg" address value.
> 
> Acked-by: Viresh Kumar 
> Signed-off-by: Rob Herring 
> ---
> v2:
>  - Add missing include
> ---
>  drivers/cpufreq/pmac32-cpufreq.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)

Applied. Thanks.

-- 
viresh

Re: [PATCH] cpufreq: pmac32: Use of_property_read_reg() to parse "reg"

[Viresh Kumar]

On 09-06-23, 12:31, Rob Herring wrote:
> Use the recently added of_property_read_reg() helper to get the
> untranslated "reg" address value.
> 
> Signed-off-by: Rob Herring 
> ---
>  drivers/cpufreq/pmac32-cpufreq.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/pmac32-cpufreq.c 
> b/drivers/cpufreq/pmac32-cpufreq.c
> index ec75e79659ac..f53635ba16c1 100644
> --- a/drivers/cpufreq/pmac32-cpufreq.c
> +++ b/drivers/cpufreq/pmac32-cpufreq.c
> @@ -378,10 +378,9 @@ static int pmac_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>  
>  static u32 read_gpio(struct device_node *np)
>  {
> - const u32 *reg = of_get_property(np, "reg", NULL);
> - u32 offset;
> + u64 offset;
>  
> - if (reg == NULL)
> + if (of_property_read_reg(np, 0, , NULL) < 0)
>   return 0;
>   /* That works for all keylargos but shall be fixed properly
>* some day... The problem is that it seems we can't rely

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 18/19] OPP: Adjust includes to remove of_device.h

[Viresh Kumar]

On 29-03-23, 10:52, Rob Herring wrote:
> Now that of_cpu_device_node_get() is defined in of.h, of_device.h is just
> implicitly including other includes, and is no longer needed. Adjust the
> include files with what was implicitly included by of_device.h (cpu.h and
> of.h) and drop including of_device.h.
> 
> Signed-off-by: Rob Herring 
> ---
> Please ack and I will take the series via the DT tree.
> ---
>  drivers/opp/of.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index e55c6095adf0..63b126c6215e 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -13,7 +13,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 15/19] cpufreq: sun50i: Add explicit include for cpu.h

[Viresh Kumar]

On 29-03-23, 10:52, Rob Herring wrote:
> Removing the include of cpu.h from of_device.h causes an error:
> 
> drivers/cpufreq/sun50i-cpufreq-nvmem.c:42:19: error: implicit declaration of 
> function ‘get_cpu_device’; did you mean ‘get_device’? 
> [-Werror=implicit-function-declaration]
> 
> As of_device.h is not otherwise needed, it can be replaced with of.h
> (also implicitly included).
> 
> Signed-off-by: Rob Herring 
> ---
> Please ack and I will take the series via the DT tree.
> ---
>  drivers/cpufreq/sun50i-cpufreq-nvmem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c 
> b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> index 1583a370da39..4321d7bbe769 100644
> --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> @@ -10,9 +10,10 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 14/19] cpufreq: Adjust includes to remove of_device.h

[Viresh Kumar]

On 29-03-23, 10:52, Rob Herring wrote:
> Now that of_cpu_device_node_get() is defined in of.h, of_device.h is just
> implicitly including other includes, and is no longer needed. Adjust the
> include files with what was implicitly included by of_device.h (cpu.h and
> of.h) and drop including of_device.h.
> 
> Signed-off-by: Rob Herring 
> ---
> Please ack and I will take the series via the DT tree.
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 -
>  drivers/cpufreq/kirkwood-cpufreq.c   | 2 +-
>  drivers/cpufreq/maple-cpufreq.c  | 2 +-
>  drivers/cpufreq/pmac32-cpufreq.c | 2 +-
>  drivers/cpufreq/pmac64-cpufreq.c | 2 +-
>  drivers/cpufreq/qcom-cpufreq-hw.c| 4 ++--
>  drivers/cpufreq/spear-cpufreq.c  | 2 +-
>  drivers/cpufreq/tegra124-cpufreq.c   | 1 -
>  drivers/cpufreq/tegra20-cpufreq.c| 2 +-
>  include/linux/cpufreq.h  | 1 -
>  10 files changed, 8 insertions(+), 11 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH] cpufreq: pmac32: Use of_property_read_bool() for boolean properties

[Viresh Kumar]

On 10-03-23, 08:47, Rob Herring wrote:
> It is preferred to use typed property access functions (i.e.
> of_property_read_ functions) rather than low-level
> of_get_property/of_find_property functions for reading properties.
> Convert reading boolean properties to to of_property_read_bool().
> 
> Signed-off-by: Rob Herring 
> ---
>  drivers/cpufreq/pmac32-cpufreq.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH] cpufreq: pmac32-cpufreq: Fix refcount leak bug

[Viresh Kumar]

On 18-06-22, 10:25, Liang He wrote:
> In pmac_cpufreq_init_MacRISC3(), we need to add corresponding
> of_node_put() for the three node pointers whose refcount have
> been incremented by of_find_node_by_name().
> 
> Signed-off-by: Liang He 
> ---
>  drivers/cpufreq/pmac32-cpufreq.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/cpufreq/pmac32-cpufreq.c 
> b/drivers/cpufreq/pmac32-cpufreq.c
> index 20f64a8b0a35..4b8ee2014da6 100644
> --- a/drivers/cpufreq/pmac32-cpufreq.c
> +++ b/drivers/cpufreq/pmac32-cpufreq.c
> @@ -470,6 +470,10 @@ static int pmac_cpufreq_init_MacRISC3(struct device_node 
> *cpunode)
>   if (slew_done_gpio_np)
>   slew_done_gpio = read_gpio(slew_done_gpio_np);
>  
> + of_node_put(volt_gpio_np);
> + of_node_put(freq_gpio_np);
> + of_node_put(slew_done_gpio_np);
> +
>   /* If we use the frequency GPIOs, calculate the min/max speeds based
>* on the bus frequencies
>*/

Applied. Thanks.

-- 
viresh

Re: [PATCH] cpufreq: Prepare cleanup of powerpc's asm/prom.h

[Viresh Kumar]

On 01-04-22, 19:24, Christophe Leroy wrote:
> powerpc's asm/prom.h brings some headers that it doesn't
> need itself.
> 
> In order to clean it up, first add missing headers in
> users of asm/prom.h
> 
> Signed-off-by: Christophe Leroy 
> ---
>  drivers/cpufreq/pasemi-cpufreq.c  | 1 -
>  drivers/cpufreq/pmac32-cpufreq.c  | 2 +-
>  drivers/cpufreq/pmac64-cpufreq.c  | 2 +-
>  drivers/cpufreq/ppc_cbe_cpufreq.c | 1 -
>  drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 2 +-
>  5 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 815645170c4d..039a66bbe1be 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -18,7 +18,6 @@
>  
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
> diff --git a/drivers/cpufreq/pmac32-cpufreq.c 
> b/drivers/cpufreq/pmac32-cpufreq.c
> index 4f20c6a9108d..20f64a8b0a35 100644
> --- a/drivers/cpufreq/pmac32-cpufreq.c
> +++ b/drivers/cpufreq/pmac32-cpufreq.c
> @@ -24,7 +24,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/cpufreq/pmac64-cpufreq.c 
> b/drivers/cpufreq/pmac64-cpufreq.c
> index d7542a106e6b..ba9c31d98bd6 100644
> --- a/drivers/cpufreq/pmac64-cpufreq.c
> +++ b/drivers/cpufreq/pmac64-cpufreq.c
> @@ -22,7 +22,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c 
> b/drivers/cpufreq/ppc_cbe_cpufreq.c
> index c58abb4cca3a..e3313ce63b38 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
> @@ -12,7 +12,6 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  
>  #include "ppc_cbe_cpufreq.h"
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c 
> b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> index 037fe23bc6ed..4fba3637b115 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> @@ -13,9 +13,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
> -#include 
>  #include 
>  #include 

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH V4 3/4] cpufreq: powerenv: Migrate to ->exit() callback instead of ->stop_cpu()

[Viresh Kumar]

On 23-06-21, 15:45, Michael Ellerman wrote:
> Viresh Kumar  writes:
> >
> > Subject: Re: [PATCH V4 3/4] cpufreq: powerenv: Migrate to ->exit() callback 
> > instead of ->stop_cpu()
> 
> Typo in subject should be "powernv".

Thanks for noticing it :)

-- 
viresh

[PATCH V4.1 3/4] cpufreq: powernv: Migrate to ->exit() callback instead of ->stop_cpu()

[Viresh Kumar]

commit 367dc4aa932b ("cpufreq: Add stop CPU callback to cpufreq_driver
interface") added the stop_cpu() callback to allow the drivers to do
clean up before the CPU is completely down and its state can't be
modified.

At that time the CPU hotplug framework used to call the cpufreq core's
registered notifier for different events like CPU_DOWN_PREPARE and
CPU_POST_DEAD. The stop_cpu() callback was called during the
CPU_DOWN_PREPARE event.

This is no longer the case, cpuhp_cpufreq_offline() is called only once
by the CPU hotplug core now and we don't really need two separate
callbacks for cpufreq drivers, i.e. stop_cpu() and exit(), as everything
can be done from the exit() callback itself.

Migrate to using the exit() callback instead of stop_cpu().

Signed-off-by: Viresh Kumar 
---
V4->V4.1:
- s/powerenv/powernv/

 drivers/cpufreq/powernv-cpufreq.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index e439b43c19eb..005600cef273 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -875,7 +875,15 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy 
*policy)
 
 static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
 {
-   /* timer is deleted in cpufreq_cpu_stop() */
+   struct powernv_smp_call_data freq_data;
+   struct global_pstate_info *gpstates = policy->driver_data;
+
+   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
+   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
+   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
+   if (gpstates)
+   del_timer_sync(>timer);
+
kfree(policy->driver_data);
 
return 0;
@@ -1007,18 +1015,6 @@ static struct notifier_block powernv_cpufreq_opal_nb = {
.priority   = 0,
 };
 
-static void powernv_cpufreq_stop_cpu(struct cpufreq_policy *policy)
-{
-   struct powernv_smp_call_data freq_data;
-   struct global_pstate_info *gpstates = policy->driver_data;
-
-   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
-   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
-   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
-   if (gpstates)
-   del_timer_sync(>timer);
-}
-
 static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
 {
@@ -1042,7 +1038,6 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.target_index   = powernv_cpufreq_target_index,
.fast_switch= powernv_fast_switch,
.get= powernv_cpufreq_get,
-   .stop_cpu   = powernv_cpufreq_stop_cpu,
.attr   = powernv_cpu_freq_attr,
 };
 
-- 
2.31.1.272.g89b43f80a514

[PATCH V4 3/4] cpufreq: powerenv: Migrate to ->exit() callback instead of ->stop_cpu()

[Viresh Kumar]

commit 367dc4aa932b ("cpufreq: Add stop CPU callback to cpufreq_driver
interface") added the stop_cpu() callback to allow the drivers to do
clean up before the CPU is completely down and its state can't be
modified.

At that time the CPU hotplug framework used to call the cpufreq core's
registered notifier for different events like CPU_DOWN_PREPARE and
CPU_POST_DEAD. The stop_cpu() callback was called during the
CPU_DOWN_PREPARE event.

This is no longer the case, cpuhp_cpufreq_offline() is called only once
by the CPU hotplug core now and we don't really need two separate
callbacks for cpufreq drivers, i.e. stop_cpu() and exit(), as everything
can be done from the exit() callback itself.

Migrate to using the exit() callback instead of stop_cpu().

Signed-off-by: Viresh Kumar 
---
 drivers/cpufreq/powernv-cpufreq.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index e439b43c19eb..005600cef273 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -875,7 +875,15 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy 
*policy)
 
 static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
 {
-   /* timer is deleted in cpufreq_cpu_stop() */
+   struct powernv_smp_call_data freq_data;
+   struct global_pstate_info *gpstates = policy->driver_data;
+
+   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
+   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
+   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
+   if (gpstates)
+   del_timer_sync(>timer);
+
kfree(policy->driver_data);
 
return 0;
@@ -1007,18 +1015,6 @@ static struct notifier_block powernv_cpufreq_opal_nb = {
.priority   = 0,
 };
 
-static void powernv_cpufreq_stop_cpu(struct cpufreq_policy *policy)
-{
-   struct powernv_smp_call_data freq_data;
-   struct global_pstate_info *gpstates = policy->driver_data;
-
-   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
-   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
-   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
-   if (gpstates)
-   del_timer_sync(>timer);
-}
-
 static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
 {
@@ -1042,7 +1038,6 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.target_index   = powernv_cpufreq_target_index,
.fast_switch= powernv_fast_switch,
.get= powernv_cpufreq_get,
-   .stop_cpu   = powernv_cpufreq_stop_cpu,
.attr   = powernv_cpu_freq_attr,
 };
 
-- 
2.31.1.272.g89b43f80a514

[PATCH V4 0/4] cpufreq: Migrate away from ->stop_cpu() callback

[Viresh Kumar]

Hi Rafael,

These are based on your patch [1] now.

commit 367dc4aa932b ("cpufreq: Add stop CPU callback to cpufreq_driver
interface") added the stop_cpu() callback to allow the drivers to do
clean up before the CPU is completely down and its state can't be
modified.

At that time the CPU hotplug framework used to call the cpufreq core's
registered notifier for different events like CPU_DOWN_PREPARE and
CPU_POST_DEAD. The stop_cpu() callback was called during the
CPU_DOWN_PREPARE event.

This is no longer the case, cpuhp_cpufreq_offline() is called only once
by the CPU hotplug core now and we don't really need two separate
callbacks for cpufreq drivers, i.e. stop_cpu() and exit(), as everything
can be done from the exit() callback itself.

Migrate to using the offline() or exit() callback instead of stop_cpu().

V3->V4:
- Based on a cleanup patch [1] from Rafael, apart from 5.13-rc7.
- No need to update exit() for intel pstate anymore.
- Remove the stop_cpu() callback completely.

--
Viresh

[1] https://lore.kernel.org/linux-pm/5490292.DvuYhMxLoT@kreacher/

Viresh Kumar (4):
  cpufreq: cppc: Migrate to ->exit() callback instead of ->stop_cpu()
  cpufreq: intel_pstate: Migrate to ->offline() instead of ->stop_cpu()
  cpufreq: powerenv: Migrate to ->exit() callback instead of
->stop_cpu()
  cpufreq: Remove stop_cpu() callback

 Documentation/cpu-freq/cpu-drivers.rst|  3 --
 .../zh_CN/cpu-freq/cpu-drivers.rst|  3 --
 drivers/cpufreq/cppc_cpufreq.c| 46 ++-
 drivers/cpufreq/cpufreq.c |  3 --
 drivers/cpufreq/intel_pstate.c| 10 +---
 drivers/cpufreq/powernv-cpufreq.c | 23 --
 include/linux/cpufreq.h   |  1 -
 7 files changed, 35 insertions(+), 54 deletions(-)

-- 
2.31.1.272.g89b43f80a514

[PATCH V2 3/3] cpufreq: powerenv: Migrate to ->exit() callback instead of ->stop_cpu()

[Viresh Kumar]

commit 367dc4aa932b ("cpufreq: Add stop CPU callback to cpufreq_driver
interface") added the stop_cpu() callback to allow the drivers to do
clean up before the CPU is completely down and its state can't be
modified.

At that time the CPU hotplug framework used to call the cpufreq core's
registered notifier for different events like CPU_DOWN_PREPARE and
CPU_POST_DEAD. The stop_cpu() callback was called during the
CPU_DOWN_PREPARE event.

This is no longer the case, cpuhp_cpufreq_offline() is called only once
by the CPU hotplug core now and we don't really need two separate
callbacks for cpufreq drivers, i.e. stop_cpu() and exit(), as everything
can be done from the exit() callback itself.

Migrate to using the exit() callback instead of stop_cpu().

Signed-off-by: Viresh Kumar 
---
 drivers/cpufreq/powernv-cpufreq.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index e439b43c19eb..005600cef273 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -875,7 +875,15 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy 
*policy)
 
 static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
 {
-   /* timer is deleted in cpufreq_cpu_stop() */
+   struct powernv_smp_call_data freq_data;
+   struct global_pstate_info *gpstates = policy->driver_data;
+
+   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
+   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
+   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
+   if (gpstates)
+   del_timer_sync(>timer);
+
kfree(policy->driver_data);
 
return 0;
@@ -1007,18 +1015,6 @@ static struct notifier_block powernv_cpufreq_opal_nb = {
.priority   = 0,
 };
 
-static void powernv_cpufreq_stop_cpu(struct cpufreq_policy *policy)
-{
-   struct powernv_smp_call_data freq_data;
-   struct global_pstate_info *gpstates = policy->driver_data;
-
-   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
-   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
-   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
-   if (gpstates)
-   del_timer_sync(>timer);
-}
-
 static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
 {
@@ -1042,7 +1038,6 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.target_index   = powernv_cpufreq_target_index,
.fast_switch= powernv_fast_switch,
.get= powernv_cpufreq_get,
-   .stop_cpu   = powernv_cpufreq_stop_cpu,
.attr   = powernv_cpu_freq_attr,
 };
 
-- 
2.31.1.272.g89b43f80a514

[PATCH V2 0/3] cpufreq: Migrate away from ->stop_cpu() callback

[Viresh Kumar]

Hi Rafael,

Sending these separately from CPPC stuff to avoid unnecessary confusion and
independent merging of these patches. These should get in nevertheless.

commit 367dc4aa932b ("cpufreq: Add stop CPU callback to cpufreq_driver
interface") added the stop_cpu() callback to allow the drivers to do
clean up before the CPU is completely down and its state can't be
modified.

At that time the CPU hotplug framework used to call the cpufreq core's
registered notifier for different events like CPU_DOWN_PREPARE and
CPU_POST_DEAD. The stop_cpu() callback was called during the
CPU_DOWN_PREPARE event.

This is no longer the case, cpuhp_cpufreq_offline() is called only once
by the CPU hotplug core now and we don't really need two separate
callbacks for cpufreq drivers, i.e. stop_cpu() and exit(), as everything
can be done from the exit() callback itself.

Migrate to using the exit() callback instead of stop_cpu().

The stop_cpu() callback isn't removed from core as it will be reused in
a different way in a separate patchset.

--
Viresh

Viresh Kumar (3):
  cpufreq: cppc: Migrate to ->exit() callback instead of ->stop_cpu()
  cpufreq: intel_pstate: Migrate to ->exit() callback instead of
->stop_cpu()
  cpufreq: powerenv: Migrate to ->exit() callback instead of
->stop_cpu()

 drivers/cpufreq/cppc_cpufreq.c| 46 ---
 drivers/cpufreq/intel_pstate.c|  9 +-
 drivers/cpufreq/powernv-cpufreq.c | 23 ++--
 3 files changed, 34 insertions(+), 44 deletions(-)

-- 
2.31.1.272.g89b43f80a514

Re: [PATCH 0/5] cpufreq: cppc: Fix suspend/resume specific races with FIE code

[Viresh Kumar]

On 15-06-21, 08:17, Qian Cai wrote:
> On 6/15/2021 3:50 AM, Viresh Kumar wrote:
> > This is a strange place to get the issue from. And this is a new
> > issue.
> 
> Well, it was still the same exercises with CPU online/offline.
> 
> > 
> >> [  488.151939][  T670]  kthread+0x3ac/0x460
> >> [  488.155854][  T670]  ret_from_fork+0x10/0x18
> >> [  488.160120][  T670] Code: 911e8000 aa1303e1 910a 941b595b (d421)
> >> [  488.166901][  T670] ---[ end trace e637e2d38b2cc087 ]---
> >> [  488.172206][  T670] Kernel panic - not syncing: Oops - BUG: Fatal 
> >> exception
> >> [  488.179182][  T670] SMP: stopping secondary CPUs
> >> [  489.209347][  T670] SMP: failed to stop secondary CPUs 
> >> 0-1,10-11,16-17,31
> >> [  489.216128][  T][  T670] Memoryn ]---
> > 
> > Can you give details on what exactly did you try to do, to get this ?
> > Normal boot or something more ?
> 
> Basically, it has the cpufreq driver as CPPC and the governor as
> schedutil. Running a few workloads to get CPU scaling up and down.
> Later, try to offline all CPUs until the last one and then online
> all CPUs.

Hmm, okay.

So I basically have very similar setup with 8 cores (1-policy
per-cpu), the only difference is I don't end up reading the
performance counters, everything else remains same. So I should see
issues now just like you, in case there are any.

Since the insmod/rmmod setup is a bit different, this is what I tried
today for around an hour with CONFIG_DEBUG_LIST and RCU debugging
options.

while true; do
for i in `seq 1 7`;
do
echo 0 > /sys/devices/system/cpu/cpu$i/online;
done;

for i in `seq 1 7`;
do
echo 1 > /sys/devices/system/cpu/cpu$i/online;
done;
done

I don't see any crashes, oops or warnings with latest stuff.

> I am hesitate to try this at the moment because this all feel like
> shooting in the dark.

I understand your point and you aren't completely wrong here. It
wasn't completely in dark but since I am unable to reproduce the issue
at my end, I asked for help.

FWIW, I think one of the possible cause of corruption of kthread thing
could have been because of the race in the topology related code. I
already fixed that in my tree yesterday.

> Ideally, you will be able to get access to one
> of those arm64 servers (Huawei, Ampere, TX2, FJ etc) eventually and
> really try the same exercises yourself with those debugging options
> like list debugging and KASAN on. That way you could fix things way
> efficiently.

Yeah, I thought of this work being over and I am not a user of it
normally. I had to enable it for ARM servers and I took help of my
colleagues (Vincent Guittot and Ionela) for testing the same.

I have also asked Vincent to give it a try again.

> I could share you the .config once you are there. Last
> but not least, once you get better narrow down of the issues, I'd
> hope to see someone else familiar with the code there to get review
> of those patches first (feel free to Cc me once you are ready to
> post) before I'll rerun the whole things again. That way we don't
> waste time on each other backing and forth chasing the shadow.

I did send the stuff up for review and this last thing (you reported)
was a different race altogether, so asked for testing without reviews.

Anyway, I am quite sure my tests have covered such issues now. I will
send out patches again soon.

Thanks Qian.

-- 
viresh

Re: [PATCH 0/5] cpufreq: cppc: Fix suspend/resume specific races with FIE code

[Viresh Kumar]

On 15-06-21, 13:20, Viresh Kumar wrote:
> I can see one place where race can happen, i.e. between
> topology_clear_scale_freq_source() and topology_scale_freq_tick(). It
> is possible that sfd->set_freq_scale() may get called for a previously
> set handler as there is no protection there.
> 
> I will see how to fix that. But I am not sure if the issue reported
> above comes from there.

I have tried to fix this race and pushed the relevant patch to my
branch. Please pick the latest branch and hopefully everything will
just work.

-- 
viresh

Re: [PATCH 0/5] cpufreq: cppc: Fix suspend/resume specific races with FIE code

[Viresh Kumar]

Hi Qian,

First of all thanks for testing this, I need more of your help to test
this out :)

FWIW, I did test this on my Hikey board today, with some hacks, and
tried multiple insmod/rmmod operations for the driver, and I wasn't
able to reproduce the issue you reported. I did enable the list-debug
config option.

On 14-06-21, 09:48, Qian Cai wrote:
> Unfortunately, this series looks like needing more works.
> 
> [  487.773586][T0] CPU17: Booted secondary processor 0x000801 
> [0x503f0002]
> [  487.976495][  T670] list_del corruption. next->prev should be 
> 009b66e9ec70, but was 009b66dfec70
> [  487.987037][  T670] [ cut here ]
> [  487.992351][  T670] kernel BUG at lib/list_debug.c:54!
> [  487.997810][  T670] Internal error: Oops - BUG: 0 [#1] SMP
> [  488.003295][  T670] Modules linked in: cpufreq_userspace xfs loop 
> cppc_cpufreq processor efivarfs ip_tables x_tables ext4 mbcache jbd2 dm_mod 
> igb i2c_algo_bit nvme mlx5_core i2c_core nvme_core firmware_class
> [  488.021759][  T670] CPU: 1 PID: 670 Comm: cppc_fie Not tainted 
> 5.13.0-rc5-next-20210611+ #46
> [  488.030190][  T670] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, 
> BIOS 1.6 06/28/2020
> [  488.038705][  T670] pstate: 60c5 (nZCv daIF -PAN -UAO -TCO BTYPE=--)
> [  488.045398][  T670] pc : __list_del_entry_valid+0x154/0x158
> [  488.050969][  T670] lr : __list_del_entry_valid+0x154/0x158
> [  488.056534][  T670] sp : 8000229afd70
> [  488.060534][  T670] x29: 8000229afd70 x28: 0008c8f4f340 x27: 
> dfff8000
> [  488.068361][  T670] x26: 009b66e9ec70 x25: 800011c8b4d0 x24: 
> 0008d4bfe488
> [  488.076188][  T670] x23: 0008c8f4f340 x22: 0008c8f4f340 x21: 
> 009b6789ec70
> [  488.084015][  T670] x20: 0008d4bfe4c8 x19: 009b66e9ec70 x18: 
> 0008c8f4fd70
> [  488.091842][  T670] x17: 20747562202c3037 x16: 6365396536366239 x15: 
> 0028
> [  488.099669][  T670] x14:  x13: 0001 x12: 
> 60136cdd3447
> [  488.107495][  T670] x11: 1fffe0136cdd3446 x10: 60136cdd3446 x9 : 
> 8000103ee444
> [  488.115322][  T670] x8 : 009b66e9a237 x7 : 0001 x6 : 
> 009b66e9a230
> [  488.123149][  T670] x5 : 9fec9322cbba x4 : 60136cdd3447 x3 : 
> 1fffe001191e9e69
> [  488.130975][  T670] x2 :  x1 :  x0 : 
> 0054
> [  488.138803][  T670] Call trace:
> [  488.141935][  T670]  __list_del_entry_valid+0x154/0x158
> [  488.147153][  T670]  kthread_worker_fn+0x15c/0xda0

This is a strange place to get the issue from. And this is a new
issue.

> [  488.151939][  T670]  kthread+0x3ac/0x460
> [  488.155854][  T670]  ret_from_fork+0x10/0x18
> [  488.160120][  T670] Code: 911e8000 aa1303e1 910a 941b595b (d421)
> [  488.166901][  T670] ---[ end trace e637e2d38b2cc087 ]---
> [  488.172206][  T670] Kernel panic - not syncing: Oops - BUG: Fatal exception
> [  488.179182][  T670] SMP: stopping secondary CPUs
> [  489.209347][  T670] SMP: failed to stop secondary CPUs 0-1,10-11,16-17,31
> [  489.216128][  T][  T670] Memoryn ]---

Can you give details on what exactly did you try to do, to get this ?
Normal boot or something more ?

I have made some changes to the way calls were happening, may get this
thing sorted. Can you please try this branch ?

https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/log/?h=cpufreq/cppc

I can see one place where race can happen, i.e. between
topology_clear_scale_freq_source() and topology_scale_freq_tick(). It
is possible that sfd->set_freq_scale() may get called for a previously
set handler as there is no protection there.

I will see how to fix that. But I am not sure if the issue reported
above comes from there.

Anyway, please give my branch a try, lets see.

-- 
viresh

[PATCH 3/5] cpufreq: powerenv: Migrate to ->exit() callback instead of ->stop_cpu()

[Viresh Kumar]

commit 367dc4aa932b ("cpufreq: Add stop CPU callback to cpufreq_driver
interface") added the stop_cpu() callback to allow the drivers to do
clean up before the CPU is completely down and its state cannot be
modified.

At that time the CPU hotplug framework used to call the cpufreq core's
registered notifier for different events like CPU_DOWN_PREPARE and
CPU_POST_DEAD. The stop_cpu() callback was called during the
CPU_DOWN_PREPARE event.

This is no longer the case, cpuhp_cpufreq_offline() is called only once
by the CPU hotplug core now and we don't really need two separate
callbacks for cpufreq drivers, i.e. stop_cpu() and exit(), as everything
can be done from the exit() callback itself.

Migrate to using the exit() callback instead of stop_cpu().

Signed-off-by: Viresh Kumar 
---
 drivers/cpufreq/powernv-cpufreq.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index e439b43c19eb..005600cef273 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -875,7 +875,15 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy 
*policy)
 
 static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
 {
-   /* timer is deleted in cpufreq_cpu_stop() */
+   struct powernv_smp_call_data freq_data;
+   struct global_pstate_info *gpstates = policy->driver_data;
+
+   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
+   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
+   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
+   if (gpstates)
+   del_timer_sync(>timer);
+
kfree(policy->driver_data);
 
return 0;
@@ -1007,18 +1015,6 @@ static struct notifier_block powernv_cpufreq_opal_nb = {
.priority   = 0,
 };
 
-static void powernv_cpufreq_stop_cpu(struct cpufreq_policy *policy)
-{
-   struct powernv_smp_call_data freq_data;
-   struct global_pstate_info *gpstates = policy->driver_data;
-
-   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
-   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
-   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
-   if (gpstates)
-   del_timer_sync(>timer);
-}
-
 static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
 {
@@ -1042,7 +1038,6 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.target_index   = powernv_cpufreq_target_index,
.fast_switch= powernv_fast_switch,
.get= powernv_cpufreq_get,
-   .stop_cpu   = powernv_cpufreq_stop_cpu,
.attr   = powernv_cpu_freq_attr,
 };
 
-- 
2.31.1.272.g89b43f80a514

[PATCH 0/5] cpufreq: cppc: Fix suspend/resume specific races with FIE code

[Viresh Kumar]

Hi Qian,

It would be helpful if you can test this patchset and confirm if the races you
mentioned went away or not and that the FIE code works as we wanted it to.

I don't have a real setup and so it won't be easy for me to test this out.

I have already sent a temporary fix for 5.13 and this patchset is targeted for
5.14 and is based over that.

-8<-

The CPPC driver currently stops the frequency invariance related
kthread_work and irq_work from cppc_freq_invariance_exit() which is only
called during driver's removal.

This is not sufficient as the CPUs can get hot-plugged out while the
driver is in use, the same also happens during system suspend/resume.

In such a cases we can reach a state where the CPU is removed by the
kernel but its kthread_work or irq_work aren't stopped.

Fix this by implementing the start_cpu() and stop_cpu() callbacks in the
cpufreq core, which will be called for each CPU's addition/removal.

A similar call was already available in the cpufreq core, which isn't required
anymore and so its users are migrated to use exit() callback instead.

This is targeted for v5.14-rc1.

--
Viresh

Viresh Kumar (5):
  cpufreq: cppc: Migrate to ->exit() callback instead of ->stop_cpu()
  cpufreq: intel_pstate: Migrate to ->exit() callback instead of
->stop_cpu()
  cpufreq: powerenv: Migrate to ->exit() callback instead of
->stop_cpu()
  cpufreq: Add start_cpu() and stop_cpu() callbacks
  cpufreq: cppc: Fix suspend/resume specific races with the FIE code

 Documentation/cpu-freq/cpu-drivers.rst |   7 +-
 drivers/cpufreq/Kconfig.arm|   1 -
 drivers/cpufreq/cppc_cpufreq.c | 163 ++---
 drivers/cpufreq/cpufreq.c  |  11 +-
 drivers/cpufreq/intel_pstate.c |   9 +-
 drivers/cpufreq/powernv-cpufreq.c  |  23 ++--
 include/linux/cpufreq.h|   5 +-
 7 files changed, 119 insertions(+), 100 deletions(-)

-- 
2.31.1.272.g89b43f80a514

Re: [PATCH v3 2/2] powerpc: Remove remaining parts of oprofile

[Viresh Kumar]

On 01-03-21, 12:09, Christophe Leroy wrote:
> Commit 9850b6c69356 ("arch: powerpc: Remove oprofile") removed
> oprofile.
> 
> Remove all remaining parts of it.
> 
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/include/asm/cputable.h   |  3 --
>  arch/powerpc/kernel/cputable.c| 66 +--
>  arch/powerpc/kernel/dt_cpu_ftrs.c |  4 --
>  arch/powerpc/platforms/cell/spufs/spufs.h |  2 +-
>  4 files changed, 3 insertions(+), 72 deletions(-)

Great, I wasn't sure how the handle the cpu type stuff and so left it
for the right people to handle. :)

Acked-by: Viresh Kumar 

-- 
viresh

[PATCH] cpufreq: Remove unused flag CPUFREQ_PM_NO_WARN

[Viresh Kumar]

This flag is set by one of the drivers but it isn't used in the code
otherwise. Remove the unused flag and update the driver.

Signed-off-by: Viresh Kumar 
---
Rebased over:

https://lore.kernel.org/lkml/a59bb322b22c247d570b70a8e94067804287623b.1612241683.git.viresh.ku...@linaro.org/

 drivers/cpufreq/pmac32-cpufreq.c |  3 +--
 include/linux/cpufreq.h  | 13 +
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c
index 73621bc11976..4f20c6a9108d 100644
--- a/drivers/cpufreq/pmac32-cpufreq.c
+++ b/drivers/cpufreq/pmac32-cpufreq.c
@@ -439,8 +439,7 @@ static struct cpufreq_driver pmac_cpufreq_driver = {
.init   = pmac_cpufreq_cpu_init,
.suspend= pmac_cpufreq_suspend,
.resume = pmac_cpufreq_resume,
-   .flags  = CPUFREQ_PM_NO_WARN |
- CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING,
+   .flags  = CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING,
.attr   = cpufreq_generic_attr,
.name   = "powermac",
 };
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index c8e40e91fe9b..353969c7acd3 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -398,8 +398,11 @@ struct cpufreq_driver {
 /* loops_per_jiffy or other kernel "constants" aren't affected by frequency 
transitions */
 #define CPUFREQ_CONST_LOOPSBIT(1)
 
-/* don't warn on suspend/resume speed mismatches */
-#define CPUFREQ_PM_NO_WARN BIT(2)
+/*
+ * Set by drivers that want the core to automatically register the cpufreq
+ * driver as a thermal cooling device.
+ */
+#define CPUFREQ_IS_COOLING_DEV BIT(2)
 
 /*
  * This should be set by platforms having multiple clock-domains, i.e.
@@ -431,12 +434,6 @@ struct cpufreq_driver {
  */
 #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING  BIT(6)
 
-/*
- * Set by drivers that want the core to automatically register the cpufreq
- * driver as a thermal cooling device.
- */
-#define CPUFREQ_IS_COOLING_DEV BIT(7)
-
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
 
-- 
2.25.0.rc1.19.g042ed3e048af

Re: [PATCH 10/18] arch: powerpc: Stop building and using oprofile

[Viresh Kumar]

On 14-01-21, 17:05, Viresh Kumar wrote:
> The "oprofile" user-space tools don't use the kernel OPROFILE support
> any more, and haven't in a long time. User-space has been converted to
> the perf interfaces.
> 
> This commits stops building oprofile for powerpc and removes any
> reference to it from directories in arch/powerpc/ apart from
> arch/powerpc/oprofile, which will be removed in the next commit (this is
> broken into two commits as the size of the commit became very big, ~5k
> lines).
> 
> Note that the member "oprofile_cpu_type" in "struct cpu_spec" isn't
> removed as it was also used by other parts of the code.
> 
> Suggested-by: Christoph Hellwig 
> Suggested-by: Linus Torvalds 
> Signed-off-by: Viresh Kumar 
> ---
>  arch/powerpc/Kconfig  |   1 -
>  arch/powerpc/Makefile |   2 -
>  arch/powerpc/configs/44x/akebono_defconfig|   1 -
>  arch/powerpc/configs/44x/currituck_defconfig  |   1 -
>  arch/powerpc/configs/44x/fsp2_defconfig   |   1 -
>  arch/powerpc/configs/44x/iss476-smp_defconfig |   1 -
>  arch/powerpc/configs/cell_defconfig   |   1 -
>  arch/powerpc/configs/g5_defconfig |   1 -
>  arch/powerpc/configs/maple_defconfig  |   1 -
>  arch/powerpc/configs/pasemi_defconfig |   1 -
>  arch/powerpc/configs/pmac32_defconfig |   1 -
>  arch/powerpc/configs/powernv_defconfig|   1 -
>  arch/powerpc/configs/ppc64_defconfig  |   1 -
>  arch/powerpc/configs/ppc64e_defconfig |   1 -
>  arch/powerpc/configs/ppc6xx_defconfig |   1 -
>  arch/powerpc/configs/ps3_defconfig|   1 -
>  arch/powerpc/configs/pseries_defconfig|   1 -
>  arch/powerpc/include/asm/cputable.h   |  20 ---
>  arch/powerpc/include/asm/oprofile_impl.h  | 135 --
>  arch/powerpc/include/asm/spu.h|  33 -
>  arch/powerpc/kernel/cputable.c|  67 -
>  arch/powerpc/kernel/dt_cpu_ftrs.c |   2 -
>  arch/powerpc/platforms/cell/Kconfig   |   5 -
>  arch/powerpc/platforms/cell/spu_notify.c  |  55 ---

+ this..

diff --git a/arch/powerpc/platforms/cell/Makefile 
b/arch/powerpc/platforms/cell/Makefile
index 10064a33ca96..7ea6692f67e2 100644
--- a/arch/powerpc/platforms/cell/Makefile
+++ b/arch/powerpc/platforms/cell/Makefile
@@ -19,7 +19,6 @@ spu-priv1-$(CONFIG_PPC_CELL_COMMON)   += spu_priv1_mmio.o
 spu-manage-$(CONFIG_PPC_CELL_COMMON)   += spu_manage.o
 
 obj-$(CONFIG_SPU_BASE) += spu_callbacks.o spu_base.o \
-  spu_notify.o \
   spu_syscalls.o \
   $(spu-priv1-y) \
   $(spu-manage-y) \

-- 
viresh

Re: [PATCH 10/18] arch: powerpc: Stop building and using oprofile

[Viresh Kumar]

On 14-01-21, 17:05, Viresh Kumar wrote:
> The "oprofile" user-space tools don't use the kernel OPROFILE support
> any more, and haven't in a long time. User-space has been converted to
> the perf interfaces.
> 
> This commits stops building oprofile for powerpc and removes any
> reference to it from directories in arch/powerpc/ apart from
> arch/powerpc/oprofile, which will be removed in the next commit (this is
> broken into two commits as the size of the commit became very big, ~5k
> lines).
> 
> Note that the member "oprofile_cpu_type" in "struct cpu_spec" isn't
> removed as it was also used by other parts of the code.
> 
> Suggested-by: Christoph Hellwig 
> Suggested-by: Linus Torvalds 
> Signed-off-by: Viresh Kumar 

+ this to fix a warning:

diff --git a/arch/powerpc/platforms/cell/spufs/run.c 
b/arch/powerpc/platforms/cell/spufs/run.c
index 466006918003..ce52b87496d2 100644
--- a/arch/powerpc/platforms/cell/spufs/run.c
+++ b/arch/powerpc/platforms/cell/spufs/run.c
@@ -353,7 +353,6 @@ static int spu_process_callback(struct spu_context *ctx)
 long spufs_run_spu(struct spu_context *ctx, u32 *npc, u32 *event)
 {
int ret;
-   struct spu *spu;
u32 status;
 
if (mutex_lock_interruptible(>run_mutex))
@@ -386,7 +385,6 @@ long spufs_run_spu(struct spu_context *ctx, u32 *npc, u32 
*event)
mutex_lock(>state_mutex);
break;
}
-   spu = ctx->spu;
if (unlikely(test_and_clear_bit(SPU_SCHED_NOTIFY_ACTIVE,
>sched_flags))) {
if (!(status & SPU_STATUS_STOPPED_BY_STOP))

-- 
viresh

[PATCH 11/18] arch: powerpc: Remove oprofile

[Viresh Kumar]

The previous commit already disabled building oprofile, lets remove the
oprofile directory now.

Suggested-by: Christoph Hellwig 
Suggested-by: Linus Torvalds 
Signed-off-by: Viresh Kumar 
---
 arch/powerpc/oprofile/Makefile |   19 -
 arch/powerpc/oprofile/backtrace.c  |  120 --
 arch/powerpc/oprofile/cell/pr_util.h   |  110 --
 arch/powerpc/oprofile/cell/spu_profiler.c  |  248 ---
 arch/powerpc/oprofile/cell/spu_task_sync.c |  657 
 arch/powerpc/oprofile/cell/vma_map.c   |  279 
 arch/powerpc/oprofile/common.c |  243 ---
 arch/powerpc/oprofile/op_model_7450.c  |  207 ---
 arch/powerpc/oprofile/op_model_cell.c  | 1709 
 arch/powerpc/oprofile/op_model_fsl_emb.c   |  380 -
 arch/powerpc/oprofile/op_model_pa6t.c  |  227 ---
 arch/powerpc/oprofile/op_model_power4.c|  438 -
 12 files changed, 4637 deletions(-)
 delete mode 100644 arch/powerpc/oprofile/Makefile
 delete mode 100644 arch/powerpc/oprofile/backtrace.c
 delete mode 100644 arch/powerpc/oprofile/cell/pr_util.h
 delete mode 100644 arch/powerpc/oprofile/cell/spu_profiler.c
 delete mode 100644 arch/powerpc/oprofile/cell/spu_task_sync.c
 delete mode 100644 arch/powerpc/oprofile/cell/vma_map.c
 delete mode 100644 arch/powerpc/oprofile/common.c
 delete mode 100644 arch/powerpc/oprofile/op_model_7450.c
 delete mode 100644 arch/powerpc/oprofile/op_model_cell.c
 delete mode 100644 arch/powerpc/oprofile/op_model_fsl_emb.c
 delete mode 100644 arch/powerpc/oprofile/op_model_pa6t.c
 delete mode 100644 arch/powerpc/oprofile/op_model_power4.c

diff --git a/arch/powerpc/oprofile/Makefile b/arch/powerpc/oprofile/Makefile
deleted file mode 100644
index bb2d94c8cbe6..
--- a/arch/powerpc/oprofile/Makefile
+++ /dev/null
@@ -1,19 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-
-ccflags-$(CONFIG_PPC64):= $(NO_MINIMAL_TOC)
-
-obj-$(CONFIG_OPROFILE) += oprofile.o
-
-DRIVER_OBJS := $(addprefix ../../../drivers/oprofile/, \
-   oprof.o cpu_buffer.o buffer_sync.o \
-   event_buffer.o oprofile_files.o \
-   oprofilefs.o oprofile_stats.o \
-   timer_int.o )
-
-oprofile-y := $(DRIVER_OBJS) common.o backtrace.o
-oprofile-$(CONFIG_OPROFILE_CELL) += op_model_cell.o \
-   cell/spu_profiler.o cell/vma_map.o \
-   cell/spu_task_sync.o
-oprofile-$(CONFIG_PPC_BOOK3S_64) += op_model_power4.o op_model_pa6t.o
-oprofile-$(CONFIG_FSL_EMB_PERFMON) += op_model_fsl_emb.o
-oprofile-$(CONFIG_PPC_BOOK3S_32) += op_model_7450.o
diff --git a/arch/powerpc/oprofile/backtrace.c 
b/arch/powerpc/oprofile/backtrace.c
deleted file mode 100644
index 9db7ada79d10..
--- a/arch/powerpc/oprofile/backtrace.c
+++ /dev/null
@@ -1,120 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/**
- * Copyright (C) 2005 Brian Rogan , IBM
- *
-**/
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#define STACK_SP(STACK)*(STACK)
-
-#define STACK_LR64(STACK)  *((unsigned long *)(STACK) + 2)
-#define STACK_LR32(STACK)  *((unsigned int *)(STACK) + 1)
-
-#ifdef CONFIG_PPC64
-#define STACK_LR(STACK)STACK_LR64(STACK)
-#else
-#define STACK_LR(STACK)STACK_LR32(STACK)
-#endif
-
-static unsigned int user_getsp32(unsigned int sp, int is_first)
-{
-   unsigned int stack_frame[2];
-   void __user *p = compat_ptr(sp);
-
-   /*
-* The most likely reason for this is that we returned -EFAULT,
-* which means that we've done all that we can do from
-* interrupt context.
-*/
-   if (copy_from_user_nofault(stack_frame, (void __user *)p,
-   sizeof(stack_frame)))
-   return 0;
-
-   if (!is_first)
-   oprofile_add_trace(STACK_LR32(stack_frame));
-
-   /*
-* We do not enforce increasing stack addresses here because
-* we may transition to a different stack, eg a signal handler.
-*/
-   return STACK_SP(stack_frame);
-}
-
-#ifdef CONFIG_PPC64
-static unsigned long user_getsp64(unsigned long sp, int is_first)
-{
-   unsigned long stack_frame[3];
-
-   if (copy_from_user_nofault(stack_frame, (void __user *)sp,
-   sizeof(stack_frame)))
-   return 0;
-
-   if (!is_first)
-   oprofile_add_trace(STACK_LR64(stack_frame));
-
-   return STACK_SP(stack_frame);
-}
-#endif
-
-static unsigned long kernel_getsp(unsigned long sp, int is_first)
-{
-   unsigned long *stack_frame = (unsigned long *)sp;
-
-   if (!validate_sp(sp, current, STACK_FRAME_OVERHEAD))
-   return 0;
-
-   if (!is_first)
-   oprofile_add_trace(STACK_LR(stack_frame));
-
-   /*
-* We do not enforce increasing stack addresses here because
-* we might be transitioning from an interrupt stack to a kernel
-* stack. validate_sp() is designed to understand this, so just

[PATCH 10/18] arch: powerpc: Stop building and using oprofile

[Viresh Kumar]

The "oprofile" user-space tools don't use the kernel OPROFILE support
any more, and haven't in a long time. User-space has been converted to
the perf interfaces.

This commits stops building oprofile for powerpc and removes any
reference to it from directories in arch/powerpc/ apart from
arch/powerpc/oprofile, which will be removed in the next commit (this is
broken into two commits as the size of the commit became very big, ~5k
lines).

Note that the member "oprofile_cpu_type" in "struct cpu_spec" isn't
removed as it was also used by other parts of the code.

Suggested-by: Christoph Hellwig 
Suggested-by: Linus Torvalds 
Signed-off-by: Viresh Kumar 
---
 arch/powerpc/Kconfig  |   1 -
 arch/powerpc/Makefile |   2 -
 arch/powerpc/configs/44x/akebono_defconfig|   1 -
 arch/powerpc/configs/44x/currituck_defconfig  |   1 -
 arch/powerpc/configs/44x/fsp2_defconfig   |   1 -
 arch/powerpc/configs/44x/iss476-smp_defconfig |   1 -
 arch/powerpc/configs/cell_defconfig   |   1 -
 arch/powerpc/configs/g5_defconfig |   1 -
 arch/powerpc/configs/maple_defconfig  |   1 -
 arch/powerpc/configs/pasemi_defconfig |   1 -
 arch/powerpc/configs/pmac32_defconfig |   1 -
 arch/powerpc/configs/powernv_defconfig|   1 -
 arch/powerpc/configs/ppc64_defconfig  |   1 -
 arch/powerpc/configs/ppc64e_defconfig |   1 -
 arch/powerpc/configs/ppc6xx_defconfig |   1 -
 arch/powerpc/configs/ps3_defconfig|   1 -
 arch/powerpc/configs/pseries_defconfig|   1 -
 arch/powerpc/include/asm/cputable.h   |  20 ---
 arch/powerpc/include/asm/oprofile_impl.h  | 135 --
 arch/powerpc/include/asm/spu.h|  33 -
 arch/powerpc/kernel/cputable.c|  67 -
 arch/powerpc/kernel/dt_cpu_ftrs.c |   2 -
 arch/powerpc/platforms/cell/Kconfig   |   5 -
 arch/powerpc/platforms/cell/spu_notify.c  |  55 ---
 arch/powerpc/platforms/cell/spufs/run.c   |   4 +-
 arch/powerpc/platforms/cell/spufs/sched.c |   5 -
 arch/powerpc/platforms/cell/spufs/spufs.h |   1 -
 27 files changed, 1 insertion(+), 344 deletions(-)
 delete mode 100644 arch/powerpc/include/asm/oprofile_impl.h
 delete mode 100644 arch/powerpc/platforms/cell/spu_notify.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 107bb4319e0e..1897c0ff2f2b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -226,7 +226,6 @@ config PPC
select HAVE_MOD_ARCH_SPECIFIC
select HAVE_NMI if PERF_EVENTS || (PPC64 && 
PPC_BOOK3S)
select HAVE_HARDLOCKUP_DETECTOR_ARCHif (PPC64 && PPC_BOOK3S)
-   select HAVE_OPROFILE
select HAVE_OPTPROBES   if PPC64
select HAVE_PERF_EVENTS
select HAVE_PERF_EVENTS_NMI if PPC64
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 08cf0eade56a..b959fdaec713 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -276,8 +276,6 @@ head-$(CONFIG_PPC_OF_BOOT_TRAMPOLINE)  += 
arch/powerpc/kernel/prom_init.o
 # See arch/powerpc/Kbuild for content of core part of the kernel
 core-y += arch/powerpc/
 
-drivers-$(CONFIG_OPROFILE) += arch/powerpc/oprofile/
-
 # Default to zImage, override when needed
 all: zImage
 
diff --git a/arch/powerpc/configs/44x/akebono_defconfig 
b/arch/powerpc/configs/44x/akebono_defconfig
index 3894ba8f8ffc..72b8f93a9bdd 100644
--- a/arch/powerpc/configs/44x/akebono_defconfig
+++ b/arch/powerpc/configs/44x/akebono_defconfig
@@ -8,7 +8,6 @@ CONFIG_EXPERT=y
 CONFIG_KALLSYMS_ALL=y
 # CONFIG_SLUB_CPU_PARTIAL is not set
 CONFIG_PROFILING=y
-CONFIG_OPROFILE=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
diff --git a/arch/powerpc/configs/44x/currituck_defconfig 
b/arch/powerpc/configs/44x/currituck_defconfig
index 34c86b3abecb..717827219921 100644
--- a/arch/powerpc/configs/44x/currituck_defconfig
+++ b/arch/powerpc/configs/44x/currituck_defconfig
@@ -6,7 +6,6 @@ CONFIG_LOG_BUF_SHIFT=14
 CONFIG_EXPERT=y
 CONFIG_KALLSYMS_ALL=y
 CONFIG_PROFILING=y
-CONFIG_OPROFILE=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
diff --git a/arch/powerpc/configs/44x/fsp2_defconfig 
b/arch/powerpc/configs/44x/fsp2_defconfig
index 30845ce0885a..8da316e61a08 100644
--- a/arch/powerpc/configs/44x/fsp2_defconfig
+++ b/arch/powerpc/configs/44x/fsp2_defconfig
@@ -17,7 +17,6 @@ CONFIG_KALLSYMS_ALL=y
 CONFIG_BPF_SYSCALL=y
 CONFIG_EMBEDDED=y
 CONFIG_PROFILING=y
-CONFIG_OPROFILE=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
diff --git a/arch/powerpc/configs/44x/iss476-smp_defconfig 
b/arch/powerpc/configs/44x/iss476-smp_defconfig
index 2c3834eebca3..c11e777b2f3d 100644
--- a/arch/powerpc/configs/44x/iss476-smp_defconfig
+++ b/arch/powerpc/configs/44x/iss476-smp_defconfig
@@ -7,7 

Re: [PATCH v2 06/13] cpufreq: powernv-cpufreq: Functions only used in call-backs should be static

[Viresh Kumar]

On 15-07-20, 09:26, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/powernv-cpufreq.c:669:6: warning: no previous prototype for 
> ‘gpstate_timer_handler’ [-Wmissing-prototypes]
>  drivers/cpufreq/powernv-cpufreq.c:902:6: warning: no previous prototype for 
> ‘powernv_cpufreq_work_fn’ [-Wmissing-prototypes]
> 
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones 
> Acked-by: Viresh Kumar 
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 8646eb197cd96..068cc53abe320 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -666,7 +666,7 @@ static inline void  queue_gpstate_timer(struct 
> global_pstate_info *gpstates)
>   * according quadratic equation. Queues a new timer if it is still not equal
>   * to local pstate
>   */
> -void gpstate_timer_handler(struct timer_list *t)
> +static void gpstate_timer_handler(struct timer_list *t)
>  {
>   struct global_pstate_info *gpstates = from_timer(gpstates, t, timer);
>   struct cpufreq_policy *policy = gpstates->policy;
> @@ -899,7 +899,7 @@ static struct notifier_block powernv_cpufreq_reboot_nb = {
>   .notifier_call = powernv_cpufreq_reboot_notifier,
>  };
>  
> -void powernv_cpufreq_work_fn(struct work_struct *work)
> +static void powernv_cpufreq_work_fn(struct work_struct *work)
>  {
>   struct chip *chip = container_of(work, struct chip, throttle);
>   struct cpufreq_policy *policy;

Don't you want to drop this patch now ? As you already reviewed the
other one on the list ?

-- 
viresh

Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header

[Viresh Kumar]

On 15-07-20, 07:36, Lee Jones wrote:
> I searched for "include.*platforms/" in drivers/, and was scared off
> this method since no one else does this.

Yeah its not right for generic drivers, but this is very much platform
specific so it is fine here.

-- 
viresh

Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header

[Viresh Kumar]

On 14-07-20, 20:49, Olof Johansson wrote:
> On Tue, Jul 14, 2020 at 8:07 PM Viresh Kumar  wrote:
> >
> > On 14-07-20, 15:50, Lee Jones wrote:
> > > If function callers and providers do not share the same prototypes the
> > > compiler complains of missing prototypes.  Fix this by moving the
> > > already existing prototypes out to a mutually convenient location.
> > >
> > > Fixes the following W=1 kernel build warning(s):
> > >
> > >  drivers/cpufreq/pasemi-cpufreq.c:109:5: warning: no previous prototype 
> > > for ‘check_astate’ [-Wmissing-prototypes]
> > >  109 | int check_astate(void)
> > >  | ^~~~
> > >  drivers/cpufreq/pasemi-cpufreq.c:114:6: warning: no previous prototype 
> > > for ‘restore_astate’ [-Wmissing-prototypes]
> > >  114 | void restore_astate(int cpu)
> > >  | ^~
> > >
> > > Cc: Olof Johansson 
> > > Cc: Michael Ellerman 
> > > Cc: Benjamin Herrenschmidt 
> > > Cc: Paul Mackerras 
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones 
> > > ---
> > >  arch/powerpc/platforms/pasemi/pasemi.h| 15 
> >
> > Is there no sane way we can include this file directly to the cpufreq
> > file ?
> 
> Yep. arch/powerpc seems to be in the search path for modules on powerpc, so:
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index c66f566a854cb..815645170c4de 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -22,6 +22,8 @@
>  #include 
>  #include 
> 
> +#include 
> +
>  #define SDCASR_REG 0x0100
>  #define SDCASR_REG_STRIDE  0x1000
>  #define SDCPWR_CFGA0_REG   0x0100

Fantastic. Thanks.

-- 
viresh

Re: [PATCH -next] cpufreq: powernv: Make some symbols static

[Viresh Kumar]

On 14-07-20, 22:23, Wei Yongjun wrote:
> The sparse tool complains as follows:
> 
> drivers/cpufreq/powernv-cpufreq.c:88:1: warning:
>  symbol 'pstate_revmap' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:383:18: warning:
>  symbol 'cpufreq_freq_attr_cpuinfo_nominal_freq' was not declared. Should it 
> be static?
> drivers/cpufreq/powernv-cpufreq.c:669:6: warning:
>  symbol 'gpstate_timer_handler' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:902:6: warning:
>  symbol 'powernv_cpufreq_work_fn' was not declared. Should it be static?
> 
> Those symbols are not used outside of this file, so mark
> them static.
> 
> Reported-by: Hulk Robot 
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Lee also sent a fix for this, but yours look complete :)

https://lore.kernel.org/lkml/20200714145049.2496163-7-lee.jo...@linaro.org/

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 07/13] cpufreq: powernv-cpufreq: Fix a bunch of kerneldoc related issues

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> Repair problems with formatting and missing attributes/parameters, and
> demote header comments which do not meet the required standards
> applicable to kerneldoc.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/powernv-cpufreq.c:84: warning: Function parameter or member 
> 'last_lpstate_idx' not described in 'global_pstate_info'
>  drivers/cpufreq/powernv-cpufreq.c:84: warning: Function parameter or member 
> 'last_gpstate_idx' not described in 'global_pstate_info'
>  drivers/cpufreq/powernv-cpufreq.c:84: warning: Function parameter or member 
> 'policy' not described in 'global_pstate_info'
>  drivers/cpufreq/powernv-cpufreq.c:182: warning: Function parameter or member 
> 'i' not described in 'idx_to_pstate'
>  drivers/cpufreq/powernv-cpufreq.c:201: warning: Function parameter or member 
> 'pstate' not described in 'pstate_to_idx'
>  drivers/cpufreq/powernv-cpufreq.c:670: warning: Function parameter or member 
> 't' not described in 'gpstate_timer_handler'
>  drivers/cpufreq/powernv-cpufreq.c:670: warning: Excess function parameter 
> 'data' description in 'gpstate_timer_handler'
> 
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 068cc53abe320..2e5a8b8a4abaa 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -64,13 +64,14 @@
>   *   highest_lpstate_idx
>   * @last_sampled_time:   Time from boot in ms when global 
> pstates were
>   *   last set
> - * @last_lpstate_idx,Last set value of local pstate and 
> global
> - * last_gpstate_idx  pstate in terms of cpufreq table index
> + * @last_lpstate_idx:Last set value of local pstate and 
> global
> + * @last_gpstate_idx:pstate in terms of cpufreq table index
>   * @timer:   Is used for ramping down if cpu goes idle for
>   *   a long time with global pstate held high
>   * @gpstate_lock:A spinlock to maintain synchronization between
>   *   routines called by the timer handler and
>   *   governer's target_index calls
> + * @policy:  Associated CPUFreq policy
>   */
>  struct global_pstate_info {
>   int highest_lpstate_idx;
> @@ -170,7 +171,7 @@ static inline u8 extract_pstate(u64 pmsr_val, unsigned 
> int shift)
>  
>  /* Use following functions for conversions between pstate_id and index */
>  
> -/**
> +/*
>   * idx_to_pstate : Returns the pstate id corresponding to the
>   *  frequency in the cpufreq frequency table
>   *  powernv_freqs indexed by @i.
> @@ -188,7 +189,7 @@ static inline u8 idx_to_pstate(unsigned int i)
>   return powernv_freqs[i].driver_data;
>  }
>  
> -/**
> +/*
>   * pstate_to_idx : Returns the index in the cpufreq frequencytable
>   *  powernv_freqs for the frequency whose corresponding
>   *  pstate id is @pstate.
> @@ -660,7 +661,7 @@ static inline void  queue_gpstate_timer(struct 
> global_pstate_info *gpstates)
>  /**
>   * gpstate_timer_handler
>   *
> - * @data: pointer to cpufreq_policy on which timer was queued
> + * @t: Timer context used to fetch global pstate info struct
>   *
>   * This handler brings down the global pstate closer to the local pstate
>   * according quadratic equation. Queues a new timer if it is still not equal

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 06/13] cpufreq: powernv-cpufreq: Functions only used in call-backs should be static

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/powernv-cpufreq.c:669:6: warning: no previous prototype for 
> ‘gpstate_timer_handler’ [-Wmissing-prototypes]
>  drivers/cpufreq/powernv-cpufreq.c:902:6: warning: no previous prototype for 
> ‘powernv_cpufreq_work_fn’ [-Wmissing-prototypes]
> 
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 8646eb197cd96..068cc53abe320 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -666,7 +666,7 @@ static inline void  queue_gpstate_timer(struct 
> global_pstate_info *gpstates)
>   * according quadratic equation. Queues a new timer if it is still not equal
>   * to local pstate
>   */
> -void gpstate_timer_handler(struct timer_list *t)
> +static void gpstate_timer_handler(struct timer_list *t)
>  {
>   struct global_pstate_info *gpstates = from_timer(gpstates, t, timer);
>   struct cpufreq_policy *policy = gpstates->policy;
> @@ -899,7 +899,7 @@ static struct notifier_block powernv_cpufreq_reboot_nb = {
>   .notifier_call = powernv_cpufreq_reboot_notifier,
>  };
>  
> -void powernv_cpufreq_work_fn(struct work_struct *work)
> +static void powernv_cpufreq_work_fn(struct work_struct *work)
>  {
>   struct chip *chip = container_of(work, struct chip, throttle);
>   struct cpufreq_policy *policy;

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> If function callers and providers do not share the same prototypes the
> compiler complains of missing prototypes.  Fix this by moving the
> already existing prototypes out to a mutually convenient location.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/pasemi-cpufreq.c:109:5: warning: no previous prototype for 
> ‘check_astate’ [-Wmissing-prototypes]
>  109 | int check_astate(void)
>  | ^~~~
>  drivers/cpufreq/pasemi-cpufreq.c:114:6: warning: no previous prototype for 
> ‘restore_astate’ [-Wmissing-prototypes]
>  114 | void restore_astate(int cpu)
>  | ^~
> 
> Cc: Olof Johansson 
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones 
> ---
>  arch/powerpc/platforms/pasemi/pasemi.h| 15 

Is there no sane way we can include this file directly to the cpufreq
file ?

>  arch/powerpc/platforms/pasemi/powersave.S |  2 ++
>  drivers/cpufreq/pasemi-cpufreq.c  |  1 +
>  include/linux/platform_data/pasemi.h  | 28 +++
>  4 files changed, 31 insertions(+), 15 deletions(-)
>  create mode 100644 include/linux/platform_data/pasemi.h

-- 
viresh

[PATCH V4 2/3] cpufreq: Register governors at core_initcall

[Viresh Kumar]

From: Quentin Perret 

Currently, most CPUFreq governors are registered at core_initcall time
when used as default, and module_init otherwise. In preparation for
letting users specify the default governor on the kernel command line,
change all of them to use core_initcall unconditionally, as is already
the case for schedutil and performance. This will enable us to assume
builtin governors have been registered before the builtin CPUFreq
drivers probe.

And since all governors now have similar init/exit patterns, introduce
two new macros cpufreq_governor_{init,exit}() to factorize the code.

Acked-by: Viresh Kumar 
Signed-off-by: Quentin Perret 
Signed-off-by: Viresh Kumar 
---
 .../platforms/cell/cpufreq_spudemand.c| 26 ++-
 drivers/cpufreq/cpufreq_conservative.c| 22 
 drivers/cpufreq/cpufreq_ondemand.c| 24 +
 drivers/cpufreq/cpufreq_performance.c | 14 ++
 drivers/cpufreq/cpufreq_powersave.c   | 18 +++--
 drivers/cpufreq/cpufreq_userspace.c   | 18 +++--
 include/linux/cpufreq.h   | 14 ++
 kernel/sched/cpufreq_schedutil.c  |  6 +
 8 files changed, 36 insertions(+), 106 deletions(-)

diff --git a/arch/powerpc/platforms/cell/cpufreq_spudemand.c 
b/arch/powerpc/platforms/cell/cpufreq_spudemand.c
index 55b31eadb3c8..ca7849e113d7 100644
--- a/arch/powerpc/platforms/cell/cpufreq_spudemand.c
+++ b/arch/powerpc/platforms/cell/cpufreq_spudemand.c
@@ -126,30 +126,8 @@ static struct cpufreq_governor spu_governor = {
.stop = spu_gov_stop,
.owner = THIS_MODULE,
 };
-
-/*
- * module init and destoy
- */
-
-static int __init spu_gov_init(void)
-{
-   int ret;
-
-   ret = cpufreq_register_governor(_governor);
-   if (ret)
-   printk(KERN_ERR "registration of governor failed\n");
-   return ret;
-}
-
-static void __exit spu_gov_exit(void)
-{
-   cpufreq_unregister_governor(_governor);
-}
-
-
-module_init(spu_gov_init);
-module_exit(spu_gov_exit);
+cpufreq_governor_init(spu_governor);
+cpufreq_governor_exit(spu_governor);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Christian Krafft ");
-
diff --git a/drivers/cpufreq/cpufreq_conservative.c 
b/drivers/cpufreq/cpufreq_conservative.c
index 737ff3b9c2c0..aa39ff31ec9f 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -322,17 +322,7 @@ static struct dbs_governor cs_governor = {
.start = cs_start,
 };
 
-#define CPU_FREQ_GOV_CONSERVATIVE  (_governor.gov)
-
-static int __init cpufreq_gov_dbs_init(void)
-{
-   return cpufreq_register_governor(CPU_FREQ_GOV_CONSERVATIVE);
-}
-
-static void __exit cpufreq_gov_dbs_exit(void)
-{
-   cpufreq_unregister_governor(CPU_FREQ_GOV_CONSERVATIVE);
-}
+#define CPU_FREQ_GOV_CONSERVATIVE  (cs_governor.gov)
 
 MODULE_AUTHOR("Alexander Clouter ");
 MODULE_DESCRIPTION("'cpufreq_conservative' - A dynamic cpufreq governor for "
@@ -343,11 +333,9 @@ MODULE_LICENSE("GPL");
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
 struct cpufreq_governor *cpufreq_default_governor(void)
 {
-   return CPU_FREQ_GOV_CONSERVATIVE;
+   return _FREQ_GOV_CONSERVATIVE;
 }
-
-core_initcall(cpufreq_gov_dbs_init);
-#else
-module_init(cpufreq_gov_dbs_init);
 #endif
-module_exit(cpufreq_gov_dbs_exit);
+
+cpufreq_governor_init(CPU_FREQ_GOV_CONSERVATIVE);
+cpufreq_governor_exit(CPU_FREQ_GOV_CONSERVATIVE);
diff --git a/drivers/cpufreq/cpufreq_ondemand.c 
b/drivers/cpufreq/cpufreq_ondemand.c
index 82a4d37ddecb..ac361a8b1d3b 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -408,7 +408,7 @@ static struct dbs_governor od_dbs_gov = {
.start = od_start,
 };
 
-#define CPU_FREQ_GOV_ONDEMAND  (_dbs_gov.gov)
+#define CPU_FREQ_GOV_ONDEMAND  (od_dbs_gov.gov)
 
 static void od_set_powersave_bias(unsigned int powersave_bias)
 {
@@ -429,7 +429,7 @@ static void od_set_powersave_bias(unsigned int 
powersave_bias)
continue;
 
policy = cpufreq_cpu_get_raw(cpu);
-   if (!policy || policy->governor != CPU_FREQ_GOV_ONDEMAND)
+   if (!policy || policy->governor != _FREQ_GOV_ONDEMAND)
continue;
 
policy_dbs = policy->governor_data;
@@ -461,16 +461,6 @@ void od_unregister_powersave_bias_handler(void)
 }
 EXPORT_SYMBOL_GPL(od_unregister_powersave_bias_handler);
 
-static int __init cpufreq_gov_dbs_init(void)
-{
-   return cpufreq_register_governor(CPU_FREQ_GOV_ONDEMAND);
-}
-
-static void __exit cpufreq_gov_dbs_exit(void)
-{
-   cpufreq_unregister_governor(CPU_FREQ_GOV_ONDEMAND);
-}
-
 MODULE_AUTHOR("Venkatesh Pallipadi ");
 MODULE_AUTHOR("Alexey Starikovskiy ");
 MODULE_DESCRIPTION("'cpufreq_ondemand' - A dynamic cpufreq governor for "
@@ -480,11 +47

[PATCH V4 0/3] cpufreq: Allow default governor on cmdline and fix locking issues

[Viresh Kumar]

Hi,

I have picked Quentin's series over my patch, modified both and tested.

V3->V4:
- Do __module_get() for cpufreq_default_governor() case as well and get
  rid of an extra variable.
- Use a single character array, default_governor, instead of two of them.

V2->V3:
- default_governor is a string now and we don't set it on governor
  registration or unregistration anymore.
- Fixed locking issues in cpufreq_init_policy().

--
Viresh

Original cover letter fro Quentin:

This series enables users of prebuilt kernels (e.g. distro kernels) to
specify their CPUfreq governor of choice using the kernel command line,
instead of having to wait for the system to fully boot to userspace to
switch using the sysfs interface. This is helpful for 2 reasons:
  1. users get to choose the governor that runs during the actual boot;
  2. it simplifies the userspace boot procedure a bit (one less thing to
 worry about).

To enable this, the first patch moves all governor init calls to
core_initcall, to make sure they are registered by the time the drivers
probe. This should be relatively low impact as registering a governor
is a simple procedure (it gets added to a llist), and all governors
already load at core_initcall anyway when they're set as the default
in Kconfig. This also allows to clean-up the governors' init/exit code,
and reduces boilerplate.

The second patch introduces the new command line parameter, inspired by
its cpuidle counterpart. More details can be found in the respective
patch headers.

Changes in v2:
 - added Viresh's ack to patch 01
 - moved the assignment of 'default_governor' in patch 02 to the governor
   registration path instead of the driver registration (Viresh)

Quentin Perret (2):
  cpufreq: Register governors at core_initcall
  cpufreq: Specify default governor on command line

Viresh Kumar (1):
  cpufreq: Fix locking issues with governors

 .../admin-guide/kernel-parameters.txt |  5 ++
 Documentation/admin-guide/pm/cpufreq.rst  |  6 +-
 .../platforms/cell/cpufreq_spudemand.c| 26 +-
 drivers/cpufreq/cpufreq.c | 87 ---
 drivers/cpufreq/cpufreq_conservative.c| 22 ++---
 drivers/cpufreq/cpufreq_ondemand.c| 24 ++---
 drivers/cpufreq/cpufreq_performance.c | 14 +--
 drivers/cpufreq/cpufreq_powersave.c   | 18 +---
 drivers/cpufreq/cpufreq_userspace.c   | 18 +---
 include/linux/cpufreq.h   | 14 +++
 kernel/sched/cpufreq_schedutil.c  |  6 +-
 11 files changed, 100 insertions(+), 140 deletions(-)

-- 
2.25.0.rc1.19.g042ed3e048af

[PATCH V3 2/3] cpufreq: Register governors at core_initcall

[Viresh Kumar]

From: Quentin Perret 

Currently, most CPUFreq governors are registered at core_initcall time
when used as default, and module_init otherwise. In preparation for
letting users specify the default governor on the kernel command line,
change all of them to use core_initcall unconditionally, as is already
the case for schedutil and performance. This will enable us to assume
builtin governors have been registered before the builtin CPUFreq
drivers probe.

And since all governors now have similar init/exit patterns, introduce
two new macros cpufreq_governor_{init,exit}() to factorize the code.

Acked-by: Viresh Kumar 
Signed-off-by: Quentin Perret 
Signed-off-by: Viresh Kumar 
---
 .../platforms/cell/cpufreq_spudemand.c| 26 ++-
 drivers/cpufreq/cpufreq_conservative.c| 22 
 drivers/cpufreq/cpufreq_ondemand.c| 24 +
 drivers/cpufreq/cpufreq_performance.c | 14 ++
 drivers/cpufreq/cpufreq_powersave.c   | 18 +++--
 drivers/cpufreq/cpufreq_userspace.c   | 18 +++--
 include/linux/cpufreq.h   | 14 ++
 kernel/sched/cpufreq_schedutil.c  |  6 +
 8 files changed, 36 insertions(+), 106 deletions(-)

diff --git a/arch/powerpc/platforms/cell/cpufreq_spudemand.c 
b/arch/powerpc/platforms/cell/cpufreq_spudemand.c
index 55b31eadb3c8..ca7849e113d7 100644
--- a/arch/powerpc/platforms/cell/cpufreq_spudemand.c
+++ b/arch/powerpc/platforms/cell/cpufreq_spudemand.c
@@ -126,30 +126,8 @@ static struct cpufreq_governor spu_governor = {
.stop = spu_gov_stop,
.owner = THIS_MODULE,
 };
-
-/*
- * module init and destoy
- */
-
-static int __init spu_gov_init(void)
-{
-   int ret;
-
-   ret = cpufreq_register_governor(_governor);
-   if (ret)
-   printk(KERN_ERR "registration of governor failed\n");
-   return ret;
-}
-
-static void __exit spu_gov_exit(void)
-{
-   cpufreq_unregister_governor(_governor);
-}
-
-
-module_init(spu_gov_init);
-module_exit(spu_gov_exit);
+cpufreq_governor_init(spu_governor);
+cpufreq_governor_exit(spu_governor);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Christian Krafft ");
-
diff --git a/drivers/cpufreq/cpufreq_conservative.c 
b/drivers/cpufreq/cpufreq_conservative.c
index 737ff3b9c2c0..aa39ff31ec9f 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -322,17 +322,7 @@ static struct dbs_governor cs_governor = {
.start = cs_start,
 };
 
-#define CPU_FREQ_GOV_CONSERVATIVE  (_governor.gov)
-
-static int __init cpufreq_gov_dbs_init(void)
-{
-   return cpufreq_register_governor(CPU_FREQ_GOV_CONSERVATIVE);
-}
-
-static void __exit cpufreq_gov_dbs_exit(void)
-{
-   cpufreq_unregister_governor(CPU_FREQ_GOV_CONSERVATIVE);
-}
+#define CPU_FREQ_GOV_CONSERVATIVE  (cs_governor.gov)
 
 MODULE_AUTHOR("Alexander Clouter ");
 MODULE_DESCRIPTION("'cpufreq_conservative' - A dynamic cpufreq governor for "
@@ -343,11 +333,9 @@ MODULE_LICENSE("GPL");
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
 struct cpufreq_governor *cpufreq_default_governor(void)
 {
-   return CPU_FREQ_GOV_CONSERVATIVE;
+   return _FREQ_GOV_CONSERVATIVE;
 }
-
-core_initcall(cpufreq_gov_dbs_init);
-#else
-module_init(cpufreq_gov_dbs_init);
 #endif
-module_exit(cpufreq_gov_dbs_exit);
+
+cpufreq_governor_init(CPU_FREQ_GOV_CONSERVATIVE);
+cpufreq_governor_exit(CPU_FREQ_GOV_CONSERVATIVE);
diff --git a/drivers/cpufreq/cpufreq_ondemand.c 
b/drivers/cpufreq/cpufreq_ondemand.c
index 82a4d37ddecb..ac361a8b1d3b 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -408,7 +408,7 @@ static struct dbs_governor od_dbs_gov = {
.start = od_start,
 };
 
-#define CPU_FREQ_GOV_ONDEMAND  (_dbs_gov.gov)
+#define CPU_FREQ_GOV_ONDEMAND  (od_dbs_gov.gov)
 
 static void od_set_powersave_bias(unsigned int powersave_bias)
 {
@@ -429,7 +429,7 @@ static void od_set_powersave_bias(unsigned int 
powersave_bias)
continue;
 
policy = cpufreq_cpu_get_raw(cpu);
-   if (!policy || policy->governor != CPU_FREQ_GOV_ONDEMAND)
+   if (!policy || policy->governor != _FREQ_GOV_ONDEMAND)
continue;
 
policy_dbs = policy->governor_data;
@@ -461,16 +461,6 @@ void od_unregister_powersave_bias_handler(void)
 }
 EXPORT_SYMBOL_GPL(od_unregister_powersave_bias_handler);
 
-static int __init cpufreq_gov_dbs_init(void)
-{
-   return cpufreq_register_governor(CPU_FREQ_GOV_ONDEMAND);
-}
-
-static void __exit cpufreq_gov_dbs_exit(void)
-{
-   cpufreq_unregister_governor(CPU_FREQ_GOV_ONDEMAND);
-}
-
 MODULE_AUTHOR("Venkatesh Pallipadi ");
 MODULE_AUTHOR("Alexey Starikovskiy ");
 MODULE_DESCRIPTION("'cpufreq_ondemand' - A dynamic cpufreq governor for "
@@ -480,11 +47

[PATCH v3 0/3] cpufreq: Allow default governor on cmdline and fix locking issues

[Viresh Kumar]

Hi,

I have picked Quentin's series over my patch, modified both and tested.

V2->V3:
- default_governor is a string now and we don't set it on governor
  registration or unregistration anymore.
- Fixed locking issues in cpufreq_init_policy().

--
Viresh

Original cover letter fro Quentin:

This series enables users of prebuilt kernels (e.g. distro kernels) to
specify their CPUfreq governor of choice using the kernel command line,
instead of having to wait for the system to fully boot to userspace to
switch using the sysfs interface. This is helpful for 2 reasons:
  1. users get to choose the governor that runs during the actual boot;
  2. it simplifies the userspace boot procedure a bit (one less thing to
 worry about).

To enable this, the first patch moves all governor init calls to
core_initcall, to make sure they are registered by the time the drivers
probe. This should be relatively low impact as registering a governor
is a simple procedure (it gets added to a llist), and all governors
already load at core_initcall anyway when they're set as the default
in Kconfig. This also allows to clean-up the governors' init/exit code,
and reduces boilerplate.

The second patch introduces the new command line parameter, inspired by
its cpuidle counterpart. More details can be found in the respective
patch headers.

Changes in v2:
 - added Viresh's ack to patch 01
 - moved the assignment of 'default_governor' in patch 02 to the governor
   registration path instead of the driver registration (Viresh)

Quentin Perret (2):
  cpufreq: Register governors at core_initcall
  cpufreq: Specify default governor on command line

Viresh Kumar (1):
  cpufreq: Fix locking issues with governors

 .../admin-guide/kernel-parameters.txt |  5 +
 Documentation/admin-guide/pm/cpufreq.rst  |  6 +-
 .../platforms/cell/cpufreq_spudemand.c| 26 +-
 drivers/cpufreq/cpufreq.c | 93 ---
 drivers/cpufreq/cpufreq_conservative.c| 22 +
 drivers/cpufreq/cpufreq_ondemand.c| 24 ++---
 drivers/cpufreq/cpufreq_performance.c | 14 +--
 drivers/cpufreq/cpufreq_powersave.c   | 18 +---
 drivers/cpufreq/cpufreq_userspace.c   | 18 +---
 include/linux/cpufreq.h   | 14 +++
 kernel/sched/cpufreq_schedutil.c  |  6 +-
 11 files changed, 106 insertions(+), 140 deletions(-)

-- 
2.25.0.rc1.19.g042ed3e048af

Re: [PATCH v2 2/2] cpufreq: Specify default governor on command line

[Viresh Kumar]

On 23-06-20, 15:21, Quentin Perret wrote:
> @@ -2789,7 +2796,13 @@ static int __init cpufreq_core_init(void)
>   cpufreq_global_kobject = kobject_create_and_add("cpufreq", 
> _subsys.dev_root->kobj);
>   BUG_ON(!cpufreq_global_kobject);
>  
> + mutex_lock(_governor_mutex);
> + if (!default_governor)

Also is this check really required ? The pointer will always be NULL
at this point, isn't it ?

> + default_governor = cpufreq_default_governor();
> + mutex_unlock(_governor_mutex);
> +
>   return 0;
>  }
>  module_param(off, int, 0444);
> +module_param_string(default_governor, cpufreq_param_governor, 
> CPUFREQ_NAME_LEN, 0444);
>  core_initcall(cpufreq_core_init);
> -- 
> 2.27.0.111.gc72c7da667-goog

-- 
viresh

Re: [PATCH v2 2/2] cpufreq: Specify default governor on command line

[Viresh Kumar]

  } else if (default_governor) {
> + gov = default_governor;
>   } else {
>   return -ENODATA;
>   }
> @@ -1074,8 +1076,8 @@ static int cpufreq_init_policy(struct cpufreq_policy 
> *policy)
>   /* Use the default policy if there is no last_policy. */
>   if (policy->last_policy) {
>   pol = policy->last_policy;
> - } else if (def_gov) {
> - pol = cpufreq_parse_policy(def_gov->name);
> + } else if (default_governor) {
> + pol = cpufreq_parse_policy(default_governor->name);
>   /*
>* In case the default governor is neiter "performance"
>* nor "powersave", fall back to the initial policy
> @@ -2320,6 +2322,9 @@ int cpufreq_register_governor(struct cpufreq_governor 
> *governor)
>   list_add(>governor_list, _governor_list);
>   }
>  
> + if (!strncasecmp(cpufreq_param_governor, governor->name, 
> CPUFREQ_NAME_LEN))
> + default_governor = governor;
> +
>   mutex_unlock(_governor_mutex);
>   return err;
>  }
> @@ -2348,6 +2353,8 @@ void cpufreq_unregister_governor(struct 
> cpufreq_governor *governor)
>  
>   mutex_lock(_governor_mutex);
>   list_del(>governor_list);
> + if (governor == default_governor)
> + default_governor = cpufreq_default_governor();
>   mutex_unlock(_governor_mutex);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
> @@ -2789,7 +2796,13 @@ static int __init cpufreq_core_init(void)
>   cpufreq_global_kobject = kobject_create_and_add("cpufreq", 
> _subsys.dev_root->kobj);
>   BUG_ON(!cpufreq_global_kobject);
>  
> + mutex_lock(_governor_mutex);
> + if (!default_governor)
> + default_governor = cpufreq_default_governor();
> + mutex_unlock(_governor_mutex);

I don't think locking is required here at core-initcall level. Apart
from that:

Acked-by: Viresh Kumar 

> +
>   return 0;
>  }
>  module_param(off, int, 0444);
> +module_param_string(default_governor, cpufreq_param_governor, 
> CPUFREQ_NAME_LEN, 0444);
>  core_initcall(cpufreq_core_init);
> -- 
> 2.27.0.111.gc72c7da667-goog

-- 
viresh

Re: [PATCH 2/2] cpufreq: Specify default governor on command line

[Viresh Kumar]

On 16-06-20, 10:48, Quentin Perret wrote:
> ---8<---
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 0f05caedc320..a9219404e07f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2340,6 +2340,11 @@ int cpufreq_register_governor(struct cpufreq_governor 
> *governor)
>   list_add(>governor_list, _governor_list);
>   }
>  
> + if (!strncasecmp(cpufreq_param_governor, governor->name, 
> CPUFREQ_NAME_LEN))
> + default_governor = governor;
> + else if (!default_governor && cpufreq_default_governor() == governor)
> + default_governor = cpufreq_default_governor();

Instead of the else part here, maybe just do this from
cpufreq_core_init() only once, and so we will always have
default_governor set.

> +
>   mutex_unlock(_governor_mutex);
>   return err;
>  }
> @@ -2368,6 +2373,8 @@ void cpufreq_unregister_governor(struct 
> cpufreq_governor *governor)
>  
>   mutex_lock(_governor_mutex);
>   list_del(>governor_list);
> + if (governor == default_governor)
> + default_governor = cpufreq_default_governor();
>   mutex_unlock(_governor_mutex);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
> --->8---
> 
> should do the trick. That removes the unnecessary reference count, and
> feels like a good place to hook things -- that is how cpuidle does it
> too IIRC.
> 
> I'll double check the locking/synchronization, but that shouldn't be too
> bad (famous last words).
> 
> Cheers,
> Quentin

-- 
viresh

Re: [PATCH 2/2] cpufreq: Specify default governor on command line

[Viresh Kumar]

On 16-06-20, 09:31, Quentin Perret wrote:
> Right, so the reason I avoided cpufreq_core_init() was because it is
> called at core_initcall() time, which means I can't really assume the
> governors have been loaded by that time. By waiting for the driver to
> probe before detecting the default gov, we get that nice ordering. But
> yes, it feels odd to have it here :/
> 
> Thinking about it more, the natural fit for this would rather be the
> register/unregister path for governors directly. If that sounds good to
> you (?) I'll try to move it there in v2.

There is another problem here which we need to look at. Any governor
which is built as a module and isn't currently used, should be allowed
to unload. And this needs to be tested by you as well, should be easy
enough.

With the current implementation, you take a reference to the default
governor when the driver is registered and drop it only when the
driver goes away. Which means we won't be able to unload the module of
the governor even if it isn't used. Which is wrong. The solution I
proposed had the same issue as well.

You need to figure out a way where we don't need to keep holding the
module hostage even when it isn't used. I see two ways at least for
the same:

- Do that from the existing place: cpufreq_init_policy().

- And I think this can be done from governor-register/unregister as
  well.

Second one sounds good, if it is feasible to do that.

-- 
viresh

Re: [PATCH 2/2] cpufreq: Specify default governor on command line

[Viresh Kumar]

On 15-06-20, 17:55, Quentin Perret wrote:
> +static void cpufreq_get_default_governor(void)
> +{
> + default_governor = cpufreq_parse_governor(cpufreq_param_governor);
> + if (!default_governor) {
> + if (*cpufreq_param_governor)
> + pr_warn("Failed to find %s\n", cpufreq_param_governor);
> + default_governor = cpufreq_default_governor();

A module_get() never happened for this case and so maybe a
module_put() should never get called.

> + }
> +}
> +
> +static void cpufreq_put_default_governor(void)
> +{
> + if (!default_governor)
> + return;
> + module_put(default_governor->owner);
> + default_governor = NULL;
> +}
> +
>  static int cpufreq_init_governor(struct cpufreq_policy *policy)
>  {
>   int ret;
> @@ -2701,6 +2721,8 @@ int cpufreq_register_driver(struct cpufreq_driver 
> *driver_data)
>  
>   if (driver_data->setpolicy)
>   driver_data->flags |= CPUFREQ_CONST_LOOPS;
> + else
> + cpufreq_get_default_governor();
>  
>   if (cpufreq_boost_supported()) {
>   ret = create_boost_sysfs_file();
> @@ -2769,6 +2791,7 @@ int cpufreq_unregister_driver(struct cpufreq_driver 
> *driver)
>   subsys_interface_unregister(_interface);
>   remove_boost_sysfs_file();
>   cpuhp_remove_state_nocalls_cpuslocked(hp_online);
> + cpufreq_put_default_governor();
>  
>   write_lock_irqsave(_driver_lock, flags);
>  
> @@ -2792,4 +2815,5 @@ static int __init cpufreq_core_init(void)
>   return 0;
>  }

And since this is a per boot thing, there is perhaps no need of doing
these at driver register/unregister, I would rather do it at:
cpufreq_core_init() time itself and so we will never need to run
cpufreq_put_default_governor() and so can be removed.

And another thing I am not able to understand (despite you commenting
about that in the commit log) is what happens if the default governor
chosen is built as a module ?

-- 
viresh

Re: [PATCH 1/2] cpufreq: Register governors at core_initcall

[Viresh Kumar]

On 15-06-20, 17:55, Quentin Perret wrote:
> Currently, most CPUFreq governors are registered at core_initcall time
> when used as default, and module_init otherwise. In preparation for
> letting users specify the default governor on the kernel command line,
> change all of them to use core_initcall unconditionally, as is already
> the case for schedutil and performance. This will enable us to assume
> builtin governors have been registered before the builtin CPUFreq
> drivers probe.
> 
> And since all governors now have similar init/exit patterns, introduce
> two new macros cpufreq_governor_{init,exit}() to factorize the code.
> 
> Signed-off-by: Quentin Perret 
> ---
> Note: I couldn't boot-test the change to spudemand, by lack of hardware.
> But I can confirm cell_defconfig compiles just fine.
> ---
>  .../platforms/cell/cpufreq_spudemand.c| 26 ++-
>  drivers/cpufreq/cpufreq_conservative.c| 22 
>  drivers/cpufreq/cpufreq_ondemand.c| 24 +
>  drivers/cpufreq/cpufreq_performance.c | 14 ++
>  drivers/cpufreq/cpufreq_powersave.c   | 18 +++--
>  drivers/cpufreq/cpufreq_userspace.c   | 18 +++--
>  include/linux/cpufreq.h   | 14 ++
>  kernel/sched/cpufreq_schedutil.c  |  6 +
>  8 files changed, 36 insertions(+), 106 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v2 1/2] cpufreq: qoriq: convert to a platform driver

[Viresh Kumar]

On 28-04-20, 16:31, Viresh Kumar wrote:
> On 21-04-20, 10:29, Mian Yousaf Kaukab wrote:
> > The driver has to be manually loaded if it is built as a module. It
> > is neither exporting MODULE_DEVICE_TABLE nor MODULE_ALIAS. Moreover,
> > no platform-device is created (and thus no uevent is sent) for the
> > clockgen nodes it depends on.
> > 
> > Convert the module to a platform driver with its own alias. Moreover,
> > drop whitelisted SOCs. Platform device will be created only for the
> > compatible platforms.
> > 
> > Reviewed-by: Yuantian Tang 
> > Acked-by: Viresh Kumar 
> > Signed-off-by: Mian Yousaf Kaukab 
> > ---
> > v2:
> >  +Rafael, Stephen, linux-clk
> >  Add Reviewed-by and Acked-by tags
> > 
> >  drivers/cpufreq/qoriq-cpufreq.c | 76 
> > -
> >  1 file changed, 29 insertions(+), 47 deletions(-)
> 
> @Rafael,
> 
> Though this looks to be PPC stuff, but it is used on both ARM and PPC. Do you
> want to pick them up or should I do that ?

Applied now. Thanks.

-- 
viresh

Re: [PATCH v2 1/2] cpufreq: qoriq: convert to a platform driver

[Viresh Kumar]

On 21-04-20, 10:29, Mian Yousaf Kaukab wrote:
> The driver has to be manually loaded if it is built as a module. It
> is neither exporting MODULE_DEVICE_TABLE nor MODULE_ALIAS. Moreover,
> no platform-device is created (and thus no uevent is sent) for the
> clockgen nodes it depends on.
> 
> Convert the module to a platform driver with its own alias. Moreover,
> drop whitelisted SOCs. Platform device will be created only for the
> compatible platforms.
> 
> Reviewed-by: Yuantian Tang 
> Acked-by: Viresh Kumar 
> Signed-off-by: Mian Yousaf Kaukab 
> ---
> v2:
>  +Rafael, Stephen, linux-clk
>  Add Reviewed-by and Acked-by tags
> 
>  drivers/cpufreq/qoriq-cpufreq.c | 76 
> -
>  1 file changed, 29 insertions(+), 47 deletions(-)

@Rafael,

Though this looks to be PPC stuff, but it is used on both ARM and PPC. Do you
want to pick them up or should I do that ?

-- 
viresh

Re: [PATCH 1/2] cpufreq: qoriq: convert to a platform driver

[Viresh Kumar]

On 03-04-20, 23:21, Mian Yousaf Kaukab wrote:
> The driver has to be manually loaded if it is built as a module. It
> is neither exporting MODULE_DEVICE_TABLE nor MODULE_ALIAS. Moreover,
> no platform-device is created (and thus no uevent is sent) for the
> clockgen nodes it depends on.
> 
> Convert the module to a platform driver with its own alias. Moreover,
> drop whitelisted SOCs. Platform device will be created only for the
> compatible platforms.
> 
> Signed-off-by: Mian Yousaf Kaukab 
> ---
>  drivers/cpufreq/qoriq-cpufreq.c | 76 
> -
>  1 file changed, 29 insertions(+), 47 deletions(-)

For both patches,

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v3] cpufreq: powernv: fix stack bloat and hard limit on num cpus

[Viresh Kumar]

On Thu, 31 Oct 2019 at 10:52, John Hubbard  wrote:
>
> The following build warning occurred on powerpc 64-bit builds:
>
> drivers/cpufreq/powernv-cpufreq.c: In function 'init_chip_info':
> drivers/cpufreq/powernv-cpufreq.c:1070:1: warning: the frame size of
> 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>
> This is with a cross-compiler based on gcc 8.1.0, which I got from:
>   https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/8.1.0/
>
> The warning is due to putting 1024 bytes on the stack:
>
> unsigned int chip[256];
>
> ...and it's also undesirable to have a hard limit on the number of
> CPUs here.
>
> Fix both problems by dynamically allocating based on num_possible_cpus,
> as recommended by Michael Ellerman.
>
> Fixes: 053819e0bf840 ("cpufreq: powernv: Handle throttling due to Pmax 
> capping at chip level")
> Cc: Michael Ellerman 
> Cc: Shilpasri G Bhat 
> Cc: Preeti U Murthy 
> Cc: Viresh Kumar 
> Cc: Rafael J. Wysocki 
> Cc: linux...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: John Hubbard 
> ---

Acked-by: Viresh Kumar 

Re: [PATCH v2] cpufreq: powernv: fix stack bloat and NR_CPUS limitation

[Viresh Kumar]

On 17-10-19, 21:55, John Hubbard wrote:
> The following build warning occurred on powerpc 64-bit builds:
> 
> drivers/cpufreq/powernv-cpufreq.c: In function 'init_chip_info':
> drivers/cpufreq/powernv-cpufreq.c:1070:1: warning: the frame size of 1040 
> bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> This is due to putting 1024 bytes on the stack:
> 
> unsigned int chip[256];
> 
> ...and while looking at this, it also has a bug: it fails with a stack
> overrun, if CONFIG_NR_CPUS > 256.
> 
> Fix both problems by dynamically allocating based on CONFIG_NR_CPUS.
> 
> Fixes: 053819e0bf840 ("cpufreq: powernv: Handle throttling due to Pmax 
> capping at chip level")
> Cc: Shilpasri G Bhat 
> Cc: Preeti U Murthy 
> Cc: Viresh Kumar 
> Cc: Rafael J. Wysocki 
> Cc: linux...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: John Hubbard 
> ---
> 
> Changes since v1: includes Viresh's review commit fixes.
> 
>  drivers/cpufreq/powernv-cpufreq.c | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH] cpufreq: powernv: fix stack bloat and NR_CPUS limitation

[Viresh Kumar]

On 17-10-19, 21:41, John Hubbard wrote:
> On 10/17/19 9:38 PM, Viresh Kumar wrote:
> > On 17-10-19, 21:34, John Hubbard wrote:
> >> On 10/17/19 9:27 PM, Viresh Kumar wrote:
> >>> On 17-10-19, 17:04, John Hubbard wrote:
> >>>> The following build warning occurred on powerpc 64-bit builds:
> >>>>
> >>>> drivers/cpufreq/powernv-cpufreq.c: In function 'init_chip_info':
> >>>> drivers/cpufreq/powernv-cpufreq.c:1070:1: warning: the frame size of 
> >>>> 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> >>>
> >>> How come we are catching this warning after 4 years ?
> >>>
> >>
> >> Newer compilers. And btw, I don't spend a lot of time in powerpc
> >> code, so I just recently ran this, and I guess everyone has been on less
> >> new compilers so far, it seems.
> >>
> >> I used a gcc 8.1 cross compiler in this case:
> > 
> > Hmm, okay.
> > 
> > I hope you haven't missed my actual review comments on your patch,
> > just wanted to make sure we don't end up waiting for each other
> > indefinitely here :)
> > 
> 
> Ha, I did overlook those. It's late around here, I guess. :)

Good that I reminded you then :)

-- 
viresh

Re: [PATCH] cpufreq: powernv: fix stack bloat and NR_CPUS limitation

[Viresh Kumar]

On 17-10-19, 21:34, John Hubbard wrote:
> On 10/17/19 9:27 PM, Viresh Kumar wrote:
> > On 17-10-19, 17:04, John Hubbard wrote:
> >> The following build warning occurred on powerpc 64-bit builds:
> >>
> >> drivers/cpufreq/powernv-cpufreq.c: In function 'init_chip_info':
> >> drivers/cpufreq/powernv-cpufreq.c:1070:1: warning: the frame size of 1040 
> >> bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > 
> > How come we are catching this warning after 4 years ?
> > 
> 
> Newer compilers. And btw, I don't spend a lot of time in powerpc
> code, so I just recently ran this, and I guess everyone has been on less
> new compilers so far, it seems.
> 
> I used a gcc 8.1 cross compiler in this case:

Hmm, okay.

I hope you haven't missed my actual review comments on your patch,
just wanted to make sure we don't end up waiting for each other
indefinitely here :)

-- 
viresh

Re: [PATCH] cpufreq: powernv: fix stack bloat and NR_CPUS limitation

[Viresh Kumar]

On 17-10-19, 17:04, John Hubbard wrote:
> The following build warning occurred on powerpc 64-bit builds:
> 
> drivers/cpufreq/powernv-cpufreq.c: In function 'init_chip_info':
> drivers/cpufreq/powernv-cpufreq.c:1070:1: warning: the frame size of 1040 
> bytes is larger than 1024 bytes [-Wframe-larger-than=]

How come we are catching this warning after 4 years ?

> 
> This is due to putting 1024 bytes on the stack:
> 
> unsigned int chip[256];
> 
> ...and while looking at this, it also has a bug: it fails with a stack
> overrun, if CONFIG_NR_CPUS > 256.
> 
> Fix both problems by dynamically allocating based on CONFIG_NR_CPUS.
> 
> Fixes: 053819e0bf840 ("cpufreq: powernv: Handle throttling due to Pmax 
> capping at chip level")
> Cc: Shilpasri G Bhat 
> Cc: Preeti U Murthy 
> Cc: Viresh Kumar 
> Cc: Rafael J. Wysocki 
> Cc: linux...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: John Hubbard 
> ---
> 
> Hi,
> 
> I have only compile-tested this, so I would appreciate if anyone
> could do a basic runtime test on it. But (famous last words) it
> seems simple enough that I'm confident it's correct. oh boy. :)
> 
> thanks,
> John Hubbard
> NVIDIA
> 
>  drivers/cpufreq/powernv-cpufreq.c | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 6061850e59c9..78e04402125f 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -1041,9 +1041,14 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>  
>  static int init_chip_info(void)
>  {
> - unsigned int chip[256];
> + unsigned int *chip;
>   unsigned int cpu, i;
>   unsigned int prev_chip_id = UINT_MAX;
> + int ret = 0;
> +
> + chip = kcalloc(CONFIG_NR_CPUS, sizeof(int), GFP_KERNEL);

   sizeof(*chip)

> + if (!chips)

   (!chip)

> + return -ENOMEM;
>  
>   for_each_possible_cpu(cpu) {
>   unsigned int id = cpu_to_chip_id(cpu);
> @@ -1055,8 +1060,10 @@ static int init_chip_info(void)
>   }
>  
>   chips = kcalloc(nr_chips, sizeof(struct chip), GFP_KERNEL);
> - if (!chips)
> - return -ENOMEM;
> + if (!chips) {
> + ret = -ENOMEM;
> + goto free_and_return;
> + }
>  
>   for (i = 0; i < nr_chips; i++) {
>   chips[i].id = chip[i];
> @@ -1066,7 +1073,9 @@ static int init_chip_info(void)
>   per_cpu(chip_info, cpu) =  [i];
>   }
>  
> - return 0;
> +free_and_return:
> + kfree(chip);
> + return ret;
>  }
>  
>  static inline void clean_chip_info(void)
> -- 
> 2.23.0

-- 
viresh

[PATCH V2 03/10] powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier

[Viresh Kumar]

The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.

Switch over to using the QoS request for maximum frequency constraint
for windfarm_cpufreq_clamp driver.

Signed-off-by: Viresh Kumar 
---
 drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++
 1 file changed, 50 insertions(+), 27 deletions(-)

diff --git a/drivers/macintosh/windfarm_cpufreq_clamp.c 
b/drivers/macintosh/windfarm_cpufreq_clamp.c
index 52fd5fca89a0..705c6200814b 100644
--- a/drivers/macintosh/windfarm_cpufreq_clamp.c
+++ b/drivers/macintosh/windfarm_cpufreq_clamp.c
@@ -3,9 +3,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -16,36 +18,24 @@
 
 static int clamped;
 static struct wf_control *clamp_control;
-
-static int clamp_notifier_call(struct notifier_block *self,
-  unsigned long event, void *data)
-{
-   struct cpufreq_policy *p = data;
-   unsigned long max_freq;
-
-   if (event != CPUFREQ_ADJUST)
-   return 0;
-
-   max_freq = clamped ? (p->cpuinfo.min_freq) : (p->cpuinfo.max_freq);
-   cpufreq_verify_within_limits(p, 0, max_freq);
-
-   return 0;
-}
-
-static struct notifier_block clamp_notifier = {
-   .notifier_call = clamp_notifier_call,
-};
+static struct dev_pm_qos_request qos_req;
+static unsigned int min_freq, max_freq;
 
 static int clamp_set(struct wf_control *ct, s32 value)
 {
-   if (value)
+   unsigned int freq;
+
+   if (value) {
+   freq = min_freq;
printk(KERN_INFO "windfarm: Clamping CPU frequency to "
   "minimum !\n");
-   else
+   } else {
+   freq = max_freq;
printk(KERN_INFO "windfarm: CPU frequency unclamped !\n");
+   }
clamped = value;
-   cpufreq_update_policy(0);
-   return 0;
+
+   return dev_pm_qos_update_request(_req, freq);
 }
 
 static int clamp_get(struct wf_control *ct, s32 *value)
@@ -74,27 +64,60 @@ static const struct wf_control_ops clamp_ops = {
 
 static int __init wf_cpufreq_clamp_init(void)
 {
+   struct cpufreq_policy *policy;
struct wf_control *clamp;
+   struct device *dev;
+   int ret;
+
+   policy = cpufreq_cpu_get(0);
+   if (!policy) {
+   pr_warn("%s: cpufreq policy not found cpu0\n", __func__);
+   return -EPROBE_DEFER;
+   }
+
+   min_freq = policy->cpuinfo.min_freq;
+   max_freq = policy->cpuinfo.max_freq;
+   cpufreq_cpu_put(policy);
+
+   dev = get_cpu_device(0);
+   if (unlikely(!dev)) {
+   pr_warn("%s: No cpu device for cpu0\n", __func__);
+   return -ENODEV;
+   }
 
clamp = kmalloc(sizeof(struct wf_control), GFP_KERNEL);
if (clamp == NULL)
return -ENOMEM;
-   cpufreq_register_notifier(_notifier, CPUFREQ_POLICY_NOTIFIER);
+
+   ret = dev_pm_qos_add_request(dev, _req, DEV_PM_QOS_MAX_FREQUENCY,
+max_freq);
+   if (ret < 0) {
+   pr_err("%s: Failed to add freq constraint (%d)\n", __func__,
+  ret);
+   goto free;
+   }
+
clamp->ops = _ops;
clamp->name = "cpufreq-clamp";
-   if (wf_register_control(clamp))
+   ret = wf_register_control(clamp);
+   if (ret)
goto fail;
clamp_control = clamp;
return 0;
  fail:
+   dev_pm_qos_remove_request(_req);
+
+ free:
kfree(clamp);
-   return -ENODEV;
+   return ret;
 }
 
 static void __exit wf_cpufreq_clamp_exit(void)
 {
-   if (clamp_control)
+   if (clamp_control) {
wf_unregister_control(clamp_control);
+   dev_pm_qos_remove_request(_req);
+   }
 }
 
 
-- 
2.21.0.rc0.269.g1a574e7a288b

[PATCH V2 00/10] cpufreq: Migrate users of policy notifiers to QoS requests

[Viresh Kumar]

Hello,

Now that cpufreq core supports taking QoS requests for min/max cpu
frequencies, lets migrate rest of the users to using them instead of the
policy notifiers.

The CPUFREQ_NOTIFY and CPUFREQ_ADJUST events of the policy notifiers are
removed as a result, but we have to add CPUFREQ_CREATE_POLICY and
CPUFREQ_REMOVE_POLICY events to it for the acpi stuff specifically,
though they are also used by arch_topology stuff now. So the policy
notifiers aren't completely removed.

Boot tested on my x86 PC and ARM hikey board.

This has already gone through build bot for a few days now.

V1->V2:
- Added Acked-by tags
- Reordered to keep cleanups at the bottom
- Rebased over 5.3-rc1

--
viresh

Viresh Kumar (10):
  cpufreq: Add policy create/remove notifiers
  thermal: cpu_cooling: Switch to QoS requests instead of cpufreq
notifier
  powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier
  cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq
notifier
  ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
  arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY
  video: sa1100fb: Remove cpufreq policy notifier
  video: pxafb: Remove cpufreq policy notifier
  cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier
events
  Documentation: cpufreq: Update policy notifier documentation

 Documentation/cpu-freq/core.txt|  16 +--
 drivers/acpi/processor_driver.c|  44 -
 drivers/acpi/processor_perflib.c   | 106 +---
 drivers/acpi/processor_thermal.c   |  81 ---
 drivers/base/arch_topology.c   |   2 +-
 drivers/cpufreq/cpufreq.c  |  51 --
 drivers/cpufreq/ppc_cbe_cpufreq.c  |  19 +++-
 drivers/cpufreq/ppc_cbe_cpufreq.h  |   8 ++
 drivers/cpufreq/ppc_cbe_cpufreq_pmi.c  |  96 +++---
 drivers/macintosh/windfarm_cpufreq_clamp.c |  77 ++-
 drivers/thermal/cpu_cooling.c  | 110 +
 drivers/video/fbdev/pxafb.c|  21 
 drivers/video/fbdev/pxafb.h|   1 -
 drivers/video/fbdev/sa1100fb.c |  27 -
 drivers/video/fbdev/sa1100fb.h |   1 -
 include/acpi/processor.h   |  22 +++--
 include/linux/cpufreq.h|   4 +-
 17 files changed, 327 insertions(+), 359 deletions(-)

-- 
2.21.0.rc0.269.g1a574e7a288b

Re: [PATCH v7] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 17-07-19, 11:55, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f6 ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: Michael Ellerman 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v7: adapt to commit ("cpufreq: Make cpufreq_generic_init() return void")
> v6: keep the blank line and fix warning: label 'out_unmap_sdcpwr' defined but 
> not used.
> v5: put together the code to get, use, and release cpu device_node.
> v4: restore the blank line.
> v3: fix a leaked reference.
> v2: clean up the code according to the advice of viresh.
> 
>  drivers/cpufreq/pasemi-cpufreq.c | 23 +++++--
>  1 file changed, 9 insertions(+), 14 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 00/10] cpufreq: Migrate users of policy notifiers to QoS requests

[Viresh Kumar]

On 16-07-19, 12:06, Rafael J. Wysocki wrote:
> On Tue, Jul 16, 2019 at 11:49 AM Viresh Kumar  wrote:
> >
> > Hello,
> >
> > Now that cpufreq core supports taking QoS requests for min/max cpu
> > frequencies, lets migrate rest of the users to using them instead of the
> > policy notifiers.
> 
> Technically, this still is linux-next only. :-)

True :)

> > The CPUFREQ_NOTIFY and CPUFREQ_ADJUST events of the policy notifiers are
> > removed as a result, but we have to add CPUFREQ_CREATE_POLICY and
> > CPUFREQ_REMOVE_POLICY events to it for the acpi stuff specifically. So
> > the policy notifiers aren't completely removed.
> 
> That's not entirely accurate, because arch_topology is going to use
> CPUFREQ_CREATE_POLICY now too.

Yeah, I thought about that while writing this patchset and
coverletter. But had it not been required for ACPI, I would have done
it differently for the arch-topology code. Maybe direct calling of
arch-topology routine from cpufreq core. I wanted to get rid of the
policy notifiers completely but I couldn't find a better way of doing
it for ACPI stuff.

> > Boot tested on my x86 PC and ARM hikey board. Nothing looked broken :)
> >
> > This has already gone through build bot for a few days now.
> 
> So I'd prefer patches [5-8] to go right after the first one and then
> do the cleanups on top of that, as somebody may want to backport the
> essential changes without the cleanups.

In the exceptional case where nobody finds anything wrong with the
patches (highly unlikely), do you want me to resend with reordering or
you can reorder them while applying? There are no dependencies between
those patches anyway.

--
viresh

[PATCH 06/10] powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier

[Viresh Kumar]

The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.

Switch over to using the QoS request for maximum frequency constraint
for windfarm_cpufreq_clamp driver.

Signed-off-by: Viresh Kumar 
---
 drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++
 1 file changed, 50 insertions(+), 27 deletions(-)

diff --git a/drivers/macintosh/windfarm_cpufreq_clamp.c 
b/drivers/macintosh/windfarm_cpufreq_clamp.c
index 52fd5fca89a0..705c6200814b 100644
--- a/drivers/macintosh/windfarm_cpufreq_clamp.c
+++ b/drivers/macintosh/windfarm_cpufreq_clamp.c
@@ -3,9 +3,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -16,36 +18,24 @@
 
 static int clamped;
 static struct wf_control *clamp_control;
-
-static int clamp_notifier_call(struct notifier_block *self,
-  unsigned long event, void *data)
-{
-   struct cpufreq_policy *p = data;
-   unsigned long max_freq;
-
-   if (event != CPUFREQ_ADJUST)
-   return 0;
-
-   max_freq = clamped ? (p->cpuinfo.min_freq) : (p->cpuinfo.max_freq);
-   cpufreq_verify_within_limits(p, 0, max_freq);
-
-   return 0;
-}
-
-static struct notifier_block clamp_notifier = {
-   .notifier_call = clamp_notifier_call,
-};
+static struct dev_pm_qos_request qos_req;
+static unsigned int min_freq, max_freq;
 
 static int clamp_set(struct wf_control *ct, s32 value)
 {
-   if (value)
+   unsigned int freq;
+
+   if (value) {
+   freq = min_freq;
printk(KERN_INFO "windfarm: Clamping CPU frequency to "
   "minimum !\n");
-   else
+   } else {
+   freq = max_freq;
printk(KERN_INFO "windfarm: CPU frequency unclamped !\n");
+   }
clamped = value;
-   cpufreq_update_policy(0);
-   return 0;
+
+   return dev_pm_qos_update_request(_req, freq);
 }
 
 static int clamp_get(struct wf_control *ct, s32 *value)
@@ -74,27 +64,60 @@ static const struct wf_control_ops clamp_ops = {
 
 static int __init wf_cpufreq_clamp_init(void)
 {
+   struct cpufreq_policy *policy;
struct wf_control *clamp;
+   struct device *dev;
+   int ret;
+
+   policy = cpufreq_cpu_get(0);
+   if (!policy) {
+   pr_warn("%s: cpufreq policy not found cpu0\n", __func__);
+   return -EPROBE_DEFER;
+   }
+
+   min_freq = policy->cpuinfo.min_freq;
+   max_freq = policy->cpuinfo.max_freq;
+   cpufreq_cpu_put(policy);
+
+   dev = get_cpu_device(0);
+   if (unlikely(!dev)) {
+   pr_warn("%s: No cpu device for cpu0\n", __func__);
+   return -ENODEV;
+   }
 
clamp = kmalloc(sizeof(struct wf_control), GFP_KERNEL);
if (clamp == NULL)
return -ENOMEM;
-   cpufreq_register_notifier(_notifier, CPUFREQ_POLICY_NOTIFIER);
+
+   ret = dev_pm_qos_add_request(dev, _req, DEV_PM_QOS_MAX_FREQUENCY,
+max_freq);
+   if (ret < 0) {
+   pr_err("%s: Failed to add freq constraint (%d)\n", __func__,
+  ret);
+   goto free;
+   }
+
clamp->ops = _ops;
clamp->name = "cpufreq-clamp";
-   if (wf_register_control(clamp))
+   ret = wf_register_control(clamp);
+   if (ret)
goto fail;
clamp_control = clamp;
return 0;
  fail:
+   dev_pm_qos_remove_request(_req);
+
+ free:
kfree(clamp);
-   return -ENODEV;
+   return ret;
 }
 
 static void __exit wf_cpufreq_clamp_exit(void)
 {
-   if (clamp_control)
+   if (clamp_control) {
wf_unregister_control(clamp_control);
+   dev_pm_qos_remove_request(_req);
+   }
 }
 
 
-- 
2.21.0.rc0.269.g1a574e7a288b

[PATCH 00/10] cpufreq: Migrate users of policy notifiers to QoS requests

[Viresh Kumar]

Hello,

Now that cpufreq core supports taking QoS requests for min/max cpu
frequencies, lets migrate rest of the users to using them instead of the
policy notifiers.

The CPUFREQ_NOTIFY and CPUFREQ_ADJUST events of the policy notifiers are
removed as a result, but we have to add CPUFREQ_CREATE_POLICY and
CPUFREQ_REMOVE_POLICY events to it for the acpi stuff specifically. So
the policy notifiers aren't completely removed.

Boot tested on my x86 PC and ARM hikey board. Nothing looked broken :)

This has already gone through build bot for a few days now.

--
viresh

Viresh Kumar (10):
  cpufreq: Add policy create/remove notifiers
  video: sa1100fb: Remove cpufreq policy notifier
  video: pxafb: Remove cpufreq policy notifier
  arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY
  thermal: cpu_cooling: Switch to QoS requests instead of cpufreq
notifier
  powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier
  cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq
notifier
  ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
  cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier
events
  Documentation: cpufreq: Update policy notifier documentation

 Documentation/cpu-freq/core.txt|  16 +--
 drivers/acpi/processor_driver.c|  44 -
 drivers/acpi/processor_perflib.c   | 106 +---
 drivers/acpi/processor_thermal.c   |  81 ---
 drivers/base/arch_topology.c   |   2 +-
 drivers/cpufreq/cpufreq.c  |  51 --
 drivers/cpufreq/ppc_cbe_cpufreq.c  |  19 +++-
 drivers/cpufreq/ppc_cbe_cpufreq.h  |   8 ++
 drivers/cpufreq/ppc_cbe_cpufreq_pmi.c  |  96 +++---
 drivers/macintosh/windfarm_cpufreq_clamp.c |  77 ++-
 drivers/thermal/cpu_cooling.c  | 110 +
 drivers/video/fbdev/pxafb.c|  21 
 drivers/video/fbdev/pxafb.h|   1 -
 drivers/video/fbdev/sa1100fb.c |  27 -
 drivers/video/fbdev/sa1100fb.h |   1 -
 include/acpi/processor.h   |  22 +++--
 include/linux/cpufreq.h|   4 +-
 17 files changed, 327 insertions(+), 359 deletions(-)

-- 
2.21.0.rc0.269.g1a574e7a288b

Re: [PATCH v6] cpufreq/pasemi: fix an use-after-free inpas_cpufreq_cpu_init()

[Viresh Kumar]

On 16-07-19, 16:26, wen.yan...@zte.com.cn wrote:
> Okay thank you.
> Now this patch
> (https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.ku...@linaro.org/)
>  
> seems to have not been merged into the linux-next.
> 
> In order to avoid code conflicts, we will wait until this patch is merged in 
> and then send v7.

Please rebase on PM tree's linux-next branch instead and resend your
patch.

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

-- 
viresh

[PATCH V3] cpufreq: Make cpufreq_generic_init() return void

[Viresh Kumar]

It always returns 0 (success) and its return type should really be void.
Over that, many drivers have added error handling code based on its
return value, which is not required at all.

change its return type to void and update all the callers.

Signed-off-by: Viresh Kumar 
---
V2->V3:
- Update bmips cpufreq driver to avoid "warning: 'ret' may be used
  uninitialized".
- Build bot reported this issue almost after 4 days of posting this
  patch, I was expecting this a lot earlier :)

 drivers/cpufreq/bmips-cpufreq.c | 17 ++---
 drivers/cpufreq/cpufreq.c   |  4 +---
 drivers/cpufreq/davinci-cpufreq.c   |  3 ++-
 drivers/cpufreq/imx6q-cpufreq.c |  6 ++
 drivers/cpufreq/kirkwood-cpufreq.c  |  3 ++-
 drivers/cpufreq/loongson1-cpufreq.c |  8 +++-
 drivers/cpufreq/loongson2_cpufreq.c |  3 ++-
 drivers/cpufreq/maple-cpufreq.c |  3 ++-
 drivers/cpufreq/omap-cpufreq.c  | 15 +--
 drivers/cpufreq/pasemi-cpufreq.c|  3 ++-
 drivers/cpufreq/pmac32-cpufreq.c|  3 ++-
 drivers/cpufreq/pmac64-cpufreq.c|  3 ++-
 drivers/cpufreq/s3c2416-cpufreq.c   |  9 ++---
 drivers/cpufreq/s3c64xx-cpufreq.c   | 15 +++
 drivers/cpufreq/s5pv210-cpufreq.c   |  3 ++-
 drivers/cpufreq/sa1100-cpufreq.c|  3 ++-
 drivers/cpufreq/sa1110-cpufreq.c|  3 ++-
 drivers/cpufreq/spear-cpufreq.c |  3 ++-
 drivers/cpufreq/tegra20-cpufreq.c   |  8 +---
 include/linux/cpufreq.h |  2 +-
 20 files changed, 46 insertions(+), 71 deletions(-)

diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c
index 56a4ebbf00e0..f7c23fa468f0 100644
--- a/drivers/cpufreq/bmips-cpufreq.c
+++ b/drivers/cpufreq/bmips-cpufreq.c
@@ -131,23 +131,18 @@ static int bmips_cpufreq_exit(struct cpufreq_policy 
*policy)
 static int bmips_cpufreq_init(struct cpufreq_policy *policy)
 {
struct cpufreq_frequency_table *freq_table;
-   int ret;
 
freq_table = bmips_cpufreq_get_freq_table(policy);
if (IS_ERR(freq_table)) {
-   ret = PTR_ERR(freq_table);
-   pr_err("%s: couldn't determine frequency table (%d).\n",
-   BMIPS_CPUFREQ_NAME, ret);
-   return ret;
+   pr_err("%s: couldn't determine frequency table (%ld).\n",
+   BMIPS_CPUFREQ_NAME, PTR_ERR(freq_table));
+   return PTR_ERR(freq_table);
}
 
-   ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
-   if (ret)
-   bmips_cpufreq_exit(policy);
-   else
-   pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
+   cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
+   pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
 
-   return ret;
+   return 0;
 }
 
 static struct cpufreq_driver bmips_cpufreq_driver = {
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 4d6043ee7834..8dda62367816 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -159,7 +159,7 @@ EXPORT_SYMBOL_GPL(arch_set_freq_scale);
  * - set policies transition latency
  * - policy->cpus with all possible CPUs
  */
-int cpufreq_generic_init(struct cpufreq_policy *policy,
+void cpufreq_generic_init(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table,
unsigned int transition_latency)
 {
@@ -171,8 +171,6 @@ int cpufreq_generic_init(struct cpufreq_policy *policy,
 * share the clock and voltage and clock.
 */
cpumask_setall(policy->cpus);
-
-   return 0;
 }
 EXPORT_SYMBOL_GPL(cpufreq_generic_init);
 
diff --git a/drivers/cpufreq/davinci-cpufreq.c 
b/drivers/cpufreq/davinci-cpufreq.c
index 3de48ae60c29..297d23cad8b5 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -90,7 +90,8 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 * Setting the latency to 2000 us to accommodate addition of drivers
 * to pre/post change notification list.
 */
-   return cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+   cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+   return 0;
 }
 
 static struct cpufreq_driver davinci_driver = {
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 47ccfa6b17b7..648a09a1778a 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -190,14 +190,12 @@ static int imx6q_set_target(struct cpufreq_policy 
*policy, unsigned int index)
 
 static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
 {
-   int ret;
-
policy->clk = clks[ARM].clk;
-   ret = cpufreq_generic_init(policy, freq_table, transition_latency);
+   cpufreq_generic_init(policy, freq_table, transition_latency);
policy->suspend_freq = max_freq;
dev_pm_opp_of_register_em(policy->cpus);
 
-   

[PATCH V2] cpufreq: Make cpufreq_generic_init() return void

[Viresh Kumar]

It always returns 0 (success) and its return type should really be void.
Over that, many drivers have added error handling code based on its
return value, which is not required at all.

change its return type to void and update all the callers.

Signed-off-by: Viresh Kumar 
---
V1->V2:
- Fixed compilation warning with s3c64xx driver by replace %d with %ld
  (Reported by buildbot).

 drivers/cpufreq/bmips-cpufreq.c |  7 ++-
 drivers/cpufreq/cpufreq.c   |  4 +---
 drivers/cpufreq/davinci-cpufreq.c   |  3 ++-
 drivers/cpufreq/imx6q-cpufreq.c |  6 ++
 drivers/cpufreq/kirkwood-cpufreq.c  |  3 ++-
 drivers/cpufreq/loongson1-cpufreq.c |  8 +++-
 drivers/cpufreq/loongson2_cpufreq.c |  3 ++-
 drivers/cpufreq/maple-cpufreq.c |  3 ++-
 drivers/cpufreq/omap-cpufreq.c  | 15 +--
 drivers/cpufreq/pasemi-cpufreq.c|  3 ++-
 drivers/cpufreq/pmac32-cpufreq.c|  3 ++-
 drivers/cpufreq/pmac64-cpufreq.c|  3 ++-
 drivers/cpufreq/s3c2416-cpufreq.c   |  9 ++---
 drivers/cpufreq/s3c64xx-cpufreq.c   | 15 +++
 drivers/cpufreq/s5pv210-cpufreq.c   |  3 ++-
 drivers/cpufreq/sa1100-cpufreq.c|  3 ++-
 drivers/cpufreq/sa1110-cpufreq.c|  3 ++-
 drivers/cpufreq/spear-cpufreq.c |  3 ++-
 drivers/cpufreq/tegra20-cpufreq.c   |  8 +---
 include/linux/cpufreq.h |  2 +-
 20 files changed, 42 insertions(+), 65 deletions(-)

diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c
index 56a4ebbf00e0..2b187d802fe3 100644
--- a/drivers/cpufreq/bmips-cpufreq.c
+++ b/drivers/cpufreq/bmips-cpufreq.c
@@ -141,11 +141,8 @@ static int bmips_cpufreq_init(struct cpufreq_policy 
*policy)
return ret;
}
 
-   ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
-   if (ret)
-   bmips_cpufreq_exit(policy);
-   else
-   pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
+   cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
+   pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
 
return ret;
 }
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 4d6043ee7834..8dda62367816 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -159,7 +159,7 @@ EXPORT_SYMBOL_GPL(arch_set_freq_scale);
  * - set policies transition latency
  * - policy->cpus with all possible CPUs
  */
-int cpufreq_generic_init(struct cpufreq_policy *policy,
+void cpufreq_generic_init(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table,
unsigned int transition_latency)
 {
@@ -171,8 +171,6 @@ int cpufreq_generic_init(struct cpufreq_policy *policy,
 * share the clock and voltage and clock.
 */
cpumask_setall(policy->cpus);
-
-   return 0;
 }
 EXPORT_SYMBOL_GPL(cpufreq_generic_init);
 
diff --git a/drivers/cpufreq/davinci-cpufreq.c 
b/drivers/cpufreq/davinci-cpufreq.c
index 3de48ae60c29..297d23cad8b5 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -90,7 +90,8 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 * Setting the latency to 2000 us to accommodate addition of drivers
 * to pre/post change notification list.
 */
-   return cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+   cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+   return 0;
 }
 
 static struct cpufreq_driver davinci_driver = {
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 47ccfa6b17b7..648a09a1778a 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -190,14 +190,12 @@ static int imx6q_set_target(struct cpufreq_policy 
*policy, unsigned int index)
 
 static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
 {
-   int ret;
-
policy->clk = clks[ARM].clk;
-   ret = cpufreq_generic_init(policy, freq_table, transition_latency);
+   cpufreq_generic_init(policy, freq_table, transition_latency);
policy->suspend_freq = max_freq;
dev_pm_opp_of_register_em(policy->cpus);
 
-   return ret;
+   return 0;
 }
 
 static struct cpufreq_driver imx6q_cpufreq_driver = {
diff --git a/drivers/cpufreq/kirkwood-cpufreq.c 
b/drivers/cpufreq/kirkwood-cpufreq.c
index 7ab564c1f7ae..cb74bdc5baaa 100644
--- a/drivers/cpufreq/kirkwood-cpufreq.c
+++ b/drivers/cpufreq/kirkwood-cpufreq.c
@@ -85,7 +85,8 @@ static int kirkwood_cpufreq_target(struct cpufreq_policy 
*policy,
 /* Module init and exit code */
 static int kirkwood_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
-   return cpufreq_generic_init(policy, kirkwood_freq_table, 5000);
+   cpufreq_generic_init(policy, kirkwood_freq_table, 5000);
+   return 0;
 }
 
 static struct cpufreq_driver kirkwood_cpufreq_driver = {
diff --git a/drivers/cpufreq/loongson1-cpufreq.c 
b/drivers/cpufreq/loongson1-cpufreq.c
i

Re: [PATCH v6] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 12-07-19, 10:44, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f6 ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: Michael Ellerman 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v6: keep the blank line and fix warning: label 'out_unmap_sdcpwr' defined but 
> not used.
> v5: put together the code to get, use, and release cpu device_node.
> v4: restore the blank line.
> v3: fix a leaked reference.
> v2: clean up the code according to the advice of viresh.
> 
>  drivers/cpufreq/pasemi-cpufreq.c | 26 ++
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..7d557f9 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -131,10 +131,18 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   int err = -ENODEV;
>  
>   cpu = of_get_cpu_node(policy->cpu, NULL);
> + if (!cpu)
> + goto out;
>  
> + max_freqp = of_get_property(cpu, "clock-frequency", NULL);
>   of_node_put(cpu);
> - if (!cpu)
> + if (!max_freqp) {
> + err = -EINVAL;
>   goto out;
> + }
> +
> + /* we need the freq in kHz */
> + max_freq = *max_freqp / 1000;
>  
>   dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
>   if (!dn)
> @@ -171,16 +179,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   }
>  
>   pr_debug("init cpufreq on CPU %d\n", policy->cpu);
> -
> - max_freqp = of_get_property(cpu, "clock-frequency", NULL);
> - if (!max_freqp) {
> - err = -EINVAL;
> - goto out_unmap_sdcpwr;
> - }
> -
> - /* we need the freq in kHz */
> - max_freq = *max_freqp / 1000;
> -
>   pr_debug("max clock-frequency is at %u kHz\n", max_freq);
>   pr_debug("initializing frequency table\n");
>  
> @@ -196,7 +194,11 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   policy->cur = pas_freqs[cur_astate].frequency;
>   ppc_proc_freq = policy->cur * 1000ul;
>  
> - return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
> + err = cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());

So you are trying to fix an earlier issue here with this. Should have
been a separate patch. Over that I have just sent a patch now to make
this routine return void.

https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.ku...@linaro.org/

So all you need to do is to remove the label out_unmap_sdcpwr instead.

> + if (err)
> + goto out_unmap_sdcpwr;
> +
> + return 0;
>  
>  out_unmap_sdcpwr:
>   iounmap(sdcpwr_mapbase);
> -- 
> 2.9.5

-- 
viresh

[PATCH] cpufreq: Make cpufreq_generic_init() return void

[Viresh Kumar]

It always returns 0 (success) and its return type should really be void.
Over that, many drivers have added error handling code based on its
return value, which is not required at all.

change its return type to void and update all the callers.

Signed-off-by: Viresh Kumar 
---
 drivers/cpufreq/bmips-cpufreq.c |  7 ++-
 drivers/cpufreq/cpufreq.c   |  4 +---
 drivers/cpufreq/davinci-cpufreq.c   |  3 ++-
 drivers/cpufreq/imx6q-cpufreq.c |  6 ++
 drivers/cpufreq/kirkwood-cpufreq.c  |  3 ++-
 drivers/cpufreq/loongson1-cpufreq.c |  8 +++-
 drivers/cpufreq/loongson2_cpufreq.c |  3 ++-
 drivers/cpufreq/maple-cpufreq.c |  3 ++-
 drivers/cpufreq/omap-cpufreq.c  | 15 +--
 drivers/cpufreq/pasemi-cpufreq.c|  3 ++-
 drivers/cpufreq/pmac32-cpufreq.c|  3 ++-
 drivers/cpufreq/pmac64-cpufreq.c|  3 ++-
 drivers/cpufreq/s3c2416-cpufreq.c   |  9 ++---
 drivers/cpufreq/s3c64xx-cpufreq.c   | 15 +++
 drivers/cpufreq/s5pv210-cpufreq.c   |  3 ++-
 drivers/cpufreq/sa1100-cpufreq.c|  3 ++-
 drivers/cpufreq/sa1110-cpufreq.c|  3 ++-
 drivers/cpufreq/spear-cpufreq.c |  3 ++-
 drivers/cpufreq/tegra20-cpufreq.c   |  8 +---
 include/linux/cpufreq.h |  2 +-
 20 files changed, 42 insertions(+), 65 deletions(-)

diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c
index 56a4ebbf00e0..2b187d802fe3 100644
--- a/drivers/cpufreq/bmips-cpufreq.c
+++ b/drivers/cpufreq/bmips-cpufreq.c
@@ -141,11 +141,8 @@ static int bmips_cpufreq_init(struct cpufreq_policy 
*policy)
return ret;
}
 
-   ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
-   if (ret)
-   bmips_cpufreq_exit(policy);
-   else
-   pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
+   cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
+   pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
 
return ret;
 }
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 4d6043ee7834..8dda62367816 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -159,7 +159,7 @@ EXPORT_SYMBOL_GPL(arch_set_freq_scale);
  * - set policies transition latency
  * - policy->cpus with all possible CPUs
  */
-int cpufreq_generic_init(struct cpufreq_policy *policy,
+void cpufreq_generic_init(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table,
unsigned int transition_latency)
 {
@@ -171,8 +171,6 @@ int cpufreq_generic_init(struct cpufreq_policy *policy,
 * share the clock and voltage and clock.
 */
cpumask_setall(policy->cpus);
-
-   return 0;
 }
 EXPORT_SYMBOL_GPL(cpufreq_generic_init);
 
diff --git a/drivers/cpufreq/davinci-cpufreq.c 
b/drivers/cpufreq/davinci-cpufreq.c
index 3de48ae60c29..297d23cad8b5 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -90,7 +90,8 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 * Setting the latency to 2000 us to accommodate addition of drivers
 * to pre/post change notification list.
 */
-   return cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+   cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+   return 0;
 }
 
 static struct cpufreq_driver davinci_driver = {
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 47ccfa6b17b7..648a09a1778a 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -190,14 +190,12 @@ static int imx6q_set_target(struct cpufreq_policy 
*policy, unsigned int index)
 
 static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
 {
-   int ret;
-
policy->clk = clks[ARM].clk;
-   ret = cpufreq_generic_init(policy, freq_table, transition_latency);
+   cpufreq_generic_init(policy, freq_table, transition_latency);
policy->suspend_freq = max_freq;
dev_pm_opp_of_register_em(policy->cpus);
 
-   return ret;
+   return 0;
 }
 
 static struct cpufreq_driver imx6q_cpufreq_driver = {
diff --git a/drivers/cpufreq/kirkwood-cpufreq.c 
b/drivers/cpufreq/kirkwood-cpufreq.c
index 7ab564c1f7ae..cb74bdc5baaa 100644
--- a/drivers/cpufreq/kirkwood-cpufreq.c
+++ b/drivers/cpufreq/kirkwood-cpufreq.c
@@ -85,7 +85,8 @@ static int kirkwood_cpufreq_target(struct cpufreq_policy 
*policy,
 /* Module init and exit code */
 static int kirkwood_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
-   return cpufreq_generic_init(policy, kirkwood_freq_table, 5000);
+   cpufreq_generic_init(policy, kirkwood_freq_table, 5000);
+   return 0;
 }
 
 static struct cpufreq_driver kirkwood_cpufreq_driver = {
diff --git a/drivers/cpufreq/loongson1-cpufreq.c 
b/drivers/cpufreq/loongson1-cpufreq.c
index 21c9ce8526c0..0ea88778882a 100644
--- a/drivers/cpufreq/loongson1-cpufreq.c
+++ b/drivers/cpufreq/loongson1

Re: [PATCH v5] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 09-07-19, 16:04, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: Michael Ellerman 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v5: put together the code to get, use, and release cpu device_node.
> v4: restore the blank line.
> v3: fix a leaked reference.
> v2: clean up the code according to the advice of viresh.
> 
>  drivers/cpufreq/pasemi-cpufreq.c | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..1f0beb7 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -131,10 +131,17 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   int err = -ENODEV;
>  
>   cpu = of_get_cpu_node(policy->cpu, NULL);
> -
> - of_node_put(cpu);
>   if (!cpu)
>   goto out;

I would have loved a blank line here :)

> + max_freqp = of_get_property(cpu, "clock-frequency", NULL);
> + of_node_put(cpu);
> + if (!max_freqp) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + /* we need the freq in kHz */
> + max_freq = *max_freqp / 1000;
>  
>   dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
>   if (!dn)
> @@ -171,16 +178,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   }
>  
>   pr_debug("init cpufreq on CPU %d\n", policy->cpu);
> -
> - max_freqp = of_get_property(cpu, "clock-frequency", NULL);
> - if (!max_freqp) {
> - err = -EINVAL;
> - goto out_unmap_sdcpwr;
> - }
> -
> - /* we need the freq in kHz */
> - max_freq = *max_freqp / 1000;
> -
>   pr_debug("max clock-frequency is at %u kHz\n", max_freq);
>   pr_debug("initializing frequency table\n");

Though, enough versions have happened now.

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v4] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 09-07-19, 07:39, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v4: restore the blank line.

I don't find it restored in the below code.

> v3: fix a leaked reference.
> v2: clean up the code according to the advice of viresh.
> 
>  drivers/cpufreq/pasemi-cpufreq.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..f0c98fc 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -128,20 +128,21 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   int cur_astate, idx;
>   struct resource res;
>   struct device_node *cpu, *dn;
> - int err = -ENODEV;
> + int err;
>  
>   cpu = of_get_cpu_node(policy->cpu, NULL);
>  
> - of_node_put(cpu);
>   if (!cpu)
> - goto out;
> + return -ENODEV;
>  
>   dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
>   if (!dn)
>   dn = of_find_compatible_node(NULL, NULL,
>"pasemi,pwrficient-sdc");
> - if (!dn)
> + if (!dn) {
> + err = -ENODEV;
>   goto out;
> + }
>   err = of_address_to_resource(dn, 0, );
>   of_node_put(dn);
>   if (err)
> @@ -196,6 +197,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   policy->cur = pas_freqs[cur_astate].frequency;
>   ppc_proc_freq = policy->cur * 1000ul;
>  
> + of_node_put(cpu);
>   return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
>  
>  out_unmap_sdcpwr:
> @@ -204,6 +206,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>  out_unmap_sdcasr:
>   iounmap(sdcasr_mapbase);
>  out:
> + of_node_put(cpu);
>   return err;
>  }
>  
> -- 
> 2.9.5

-- 
viresh

Re: [PATCH v3] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 08-07-19, 16:48, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v3: fix a leaked reference.
> v2: clean up the code according to the advice of viresh.
> 
>  drivers/cpufreq/pasemi-cpufreq.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..9dc5163 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -128,20 +128,20 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   int cur_astate, idx;
>   struct resource res;
>   struct device_node *cpu, *dn;
> - int err = -ENODEV;
> + int err;
>  
>   cpu = of_get_cpu_node(policy->cpu, NULL);
> -
> - of_node_put(cpu);
>   if (!cpu)
> - goto out;
> + return -ENODEV;
>  
>   dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
>   if (!dn)
>   dn = of_find_compatible_node(NULL, NULL,
>"pasemi,pwrficient-sdc");
> - if (!dn)
> + if (!dn) {
> + err = -ENODEV;
>   goto out;
> + }

Please restore the blank line here.

>   err = of_address_to_resource(dn, 0, );
>   of_node_put(dn);
>   if (err)
> @@ -196,6 +196,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   policy->cur = pas_freqs[cur_astate].frequency;
>   ppc_proc_freq = policy->cur * 1000ul;
>  
> + of_node_put(cpu);
>   return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
>  
>  out_unmap_sdcpwr:
> @@ -204,6 +205,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>  out_unmap_sdcasr:
>   iounmap(sdcasr_mapbase);
>  out:
> + of_node_put(cpu);
>   return err;
>  }
>  
> -- 
> 2.9.5

-- 
viresh

Re: [PATCH v2] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 08-07-19, 15:19, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v2: clean up the code according to the advice of viresh.
> 
>  drivers/cpufreq/pasemi-cpufreq.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..c6d464b 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -128,20 +128,18 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   int cur_astate, idx;
>   struct resource res;
>   struct device_node *cpu, *dn;
> - int err = -ENODEV;
> + int err;
>  
>   cpu = of_get_cpu_node(policy->cpu, NULL);
> -
> - of_node_put(cpu);
>   if (!cpu)
> - goto out;
> + return -ENODEV;
>  


>   dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
>   if (!dn)
>   dn = of_find_compatible_node(NULL, NULL,
>"pasemi,pwrficient-sdc");
>   if (!dn)
> - goto out;
> + return -ENODEV;

This change looks incorrect. You still need to drop reference to cpu ?

>   err = of_address_to_resource(dn, 0, );
>   of_node_put(dn);
>   if (err)
> @@ -196,6 +194,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   policy->cur = pas_freqs[cur_astate].frequency;
>   ppc_proc_freq = policy->cur * 1000ul;
>  
> + of_node_put(cpu);
>   return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
>  
>  out_unmap_sdcpwr:
> @@ -204,6 +203,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>  out_unmap_sdcasr:
>   iounmap(sdcasr_mapbase);
>  out:
> + of_node_put(cpu);
>   return err;
>  }
>  
> -- 
> 2.9.5

-- 
viresh

Re: [PATCH] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 08-07-19, 14:19, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/cpufreq/pasemi-cpufreq.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

I will suggest some changes here.

> diff --git a/drivers/cpufreq/pasemi-cpufreq.c 
> b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..d2dd47b 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -132,7 +132,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)

Don't initialize "err" anymore.

>   cpu = of_get_cpu_node(policy->cpu, NULL);
>  
> - of_node_put(cpu);
>   if (!cpu)
>   goto out;

Do return -ENODEV; here.

>  
> @@ -141,15 +140,15 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   dn = of_find_compatible_node(NULL, NULL,
>"pasemi,pwrficient-sdc");
>   if (!dn)
> - goto out;
> + goto out_put_cpu_node;
>   err = of_address_to_resource(dn, 0, );
>   of_node_put(dn);
>   if (err)
> - goto out;
> + goto out_put_cpu_node;
>   sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
>   if (!sdcasr_mapbase) {
>   err = -EINVAL;
> - goto out;
> + goto out_put_cpu_node;
>   }

Don't do above changes.

>  
>   dn = of_find_compatible_node(NULL, NULL, "1682m-gizmo");
> @@ -177,6 +176,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>   err = -EINVAL;
>   goto out_unmap_sdcpwr;
>   }
> + of_node_put(cpu);
>  
>   /* we need the freq in kHz */
>   max_freq = *max_freqp / 1000;
> @@ -203,6 +203,8 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy 
> *policy)
>  
>  out_unmap_sdcasr:
>   iounmap(sdcasr_mapbase);
> +out_put_cpu_node:

Don't add this label, instead use "out" for also having the below
code.

> + of_node_put(cpu);
>  out:
>   return err;
>  }
> -- 
> 2.9.5

-- 
viresh

Re: [PATCH] cpufreq: powernv: fix missing check of return value in init_powernv_pstates()

[Viresh Kumar]

On 16-02-19, 12:06, Yangtao Li wrote:
> kmalloc() could fail, so insert a check of its return value. And
> if it fails, returns -ENOMEM.
> 
> And remove (struct pstate_idx_revmap_data *) to fix coccinelle WARNING
> by the way.
> 
> WARNING: casting value returned by memory allocation function to (struct
> pstate_idx_revmap_data *) is useless.
> 
> Signed-off-by: Yangtao Li 
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 7e7ad3879c4e..d2230812fa4b 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -244,6 +244,7 @@ static int init_powernv_pstates(void)
>   u32 len_ids, len_freqs;
>   u32 pstate_min, pstate_max, pstate_nominal;
>   u32 pstate_turbo, pstate_ultra_turbo;
> + int rc = -ENODEV;
>  
>   power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
>   if (!power_mgt) {
> @@ -327,8 +328,11 @@ static int init_powernv_pstates(void)
>   powernv_freqs[i].frequency = freq * 1000; /* kHz */
>   powernv_freqs[i].driver_data = id & 0xFF;
>  
> - revmap_data = (struct pstate_idx_revmap_data *)
> -   kmalloc(sizeof(*revmap_data), GFP_KERNEL);
> + revmap_data = kmalloc(sizeof(*revmap_data), GFP_KERNEL);
> + if (!revmap_data) {
> + rc = -ENOMEM;
> + goto out;
> + }
>  
>   revmap_data->pstate_id = id & 0xFF;
>   revmap_data->cpufreq_table_idx = i;
> @@ -357,7 +361,7 @@ static int init_powernv_pstates(void)
>   return 0;
>  out:
>   of_node_put(power_mgt);
> - return -ENODEV;
> + return rc;
>  }
>  
>  /* Returns the CPU frequency corresponding to the pstate_id. */

Applied. Thanks.

-- 
viresh

Re: [PATCH v2 28/34] dt-bindings: arm: Convert SPEAr board/soc bindings to json-schema

[Viresh Kumar]

On 03-12-18, 15:32, Rob Herring wrote:
> Convert SPEAr SoC bindings to DT schema format using json-schema.
> 
> Cc: Viresh Kumar 
> Cc: Shiraz Hashim 
> Cc: Mark Rutland 
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Rob Herring 
> ---
>  .../devicetree/bindings/arm/spear.txt | 26 ---
>  .../devicetree/bindings/arm/spear.yaml| 25 ++
>  2 files changed, 25 insertions(+), 26 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/spear.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/spear.yaml

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v2] cpufreq: pmac64: add of_node_put()

[Viresh Kumar]

On 23-11-18, 08:33, Yangtao Li wrote:
> of_find_node_by_path() acquires a reference to the node
> returned by it and that reference needs to be dropped by its caller.
> g5_neo2_cpufreq_init() doesn't do that, so fix it.
> 
> Signed-off-by: Yangtao Li 
> Acked-by: Viresh Kumar 
> ---
> Changes in v2:
> -update changelog
> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/pmac64-cpufreq.c 
> b/drivers/cpufreq/pmac64-cpufreq.c
> index be623dd7b9f2..1d32a863332d 100644
> --- a/drivers/cpufreq/pmac64-cpufreq.c
> +++ b/drivers/cpufreq/pmac64-cpufreq.c
> @@ -411,6 +411,7 @@ static int __init g5_neo2_cpufreq_init(struct device_node 
> *cpunode)
>   pfunc_set_vdnap0 = pmf_find_function(root, "set-vdnap0");
>   pfunc_vdnap0_complete =
>   pmf_find_function(root, "slewing-done");
> + of_node_put(root);
>   if (pfunc_set_vdnap0 == NULL ||
>   pfunc_vdnap0_complete == NULL) {
>   pr_err("Can't find required platform function\n");

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v2] cpufreq: powernv: add of_node_put()

[Viresh Kumar]

On 20-11-18, 11:05, Yangtao Li wrote:
> The of_find_node_by_path() returns a node pointer with refcount
> incremented,but there is the lack of use of the of_node_put() when
> done.Add the missing of_node_put() to release the refcount.
> 
> Signed-off-by: Yangtao Li 
> ---
> Changes in v2
> -update changelog
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH] cpufreq: pmac64: add of_node_put()

[Viresh Kumar]

On 20-11-18, 07:54, Yangtao Li wrote:
> use of_node_put to release the refcount of node.
> 
> Signed-off-by: Yangtao Li 
> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/pmac64-cpufreq.c 
> b/drivers/cpufreq/pmac64-cpufreq.c
> index be623dd7b9f2..1d32a863332d 100644
> --- a/drivers/cpufreq/pmac64-cpufreq.c
> +++ b/drivers/cpufreq/pmac64-cpufreq.c
> @@ -411,6 +411,7 @@ static int __init g5_neo2_cpufreq_init(struct device_node 
> *cpunode)
>   pfunc_set_vdnap0 = pmf_find_function(root, "set-vdnap0");
>   pfunc_vdnap0_complete =
>   pmf_find_function(root, "slewing-done");
> + of_node_put(root);
>   if (pfunc_set_vdnap0 == NULL ||
>   pfunc_vdnap0_complete == NULL) {
>       pr_err("Can't find required platform function\n");

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 1/7] drivers/cpufreq: change CONFIG_6xx to CONFIG_PPC_BOOK3S_32

[Viresh Kumar]

On 17-11-18, 10:24, Christophe Leroy wrote:
> Today, powerpc has three CONFIG labels which means exactly the same:
> - CONFIG_6xx
> - CONFIG_PPC_BOOK3S_32
> - CONFIG_PPC_STD_MMU_32
> 
> By consistency with PPC64, CONFIG_PPC_BOOK3S_32 is the preferred one.
> Using a label with includes _PPC_ also makes it clearer that it is
> linked to powerpc.
> 
> In preparation of the removal of CONFIG_6xx, this patch replaces it
> by CONFIG_PPC_BOOK3S_32
> 
> Signed-off-by: Christophe Leroy 
> ---
>  drivers/cpufreq/pmac32-cpufreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/pmac32-cpufreq.c 
> b/drivers/cpufreq/pmac32-cpufreq.c
> index 61ae06ca008e..52f0d91d30c1 100644
> --- a/drivers/cpufreq/pmac32-cpufreq.c
> +++ b/drivers/cpufreq/pmac32-cpufreq.c
> @@ -128,7 +128,7 @@ static int cpu_750fx_cpu_speed(int low_speed)
>   mtspr(SPRN_HID2, hid2);
>   }
>   }
> -#ifdef CONFIG_6xx
> +#ifdef CONFIG_PPC_BOOK3S_32
>   low_choose_750fx_pll(low_speed);
>  #endif
>   if (low_speed == 1) {
> @@ -166,7 +166,7 @@ static int dfs_set_cpu_speed(int low_speed)
>   }
>  
>   /* set frequency */
> -#ifdef CONFIG_6xx
> +#ifdef CONFIG_PPC_BOOK3S_32
>   low_choose_7447a_dfs(low_speed);
>  #endif
>   udelay(100);

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v4 4/6] clk: qoriq: Add clockgen support for lx2160a

[Viresh Kumar]

On 04-10-18, 06:33, Vabhav Sharma wrote:
> diff --git a/drivers/cpufreq/qoriq-cpufreq.c b/drivers/cpufreq/qoriq-cpufreq.c
> index 3d773f6..83921b7 100644
> --- a/drivers/cpufreq/qoriq-cpufreq.c
> +++ b/drivers/cpufreq/qoriq-cpufreq.c
> @@ -295,6 +295,7 @@ static const struct of_device_id node_matches[] 
> __initconst = {
>   { .compatible = "fsl,ls1046a-clockgen", },
>   { .compatible = "fsl,ls1088a-clockgen", },
>   { .compatible = "fsl,ls2080a-clockgen", },
> + { .compatible = "fsl,lx2160a-clockgen", },
>   { .compatible = "fsl,p4080-clockgen", },
>   { .compatible = "fsl,qoriq-clockgen-1.0", },
>   { .compatible = "fsl,qoriq-clockgen-2.0", },

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH V3] cpufreq: powernv: Fix the hardlockup by synchronus smp_call in timer interrupt

[Viresh Kumar]

On 25-04-18, 16:29, Shilpasri G Bhat wrote:
> gpstate_timer_handler() uses synchronous smp_call to set the pstate
> on the requested core. This causes the below hard lockup:
> 
> [c03fe566b320] [c01d5340] smp_call_function_single+0x110/0x180 
> (unreliable)
> [c03fe566b390] [c01d55e0] smp_call_function_any+0x180/0x250
> [c03fe566b3f0] [c0acd3e8] gpstate_timer_handler+0x1e8/0x580
> [c03fe566b4a0] [c01b46b0] call_timer_fn+0x50/0x1c0
> [c03fe566b520] [c01b4958] expire_timers+0x138/0x1f0
> [c03fe566b590] [c01b4bf8] run_timer_softirq+0x1e8/0x270
> [c03fe566b630] [c0d0d6c8] __do_softirq+0x158/0x3e4
> [c03fe566b710] [c0114be8] irq_exit+0xe8/0x120
> [c03fe566b730] [c0024d0c] timer_interrupt+0x9c/0xe0
> [c03fe566b760] [c0009014] decrementer_common+0x114/0x120
> -- interrupt: 901 at doorbell_global_ipi+0x34/0x50
> LR = arch_send_call_function_ipi_mask+0x120/0x130
> [c03fe566ba50] [c004876c]
> arch_send_call_function_ipi_mask+0x4c/0x130
> [c03fe566ba90] [c01d59f0] smp_call_function_many+0x340/0x450
> [c03fe566bb00] [c0075f18] pmdp_invalidate+0x98/0xe0
> [c03fe566bb30] [c03a1120] change_huge_pmd+0xe0/0x270
> [c03fe566bba0] [c0349278] change_protection_range+0xb88/0xe40
> [c03fe566bcf0] [c03496c0] mprotect_fixup+0x140/0x340
> [c03fe566bdb0] [c0349a74] SyS_mprotect+0x1b4/0x350
> [c03fe566be30] [c000b184] system_call+0x58/0x6c
> 
> One way to avoid this is removing the smp-call. We can ensure that the timer
> always runs on one of the policy-cpus. If the timer gets migrated to a
> cpu outside the policy then re-queue it back on the policy->cpus. This way
> we can get rid of the smp-call which was being used to set the pstate
> on the policy->cpus.
> 
> Fixes: 7bc54b652f13 (timers, cpufreq/powernv: Initialize the gpstate timer as 
> pinned)
> Cc: <sta...@vger.kernel.org>[4.8+]
> Reported-by: Nicholas Piggin <npig...@gmail.com>
> Reported-by: Pridhiviraj Paidipeddi <ppaid...@linux.vnet.ibm.com>
> Signed-off-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> ---
> Changes from V2:
> - Remove the check for active policy while requeing the migrated timer
> Changes from V1:
> - Remove smp_call in the pstate handler.

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

Re: [PATCH V2] cpufreq: powernv: Fix the hardlockup by synchronus smp_call in timer interrupt

[Viresh Kumar]

On 25-04-18, 15:32, Shilpasri G Bhat wrote:
> Hi,
> 
> On 04/25/2018 02:47 PM, Viresh Kumar wrote:
> > On 25-04-18, 14:32, Shilpasri G Bhat wrote:
> >> gpstate_timer_handler() uses synchronous smp_call to set the pstate
> >> on the requested core. This causes the below hard lockup:
> >>
> >> [c03fe566b320] [c01d5340] smp_call_function_single+0x110/0x180 
> >> (unreliable)
> >> [c03fe566b390] [c01d55e0] smp_call_function_any+0x180/0x250
> >> [c03fe566b3f0] [c0acd3e8] gpstate_timer_handler+0x1e8/0x580
> >> [c03fe566b4a0] [c01b46b0] call_timer_fn+0x50/0x1c0
> >> [c03fe566b520] [c01b4958] expire_timers+0x138/0x1f0
> >> [c03fe566b590] [c01b4bf8] run_timer_softirq+0x1e8/0x270
> >> [c03fe566b630] [c0d0d6c8] __do_softirq+0x158/0x3e4
> >> [c03fe566b710] [c0114be8] irq_exit+0xe8/0x120
> >> [c03fe566b730] [c0024d0c] timer_interrupt+0x9c/0xe0
> >> [c03fe566b760] [c0009014] decrementer_common+0x114/0x120
> >> -- interrupt: 901 at doorbell_global_ipi+0x34/0x50
> >> LR = arch_send_call_function_ipi_mask+0x120/0x130
> >> [c03fe566ba50] [c004876c]
> >> arch_send_call_function_ipi_mask+0x4c/0x130
> >> [c03fe566ba90] [c01d59f0] smp_call_function_many+0x340/0x450
> >> [c03fe566bb00] [c0075f18] pmdp_invalidate+0x98/0xe0
> >> [c03fe566bb30] [c03a1120] change_huge_pmd+0xe0/0x270
> >> [c03fe566bba0] [c0349278] change_protection_range+0xb88/0xe40
> >> [c03fe566bcf0] [c03496c0] mprotect_fixup+0x140/0x340
> >> [c03fe566bdb0] [c0349a74] SyS_mprotect+0x1b4/0x350
> >> [c03fe566be30] [c000b184] system_call+0x58/0x6c
> >>
> >> One way to avoid this is removing the smp-call. We can ensure that the 
> >> timer
> >> always runs on one of the policy-cpus. If the timer gets migrated to a
> >> cpu outside the policy then re-queue it back on the policy->cpus. This way
> >> we can get rid of the smp-call which was being used to set the pstate
> >> on the policy->cpus.
> >>
> >> Fixes: 7bc54b652f13 (timers, cpufreq/powernv: Initialize the gpstate timer 
> >> as pinned)
> >> Cc: <sta...@vger.kernel.org>[4.8+]
> >> Reported-by: Nicholas Piggin <npig...@gmail.com>
> >> Reported-by: Pridhiviraj Paidipeddi <ppaid...@linux.vnet.ibm.com>
> >> Signed-off-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> >> ---
> >> Changes from V1:
> >> - Remove smp_call in the pstate handler.
> >>
> >>  drivers/cpufreq/powernv-cpufreq.c | 23 ---
> >>  1 file changed, 20 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> >> b/drivers/cpufreq/powernv-cpufreq.c
> >> index 71f8682..dc8ffb5 100644
> >> --- a/drivers/cpufreq/powernv-cpufreq.c
> >> +++ b/drivers/cpufreq/powernv-cpufreq.c
> >> @@ -679,6 +679,25 @@ void gpstate_timer_handler(struct timer_list *t)
> >>  
> >>if (!spin_trylock(>gpstate_lock))
> >>return;
> >> +  /*
> >> +   * If the timer has migrated to the different cpu then bring
> >> +   * it back to one of the policy->cpus
> >> +   */
> >> +  if (!cpumask_test_cpu(raw_smp_processor_id(), policy->cpus)) {
> >> +  /*
> >> +   * Timer should be deleted if policy is inactive.
> >> +   * If policy is active then re-queue on one of the
> >> +   * policy->cpus.
> >> +   */
> > 
> > This looks racy. Shouldn't you guarantee that the timer is already
> > removed in a synchronous way before de-activating the policy ?
> > 
> 
> The timer is deleted in driver->stop_cpu(). So we ensure to remove the timer
> before de-activating the policy.
> 
> 
> >> +  if (!cpumask_empty(policy->cpus)) {
> 
> So are you suggesting to remove ^^ the check for active policy here?
> (I put that as a safety check.)

Either you are sure or you are not, and you don't need a safety check
if you are sure :)

-- 
viresh

Re: [PATCH V2] cpufreq: powernv: Fix the hardlockup by synchronus smp_call in timer interrupt

[Viresh Kumar]

On 25-04-18, 14:32, Shilpasri G Bhat wrote:
> gpstate_timer_handler() uses synchronous smp_call to set the pstate
> on the requested core. This causes the below hard lockup:
> 
> [c03fe566b320] [c01d5340] smp_call_function_single+0x110/0x180 
> (unreliable)
> [c03fe566b390] [c01d55e0] smp_call_function_any+0x180/0x250
> [c03fe566b3f0] [c0acd3e8] gpstate_timer_handler+0x1e8/0x580
> [c03fe566b4a0] [c01b46b0] call_timer_fn+0x50/0x1c0
> [c03fe566b520] [c01b4958] expire_timers+0x138/0x1f0
> [c03fe566b590] [c01b4bf8] run_timer_softirq+0x1e8/0x270
> [c03fe566b630] [c0d0d6c8] __do_softirq+0x158/0x3e4
> [c03fe566b710] [c0114be8] irq_exit+0xe8/0x120
> [c03fe566b730] [c0024d0c] timer_interrupt+0x9c/0xe0
> [c03fe566b760] [c0009014] decrementer_common+0x114/0x120
> -- interrupt: 901 at doorbell_global_ipi+0x34/0x50
> LR = arch_send_call_function_ipi_mask+0x120/0x130
> [c03fe566ba50] [c004876c]
> arch_send_call_function_ipi_mask+0x4c/0x130
> [c03fe566ba90] [c01d59f0] smp_call_function_many+0x340/0x450
> [c03fe566bb00] [c0075f18] pmdp_invalidate+0x98/0xe0
> [c03fe566bb30] [c03a1120] change_huge_pmd+0xe0/0x270
> [c03fe566bba0] [c0349278] change_protection_range+0xb88/0xe40
> [c03fe566bcf0] [c03496c0] mprotect_fixup+0x140/0x340
> [c03fe566bdb0] [c0349a74] SyS_mprotect+0x1b4/0x350
> [c03fe566be30] [c000b184] system_call+0x58/0x6c
> 
> One way to avoid this is removing the smp-call. We can ensure that the timer
> always runs on one of the policy-cpus. If the timer gets migrated to a
> cpu outside the policy then re-queue it back on the policy->cpus. This way
> we can get rid of the smp-call which was being used to set the pstate
> on the policy->cpus.
> 
> Fixes: 7bc54b652f13 (timers, cpufreq/powernv: Initialize the gpstate timer as 
> pinned)
> Cc: [4.8+]
> Reported-by: Nicholas Piggin 
> Reported-by: Pridhiviraj Paidipeddi 
> Signed-off-by: Shilpasri G Bhat 
> ---
> Changes from V1:
> - Remove smp_call in the pstate handler.
> 
>  drivers/cpufreq/powernv-cpufreq.c | 23 ---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 71f8682..dc8ffb5 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -679,6 +679,25 @@ void gpstate_timer_handler(struct timer_list *t)
>  
>   if (!spin_trylock(>gpstate_lock))
>   return;
> + /*
> +  * If the timer has migrated to the different cpu then bring
> +  * it back to one of the policy->cpus
> +  */
> + if (!cpumask_test_cpu(raw_smp_processor_id(), policy->cpus)) {
> + /*
> +  * Timer should be deleted if policy is inactive.
> +  * If policy is active then re-queue on one of the
> +  * policy->cpus.
> +  */

This looks racy. Shouldn't you guarantee that the timer is already
removed in a synchronous way before de-activating the policy ?

> + if (!cpumask_empty(policy->cpus)) {
> + gpstates->timer.expires = jiffies +
> + msecs_to_jiffies(1);
> + add_timer_on(>timer,
> + cpumask_first(policy->cpus));
> + }
> + spin_unlock(>gpstate_lock);
> + return;
> + }
>  
>   /*
>* If PMCR was last updated was using fast_swtich then
> @@ -718,10 +737,8 @@ void gpstate_timer_handler(struct timer_list *t)
>   if (gpstate_idx != gpstates->last_lpstate_idx)
>   queue_gpstate_timer(gpstates);
>  
> + set_pstate(_data);
>   spin_unlock(>gpstate_lock);
> -
> - /* Timer may get migrated to a different cpu on cpu hot unplug */
> - smp_call_function_any(policy->cpus, set_pstate, _data, 1);
>  }
>  
>  /*
> -- 
> 1.8.3.1

-- 
viresh

Re: [PATCH] cpufreq: powernv: Remove global pstate ramp-down timer in POWER9

[Viresh Kumar]

On 25-04-18, 11:44, Shilpasri G Bhat wrote:
> POWER9 doesnot support global pstate requests for the chip. So remove
> the timer logic which slowly ramps down the global pstate in P9
> platforms.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 23 ++-
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 0591874..71f8682 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -750,8 +750,13 @@ static int powernv_cpufreq_target_index(struct 
> cpufreq_policy *policy,
>  
>   cur_msec = jiffies_to_msecs(get_jiffies_64());
>  
> - spin_lock(>gpstate_lock);
>   freq_data.pstate_id = idx_to_pstate(new_index);
> + if (!gpstates) {
> + freq_data.gpstate_id = freq_data.pstate_id;
> + goto no_gpstate;
> + }
> +
> + spin_lock(>gpstate_lock);
>  
>   if (!gpstates->last_sampled_time) {
>   gpstate_idx = new_index;
> @@ -801,6 +806,7 @@ static int powernv_cpufreq_target_index(struct 
> cpufreq_policy *policy,
>  
>   spin_unlock(>gpstate_lock);
>  
> +no_gpstate:
>   /*
>* Use smp_call_function to send IPI and execute the
>* mtspr on target CPU.  We could do that without IPI
> @@ -835,6 +841,13 @@ static int powernv_cpufreq_cpu_init(struct 
> cpufreq_policy *policy)
>   kernfs_put(kn);
>   }
>  
> + policy->freq_table = powernv_freqs;
> + policy->fast_switch_possible = true;
> +
> + if (pvr_version_is(PVR_POWER9))
> + return 0;
> +
> + /* Initialise Gpstate ramp-down timer only on POWER8 */
>   gpstates =  kzalloc(sizeof(*gpstates), GFP_KERNEL);
>   if (!gpstates)
>   return -ENOMEM;
> @@ -849,15 +862,14 @@ static int powernv_cpufreq_cpu_init(struct 
> cpufreq_policy *policy)
>   msecs_to_jiffies(GPSTATE_TIMER_INTERVAL);
>   spin_lock_init(>gpstate_lock);
>  
> - policy->freq_table = powernv_freqs;
> - policy->fast_switch_possible = true;
>   return 0;
>  }
>  
>  static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>  {
>   /* timer is deleted in cpufreq_cpu_stop() */
> - kfree(policy->driver_data);
> + if (policy->driver_data)
> + kfree(policy->driver_data);
>  
>   return 0;
>  }
> @@ -990,7 +1002,8 @@ static void powernv_cpufreq_stop_cpu(struct 
> cpufreq_policy *policy)
>   freq_data.pstate_id = idx_to_pstate(powernv_pstate_info.min);
>   freq_data.gpstate_id = idx_to_pstate(powernv_pstate_info.min);
>   smp_call_function_single(policy->cpu, set_pstate, _data, 1);
> - del_timer_sync(>timer);
> + if (gpstates)
> + del_timer_sync(>timer);
>  }
>  
>  static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

Re: [PATCH 00/27] cpufreq: Stop validating cpufreq table in drivers

[Viresh Kumar]

On 26-02-18, 10:38, Viresh Kumar wrote:
> Hi,
> 
> A patchset [1] sent last week already updated the cpufreq core to start
> validating cpufreq table if the policy contains a valid
> "policy->freq_table" pointer.
> 
> This series updates all such drivers to stop validating the cpufreq
> table directly and let only the core handle it.
> 
> It depends on the previous series [1] and two cleanup patches [2][3] and
> is rebased over 4.16-rc3.
> 
> It is already tested by the 0-day testing infrastructure and no issues
> were reported.

We will assume no objections or concerns if you (cc'd here and on the individual
commits) don't respond and the series will then get merged into linux-next next
week.

Thanks.

--
viresh

[PATCH V2 15/27] cpufreq: powernv: Don't validate the frequency table twice

[Viresh Kumar]

The cpufreq core is already validating the CPU frequency table after
calling the ->init() callback of the cpufreq drivers and the drivers
don't need to do the same anymore. Though they need to set the
policy->freq_table field directly from the ->init() callback now.

Stop validating the frequency table from powernv driver.

Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
---
V1->V2:
- s/powerenv/powernv/

 drivers/cpufreq/powernv-cpufreq.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index 29cdec198657..0591874856d3 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -812,7 +812,7 @@ static int powernv_cpufreq_target_index(struct 
cpufreq_policy *policy,
 
 static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
-   int base, i, ret;
+   int base, i;
struct kernfs_node *kn;
struct global_pstate_info *gpstates;
 
@@ -848,15 +848,10 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy 
*policy)
gpstates->timer.expires = jiffies +
msecs_to_jiffies(GPSTATE_TIMER_INTERVAL);
spin_lock_init(>gpstate_lock);
-   ret = cpufreq_table_validate_and_show(policy, powernv_freqs);
-
-   if (ret < 0) {
-   kfree(policy->driver_data);
-   return ret;
-   }
 
+   policy->freq_table = powernv_freqs;
policy->fast_switch_possible = true;
-   return ret;
+   return 0;
 }
 
 static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
-- 
2.15.0.194.g9af6a3dea062

Re: [PATCH 15/27] cpufreq: powerenv: Don't validate the frequency table twice

[Viresh Kumar]

On 26-02-18, 22:53, Michael Ellerman wrote:
> Viresh Kumar <viresh.ku...@linaro.org> writes:
> > Subject: Re: [PATCH 15/27] cpufreq: powerenv: Don't validate the frequency 
> > table twice
>^
>   powernv
> 
> > The cpufreq core is already validating the CPU frequency table after
> > calling the ->init() callback of the cpufreq drivers and the drivers
> > don't need to do the same anymore. Though they need to set the
> > policy->freq_table field directly from the ->init() callback now.
> >
> > Stop validating the frequency table from powerenv driver.

 powernv :)

Will fix both of them.

-- 
viresh

[PATCH 15/27] cpufreq: powerenv: Don't validate the frequency table twice

[Viresh Kumar]

The cpufreq core is already validating the CPU frequency table after
calling the ->init() callback of the cpufreq drivers and the drivers
don't need to do the same anymore. Though they need to set the
policy->freq_table field directly from the ->init() callback now.

Stop validating the frequency table from powerenv driver.

Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
---
 drivers/cpufreq/powernv-cpufreq.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index 29cdec198657..0591874856d3 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -812,7 +812,7 @@ static int powernv_cpufreq_target_index(struct 
cpufreq_policy *policy,
 
 static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
-   int base, i, ret;
+   int base, i;
struct kernfs_node *kn;
struct global_pstate_info *gpstates;
 
@@ -848,15 +848,10 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy 
*policy)
gpstates->timer.expires = jiffies +
msecs_to_jiffies(GPSTATE_TIMER_INTERVAL);
spin_lock_init(>gpstate_lock);
-   ret = cpufreq_table_validate_and_show(policy, powernv_freqs);
-
-   if (ret < 0) {
-   kfree(policy->driver_data);
-   return ret;
-   }
 
+   policy->freq_table = powernv_freqs;
policy->fast_switch_possible = true;
-   return ret;
+   return 0;
 }
 
 static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
-- 
2.15.0.194.g9af6a3dea062

[PATCH 00/27] cpufreq: Stop validating cpufreq table in drivers

[Viresh Kumar]

Hi,

A patchset [1] sent last week already updated the cpufreq core to start
validating cpufreq table if the policy contains a valid
"policy->freq_table" pointer.

This series updates all such drivers to stop validating the cpufreq
table directly and let only the core handle it.

It depends on the previous series [1] and two cleanup patches [2][3] and
is rebased over 4.16-rc3.

It is already tested by the 0-day testing infrastructure and no issues
were reported.

--
viresh

[1] 
https://lkml.kernel.org/r/bd8c6133ad0bdd56c936802bcf26878d7cbdb679.1519279148.git.viresh.ku...@linaro.org
[2] 
https://lkml.kernel.org/r/77d470741dab32c2076a35253b9c0c2f0136583b.1519293292.git.viresh.ku...@linaro.org
[3] 
https://lkml.kernel.org/r/6b737a9c285840b4b2036fa51b692ee835664ec8.1519358505.git.viresh.ku...@linaro.org

Viresh Kumar (27):
  cpufreq: imx6q: Find max freq from frequency table itself
  cpufreq: Don't validate cpufreq table from cpufreq_generic_init()
  cpufreq: acpi: Don't validate the frequency table twice
  cpufreq: arm_big_little: Don't validate the frequency table twice
  cpufreq: blackfin: Don't validate the frequency table twice
  cpufreq: brcmstb: Don't validate the frequency table twice
  cpufreq: cpufreq-dt: Don't validate the frequency table twice
  cpufreq: e_powersaver: Don't validate the frequency table twice
  cpufreq: elanfreq: Don't validate the frequency table twice
  cpufreq: ia64-acpi: Don't validate the frequency table twice
  cpufreq: longhaul: Don't validate the frequency table twice
  cpufreq: mediatek: Don't validate the frequency table twice
  cpufreq: p4-clockmod: Don't validate the frequency table twice
  cpufreq: powernow: Don't validate the frequency table twice
  cpufreq: powerenv: Don't validate the frequency table twice
  cpufreq: ppc_cbe: Don't validate the frequency table twice
  cpufreq: pxa: Don't validate the frequency table twice
  cpufreq: qoirq: Don't validate the frequency table twice
  cpufreq: s3c24xx: Don't validate the frequency table twice
  cpufreq: sc520: Don't validate the frequency table twice
  cpufreq: scpi: Don't validate the frequency table twice
  cpufreq: sfi: Don't validate the frequency table twice
  cpufreq: sh: Don't validate the frequency table twice
  cpufreq: sparc: Don't validate the frequency table twice
  cpufreq: speedstep: Don't validate the frequency table twice
  cpufreq: tegra186: Don't validate the frequency table twice
  cpufreq: Drop cpufreq_table_validate_and_show()

 Documentation/cpu-freq/core.txt| 12 +---
 Documentation/cpu-freq/cpu-drivers.txt |  6 ++
 drivers/cpufreq/acpi-cpufreq.c | 20 +++-
 drivers/cpufreq/arm_big_little.c   |  9 +
 drivers/cpufreq/blackfin-cpufreq.c |  3 ++-
 drivers/cpufreq/brcmstb-avs-cpufreq.c  |  6 +-
 drivers/cpufreq/cpufreq-dt.c   |  8 +---
 drivers/cpufreq/cpufreq.c  |  9 +
 drivers/cpufreq/e_powersaver.c |  8 +---
 drivers/cpufreq/elanfreq.c |  3 ++-
 drivers/cpufreq/freq_table.c   | 14 --
 drivers/cpufreq/ia64-acpi-cpufreq.c|  7 +--
 drivers/cpufreq/imx6q-cpufreq.c|  7 ---
 drivers/cpufreq/longhaul.c |  3 ++-
 drivers/cpufreq/mediatek-cpufreq.c | 11 +--
 drivers/cpufreq/p4-clockmod.c  |  3 ++-
 drivers/cpufreq/powernow-k6.c  |  3 ++-
 drivers/cpufreq/powernow-k7.c  |  3 ++-
 drivers/cpufreq/powernow-k8.c  | 10 +-
 drivers/cpufreq/powernv-cpufreq.c  | 11 +++
 drivers/cpufreq/ppc_cbe_cpufreq.c  |  5 ++---
 drivers/cpufreq/pxa2xx-cpufreq.c   |  4 ++--
 drivers/cpufreq/pxa3xx-cpufreq.c   |  4 +++-
 drivers/cpufreq/qoriq-cpufreq.c| 13 ++---
 drivers/cpufreq/s3c24xx-cpufreq.c  |  5 +
 drivers/cpufreq/sc520_freq.c   |  3 ++-
 drivers/cpufreq/scpi-cpufreq.c | 10 +-
 drivers/cpufreq/sfi-cpufreq.c  |  3 ++-
 drivers/cpufreq/sh-cpufreq.c   | 22 --
 drivers/cpufreq/sparc-us2e-cpufreq.c   |  3 ++-
 drivers/cpufreq/sparc-us3-cpufreq.c|  3 ++-
 drivers/cpufreq/speedstep-centrino.c   |  4 ++--
 drivers/cpufreq/speedstep-ich.c|  4 +++-
 drivers/cpufreq/speedstep-smi.c|  4 +++-
 drivers/cpufreq/tegra186-cpufreq.c |  2 +-
 include/linux/cpufreq.h|  2 --
 36 files changed, 85 insertions(+), 162 deletions(-)

-- 
2.15.0.194.g9af6a3dea062

Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()

[Viresh Kumar]

On 21-02-18, 11:17, Rafael J. Wysocki wrote:
> The driver is expected to call cpufreq_table_validate_and_show() at
> ->init() time and fail ->init() if that fails.
> 
> That's kind of fragile, because it depends on the driver to do the right 
> thing.

That's exactly what I am trying to explore here, i.e. Call
validate/show from core instead of drivers.

-- 
viresh

Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()

[Viresh Kumar]

On 21-02-18, 10:27, Rafael J. Wysocki wrote:
> To be precise, ->init() should fail as that's where the table is
> created.  The registration fails as a result then.
> 
> But what if the bug is that ->init() doesn't fail when it should?
> 
> I guess the core could double check the frequency table after ->init()
> if ->target_index is not NULL.
> 
> The overall point here is that if you get a negative index in
> ->fast_switch(), that's way too late anyway and we should be able to
> catch that error much earlier.

I don't want to end up doing double checking as some of it is already
done at init, but let me check on what can be done.

-- 
viresh

Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()

[Viresh Kumar]

On 21-02-18, 16:39, Michael Ellerman wrote:
> Viresh Kumar <viresh.ku...@linaro.org> writes:

> > AFAICT, you will get -1 here only if the freq table had no valid
> > frequencies (or the freq table is empty). Why would that happen ?
> 
> Bugs?

The cupfreq driver shouldn't have registered itself in that case (i.e.
if the cpufreq table is empty).

> Or if you ask for a target_freq that is higher than anything in the
> table.

You will still get a valid index in that case.

There is only once case where we return -1, when the cpufreq table
doesn't have any valid frequencies.

> Or the API changes, and we forget to update this call site.

I am not sure we can do much about that right now.

> If you're saying that cpufreq_table_find_index_dl() can NEVER fail,

Yes, if we have at least one valid frequency in the table, otherwise
the cpufreq driver shouldn't have registered itself.

> then
> write it so that it can never fail and change it to return unsigned int.

But what should we do when there is no frequency in the cpufreq table?
Just in case where a driver is buggy and tries to call this routine
for an invalid table.

> Having it potentially return -1, which is then used to index an array
> and not handling that is just asking for bugs to happen.

I understand what you are trying to say here, but I don't know what
can be done to prevent this here.

What we can do is change the return type to void and pass a int
pointer to the routine, but that wouldn't change anything at all. That
pointers variable can still have -1 in it.

-- 
viresh

Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()

[Viresh Kumar]

On 12-02-18, 16:03, Shilpasri G Bhat wrote:
> I agree too. There is no way we can get -1 with initialized cpu frequency 
> table.
> We don't initialize powernv-cpufreq if we don't have valid CPU frequency
> entries. Is there any other way to suppress the Coverity tool warning apart 
> from
> ignoring it?

So IIUC, this warning is generated by an external tool after static
analysis of the code ?

If yes, then just ignore the warning. We shouldn't try fixing the
kernel because a tool isn't smart enough to catch intentional
ignorance of the return value here.

-- 
viresh

Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()

[Viresh Kumar]

On 12-02-18, 15:51, Shilpasri G Bhat wrote:
> This patch fixes the below Coverity warning:
> 
> *** CID 182816:  Memory - illegal accesses  (NEGATIVE_RETURNS)
> /drivers/cpufreq/powernv-cpufreq.c: 1008 in powernv_fast_switch()
> 1002  unsigned int target_freq)
> 1003 {
> 1004  int index;
> 1005  struct powernv_smp_call_data freq_data;
> 1006
> 1007  index = cpufreq_table_find_index_dl(policy, target_freq);
> >>> CID 182816:  Memory - illegal accesses  (NEGATIVE_RETURNS)
> >>> Using variable "index" as an index to array "powernv_freqs".
> 1008  freq_data.pstate_id = powernv_freqs[index].driver_data;
> 1009  freq_data.gpstate_id = powernv_freqs[index].driver_data;
> 1010  set_pstate(_data);
> 1011
> 1012  return powernv_freqs[index].frequency;
> 1013 }
> 
> Signed-off-by: Shilpasri G Bhat 
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index 29cdec1..69edfe9 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -1005,6 +1005,9 @@ static unsigned int powernv_fast_switch(struct 
> cpufreq_policy *policy,
>   struct powernv_smp_call_data freq_data;
>  
>   index = cpufreq_table_find_index_dl(policy, target_freq);
> + if (unlikely(index < 0))
> + index = get_nominal_index();
> +

AFAICT, you will get -1 here only if the freq table had no valid
frequencies (or the freq table is empty). Why would that happen ?

>   freq_data.pstate_id = powernv_freqs[index].driver_data;
>   freq_data.gpstate_id = powernv_freqs[index].driver_data;
>   set_pstate(_data);
> -- 
> 1.8.3.1

-- 
viresh

Re: [PATCH] cpufreq: powernv: Dont assume distinct pstate values for nominal and pmin

[Viresh Kumar]

On 12-01-18, 12:43, Shilpasri G Bhat wrote:
> Some OpenPOWER boxes can have same pstate values for nominal and
> pmin pstates. In these boxes the current code will not initialize
> 'powernv_pstate_info.min' variable and result in erroneous CPU
> frequency reporting. This patch fixes this problem.
> 
> Fixes: 09ca4c9b5958 ("cpufreq: powernv: Replacing pstate_id with frequency 
> table index")
> Reported-by: Alvin Wang <wan...@tw.ibm.com>
> Signed-off-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index b6d7c4c..da7fdb4 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -288,9 +288,9 @@ static int init_powernv_pstates(void)
>  
>   if (id == pstate_max)
>   powernv_pstate_info.max = i;
> - else if (id == pstate_nominal)
> + if (id == pstate_nominal)
>   powernv_pstate_info.nominal = i;
> - else if (id == pstate_min)
> + if (id == pstate_min)
>   powernv_pstate_info.min = i;
>  
>   if (powernv_pstate_info.wof_enabled && id == pstate_turbo) {

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

Re: [v3 PATCH 0/3] powernv-cpufreq: Multiple pstate related fixes.

[Viresh Kumar]

On 13-12-17, 12:27, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <e...@linux.vnet.ibm.com>
> 
>  This is a third version of the patch to fix pstate related issues in
>  the powernv-cpufreq driver.

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

Re: [PATCH v2] cpufreq: powernv: Add support of frequency domain

[Viresh Kumar]

On 20-12-17, 12:12, Abhishek Goel wrote:
> diff --git a/drivers/cpufreq/powernv-cpufreq.c 
> b/drivers/cpufreq/powernv-cpufreq.c
> index b6d7c4c..fd642bc 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -37,6 +37,7 @@
>  #include  /* Required for cpu_sibling_mask() in UP configs */
>  #include 
>  #include 
> +#include 
>  
>  #define POWERNV_MAX_PSTATES  256
>  #define PMSR_PSAFE_ENABLE(1UL << 30)
> @@ -130,6 +131,9 @@ static struct chip {
>  static int nr_chips;
>  static DEFINE_PER_CPU(struct chip *, chip_info);
>  
> +static u32 freq_domain_indicator;
> +static u32 flag;

I wouldn't name it as flag, its unreadable. Maybe its better to name
it based on the quirk you are trying to workaround with ?

> +
>  /*
>   * Note:
>   * The set of pstates consists of contiguous integers.
> @@ -194,6 +198,38 @@ static inline void reset_gpstates(struct cpufreq_policy 
> *policy)
>   gpstates->last_gpstate_idx = 0;
>  }
>  
> +#define SIZE NR_CPUS
> +#define ORDER_FREQ_MAP ilog2(SIZE)
> +
> +static DEFINE_HASHTABLE(freq_domain_map, ORDER_FREQ_MAP);
> +
> +struct hashmap {
> + cpumask_t mask;
> + int chip_id;
> + u32 pir_key;
> + struct hlist_node hash_node;
> +};
> +
> +static void insert(u32 key, int cpu)
> +{
> + struct hashmap *data;
> +
> + hash_for_each_possible(freq_domain_map, data, hash_node, key%SIZE) {
> + if (data->chip_id == cpu_to_chip_id(cpu) &&
> + data->pir_key == key) {
> + cpumask_set_cpu(cpu, >mask);
> + return;
> + }
> + }
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + hash_add(freq_domain_map, >hash_node, key%SIZE);
> + cpumask_set_cpu(cpu, >mask);
> + data->chip_id = cpu_to_chip_id(cpu);
> + data->pir_key = key;
> +
> +}
> +
>  /*
>   * Initialize the freq table based on data obtained
>   * from the firmware passed via device-tree
> @@ -206,7 +242,9 @@ static int init_powernv_pstates(void)
>   u32 len_ids, len_freqs;
>   u32 pstate_min, pstate_max, pstate_nominal;
>   u32 pstate_turbo, pstate_ultra_turbo;
> + u32 key;
>  
> + flag = 0;

Isn't flag already 0 (global-uninitialized) ?

>   power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
>   if (!power_mgt) {
>   pr_warn("power-mgt node not found\n");
> @@ -229,6 +267,17 @@ static int init_powernv_pstates(void)
>   return -ENODEV;
>   }
>  
> + if (of_device_is_compatible(power_mgt, "freq-domain-v1") &&
> + of_property_read_u32(power_mgt, "ibm,freq-domain-indicator",
> +  _domain_indicator)) {
> + pr_warn("ibm,freq-domain-indicator not found\n");
> + freq_domain_indicator = 0;

You shouldn't be required to set it to 0 here.

> + }
> +
> + if (of_device_is_compatible(power_mgt, "P9-occ-quirk")) {
> + flag = 1;
> + }

Remove {} and a better name like p9_occ_quirk would be good for flag.
Also making it a bool may be better ?

> +
>   if (of_property_read_u32(power_mgt, "ibm,pstate-ultra-turbo",
>_ultra_turbo)) {
>   powernv_pstate_info.wof_enabled = false;
> @@ -249,6 +298,7 @@ static int init_powernv_pstates(void)
>  next:
>   pr_info("cpufreq pstate min %d nominal %d max %d\n", pstate_min,
>   pstate_nominal, pstate_max);
> + pr_info("frequency domain indicator %d", freq_domain_indicator);
>   pr_info("Workload Optimized Frequency is %s in the platform\n",
>   (powernv_pstate_info.wof_enabled) ? "enabled" : "disabled");
>  
> @@ -276,6 +326,15 @@ static int init_powernv_pstates(void)
>   return -ENODEV;
>   }
>  
> + if (freq_domain_indicator) {
> + hash_init(freq_domain_map);
> + for_each_possible_cpu(i) {
> + key = ((u32) get_hard_smp_processor_id(i) &
> + freq_domain_indicator);

Maybe break it like:

key = (u32) get_hard_smp_processor_id(i);
key &= freq_domain_indicator;

to make it easily readable ?

> + insert(key, i);
> + }
> + }
> +
>   powernv_pstate_info.nr_pstates = nr_pstates;
>   pr_debug("NR PStates %d\n", nr_pstates);
>   for (i = 0; i < nr_pstates; i++) {
> @@ -693,6 +752,7 @@ static int powernv_cpufreq_target_index(struct 
> cpufreq_policy *policy,
>  {
>   struct powernv_smp_call_data freq_data;
>   unsigned int cur_msec, gpstate_idx;
> +

:(

>   struct global_pstate_info *gpstates = policy->driver_data;
>  
>   if (unlikely(rebooting) && new_index != get_nominal_index())
> @@ -760,25 +820,55 @@ static int powernv_cpufreq_target_index(struct 
> cpufreq_policy *policy,
>  
>   spin_unlock(>gpstate_lock);
>  
> - /*
> -  * Use smp_call_function to send IPI and execute the
> -  * mtspr on 

Re: [PATCH] cpufreq: powernv: Add support of frequency domain

[Viresh Kumar]

On 18-12-17, 10:41, Abhishek wrote:
> We need to do it in this way as the current implementation takes the max of
> the PMSR of the cores. Thus, when the frequency is required to be ramped up,
> it suffices to write to just the local PMSR, but when the frequency is to be
> ramped down, if we don't send the IPI it breaks the compatibility with P8.

Looks strange really that you have to program this differently for speeding up
or down. These CPUs are part of one cpufreq policy and so I would normally
expect changes to any CPU should reflect for other CPUs as well.

@Goutham: Do you know why it is so ?

-- 
viresh

Re: [PATCH] cpufreq: powernv: Add support of frequency domain

[Viresh Kumar]

+ Gautham,

@Gautham: Can you please help reviewing this one ?

On 13-12-17, 13:49, Abhishek Goel wrote:
> @@ -693,6 +746,8 @@ static int powernv_cpufreq_target_index(struct 
> cpufreq_policy *policy,
>  {
>   struct powernv_smp_call_data freq_data;
>   unsigned int cur_msec, gpstate_idx;
> + cpumask_t temp;
> + u32 cpu;
>   struct global_pstate_info *gpstates = policy->driver_data;
>  
>   if (unlikely(rebooting) && new_index != get_nominal_index())
> @@ -761,24 +816,48 @@ static int powernv_cpufreq_target_index(struct 
> cpufreq_policy *policy,
>   spin_unlock(>gpstate_lock);
>  
>   /*
> -  * Use smp_call_function to send IPI and execute the
> -  * mtspr on target CPU.  We could do that without IPI
> -  * if current CPU is within policy->cpus (core)
> +  * Use smp_call_function to send IPI and execute the mtspr on CPU.
> +  * This needs to be done on every core of the policy

Why on each CPU ?

>*/
> - smp_call_function_any(policy->cpus, set_pstate, _data, 1);
> + cpumask_copy(, policy->cpus);
> +
> + while (!cpumask_empty()) {
> + cpu = cpumask_first();
> + smp_call_function_any(cpu_sibling_mask(cpu),
> + set_pstate, _data, 1);
> + cpumask_andnot(, , cpu_sibling_mask(cpu));
> + }
> +
>   return 0;
>  }

-- 
viresh

Re: [PATCH] cpufreq: powernv: Define methods to parse positive & negative pstates

[Viresh Kumar]

On 30-11-17, 10:13, Shilpasri G Bhat wrote:
> From: "Gautham R. Shenoy" <e...@linux.vnet.ibm.com>
> 
> Pstates are 8bit values but on POWER8 they are negative and on POWER9
> they are positive. This patch adds helper routines to differentiate
> the sign to read the correct pstate value.
> 
> Signed-off-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com>
> Tested-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 43 
> ++-
>  1 file changed, 33 insertions(+), 10 deletions(-)

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

[PATCH V3 8/9] cpufreq: Add CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING cpufreq driver flag

[Viresh Kumar]

The policy->transition_latency field is used for multiple purposes
today and its not straight forward at all. This is how it is used:

A. Set the correct transition_latency value.

B. Set it to CPUFREQ_ETERNAL because:
   1. We don't want automatic dynamic switching (with
  ondemand/conservative) to happen at all.
   2. We don't know the transition latency.

This patch handles the B.1. case in a more readable way. A new flag for
the cpufreq drivers is added to disallow use of cpufreq governors which
have dynamic_switching flag set.

All the current cpufreq drivers which are setting transition_latency
unconditionally to CPUFREQ_ETERNAL are updated to use it. They don't
need to set transition_latency anymore.

There shouldn't be any functional change after this patch.

Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
---
 drivers/cpufreq/cpufreq-nforce2.c  | 2 +-
 drivers/cpufreq/cpufreq.c  | 5 +++--
 drivers/cpufreq/elanfreq.c | 4 +---
 drivers/cpufreq/gx-suspmod.c   | 2 +-
 drivers/cpufreq/pmac32-cpufreq.c   | 7 +--
 drivers/cpufreq/sa1100-cpufreq.c   | 5 +++--
 drivers/cpufreq/sa1110-cpufreq.c   | 5 +++--
 drivers/cpufreq/sh-cpufreq.c   | 3 +--
 drivers/cpufreq/speedstep-smi.c| 2 +-
 drivers/cpufreq/unicore2-cpufreq.c | 3 +--
 include/linux/cpufreq.h| 6 ++
 11 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-nforce2.c 
b/drivers/cpufreq/cpufreq-nforce2.c
index 5503d491b016..dbf82f36d270 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -357,7 +357,6 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
/* cpuinfo and default policy values */
policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
-   policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 
return 0;
 }
@@ -369,6 +368,7 @@ static int nforce2_cpu_exit(struct cpufreq_policy *policy)
 
 static struct cpufreq_driver nforce2_driver = {
.name = "nforce2",
+   .flags = CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING,
.verify = nforce2_verify,
.target = nforce2_target,
.get = nforce2_get,
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2debfb5f9126..a4d9b47c4af4 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2016,11 +2016,12 @@ static int cpufreq_init_governor(struct cpufreq_policy 
*policy)
 
/* Platform doesn't want dynamic frequency switching ? */
if (policy->governor->dynamic_switching &&
-   policy->cpuinfo.transition_latency == CPUFREQ_ETERNAL) {
+   (cpufreq_driver->flags & CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING ||
+   policy->cpuinfo.transition_latency == CPUFREQ_ETERNAL)) {
struct cpufreq_governor *gov = cpufreq_fallback_governor();
 
if (gov) {
-   pr_warn("Transition latency set to CPUFREQ_ETERNAL, 
can't use %s governor. Fallback to %s governor\n",
+   pr_warn("Can't use %s governor as dynamic switching is 
disallowed. Fallback to %s governor\n",
policy->governor->name, gov->name);
policy->governor = gov;
} else {
diff --git a/drivers/cpufreq/elanfreq.c b/drivers/cpufreq/elanfreq.c
index bfce11cba1df..45e2ca62515e 100644
--- a/drivers/cpufreq/elanfreq.c
+++ b/drivers/cpufreq/elanfreq.c
@@ -165,9 +165,6 @@ static int elanfreq_cpu_init(struct cpufreq_policy *policy)
if (pos->frequency > max_freq)
pos->frequency = CPUFREQ_ENTRY_INVALID;
 
-   /* cpuinfo and default policy values */
-   policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
-
return cpufreq_table_validate_and_show(policy, elanfreq_table);
 }
 
@@ -196,6 +193,7 @@ __setup("elanfreq=", elanfreq_setup);
 
 static struct cpufreq_driver elanfreq_driver = {
.get= elanfreq_get_cpu_frequency,
+   .flags  = CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING,
.verify = cpufreq_generic_frequency_table_verify,
.target_index   = elanfreq_target,
.init   = elanfreq_cpu_init,
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c
index 3488c9c175eb..8f52a06664e3 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -428,7 +428,6 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy 
*policy)
policy->max = maxfreq;
policy->cpuinfo.min_freq = maxfreq / max_duration;
policy->cpuinfo.max_freq = maxfreq;
-   policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 
return 0;
 }
@@ -438,6 +437,7 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy 
*p

[PATCH V3 0/9] cpufreq: transition-latency cleanups

[Viresh Kumar]

Hi Rafael,

This series tries to cleanup the code around transition-latency and its
users. Some of the old legacy code, which may not make much sense now,
is dropped as well. And some code consolidation is also done across
governors.

Based of: v4.13-rc1
Tested on: ARM64 Hikey board.

I have pushed it here as well (which gets tested by kbuild test bot):

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git 
cpufreq/transition-latency

V2->V3:
- Rearranged patches to keep related stuff together
- Introduce CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING flag (Rafael)
- Minor optimization in cpufreq_policy_transition_delay_us() and moved
  it to cpufreq.c (Rafael)
- Allow dynamic switching for drivers which don't know their transition
  latency.

V1->V2:
- While we still get rid of the limitation of 10ms for using
  ondemand/conservative, but we preserve the earlier behavior where the
  transition latency set to CPUFREQ_ETERNAL would not allow use of
  ondemand/conservative governors. Thanks to Dominik for his feedback on
  that.

--
viresh

Viresh Kumar (9):
  cpufreq: governor: Drop min_sampling_rate
  cpufreq: Use transition_delay_us for legacy governors as well
  cpufreq: Cap the default transition delay value to 10 ms
  cpufreq: Don't set transition_latency for setpolicy drivers
  cpufreq: arm_big_little: Make ->get_transition_latency() mandatory
  cpufreq: Replace "max_transition_latency" with "dynamic_switching"
  cpufreq: schedutil: Set dynamic_switching to true
  cpufreq: Add CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING cpufreq driver flag
  cpufreq: Allow dynamic switching with CPUFREQ_ETERNAL latency

 Documentation/admin-guide/pm/cpufreq.rst |  8 
 drivers/cpufreq/arm_big_little.c | 10 --
 drivers/cpufreq/cpufreq-nforce2.c|  2 +-
 drivers/cpufreq/cpufreq.c| 34 
 drivers/cpufreq/cpufreq_conservative.c   |  6 --
 drivers/cpufreq/cpufreq_governor.c   | 17 ++--
 drivers/cpufreq/cpufreq_governor.h   |  3 +--
 drivers/cpufreq/cpufreq_ondemand.c   | 12 ---
 drivers/cpufreq/elanfreq.c   |  4 +---
 drivers/cpufreq/gx-suspmod.c |  2 +-
 drivers/cpufreq/intel_pstate.c   |  1 -
 drivers/cpufreq/longrun.c|  1 -
 drivers/cpufreq/pmac32-cpufreq.c |  7 +--
 drivers/cpufreq/sa1100-cpufreq.c |  5 +++--
 drivers/cpufreq/sa1110-cpufreq.c |  5 +++--
 drivers/cpufreq/sh-cpufreq.c |  3 +--
 drivers/cpufreq/speedstep-smi.c  |  2 +-
 drivers/cpufreq/unicore2-cpufreq.c   |  3 +--
 include/linux/cpufreq.h  | 18 -
 kernel/sched/cpufreq_schedutil.c | 12 ++-
 20 files changed, 65 insertions(+), 90 deletions(-)

-- 
2.13.0.71.gd7076ec9c9cb

Re: [PATCH] cpufreq: Convert to using %pOF instead of full_name

[Viresh Kumar]

On 18-07-17, 16:42, Rob Herring wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
> 
> Signed-off-by: Rob Herring <r...@kernel.org>
> Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
> Cc: Viresh Kumar <viresh.ku...@linaro.org>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: Patrice Chotard <patrice.chot...@st.com>
> Cc: linux...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: ker...@stlinux.com
> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 2 +-
>  drivers/cpufreq/sti-cpufreq.c| 8 
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/pmac64-cpufreq.c 
> b/drivers/cpufreq/pmac64-cpufreq.c
> index 267e0894c62d..be623dd7b9f2 100644
> --- a/drivers/cpufreq/pmac64-cpufreq.c
> +++ b/drivers/cpufreq/pmac64-cpufreq.c
> @@ -516,7 +516,7 @@ static int __init g5_pm72_cpufreq_init(struct device_node 
> *cpunode)
>   goto bail;
>   }
> 
> - DBG("cpufreq: i2c clock chip found: %s\n", hwclock->full_name);
> + DBG("cpufreq: i2c clock chip found: %pOF\n", hwclock);
> 
>   /* Now get all the platform functions */
>   pfunc_cpu_getfreq =
> diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c
> index d2d0430d09d4..47105735df12 100644
> --- a/drivers/cpufreq/sti-cpufreq.c
> +++ b/drivers/cpufreq/sti-cpufreq.c
> @@ -65,8 +65,8 @@ static int sti_cpufreq_fetch_major(void) {
>   ret = of_property_read_u32_index(np, "st,syscfg",
>MAJOR_ID_INDEX, _offset);
>   if (ret) {
> - dev_err(dev, "No major number offset provided in %s [%d]\n",
> - np->full_name, ret);
> + dev_err(dev, "No major number offset provided in %pOF [%d]\n",
> + np, ret);
>   return ret;
>   }
> 
> @@ -92,8 +92,8 @@ static int sti_cpufreq_fetch_minor(void)
>MINOR_ID_INDEX, _offset);
>   if (ret) {
>   dev_err(dev,
> - "No minor number offset provided %s [%d]\n",
> - np->full_name, ret);
> + "No minor number offset provided %pOF [%d]\n",
> + np, ret);
>   return ret;
>   }

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

Re: [patch 09/18] cpufreq/pasemi: Adjust system_state check

[Viresh Kumar]

On 14-05-17, 20:27, Thomas Gleixner wrote:
> To enable smp_processor_id() and might_sleep() debug checks earlier, it's
> required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.
> 
> Adjust the system_state check in pas_cpufreq_cpu_exit() to handle the extra
> states.
> 
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
> Cc: Viresh Kumar <viresh.ku...@linaro.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  drivers/cpufreq/pasemi-cpufreq.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -226,7 +226,7 @@ static int pas_cpufreq_cpu_exit(struct c
>* We don't support CPU hotplug. Don't unmap after the system
>* has already made it to a running state.
>*/
> - if (system_state != SYSTEM_BOOTING)
> + if (system_state >= SYSTEM_RUNNING)
>   return 0;
>  
>   if (sdcasr_mapbase)

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh

[PATCH] cpufreq: Remove CONFIG_CPU_FREQ_STAT_DETAILS config option

[Viresh Kumar]

This doesn't have any benefit apart from saving a small amount of memory
when it is disabled. The ifdef hackery in the code makes it dirty
unnecessarily.

Clean it up by removing the Kconfig option completely. Few defconfigs
are also updated and CONFIG_CPU_FREQ_STAT_DETAILS is replaced with
CONFIG_CPU_FREQ_STAT now in them, as users wanted stats to be enabled.

Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
---
 arch/arm/configs/exynos_defconfig |  2 +-
 arch/arm/configs/multi_v5_defconfig   |  2 +-
 arch/arm/configs/multi_v7_defconfig   |  2 +-
 arch/arm/configs/mvebu_v5_defconfig   |  2 +-
 arch/arm/configs/pxa_defconfig|  2 +-
 arch/arm/configs/shmobile_defconfig   |  2 +-
 arch/mips/configs/lemote2f_defconfig  |  1 -
 arch/powerpc/configs/ppc6xx_defconfig |  1 -
 arch/sh/configs/sh7785lcr_32bit_defconfig |  2 +-
 drivers/cpufreq/Kconfig   |  8 
 drivers/cpufreq/cpufreq_stats.c   | 14 --
 11 files changed, 7 insertions(+), 31 deletions(-)

diff --git a/arch/arm/configs/exynos_defconfig 
b/arch/arm/configs/exynos_defconfig
index 79c415c33f69..809f0bf3042a 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -24,7 +24,7 @@ CONFIG_ARM_APPENDED_DTB=y
 CONFIG_ARM_ATAG_DTB_COMPAT=y
 CONFIG_CMDLINE="root=/dev/ram0 rw ramdisk=8192 initrd=0x4100,8M 
console=ttySAC1,115200 init=/linuxrc mem=256M"
 CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPU_FREQ_GOV_POWERSAVE=m
 CONFIG_CPU_FREQ_GOV_USERSPACE=m
diff --git a/arch/arm/configs/multi_v5_defconfig 
b/arch/arm/configs/multi_v5_defconfig
index 361686a362f1..69a4bd13eea5 100644
--- a/arch/arm/configs/multi_v5_defconfig
+++ b/arch/arm/configs/multi_v5_defconfig
@@ -58,7 +58,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_ARM_APPENDED_DTB=y
 CONFIG_ARM_ATAG_DTB_COMPAT=y
 CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPU_IDLE=y
 CONFIG_ARM_KIRKWOOD_CPUIDLE=y
diff --git a/arch/arm/configs/multi_v7_defconfig 
b/arch/arm/configs/multi_v7_defconfig
index b01a43851294..2dcac90eba01 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -132,7 +132,7 @@ CONFIG_ARM_ATAG_DTB_COMPAT=y
 CONFIG_KEXEC=y
 CONFIG_EFI=y
 CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPU_FREQ_GOV_POWERSAVE=m
 CONFIG_CPU_FREQ_GOV_USERSPACE=m
diff --git a/arch/arm/configs/mvebu_v5_defconfig 
b/arch/arm/configs/mvebu_v5_defconfig
index f7f6039419aa..4b598da0d086 100644
--- a/arch/arm/configs/mvebu_v5_defconfig
+++ b/arch/arm/configs/mvebu_v5_defconfig
@@ -44,7 +44,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_ARM_APPENDED_DTB=y
 CONFIG_ARM_ATAG_DTB_COMPAT=y
 CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPU_IDLE=y
 CONFIG_ARM_KIRKWOOD_CPUIDLE=y
diff --git a/arch/arm/configs/pxa_defconfig b/arch/arm/configs/pxa_defconfig
index e4314b1227a3..271dc7e78e43 100644
--- a/arch/arm/configs/pxa_defconfig
+++ b/arch/arm/configs/pxa_defconfig
@@ -97,7 +97,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_CMDLINE="root=/dev/ram0 ro"
 CONFIG_KEXEC=y
 CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPU_FREQ_GOV_POWERSAVE=m
 CONFIG_CPU_FREQ_GOV_USERSPACE=m
diff --git a/arch/arm/configs/shmobile_defconfig 
b/arch/arm/configs/shmobile_defconfig
index 1b0f8ae36fb3..adeaecd831a4 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -38,7 +38,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_ARM_APPENDED_DTB=y
 CONFIG_KEXEC=y
 CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_STAT=y
 CONFIG_CPU_FREQ_GOV_POWERSAVE=y
 CONFIG_CPU_FREQ_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
diff --git a/arch/mips/configs/lemote2f_defconfig 
b/arch/mips/configs/lemote2f_defconfig
index 5da76e0e120f..bed745596d86 100644
--- a/arch/mips/configs/lemote2f_defconfig
+++ b/arch/mips/configs/lemote2f_defconfig
@@ -40,7 +40,6 @@ CONFIG_PM_STD_PARTITION="/dev/hda3"
 CONFIG_CPU_FREQ=y
 CONFIG_CPU_FREQ_DEBUG=y
 CONFIG_CPU_FREQ_STAT=m
-CONFIG_CPU_FREQ_STAT_DETAILS=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPU_FREQ_GOV_POWERSAVE=m
 CONFIG_CPU_FREQ_GOV_USERSPACE=m
diff --git a/arch/powerpc/configs/ppc6xx_defconfig 
b/arch/powerpc/configs/ppc6xx_defconfig
index 3ce91a3df27f..1d2d69dd6409 100644
--- a/arch/powerpc/configs/ppc6xx_defconfig
+++ b/arch/powerpc/configs/ppc6xx_defconfig
@@ -62,7 +62,6 @@ CONFIG_MPC8610_HPCD=y
 CONFIG_GEF_SBC610=y
 CONFIG_CPU_FREQ=y
 CONFIG_CPU_FREQ_STAT=m
-CONFIG_CPU_FREQ_STAT_DETAILS=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_PERFORMANCE=y
 CONFIG_CPU_FREQ_GOV_POWERSAVE=m
diff --git a/arch/sh/configs/

Re: [PATCH V2] cpufreq: powernv: Add boost files to export ultra-turbo frequencies

[Viresh Kumar]

On 03-01-17, 16:36, Shilpasri G Bhat wrote:
> In P8+, Workload Optimized Frequency(WOF) provides the capability to
> boost the cpu frequency based on the utilization of the other cpus
> running in the chip. The On-Chip-Controller(OCC) firmware will control
> the achievability of these frequencies depending on the power headroom
> available in the chip. Currently the ultra-turbo frequencies provided
> by this feature are exported along with the turbo and sub-turbo
> frequencies as scaling_available_frequencies. This patch will export
> the ultra-turbo frequencies separately as scaling_boost_frequencies in
> WOF enabled systems. This patch will add the boost sysfs file which
> can be used to disable/enable ultra-turbo frequencies.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.b...@linux.vnet.ibm.com>
> Reviewed-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com>
> ---
> Changes from v1:
> - Print if WOF is enabled
> - s/clean_notifiers/cleanup_notifiers
> 
>  drivers/cpufreq/powernv-cpufreq.c | 50 
> ++++---
>  1 file changed, 47 insertions(+), 3 deletions(-)

Acked-by: Viresh Kumar <viresh.ku...@linaro.org>

-- 
viresh