[PATCH 00/13] Rid W=1 warnings in CPUFreq

[PATCH 00/13] Rid W=1 warnings in CPUFreq

[Lee Jones]

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

After these patches are applied, the build system no longer
complains about any W=0 nor W=1 level warnings in drivers/cpufreq.

Hurrah!

Lee Jones (13):
  cpufreq: freq_table: Demote obvious misuse of kerneldoc to standard
    comment blocks
  cpufreq: cpufreq: Demote lots of function headers unworthy of
    kerneldoc status
  cpufreq: cpufreq_governor: Demote store_sampling_rate() header to
    standard comment block
  cpufreq: sti-cpufreq: Fix some formatting and misspelling issues
  cpufreq/arch: powerpc: pasemi: Move prototypes to shared header
  cpufreq: powernv-cpufreq: Functions only used in call-backs should be
    static
  cpufreq: powernv-cpufreq: Fix a bunch of kerneldoc related issues
  cpufreq: acpi-cpufreq: Take 'dummy' principle one stage further
  cpufreq: acpi-cpufreq: Remove unused ID structs
  cpufreq: powernow-k8: Make use of known set but not used variables
  cpufreq: pcc-cpufreq: Remove unused ID structs
  cpufreq: intel_pstate: Supply struct attribute description for
    get_aperf_mperf_shift()
  cpufreq: amd_freq_sensitivity: Remove unused ID structs

 arch/powerpc/platforms/pasemi/pasemi.h    | 15 -----------
 arch/powerpc/platforms/pasemi/powersave.S |  2 ++
 drivers/cpufreq/acpi-cpufreq.c            | 16 ++----------
 drivers/cpufreq/amd_freq_sensitivity.c    |  6 -----
 drivers/cpufreq/cpufreq.c                 | 32 ++++++++++++-----------
 drivers/cpufreq/cpufreq_governor.c        |  2 +-
 drivers/cpufreq/freq_table.c              |  6 ++---
 drivers/cpufreq/intel_pstate.c            |  2 ++
 drivers/cpufreq/pasemi-cpufreq.c          |  1 +
 drivers/cpufreq/pcc-cpufreq.c             |  7 -----
 drivers/cpufreq/powernow-k8.c             |  2 ++
 drivers/cpufreq/powernv-cpufreq.c         | 15 ++++++-----
 drivers/cpufreq/sti-cpufreq.c             |  8 +++---
 include/linux/platform_data/pasemi.h      | 28 ++++++++++++++++++++
 14 files changed, 70 insertions(+), 72 deletions(-)
 create mode 100644 include/linux/platform_data/pasemi.h

-- 
2.25.1

[PATCH 01/13] cpufreq: freq_table: Demote obvious misuse of kerneldoc to standard comment blocks

[Lee Jones]

No attempt has been made to document any of the demoted functions here.

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/freq_table.c:229: warning: Function parameter or member 'policy' not described in 'show_available_freqs'
 drivers/cpufreq/freq_table.c:229: warning: Function parameter or member 'buf' not described in 'show_available_freqs'
 drivers/cpufreq/freq_table.c:229: warning: Function parameter or member 'show_boost' not described in 'show_available_freqs'
 drivers/cpufreq/freq_table.c:269: warning: Function parameter or member 'policy' not described in 'scaling_available_frequencies_show'
 drivers/cpufreq/freq_table.c:269: warning: Function parameter or member 'buf' not described in 'scaling_available_frequencies_show'
 drivers/cpufreq/freq_table.c:281: warning: Function parameter or member 'policy' not described in 'scaling_boost_frequencies_show'
 drivers/cpufreq/freq_table.c:281: warning: Function parameter or member 'buf' not described in 'scaling_boost_frequencies_show'

Cc: Dominik Brodowski <linux@brodo.de>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/freq_table.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index e117b0059123e..f839dc9852c08 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -221,7 +221,7 @@ int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
 }
 EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_index);
 
-/**
+/*
  * show_available_freqs - show available frequencies for the specified CPU
  */
 static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
@@ -260,7 +260,7 @@ static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
 struct freq_attr cpufreq_freq_attr_##_name##_freqs =     \
 __ATTR_RO(_name##_frequencies)
 
-/**
+/*
  * show_scaling_available_frequencies - show available normal frequencies for
  * the specified CPU
  */
@@ -272,7 +272,7 @@ static ssize_t scaling_available_frequencies_show(struct cpufreq_policy *policy,
 cpufreq_attr_available_freq(scaling_available);
 EXPORT_SYMBOL_GPL(cpufreq_freq_attr_scaling_available_freqs);
 
-/**
+/*
  * show_available_boost_freqs - show available boost frequencies for
  * the specified CPU
  */
-- 
2.25.1

Re: [PATCH 01/13] cpufreq: freq_table: Demote obvious misuse of kerneldoc to standard comment blocks

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> No attempt has been made to document any of the demoted functions here.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/freq_table.c:229: warning: Function parameter or member 'policy' not described in 'show_available_freqs'
>  drivers/cpufreq/freq_table.c:229: warning: Function parameter or member 'buf' not described in 'show_available_freqs'
>  drivers/cpufreq/freq_table.c:229: warning: Function parameter or member 'show_boost' not described in 'show_available_freqs'
>  drivers/cpufreq/freq_table.c:269: warning: Function parameter or member 'policy' not described in 'scaling_available_frequencies_show'
>  drivers/cpufreq/freq_table.c:269: warning: Function parameter or member 'buf' not described in 'scaling_available_frequencies_show'
>  drivers/cpufreq/freq_table.c:281: warning: Function parameter or member 'policy' not described in 'scaling_boost_frequencies_show'
>  drivers/cpufreq/freq_table.c:281: warning: Function parameter or member 'buf' not described in 'scaling_boost_frequencies_show'
> 
> Cc: Dominik Brodowski <linux@brodo.de>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/freq_table.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index e117b0059123e..f839dc9852c08 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -221,7 +221,7 @@ int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_index);
>  
> -/**
> +/*
>   * show_available_freqs - show available frequencies for the specified CPU
>   */
>  static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
> @@ -260,7 +260,7 @@ static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
>  struct freq_attr cpufreq_freq_attr_##_name##_freqs =     \
>  __ATTR_RO(_name##_frequencies)
>  
> -/**
> +/*
>   * show_scaling_available_frequencies - show available normal frequencies for
>   * the specified CPU
>   */
> @@ -272,7 +272,7 @@ static ssize_t scaling_available_frequencies_show(struct cpufreq_policy *policy,
>  cpufreq_attr_available_freq(scaling_available);
>  EXPORT_SYMBOL_GPL(cpufreq_freq_attr_scaling_available_freqs);
>  
> -/**
> +/*
>   * show_available_boost_freqs - show available boost frequencies for
>   * the specified CPU
>   */

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

-- 
viresh

[PATCH 02/13] cpufreq: cpufreq: Demote lots of function headers unworthy of kerneldoc status

[Lee Jones]

Also provide missing function parameter description for 'cpu' and 'policy'.

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/cpufreq.c:60: warning: cannot understand function prototype: 'struct cpufreq_driver *cpufreq_driver; '
 drivers/cpufreq/cpufreq.c:90: warning: Function parameter or member 'cpufreq_policy_notifier_list' not described in 'BLOCKING_NOTIFIER_HEAD'
 drivers/cpufreq/cpufreq.c:312: warning: Function parameter or member 'val' not described in 'adjust_jiffies'
 drivers/cpufreq/cpufreq.c:312: warning: Function parameter or member 'ci' not described in 'adjust_jiffies'
 drivers/cpufreq/cpufreq.c:538: warning: Function parameter or member 'policy' not described in 'cpufreq_driver_resolve_freq'
 drivers/cpufreq/cpufreq.c:686: warning: Function parameter or member 'file_name' not described in 'show_one'
 drivers/cpufreq/cpufreq.c:686: warning: Function parameter or member 'object' not described in 'show_one'
 drivers/cpufreq/cpufreq.c:731: warning: Function parameter or member 'file_name' not described in 'store_one'
 drivers/cpufreq/cpufreq.c:731: warning: Function parameter or member 'object' not described in 'store_one'
 drivers/cpufreq/cpufreq.c:741: warning: Function parameter or member 'policy' not described in 'show_cpuinfo_cur_freq'
 drivers/cpufreq/cpufreq.c:741: warning: Function parameter or member 'buf' not described in 'show_cpuinfo_cur_freq'
 drivers/cpufreq/cpufreq.c:754: warning: Function parameter or member 'policy' not described in 'show_scaling_governor'
 drivers/cpufreq/cpufreq.c:754: warning: Function parameter or member 'buf' not described in 'show_scaling_governor'
 drivers/cpufreq/cpufreq.c:770: warning: Function parameter or member 'policy' not described in 'store_scaling_governor'
 drivers/cpufreq/cpufreq.c:770: warning: Function parameter or member 'buf' not described in 'store_scaling_governor'
 drivers/cpufreq/cpufreq.c:770: warning: Function parameter or member 'count' not described in 'store_scaling_governor'
 drivers/cpufreq/cpufreq.c:806: warning: Function parameter or member 'policy' not described in 'show_scaling_driver'
 drivers/cpufreq/cpufreq.c:806: warning: Function parameter or member 'buf' not described in 'show_scaling_driver'
 drivers/cpufreq/cpufreq.c:815: warning: Function parameter or member 'policy' not described in 'show_scaling_available_governors'
 drivers/cpufreq/cpufreq.c:815: warning: Function parameter or member 'buf' not described in 'show_scaling_available_governors'
 drivers/cpufreq/cpufreq.c:859: warning: Function parameter or member 'policy' not described in 'show_related_cpus'
 drivers/cpufreq/cpufreq.c:859: warning: Function parameter or member 'buf' not described in 'show_related_cpus'
 drivers/cpufreq/cpufreq.c:867: warning: Function parameter or member 'policy' not described in 'show_affected_cpus'
 drivers/cpufreq/cpufreq.c:867: warning: Function parameter or member 'buf' not described in 'show_affected_cpus'
 drivers/cpufreq/cpufreq.c:901: warning: Function parameter or member 'policy' not described in 'show_bios_limit'
 drivers/cpufreq/cpufreq.c:901: warning: Function parameter or member 'buf' not described in 'show_bios_limit'
 drivers/cpufreq/cpufreq.c:1625: warning: Function parameter or member 'dev' not described in 'cpufreq_remove_dev'
 drivers/cpufreq/cpufreq.c:1625: warning: Function parameter or member 'sif' not described in 'cpufreq_remove_dev'
 drivers/cpufreq/cpufreq.c:2380: warning: Function parameter or member 'cpu' not described in 'cpufreq_get_policy'
 drivers/cpufreq/cpufreq.c:2771: warning: Function parameter or member 'driver' not described in 'cpufreq_unregister_driver'

Cc: Dominik Brodowski <linux@brodo.de>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Shin <jacob.shin@amd.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 036f4cc42ede2..17c1c3becd925 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -52,7 +52,7 @@ static LIST_HEAD(cpufreq_governor_list);
 
 static char default_governor[CPUFREQ_NAME_LEN];
 
-/**
+/*
  * The "cpufreq driver" - the arch- or hardware-dependent low
  * level driver of CPUFreq support, and its spinlock. This lock
  * also protects the cpufreq_cpu_data array.
@@ -80,7 +80,7 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
 			      struct cpufreq_governor *new_gov,
 			      unsigned int new_pol);
 
-/**
+/*
  * Two notifier lists: the "policy" list is involved in the
  * validation process for a new CPU frequency policy; the
  * "transition" list for kernel code that needs to handle
@@ -300,7 +300,7 @@ struct cpufreq_policy *cpufreq_cpu_acquire(unsigned int cpu)
  *            EXTERNALLY AFFECTING FREQUENCY CHANGES                 *
  *********************************************************************/
 
-/**
+/*
  * adjust_jiffies - adjust the system "loops_per_jiffy"
  *
  * This function alters the system "loops_per_jiffy" for the clock
@@ -526,6 +526,7 @@ EXPORT_SYMBOL_GPL(cpufreq_disable_fast_switch);
 /**
  * cpufreq_driver_resolve_freq - Map a target frequency to a driver-supported
  * one.
+ * @policy: associated policy to interrogate
  * @target_freq: target frequency to resolve.
  *
  * The target to driver frequency mapping is cached in the policy.
@@ -670,7 +671,7 @@ static struct cpufreq_governor *cpufreq_parse_governor(char *str_governor)
 	return get_governor(str_governor);
 }
 
-/**
+/*
  * cpufreq_per_cpu_attr_read() / show_##file_name() -
  * print out cpufreq information
  *
@@ -712,7 +713,7 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
 	return ret;
 }
 
-/**
+/*
  * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access
  */
 #define store_one(file_name, object)			\
@@ -733,7 +734,7 @@ static ssize_t store_##file_name					\
 store_one(scaling_min_freq, min);
 store_one(scaling_max_freq, max);
 
-/**
+/*
  * show_cpuinfo_cur_freq - current CPU frequency as detected by hardware
  */
 static ssize_t show_cpuinfo_cur_freq(struct cpufreq_policy *policy,
@@ -747,7 +748,7 @@ static ssize_t show_cpuinfo_cur_freq(struct cpufreq_policy *policy,
 	return sprintf(buf, "<unknown>\n");
 }
 
-/**
+/*
  * show_scaling_governor - show the current policy for the specified CPU
  */
 static ssize_t show_scaling_governor(struct cpufreq_policy *policy, char *buf)
@@ -762,7 +763,7 @@ static ssize_t show_scaling_governor(struct cpufreq_policy *policy, char *buf)
 	return -EINVAL;
 }
 
-/**
+/*
  * store_scaling_governor - store policy for the specified CPU
  */
 static ssize_t store_scaling_governor(struct cpufreq_policy *policy,
@@ -799,7 +800,7 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy,
 	return ret ? ret : count;
 }
 
-/**
+/*
  * show_scaling_driver - show the cpufreq driver currently loaded
  */
 static ssize_t show_scaling_driver(struct cpufreq_policy *policy, char *buf)
@@ -807,7 +808,7 @@ static ssize_t show_scaling_driver(struct cpufreq_policy *policy, char *buf)
 	return scnprintf(buf, CPUFREQ_NAME_PLEN, "%s\n", cpufreq_driver->name);
 }
 
-/**
+/*
  * show_scaling_available_governors - show the available CPUfreq governors
  */
 static ssize_t show_scaling_available_governors(struct cpufreq_policy *policy,
@@ -851,7 +852,7 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
 }
 EXPORT_SYMBOL_GPL(cpufreq_show_cpus);
 
-/**
+/*
  * show_related_cpus - show the CPUs affected by each transition even if
  * hw coordination is in use
  */
@@ -860,7 +861,7 @@ static ssize_t show_related_cpus(struct cpufreq_policy *policy, char *buf)
 	return cpufreq_show_cpus(policy->related_cpus, buf);
 }
 
-/**
+/*
  * show_affected_cpus - show the CPUs affected by each transition
  */
 static ssize_t show_affected_cpus(struct cpufreq_policy *policy, char *buf)
@@ -894,7 +895,7 @@ static ssize_t show_scaling_setspeed(struct cpufreq_policy *policy, char *buf)
 	return policy->governor->show_setspeed(policy, buf);
 }
 
-/**
+/*
  * show_bios_limit - show the current cpufreq HW/BIOS limitation
  */
 static ssize_t show_bios_limit(struct cpufreq_policy *policy, char *buf)
@@ -1616,7 +1617,7 @@ static int cpufreq_offline(unsigned int cpu)
 	return 0;
 }
 
-/**
+/*
  * cpufreq_remove_dev - remove a CPU device
  *
  * Removes the cpufreq interface for a CPU device.
@@ -2373,6 +2374,7 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
  * cpufreq_get_policy - get the current cpufreq_policy
  * @policy: struct cpufreq_policy into which the current cpufreq_policy
  *	is written
+ * @cpu: CPU to find the policy for
  *
  * Reads the current cpufreq policy.
  */
@@ -2759,7 +2761,7 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 }
 EXPORT_SYMBOL_GPL(cpufreq_register_driver);
 
-/**
+/*
  * cpufreq_unregister_driver - unregister the current CPUFreq driver
  *
  * Unregister the current CPUFreq driver. Only call this if you have
-- 
2.25.1

Re: [PATCH 02/13] cpufreq: cpufreq: Demote lots of function headers unworthy of kerneldoc status

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> -/**
> +/*
>   * cpufreq_remove_dev - remove a CPU device

Because cpufreq_add_dev() is part of kernel doc, we better keep it.

>   *
>   * Removes the cpufreq interface for a CPU device.
> @@ -2373,6 +2374,7 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
>   * cpufreq_get_policy - get the current cpufreq_policy
>   * @policy: struct cpufreq_policy into which the current cpufreq_policy
>   *	is written
> + * @cpu: CPU to find the policy for
>   *
>   * Reads the current cpufreq policy.
>   */
> @@ -2759,7 +2761,7 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_register_driver);
>  
> -/**
> +/*
>   * cpufreq_unregister_driver - unregister the current CPUFreq driver

And this should be there for sure.

>   *
>   * Unregister the current CPUFreq driver. Only call this if you have
> -- 
> 2.25.1

-- 
viresh

Re: [PATCH 02/13] cpufreq: cpufreq: Demote lots of function headers unworthy of kerneldoc status

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 14-07-20, 15:50, Lee Jones wrote:
> > -/**
> > +/*
> >   * cpufreq_remove_dev - remove a CPU device
> 
> Because cpufreq_add_dev() is part of kernel doc, we better keep it.
> 
> >   *
> >   * Removes the cpufreq interface for a CPU device.
> > @@ -2373,6 +2374,7 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
> >   * cpufreq_get_policy - get the current cpufreq_policy
> >   * @policy: struct cpufreq_policy into which the current cpufreq_policy
> >   *	is written
> > + * @cpu: CPU to find the policy for
> >   *
> >   * Reads the current cpufreq policy.
> >   */
> > @@ -2759,7 +2761,7 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> >  }
> >  EXPORT_SYMBOL_GPL(cpufreq_register_driver);
> >  
> > -/**
> > +/*
> >   * cpufreq_unregister_driver - unregister the current CPUFreq driver
> 
> And this should be there for sure.

Where is the *.rst file that references this kerneldoc entry?

Also, what do you mean by "there"?  We're not removing the function
header.  It's still documented, it's just not kerneldoc.

> >   *
> >   * Unregister the current CPUFreq driver. Only call this if you have
> 

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 02/13] cpufreq: cpufreq: Demote lots of function headers unworthy of kerneldoc status

[Viresh Kumar]

On 15-07-20, 07:47, Lee Jones wrote:
> On Wed, 15 Jul 2020, Viresh Kumar wrote:
> 
> > On 14-07-20, 15:50, Lee Jones wrote:
> > > -/**
> > > +/*
> > >   * cpufreq_remove_dev - remove a CPU device
> > 
> > Because cpufreq_add_dev() is part of kernel doc, we better keep it.
> > 
> > >   *
> > >   * Removes the cpufreq interface for a CPU device.
> > > @@ -2373,6 +2374,7 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
> > >   * cpufreq_get_policy - get the current cpufreq_policy
> > >   * @policy: struct cpufreq_policy into which the current cpufreq_policy
> > >   *	is written
> > > + * @cpu: CPU to find the policy for
> > >   *
> > >   * Reads the current cpufreq policy.
> > >   */
> > > @@ -2759,7 +2761,7 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> > >  }
> > >  EXPORT_SYMBOL_GPL(cpufreq_register_driver);
> > >  
> > > -/**
> > > +/*
> > >   * cpufreq_unregister_driver - unregister the current CPUFreq driver
> > 
> > And this should be there for sure.
> 
> Where is the *.rst file that references this kerneldoc entry?
> 
> Also, what do you mean by "there"?  We're not removing the function
> header.  It's still documented, it's just not kerneldoc.

Yeah, I meant from kernel-doc by "there".

Lets conclude the discussion on the other patch for what should be in
kernel-doc.

> > >   *
> > >   * Unregister the current CPUFreq driver. Only call this if you have
> > 
> 
> -- 
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

-- 
viresh

[PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Lee Jones]

There is no need for this to be denoted as kerneldoc.

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'attr_set' not described in 'store_sampling_rate'
 drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'buf' not described in 'store_sampling_rate'
 drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'count' not described in 'store_sampling_rate'

Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Alexander Clouter <alex@digriz.org.uk>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/cpufreq_governor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index f99ae45efaea7..63f7c219062b9 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -26,7 +26,7 @@ static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
 static DEFINE_MUTEX(gov_dbs_data_mutex);
 
 /* Common sysfs tunables */
-/**
+/*
  * store_sampling_rate - update sampling rate effective immediately if needed.
  *
  * If new rate is smaller than the old, simply updating
-- 
2.25.1

Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> There is no need for this to be denoted as kerneldoc.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'attr_set' not described in 'store_sampling_rate'
>  drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'buf' not described in 'store_sampling_rate'
>  drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'count' not described in 'store_sampling_rate'
> 
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Alexander Clouter <alex@digriz.org.uk>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/cpufreq_governor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index f99ae45efaea7..63f7c219062b9 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -26,7 +26,7 @@ static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
>  static DEFINE_MUTEX(gov_dbs_data_mutex);
>  
>  /* Common sysfs tunables */
> -/**
> +/*

This is an important routine with good documentation details already
there, though internal to governors and so I would rather keep it.

>   * store_sampling_rate - update sampling rate effective immediately if needed.
>   *
>   * If new rate is smaller than the old, simply updating
> -- 
> 2.25.1

-- 
viresh

Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 14-07-20, 15:50, Lee Jones wrote:
> > There is no need for this to be denoted as kerneldoc.
> > 
> > Fixes the following W=1 kernel build warning(s):
> > 
> >  drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'attr_set' not described in 'store_sampling_rate'
> >  drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'buf' not described in 'store_sampling_rate'
> >  drivers/cpufreq/cpufreq_governor.c:46: warning: Function parameter or member 'count' not described in 'store_sampling_rate'
> > 
> > Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > Cc: Jun Nakajima <jun.nakajima@intel.com>
> > Cc: Alexander Clouter <alex@digriz.org.uk>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/cpufreq/cpufreq_governor.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> > index f99ae45efaea7..63f7c219062b9 100644
> > --- a/drivers/cpufreq/cpufreq_governor.c
> > +++ b/drivers/cpufreq/cpufreq_governor.c
> > @@ -26,7 +26,7 @@ static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
> >  static DEFINE_MUTEX(gov_dbs_data_mutex);
> >  
> >  /* Common sysfs tunables */
> > -/**
> > +/*
> 
> This is an important routine with good documentation details already
> there, though internal to governors and so I would rather keep it.

It maybe documented, but it isn't kerneldoc, for 2 reasons; a) it
doesn't meet the standards required qualify as kerneldoc i.e. it's
missing descriptions for each of the function parameters, which is why
the kerneldoc checker is complaining about it and b) it is not
referenced by any *.rst file:

 git grep kernel-doc::.*cpufreq_governor.c
 /* no results */

So what is the justification for keeping it as kerneldoc?

> >   * store_sampling_rate - update sampling rate effective immediately if needed.
> >   *
> >   * If new rate is smaller than the old, simply updating
> 

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Viresh Kumar]

On 15-07-20, 07:45, Lee Jones wrote:
> On Wed, 15 Jul 2020, Viresh Kumar wrote:
> > On 14-07-20, 15:50, Lee Jones wrote:
> > > diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> > > index f99ae45efaea7..63f7c219062b9 100644
> > > --- a/drivers/cpufreq/cpufreq_governor.c
> > > +++ b/drivers/cpufreq/cpufreq_governor.c
> > > @@ -26,7 +26,7 @@ static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
> > >  static DEFINE_MUTEX(gov_dbs_data_mutex);
> > >  
> > >  /* Common sysfs tunables */
> > > -/**
> > > +/*
> > 
> > This is an important routine with good documentation details already
> > there, though internal to governors and so I would rather keep it.
> 
> It maybe documented, but it isn't kerneldoc, for 2 reasons; a) it
> doesn't meet the standards required qualify as kerneldoc i.e. it's
> missing descriptions for each of the function parameters, which is why
> the kerneldoc checker is complaining about it

Right, so this is a mistake and not intentional probably.

> and b) it is not
> referenced by any *.rst file:
> 
>  git grep kernel-doc::.*cpufreq_governor.c
>  /* no results */

I believed (and it may be wrong) that there are two categories of
routines/structures which can be put in kernel documentation, the
exported ones and the internal ones which are important and are very
useful in understanding the algorithms/logic in the drivers.

I did try to go and look into Documentation/doc-guide/ but couldn't
find any details on this.

You said that it needs to be referenced from some *.rst file, but why
is that necessary ? What if people don't add any documentation in
Documentation/ for their framework or driver but still want stuff to
appear in kernel-doc as they can keep the documentation in comments
more up to date.

-- 
viresh

Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 15-07-20, 07:45, Lee Jones wrote:
> > On Wed, 15 Jul 2020, Viresh Kumar wrote:
> > > On 14-07-20, 15:50, Lee Jones wrote:
> > > > diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> > > > index f99ae45efaea7..63f7c219062b9 100644
> > > > --- a/drivers/cpufreq/cpufreq_governor.c
> > > > +++ b/drivers/cpufreq/cpufreq_governor.c
> > > > @@ -26,7 +26,7 @@ static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
> > > >  static DEFINE_MUTEX(gov_dbs_data_mutex);
> > > >  
> > > >  /* Common sysfs tunables */
> > > > -/**
> > > > +/*
> > > 
> > > This is an important routine with good documentation details already
> > > there, though internal to governors and so I would rather keep it.
> > 
> > It maybe documented, but it isn't kerneldoc, for 2 reasons; a) it
> > doesn't meet the standards required qualify as kerneldoc i.e. it's
> > missing descriptions for each of the function parameters, which is why
> > the kerneldoc checker is complaining about it
> 
> Right, so this is a mistake and not intentional probably.
> 
> > and b) it is not
> > referenced by any *.rst file:
> > 
> >  git grep kernel-doc::.*cpufreq_governor.c
> >  /* no results */
> 
> I believed (and it may be wrong) that there are two categories of
> routines/structures which can be put in kernel documentation, the
> exported ones and the internal ones which are important and are very
> useful in understanding the algorithms/logic in the drivers.
> 
> I did try to go and look into Documentation/doc-guide/ but couldn't
> find any details on this.
> 
> You said that it needs to be referenced from some *.rst file, but why
> is that necessary ? What if people don't add any documentation in
> Documentation/ for their framework or driver but still want stuff to
> appear in kernel-doc as they can keep the documentation in comments
> more up to date.

I'm not sure what you mean.  Kerneldoc headers are designed to be
extracted and converted into mediums which are easy to read/browse.
For example, see the online documentation for 'debug_object_init':

 https://www.kernel.org/doc/html/latest/core-api/debug-objects.html?highlight=debug_object_init#c.debug_object_init

They are generally meant to be referenced/consumed.  There is even a
script provided inside the kernel to find offending instances where
kerneldoc headers are provided, but not *yet* referenced:

 `scripts/find-unused-docs.sh`

HINT: There are many.

There *could* be and argument to use kerneldoc *just* so you can use
the kerneldoc checker `scripts/kernel-doc` (which is invoked by W=1
builds), in order to ensure the parameter descriptions are kept in
check.

However, in this case, there are no descriptions provided.  So, in
reference to my previous question, what are your reasons for wanting
to keep kerneldoc here?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Viresh Kumar]

On 15-07-20, 08:31, Lee Jones wrote:
> I'm not sure what you mean.  Kerneldoc headers are designed to be
> extracted and converted into mediums which are easy to read/browse.
> For example, see the online documentation for 'debug_object_init':
> 
>  https://www.kernel.org/doc/html/latest/core-api/debug-objects.html?highlight=debug_object_init#c.debug_object_init
> 
> They are generally meant to be referenced/consumed.  There is even a
> script provided inside the kernel to find offending instances where
> kerneldoc headers are provided, but not *yet* referenced:
> 
>  `scripts/find-unused-docs.sh`
> 
> HINT: There are many.
> 
> There *could* be and argument to use kerneldoc *just* so you can use
> the kerneldoc checker `scripts/kernel-doc` (which is invoked by W=1
> builds), in order to ensure the parameter descriptions are kept in
> check.
> 
> However, in this case, there are no descriptions provided.  So, in
> reference to my previous question, what are your reasons for wanting
> to keep kerneldoc here?

I think the code did the right thing by keeping them as kernel doc
type comments. What we missed then is getting them used in the *.rst
documentation.

A simple way of doing that could be just adding this to the cpu-freq
rst file, like:

-------------------------8<-------------------------
Here are the bits from the in-source documentation:

.. kernel-doc:: include/linux/cpufreq.h
.. kernel-doc:: drivers/cpufreq/cpufreq.c
.. kernel-doc:: drivers/cpufreq/freq_table.c
.. kernel-doc:: drivers/cpufreq/cpufreq_governor.c
-------------------------8<-------------------------

This will make the script stop complaining about these. But the layout
of things wont' be very nice right now.

-- 
viresh

Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 15-07-20, 08:31, Lee Jones wrote:
> > I'm not sure what you mean.  Kerneldoc headers are designed to be
> > extracted and converted into mediums which are easy to read/browse.
> > For example, see the online documentation for 'debug_object_init':
> > 
> >  https://www.kernel.org/doc/html/latest/core-api/debug-objects.html?highlight=debug_object_init#c.debug_object_init
> > 
> > They are generally meant to be referenced/consumed.  There is even a
> > script provided inside the kernel to find offending instances where
> > kerneldoc headers are provided, but not *yet* referenced:
> > 
> >  `scripts/find-unused-docs.sh`
> > 
> > HINT: There are many.
> > 
> > There *could* be and argument to use kerneldoc *just* so you can use
> > the kerneldoc checker `scripts/kernel-doc` (which is invoked by W=1
> > builds), in order to ensure the parameter descriptions are kept in
> > check.
> > 
> > However, in this case, there are no descriptions provided.  So, in
> > reference to my previous question, what are your reasons for wanting
> > to keep kerneldoc here?
> 
> I think the code did the right thing by keeping them as kernel doc
> type comments. What we missed then is getting them used in the *.rst
> documentation.
> 
> A simple way of doing that could be just adding this to the cpu-freq
> rst file, like:
> 
> -------------------------8<-------------------------
> Here are the bits from the in-source documentation:
> 
> .. kernel-doc:: include/linux/cpufreq.h
> .. kernel-doc:: drivers/cpufreq/cpufreq.c
> .. kernel-doc:: drivers/cpufreq/freq_table.c
> .. kernel-doc:: drivers/cpufreq/cpufreq_governor.c
> -------------------------8<-------------------------
> 
> This will make the script stop complaining about these.

This will stop `scripts/find-unused-docs.sh` from mentioning these
files as an offender, but `scripts/kernel-doc` and by extension W=1
builds (which is the point of this patch-set) will still complain.

Before you add the lines above, you need to provide descriptions for
each of the function parameters or else they will not reach the
required standards expected of kerneldoc.

My suggestion would be to take this (and the other) patch and
subsequently provide your own set i) providing the required parameter
descriptions ii) re-promoting the comment blocks to kerneldoc and iii)
adding the aforementioned lines to the *.rst file(s).

> But the layout of things wont' be very nice right now.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 04/13] cpufreq: sti-cpufreq: Fix some formatting and misspelling issues

[Lee Jones]

Kerneldoc format for attribute descriptions should be '@.*: '.

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/sti-cpufreq.c:49: warning: cannot understand function prototype: 'struct sti_cpufreq_ddata '

Cc: Patrice Chotard <patrice.chotard@st.com>
Cc: Pal Singh <ajitpal.singh@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/sti-cpufreq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c
index 8f16bbb164b84..a5ad96d29adca 100644
--- a/drivers/cpufreq/sti-cpufreq.c
+++ b/drivers/cpufreq/sti-cpufreq.c
@@ -40,11 +40,11 @@ enum {
 };
 
 /**
- * ST CPUFreq Driver Data
+ * struct sti_cpufreq_ddata - ST CPUFreq Driver Data
  *
- * @cpu_node		CPU's OF node
- * @syscfg_eng		Engineering Syscon register map
- * @regmap		Syscon register map
+ * @cpu:		CPU's OF node
+ * @syscfg_eng:		Engineering Syscon register map
+ * @syscfg:		Syscon register map
  */
 static struct sti_cpufreq_ddata {
 	struct device *cpu;
-- 
2.25.1

Re: [PATCH 04/13] cpufreq: sti-cpufreq: Fix some formatting and misspelling issues

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> Kerneldoc format for attribute descriptions should be '@.*: '.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/sti-cpufreq.c:49: warning: cannot understand function prototype: 'struct sti_cpufreq_ddata '
> 
> Cc: Patrice Chotard <patrice.chotard@st.com>
> Cc: Pal Singh <ajitpal.singh@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/sti-cpufreq.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c
> index 8f16bbb164b84..a5ad96d29adca 100644
> --- a/drivers/cpufreq/sti-cpufreq.c
> +++ b/drivers/cpufreq/sti-cpufreq.c
> @@ -40,11 +40,11 @@ enum {
>  };
>  
>  /**
> - * ST CPUFreq Driver Data
> + * struct sti_cpufreq_ddata - ST CPUFreq Driver Data
>   *
> - * @cpu_node		CPU's OF node
> - * @syscfg_eng		Engineering Syscon register map
> - * @regmap		Syscon register map
> + * @cpu:		CPU's OF node
> + * @syscfg_eng:		Engineering Syscon register map
> + * @syscfg:		Syscon register map
>   */
>  static struct sti_cpufreq_ddata {
>  	struct device *cpu;

Applied. Thanks.

-- 
viresh

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

[Lee Jones]

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 <olof@lixom.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/powerpc/platforms/pasemi/pasemi.h    | 15 ------------
 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

diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
index 70b56048ed1be..528d81ef748ad 100644
--- a/arch/powerpc/platforms/pasemi/pasemi.h
+++ b/arch/powerpc/platforms/pasemi/pasemi.h
@@ -15,21 +15,6 @@ extern void __init pasemi_map_registers(void);
 extern void idle_spin(void);
 extern void idle_doze(void);
 
-/* Restore astate to last set */
-#ifdef CONFIG_PPC_PASEMI_CPUFREQ
-extern int check_astate(void);
-extern void restore_astate(int cpu);
-#else
-static inline int check_astate(void)
-{
-	/* Always return >0 so we never power save */
-	return 1;
-}
-static inline void restore_astate(int cpu)
-{
-}
-#endif
-
 extern struct pci_controller_ops pasemi_pci_controller_ops;
 
 #endif /* _PASEMI_PASEMI_H */
diff --git a/arch/powerpc/platforms/pasemi/powersave.S b/arch/powerpc/platforms/pasemi/powersave.S
index d0215d5329ca7..7747b48963286 100644
--- a/arch/powerpc/platforms/pasemi/powersave.S
+++ b/arch/powerpc/platforms/pasemi/powersave.S
@@ -5,6 +5,8 @@
  * Maintained by: Olof Johansson <olof@lixom.net>
  */
 
+#include <linux/platform_data/pasemi.h>
+
 #include <asm/processor.h>
 #include <asm/page.h>
 #include <asm/ppc_asm.h>
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index c66f566a854cb..c6bb3ecc90ef3 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -15,6 +15,7 @@
 #include <linux/timer.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/platform_data/pasemi.h>
 
 #include <asm/hw_irq.h>
 #include <asm/io.h>
diff --git a/include/linux/platform_data/pasemi.h b/include/linux/platform_data/pasemi.h
new file mode 100644
index 0000000000000..3fed0687fcc9a
--- /dev/null
+++ b/include/linux/platform_data/pasemi.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Linaro Ltd.
+ *
+ * Author: Lee Jones <lee.jones@linaro.org>
+ */
+
+#ifndef _LINUX_PLATFORM_DATA_PASEMI_H
+#define _LINUX_PLATFORM_DATA_PASEMI_H
+
+/* Restore astate to last set */
+#ifdef CONFIG_PPC_PASEMI_CPUFREQ
+int check_astate(void);
+void restore_astate(int cpu);
+#else
+static inline int check_astate(void)
+{
+	/* Always return >0 so we never power save */
+	return 1;
+}
+static inline void restore_astate(int cpu)
+{
+}
+#endif
+
+#endif /* _LINUX_PLATFORM_DATA_PASEMI_H */
+
+
-- 
2.25.1

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 <olof@lixom.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  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

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

[Olof Johansson]

On Tue, Jul 14, 2020 at 8:07 PM Viresh Kumar <viresh.kumar@linaro.org> 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 <olof@lixom.net>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  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 <asm/time.h>
 #include <asm/smp.h>

+#include <platforms/pasemi/pasemi.h>
+
 #define SDCASR_REG             0x0100
 #define SDCASR_REG_STRIDE      0x1000
 #define SDCPWR_CFGA0_REG       0x0100


-Olof

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 <viresh.kumar@linaro.org> 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 <olof@lixom.net>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  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 <asm/time.h>
>  #include <asm/smp.h>
> 
> +#include <platforms/pasemi/pasemi.h>
> +
>  #define SDCASR_REG             0x0100
>  #define SDCASR_REG_STRIDE      0x1000
>  #define SDCPWR_CFGA0_REG       0x0100

Fantastic. Thanks.

-- 
viresh

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

[Lee Jones]

On Tue, 14 Jul 2020, Olof Johansson wrote:

> On Tue, Jul 14, 2020 at 8:07 PM Viresh Kumar <viresh.kumar@linaro.org> 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 <olof@lixom.net>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  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 <asm/time.h>
>  #include <asm/smp.h>
> 
> +#include <platforms/pasemi/pasemi.h>
> +
>  #define SDCASR_REG             0x0100
>  #define SDCASR_REG_STRIDE      0x1000
>  #define SDCPWR_CFGA0_REG       0x0100

I searched for "include.*platforms/" in drivers/, and was scared off
this method since no one else does this.

But if it's a reasonable solution, great.  Will fix.

Thanks Olof.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Olof Johansson]

On Tue, Jul 14, 2020 at 7:50 AM Lee Jones <lee.jones@linaro.org> 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 <olof@lixom.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/powerpc/platforms/pasemi/pasemi.h    | 15 ------------
>  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
>
> diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
> index 70b56048ed1be..528d81ef748ad 100644
> --- a/arch/powerpc/platforms/pasemi/pasemi.h
> +++ b/arch/powerpc/platforms/pasemi/pasemi.h
> @@ -15,21 +15,6 @@ extern void __init pasemi_map_registers(void);
>  extern void idle_spin(void);
>  extern void idle_doze(void);
>
> -/* Restore astate to last set */
> -#ifdef CONFIG_PPC_PASEMI_CPUFREQ
> -extern int check_astate(void);
> -extern void restore_astate(int cpu);
> -#else
> -static inline int check_astate(void)
> -{
> -       /* Always return >0 so we never power save */
> -       return 1;
> -}
> -static inline void restore_astate(int cpu)
> -{
> -}
> -#endif
> -
>  extern struct pci_controller_ops pasemi_pci_controller_ops;
>
>  #endif /* _PASEMI_PASEMI_H */
> diff --git a/arch/powerpc/platforms/pasemi/powersave.S b/arch/powerpc/platforms/pasemi/powersave.S
> index d0215d5329ca7..7747b48963286 100644
> --- a/arch/powerpc/platforms/pasemi/powersave.S
> +++ b/arch/powerpc/platforms/pasemi/powersave.S
> @@ -5,6 +5,8 @@
>   * Maintained by: Olof Johansson <olof@lixom.net>
>   */
>
> +#include <linux/platform_data/pasemi.h>
> +
>  #include <asm/processor.h>
>  #include <asm/page.h>
>  #include <asm/ppc_asm.h>
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> index c66f566a854cb..c6bb3ecc90ef3 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -15,6 +15,7 @@
>  #include <linux/timer.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
> +#include <linux/platform_data/pasemi.h>
>
>  #include <asm/hw_irq.h>
>  #include <asm/io.h>
> diff --git a/include/linux/platform_data/pasemi.h b/include/linux/platform_data/pasemi.h
> new file mode 100644
> index 0000000000000..3fed0687fcc9a
> --- /dev/null
> +++ b/include/linux/platform_data/pasemi.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Linaro Ltd.
> + *
> + * Author: Lee Jones <lee.jones@linaro.org>
> + */

Absolutely not. It's neither your copyright, nor your authorship.


-Olof

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

[Lee Jones]

On Tue, 14 Jul 2020, Olof Johansson wrote:

> On Tue, Jul 14, 2020 at 7:50 AM Lee Jones <lee.jones@linaro.org> 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 <olof@lixom.net>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/powerpc/platforms/pasemi/pasemi.h    | 15 ------------
> >  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
> >
> > diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
> > index 70b56048ed1be..528d81ef748ad 100644
> > --- a/arch/powerpc/platforms/pasemi/pasemi.h
> > +++ b/arch/powerpc/platforms/pasemi/pasemi.h
> > @@ -15,21 +15,6 @@ extern void __init pasemi_map_registers(void);
> >  extern void idle_spin(void);
> >  extern void idle_doze(void);
> >
> > -/* Restore astate to last set */
> > -#ifdef CONFIG_PPC_PASEMI_CPUFREQ
> > -extern int check_astate(void);
> > -extern void restore_astate(int cpu);
> > -#else
> > -static inline int check_astate(void)
> > -{
> > -       /* Always return >0 so we never power save */
> > -       return 1;
> > -}
> > -static inline void restore_astate(int cpu)
> > -{
> > -}
> > -#endif
> > -
> >  extern struct pci_controller_ops pasemi_pci_controller_ops;
> >
> >  #endif /* _PASEMI_PASEMI_H */
> > diff --git a/arch/powerpc/platforms/pasemi/powersave.S b/arch/powerpc/platforms/pasemi/powersave.S
> > index d0215d5329ca7..7747b48963286 100644
> > --- a/arch/powerpc/platforms/pasemi/powersave.S
> > +++ b/arch/powerpc/platforms/pasemi/powersave.S
> > @@ -5,6 +5,8 @@
> >   * Maintained by: Olof Johansson <olof@lixom.net>
> >   */
> >
> > +#include <linux/platform_data/pasemi.h>
> > +
> >  #include <asm/processor.h>
> >  #include <asm/page.h>
> >  #include <asm/ppc_asm.h>
> > diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> > index c66f566a854cb..c6bb3ecc90ef3 100644
> > --- a/drivers/cpufreq/pasemi-cpufreq.c
> > +++ b/drivers/cpufreq/pasemi-cpufreq.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/timer.h>
> >  #include <linux/module.h>
> >  #include <linux/of_address.h>
> > +#include <linux/platform_data/pasemi.h>
> >
> >  #include <asm/hw_irq.h>
> >  #include <asm/io.h>
> > diff --git a/include/linux/platform_data/pasemi.h b/include/linux/platform_data/pasemi.h
> > new file mode 100644
> > index 0000000000000..3fed0687fcc9a
> > --- /dev/null
> > +++ b/include/linux/platform_data/pasemi.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2020 Linaro Ltd.
> > + *
> > + * Author: Lee Jones <lee.jones@linaro.org>
> > + */
> 
> Absolutely not. It's neither your copyright, nor your authorship.

The file was new.  Anyway, the point is now moot.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Olof Johansson]

On Tue, Jul 14, 2020 at 11:33 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Tue, 14 Jul 2020, Olof Johansson wrote:
>
> > On Tue, Jul 14, 2020 at 7:50 AM Lee Jones <lee.jones@linaro.org> 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 <olof@lixom.net>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  arch/powerpc/platforms/pasemi/pasemi.h    | 15 ------------
> > >  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
> > >
> > > diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
> > > index 70b56048ed1be..528d81ef748ad 100644
> > > --- a/arch/powerpc/platforms/pasemi/pasemi.h
> > > +++ b/arch/powerpc/platforms/pasemi/pasemi.h
> > > @@ -15,21 +15,6 @@ extern void __init pasemi_map_registers(void);
> > >  extern void idle_spin(void);
> > >  extern void idle_doze(void);
> > >
> > > -/* Restore astate to last set */
> > > -#ifdef CONFIG_PPC_PASEMI_CPUFREQ
> > > -extern int check_astate(void);
> > > -extern void restore_astate(int cpu);
> > > -#else
> > > -static inline int check_astate(void)
> > > -{
> > > -       /* Always return >0 so we never power save */
> > > -       return 1;
> > > -}
> > > -static inline void restore_astate(int cpu)
> > > -{
> > > -}
> > > -#endif
> > > -
> > >  extern struct pci_controller_ops pasemi_pci_controller_ops;
> > >
> > >  #endif /* _PASEMI_PASEMI_H */
> > > diff --git a/arch/powerpc/platforms/pasemi/powersave.S b/arch/powerpc/platforms/pasemi/powersave.S
> > > index d0215d5329ca7..7747b48963286 100644
> > > --- a/arch/powerpc/platforms/pasemi/powersave.S
> > > +++ b/arch/powerpc/platforms/pasemi/powersave.S
> > > @@ -5,6 +5,8 @@
> > >   * Maintained by: Olof Johansson <olof@lixom.net>
> > >   */
> > >
> > > +#include <linux/platform_data/pasemi.h>
> > > +
> > >  #include <asm/processor.h>
> > >  #include <asm/page.h>
> > >  #include <asm/ppc_asm.h>
> > > diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> > > index c66f566a854cb..c6bb3ecc90ef3 100644
> > > --- a/drivers/cpufreq/pasemi-cpufreq.c
> > > +++ b/drivers/cpufreq/pasemi-cpufreq.c
> > > @@ -15,6 +15,7 @@
> > >  #include <linux/timer.h>
> > >  #include <linux/module.h>
> > >  #include <linux/of_address.h>
> > > +#include <linux/platform_data/pasemi.h>
> > >
> > >  #include <asm/hw_irq.h>
> > >  #include <asm/io.h>
> > > diff --git a/include/linux/platform_data/pasemi.h b/include/linux/platform_data/pasemi.h
> > > new file mode 100644
> > > index 0000000000000..3fed0687fcc9a
> > > --- /dev/null
> > > +++ b/include/linux/platform_data/pasemi.h
> > > @@ -0,0 +1,28 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2020 Linaro Ltd.
> > > + *
> > > + * Author: Lee Jones <lee.jones@linaro.org>
> > > + */
> >
> > Absolutely not. It's neither your copyright, nor your authorship.
>
> The file was new.  Anyway, the point is now moot.

The contents was copied and pasted from other material, not originally
produced by you.

I suggest you consult with Linaro lawyers on how to handle this if you
have to do something like it in the future.


-Olof

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

[Lee Jones]

On Tue, 14 Jul 2020, Olof Johansson wrote:

> On Tue, Jul 14, 2020 at 11:33 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Tue, 14 Jul 2020, Olof Johansson wrote:
> >
> > > On Tue, Jul 14, 2020 at 7:50 AM Lee Jones <lee.jones@linaro.org> 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 <olof@lixom.net>
> > > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > > Cc: Paul Mackerras <paulus@samba.org>
> > > > Cc: linuxppc-dev@lists.ozlabs.org
> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > ---
> > > >  arch/powerpc/platforms/pasemi/pasemi.h    | 15 ------------
> > > >  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
> > > >
> > > > diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
> > > > index 70b56048ed1be..528d81ef748ad 100644
> > > > --- a/arch/powerpc/platforms/pasemi/pasemi.h
> > > > +++ b/arch/powerpc/platforms/pasemi/pasemi.h
> > > > @@ -15,21 +15,6 @@ extern void __init pasemi_map_registers(void);
> > > >  extern void idle_spin(void);
> > > >  extern void idle_doze(void);
> > > >
> > > > -/* Restore astate to last set */
> > > > -#ifdef CONFIG_PPC_PASEMI_CPUFREQ
> > > > -extern int check_astate(void);
> > > > -extern void restore_astate(int cpu);
> > > > -#else
> > > > -static inline int check_astate(void)
> > > > -{
> > > > -       /* Always return >0 so we never power save */
> > > > -       return 1;
> > > > -}
> > > > -static inline void restore_astate(int cpu)
> > > > -{
> > > > -}
> > > > -#endif
> > > > -
> > > >  extern struct pci_controller_ops pasemi_pci_controller_ops;
> > > >
> > > >  #endif /* _PASEMI_PASEMI_H */
> > > > diff --git a/arch/powerpc/platforms/pasemi/powersave.S b/arch/powerpc/platforms/pasemi/powersave.S
> > > > index d0215d5329ca7..7747b48963286 100644
> > > > --- a/arch/powerpc/platforms/pasemi/powersave.S
> > > > +++ b/arch/powerpc/platforms/pasemi/powersave.S
> > > > @@ -5,6 +5,8 @@
> > > >   * Maintained by: Olof Johansson <olof@lixom.net>
> > > >   */
> > > >
> > > > +#include <linux/platform_data/pasemi.h>
> > > > +
> > > >  #include <asm/processor.h>
> > > >  #include <asm/page.h>
> > > >  #include <asm/ppc_asm.h>
> > > > diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> > > > index c66f566a854cb..c6bb3ecc90ef3 100644
> > > > --- a/drivers/cpufreq/pasemi-cpufreq.c
> > > > +++ b/drivers/cpufreq/pasemi-cpufreq.c
> > > > @@ -15,6 +15,7 @@
> > > >  #include <linux/timer.h>
> > > >  #include <linux/module.h>
> > > >  #include <linux/of_address.h>
> > > > +#include <linux/platform_data/pasemi.h>
> > > >
> > > >  #include <asm/hw_irq.h>
> > > >  #include <asm/io.h>
> > > > diff --git a/include/linux/platform_data/pasemi.h b/include/linux/platform_data/pasemi.h
> > > > new file mode 100644
> > > > index 0000000000000..3fed0687fcc9a
> > > > --- /dev/null
> > > > +++ b/include/linux/platform_data/pasemi.h
> > > > @@ -0,0 +1,28 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > > +/*
> > > > + * Copyright (C) 2020 Linaro Ltd.
> > > > + *
> > > > + * Author: Lee Jones <lee.jones@linaro.org>
> > > > + */
> > >
> > > Absolutely not. It's neither your copyright, nor your authorship.
> >
> > The file was new.  Anyway, the point is now moot.
> 
> The contents was copied and pasted from other material, not originally
> produced by you.
> 
> I suggest you consult with Linaro lawyers on how to handle this if you
> have to do something like it in the future.

Very well.  Thanks for the heads-up.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Lee Jones]

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 <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 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;
-- 
2.25.1

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 <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  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.kumar@linaro.org>

-- 
viresh

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

[Lee Jones]

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 <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 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
-- 
2.25.1

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 <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  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.kumar@linaro.org>

-- 
viresh

[PATCH 08/13] cpufreq: acpi-cpufreq: Take 'dummy' principle one stage further

[Lee Jones]

If we fail to use a variable, even a 'dummy' one, then the compiler
complains that it is set but not used.  We know this is fine, so we
set it to its own value here.

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_intel’:
 drivers/cpufreq/acpi-cpufreq.c:247:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
 drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_amd’:
 drivers/cpufreq/acpi-cpufreq.c:265:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]

Cc: Andy Grover <andrew.grover@intel.com>
Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
Cc: Dominik Brodowski <linux@brodo.de>
Cc: Denis Sadykov <denis.m.sadykov@intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/acpi-cpufreq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 429e5a36c08a9..d38a693b48e03 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -247,6 +247,7 @@ static u32 cpu_freq_read_intel(struct acpi_pct_register *not_used)
 	u32 val, dummy;
 
 	rdmsr(MSR_IA32_PERF_CTL, val, dummy);
+	dummy &= dummy; /* Silence set but not used warning */
 	return val;
 }
 
@@ -264,6 +265,7 @@ static u32 cpu_freq_read_amd(struct acpi_pct_register *not_used)
 	u32 val, dummy;
 
 	rdmsr(MSR_AMD_PERF_CTL, val, dummy);
+	dummy &= dummy; /* Silence set but not used warning */
 	return val;
 }
 
-- 
2.25.1

Re: [PATCH 08/13] cpufreq: acpi-cpufreq: Take 'dummy' principle one stage further

[Rafael J. Wysocki]

On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> If we fail to use a variable, even a 'dummy' one, then the compiler
> complains that it is set but not used.  We know this is fine, so we
> set it to its own value here.

Which is kind of ugly in my personal view.  I hope that the compiler
will actually optimize the extra code away ...

> Fixes the following W=1 kernel build warning(s):

Well, "Makes the following ... warning(s) go away:" rather ...

>  drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_intel’:
>  drivers/cpufreq/acpi-cpufreq.c:247:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
>  drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_amd’:
>  drivers/cpufreq/acpi-cpufreq.c:265:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
>
> Cc: Andy Grover <andrew.grover@intel.com>
> Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> Cc: Dominik Brodowski <linux@brodo.de>
> Cc: Denis Sadykov <denis.m.sadykov@intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 429e5a36c08a9..d38a693b48e03 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -247,6 +247,7 @@ static u32 cpu_freq_read_intel(struct acpi_pct_register *not_used)
>         u32 val, dummy;
>
>         rdmsr(MSR_IA32_PERF_CTL, val, dummy);
> +       dummy &= dummy; /* Silence set but not used warning */
>         return val;
>  }
>
> @@ -264,6 +265,7 @@ static u32 cpu_freq_read_amd(struct acpi_pct_register *not_used)
>         u32 val, dummy;
>
>         rdmsr(MSR_AMD_PERF_CTL, val, dummy);
> +       dummy &= dummy; /* Silence set but not used warning */
>         return val;
>  }
>
> --
> 2.25.1
>

Re: [PATCH 08/13] cpufreq: acpi-cpufreq: Take 'dummy' principle one stage further

[Robin Murphy]

On 2020-07-14 17:03, Rafael J. Wysocki wrote:
> On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>>
>> If we fail to use a variable, even a 'dummy' one, then the compiler
>> complains that it is set but not used.  We know this is fine, so we
>> set it to its own value here.
> 
> Which is kind of ugly in my personal view.  I hope that the compiler
> will actually optimize the extra code away ...
> 
>> Fixes the following W=1 kernel build warning(s):
> 
> Well, "Makes the following ... warning(s) go away:" rather ...

Isn't that what we have __maybe_unused and __always_unused for?

Robin.

>>   drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_intel’:
>>   drivers/cpufreq/acpi-cpufreq.c:247:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
>>   drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_amd’:
>>   drivers/cpufreq/acpi-cpufreq.c:265:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
>>
>> Cc: Andy Grover <andrew.grover@intel.com>
>> Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
>> Cc: Dominik Brodowski <linux@brodo.de>
>> Cc: Denis Sadykov <denis.m.sadykov@intel.com>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>   drivers/cpufreq/acpi-cpufreq.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
>> index 429e5a36c08a9..d38a693b48e03 100644
>> --- a/drivers/cpufreq/acpi-cpufreq.c
>> +++ b/drivers/cpufreq/acpi-cpufreq.c
>> @@ -247,6 +247,7 @@ static u32 cpu_freq_read_intel(struct acpi_pct_register *not_used)
>>          u32 val, dummy;
>>
>>          rdmsr(MSR_IA32_PERF_CTL, val, dummy);
>> +       dummy &= dummy; /* Silence set but not used warning */
>>          return val;
>>   }
>>
>> @@ -264,6 +265,7 @@ static u32 cpu_freq_read_amd(struct acpi_pct_register *not_used)
>>          u32 val, dummy;
>>
>>          rdmsr(MSR_AMD_PERF_CTL, val, dummy);
>> +       dummy &= dummy; /* Silence set but not used warning */
>>          return val;
>>   }
>>
>> --
>> 2.25.1
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 08/13] cpufreq: acpi-cpufreq: Take 'dummy' principle one stage further

[Lee Jones]

On Tue, 14 Jul 2020, Robin Murphy wrote:

> On 2020-07-14 17:03, Rafael J. Wysocki wrote:
> > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > 
> > > If we fail to use a variable, even a 'dummy' one, then the compiler
> > > complains that it is set but not used.  We know this is fine, so we
> > > set it to its own value here.
> > 
> > Which is kind of ugly in my personal view.  I hope that the compiler
> > will actually optimize the extra code away ...
> > 
> > > Fixes the following W=1 kernel build warning(s):
> > 
> > Well, "Makes the following ... warning(s) go away:" rather ...
> 
> Isn't that what we have __maybe_unused and __always_unused for?

Yes, that would be better.

I didn't know you could do that with standard variables.

Will fix.

> > >   drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_intel’:
> > >   drivers/cpufreq/acpi-cpufreq.c:247:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
> > >   drivers/cpufreq/acpi-cpufreq.c: In function ‘cpu_freq_read_amd’:
> > >   drivers/cpufreq/acpi-cpufreq.c:265:11: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
> > > 
> > > Cc: Andy Grover <andrew.grover@intel.com>
> > > Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> > > Cc: Dominik Brodowski <linux@brodo.de>
> > > Cc: Denis Sadykov <denis.m.sadykov@intel.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >   drivers/cpufreq/acpi-cpufreq.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> > > index 429e5a36c08a9..d38a693b48e03 100644
> > > --- a/drivers/cpufreq/acpi-cpufreq.c
> > > +++ b/drivers/cpufreq/acpi-cpufreq.c
> > > @@ -247,6 +247,7 @@ static u32 cpu_freq_read_intel(struct acpi_pct_register *not_used)
> > >          u32 val, dummy;
> > > 
> > >          rdmsr(MSR_IA32_PERF_CTL, val, dummy);
> > > +       dummy &= dummy; /* Silence set but not used warning */
> > >          return val;
> > >   }
> > > 
> > > @@ -264,6 +265,7 @@ static u32 cpu_freq_read_amd(struct acpi_pct_register *not_used)
> > >          u32 val, dummy;
> > > 
> > >          rdmsr(MSR_AMD_PERF_CTL, val, dummy);
> > > +       dummy &= dummy; /* Silence set but not used warning */
> > >          return val;
> > >   }
> > > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

Can't see them being used anywhere and the compiler doesn't complain
that they're missing, so ...

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/acpi-cpufreq.c:1004:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
 997 | static const struct x86_cpu_id acpi_cpufreq_ids[] = {
 | ^~~~~~~~~~~~~~~~
 drivers/cpufreq/acpi-cpufreq.c:997:32: warning: ‘acpi_cpufreq_ids’ defined but not used [-Wunused-const-variable=]
 619 | static const struct acpi_device_id processor_device_ids[] = {
 | ^~~~~~~~~~~~~~~~~~~~

Cc: Andy Grover <andrew.grover@intel.com>
Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
Cc: Dominik Brodowski <linux@brodo.de>
Cc: Denis Sadykov <denis.m.sadykov@intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/acpi-cpufreq.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index d38a693b48e03..fc68f13352695 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -995,18 +995,4 @@ MODULE_PARM_DESC(acpi_pstate_strict,
 late_initcall(acpi_cpufreq_init);
 module_exit(acpi_cpufreq_exit);
 
-static const struct x86_cpu_id acpi_cpufreq_ids[] = {
-	X86_MATCH_FEATURE(X86_FEATURE_ACPI, NULL),
-	X86_MATCH_FEATURE(X86_FEATURE_HW_PSTATE, NULL),
-	{}
-};
-MODULE_DEVICE_TABLE(x86cpu, acpi_cpufreq_ids);
-
-static const struct acpi_device_id processor_device_ids[] = {
-	{ACPI_PROCESSOR_OBJECT_HID, },
-	{ACPI_PROCESSOR_DEVICE_HID, },
-	{},
-};
-MODULE_DEVICE_TABLE(acpi, processor_device_ids);
-
 MODULE_ALIAS("acpi");
-- 
2.25.1

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Rafael J. Wysocki]

On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> Can't see them being used anywhere and the compiler doesn't complain
> that they're missing, so ...

Aren't they needed for automatic module loading in certain configurations?

> Fixes the following W=1 kernel build warning(s):
>
>  drivers/cpufreq/acpi-cpufreq.c:1004:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
>  997 | static const struct x86_cpu_id acpi_cpufreq_ids[] = {
>  | ^~~~~~~~~~~~~~~~
>  drivers/cpufreq/acpi-cpufreq.c:997:32: warning: ‘acpi_cpufreq_ids’ defined but not used [-Wunused-const-variable=]
>  619 | static const struct acpi_device_id processor_device_ids[] = {
>  | ^~~~~~~~~~~~~~~~~~~~
>
> Cc: Andy Grover <andrew.grover@intel.com>
> Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> Cc: Dominik Brodowski <linux@brodo.de>
> Cc: Denis Sadykov <denis.m.sadykov@intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 14 --------------
>  1 file changed, 14 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index d38a693b48e03..fc68f13352695 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -995,18 +995,4 @@ MODULE_PARM_DESC(acpi_pstate_strict,
>  late_initcall(acpi_cpufreq_init);
>  module_exit(acpi_cpufreq_exit);
>
> -static const struct x86_cpu_id acpi_cpufreq_ids[] = {
> -       X86_MATCH_FEATURE(X86_FEATURE_ACPI, NULL),
> -       X86_MATCH_FEATURE(X86_FEATURE_HW_PSTATE, NULL),
> -       {}
> -};
> -MODULE_DEVICE_TABLE(x86cpu, acpi_cpufreq_ids);
> -
> -static const struct acpi_device_id processor_device_ids[] = {
> -       {ACPI_PROCESSOR_OBJECT_HID, },
> -       {ACPI_PROCESSOR_DEVICE_HID, },
> -       {},
> -};
> -MODULE_DEVICE_TABLE(acpi, processor_device_ids);
> -
>  MODULE_ALIAS("acpi");
> --
> 2.25.1
>

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:

> On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > Can't see them being used anywhere and the compiler doesn't complain
> > that they're missing, so ...
> 
> Aren't they needed for automatic module loading in certain configurations?

Any idea how that works, or where the code is for that?

> > Fixes the following W=1 kernel build warning(s):
> >
> >  drivers/cpufreq/acpi-cpufreq.c:1004:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
> >  997 | static const struct x86_cpu_id acpi_cpufreq_ids[] = {
> >  | ^~~~~~~~~~~~~~~~
> >  drivers/cpufreq/acpi-cpufreq.c:997:32: warning: ‘acpi_cpufreq_ids’ defined but not used [-Wunused-const-variable=]
> >  619 | static const struct acpi_device_id processor_device_ids[] = {
> >  | ^~~~~~~~~~~~~~~~~~~~
> >
> > Cc: Andy Grover <andrew.grover@intel.com>
> > Cc: Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> > Cc: Dominik Brodowski <linux@brodo.de>
> > Cc: Denis Sadykov <denis.m.sadykov@intel.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/cpufreq/acpi-cpufreq.c | 14 --------------
> >  1 file changed, 14 deletions(-)
> >
> > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> > index d38a693b48e03..fc68f13352695 100644
> > --- a/drivers/cpufreq/acpi-cpufreq.c
> > +++ b/drivers/cpufreq/acpi-cpufreq.c
> > @@ -995,18 +995,4 @@ MODULE_PARM_DESC(acpi_pstate_strict,
> >  late_initcall(acpi_cpufreq_init);
> >  module_exit(acpi_cpufreq_exit);
> >
> > -static const struct x86_cpu_id acpi_cpufreq_ids[] = {
> > -       X86_MATCH_FEATURE(X86_FEATURE_ACPI, NULL),
> > -       X86_MATCH_FEATURE(X86_FEATURE_HW_PSTATE, NULL),
> > -       {}
> > -};
> > -MODULE_DEVICE_TABLE(x86cpu, acpi_cpufreq_ids);
> > -
> > -static const struct acpi_device_id processor_device_ids[] = {
> > -       {ACPI_PROCESSOR_OBJECT_HID, },
> > -       {ACPI_PROCESSOR_DEVICE_HID, },
> > -       {},
> > -};
> > -MODULE_DEVICE_TABLE(acpi, processor_device_ids);
> > -
> >  MODULE_ALIAS("acpi");
> >

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Viresh Kumar]

On 14-07-20, 22:03, Lee Jones wrote:
> On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> 
> > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > Can't see them being used anywhere and the compiler doesn't complain
> > > that they're missing, so ...
> > 
> > Aren't they needed for automatic module loading in certain configurations?
> 
> Any idea how that works, or where the code is for that?

The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
product-id that the kernel keeps after boot (and so there is no static
reference of it for the compiler), later when a device is hotplugged
into the kernel it refers to the map to find the related driver for it
and loads it if it isn't already loaded.

This has some of it, search for MODULE_DEVICE_TABLE() in it.
Documentation/driver-api/usb/hotplug.rst

-- 
viresh

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Viresh Kumar]

On 15-07-20, 08:54, Viresh Kumar wrote:
> On 14-07-20, 22:03, Lee Jones wrote:
> > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > 
> > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > >
> > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > that they're missing, so ...
> > > 
> > > Aren't they needed for automatic module loading in certain configurations?
> > 
> > Any idea how that works, or where the code is for that?
> 
> The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> product-id that the kernel keeps after boot (and so there is no static
> reference of it for the compiler), later when a device is hotplugged
> into the kernel it refers to the map to find the related driver for it
> and loads it if it isn't already loaded.
> 
> This has some of it, search for MODULE_DEVICE_TABLE() in it.
> Documentation/driver-api/usb/hotplug.rst

And you just need to add __maybe_unused to them to suppress the
warning.

-- 
viresh

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 15-07-20, 08:54, Viresh Kumar wrote:
> > On 14-07-20, 22:03, Lee Jones wrote:
> > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > 
> > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > >
> > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > that they're missing, so ...
> > > > 
> > > > Aren't they needed for automatic module loading in certain configurations?
> > > 
> > > Any idea how that works, or where the code is for that?
> > 
> > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > product-id that the kernel keeps after boot (and so there is no static
> > reference of it for the compiler), later when a device is hotplugged
> > into the kernel it refers to the map to find the related driver for it
> > and loads it if it isn't already loaded.
> > 
> > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > Documentation/driver-api/usb/hotplug.rst
> 
> And you just need to add __maybe_unused to them to suppress the
> warning.

Right.  That's Plan B.

Will fix.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Rafael J. Wysocki]

On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 15-07-20, 08:54, Viresh Kumar wrote:
> > On 14-07-20, 22:03, Lee Jones wrote:
> > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > >
> > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > >
> > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > that they're missing, so ...
> > > >
> > > > Aren't they needed for automatic module loading in certain configurations?
> > >
> > > Any idea how that works, or where the code is for that?
> >
> > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > product-id that the kernel keeps after boot (and so there is no static
> > reference of it for the compiler), later when a device is hotplugged
> > into the kernel it refers to the map to find the related driver for it
> > and loads it if it isn't already loaded.
> >
> > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > Documentation/driver-api/usb/hotplug.rst
>
> And you just need to add __maybe_unused to them to suppress the
> warning.

Wouldn't that cause the compiler to optimize them away if it doesn't
see any users?

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:

> On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > >
> > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > >
> > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > that they're missing, so ...
> > > > >
> > > > > Aren't they needed for automatic module loading in certain configurations?
> > > >
> > > > Any idea how that works, or where the code is for that?
> > >
> > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > product-id that the kernel keeps after boot (and so there is no static
> > > reference of it for the compiler), later when a device is hotplugged
> > > into the kernel it refers to the map to find the related driver for it
> > > and loads it if it isn't already loaded.
> > >
> > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > Documentation/driver-api/usb/hotplug.rst
> >
> > And you just need to add __maybe_unused to them to suppress the
> > warning.
> 
> Wouldn't that cause the compiler to optimize them away if it doesn't
> see any users?

It looks like they're only unused when !MODULE, in which case
optimising them away would be the correct thing to do, no?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Rafael J. Wysocki]

On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
>
> > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > >
> > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > >
> > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > that they're missing, so ...
> > > > > >
> > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > >
> > > > > Any idea how that works, or where the code is for that?
> > > >
> > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > product-id that the kernel keeps after boot (and so there is no static
> > > > reference of it for the compiler), later when a device is hotplugged
> > > > into the kernel it refers to the map to find the related driver for it
> > > > and loads it if it isn't already loaded.
> > > >
> > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > Documentation/driver-api/usb/hotplug.rst
> > >
> > > And you just need to add __maybe_unused to them to suppress the
> > > warning.
> >
> > Wouldn't that cause the compiler to optimize them away if it doesn't
> > see any users?
>
> It looks like they're only unused when !MODULE,

OK

> in which case optimising them away would be the correct thing to do, no?

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:

> On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> >
> > > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > >
> > > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > > >
> > > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > > >
> > > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > > that they're missing, so ...
> > > > > > >
> > > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > > >
> > > > > > Any idea how that works, or where the code is for that?
> > > > >
> > > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > > product-id that the kernel keeps after boot (and so there is no static
> > > > > reference of it for the compiler), later when a device is hotplugged
> > > > > into the kernel it refers to the map to find the related driver for it
> > > > > and loads it if it isn't already loaded.
> > > > >
> > > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > > Documentation/driver-api/usb/hotplug.rst
> > > >
> > > > And you just need to add __maybe_unused to them to suppress the
> > > > warning.
> > >
> > > Wouldn't that cause the compiler to optimize them away if it doesn't
> > > see any users?
> >
> > It looks like they're only unused when !MODULE,
> 
> OK
> 
> > in which case optimising them away would be the correct thing to do, no?

It would be good if someone with a little more knowledge could provide
a second opinion though.  I would think (hope) that the compiler would
be smart enough to see when its actually in use.  After all, it is the
compiler that places the information into the device table.

If that is not the case, then the MODULE_DEVICE_TABLE() magic is
broken and will need fixing.  Removing boiler-plate is good, but not
at the expense of obfuscation.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Wed, 15 Jul 2020, Lee Jones wrote:

> On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> 
> > On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > >
> > > > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > >
> > > > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > > > >
> > > > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > > > that they're missing, so ...
> > > > > > > >
> > > > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > > > >
> > > > > > > Any idea how that works, or where the code is for that?
> > > > > >
> > > > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > > > product-id that the kernel keeps after boot (and so there is no static
> > > > > > reference of it for the compiler), later when a device is hotplugged
> > > > > > into the kernel it refers to the map to find the related driver for it
> > > > > > and loads it if it isn't already loaded.
> > > > > >
> > > > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > > > Documentation/driver-api/usb/hotplug.rst
> > > > >
> > > > > And you just need to add __maybe_unused to them to suppress the
> > > > > warning.
> > > >
> > > > Wouldn't that cause the compiler to optimize them away if it doesn't
> > > > see any users?
> > >
> > > It looks like they're only unused when !MODULE,
> > 
> > OK
> > 
> > > in which case optimising them away would be the correct thing to do, no?
> 
> It would be good if someone with a little more knowledge could provide
> a second opinion though.  I would think (hope) that the compiler would
> be smart enough to see when its actually in use.  After all, it is the
> compiler that places the information into the device table.
> 
> If that is not the case, then the MODULE_DEVICE_TABLE() magic is
> broken and will need fixing.  Removing boiler-plate is good, but not
> at the expense of obfuscation.

Okay, I'm satisfied.  This test build is without __maybe_unused:

# All configs built as modules (MODULE) - the compiler knows to use the tables

 $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86 allmodconfig
 $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86  W=1  drivers/cpufreq/
 [...]
   CC [M]  drivers/cpufreq/pcc-cpufreq.o

# All configs built-in (!MODULE) - the compiler sees that they are unused

 $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86 allyesconfig
 $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86  W=1  drivers/cpufreq/
   CC      drivers/cpufreq/pcc-cpufreq.o
  drivers/cpufreq/pcc-cpufreq.c:619:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
  619 | static const struct acpi_device_id processor_device_ids[] = {
      |                                    ^~~~~~~~~~~~~~~~~~~~

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Rafael J. Wysocki]

On Wed, Jul 15, 2020 at 2:07 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Wed, 15 Jul 2020, Lee Jones wrote:
>
> > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> >
> > > On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > >
> > > > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > > >
> > > > > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > > >
> > > > > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > > > > >
> > > > > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > > > > that they're missing, so ...
> > > > > > > > >
> > > > > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > > > > >
> > > > > > > > Any idea how that works, or where the code is for that?
> > > > > > >
> > > > > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > > > > product-id that the kernel keeps after boot (and so there is no static
> > > > > > > reference of it for the compiler), later when a device is hotplugged
> > > > > > > into the kernel it refers to the map to find the related driver for it
> > > > > > > and loads it if it isn't already loaded.
> > > > > > >
> > > > > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > > > > Documentation/driver-api/usb/hotplug.rst
> > > > > >
> > > > > > And you just need to add __maybe_unused to them to suppress the
> > > > > > warning.
> > > > >
> > > > > Wouldn't that cause the compiler to optimize them away if it doesn't
> > > > > see any users?
> > > >
> > > > It looks like they're only unused when !MODULE,
> > >
> > > OK
> > >
> > > > in which case optimising them away would be the correct thing to do, no?
> >
> > It would be good if someone with a little more knowledge could provide
> > a second opinion though.  I would think (hope) that the compiler would
> > be smart enough to see when its actually in use.  After all, it is the
> > compiler that places the information into the device table.
> >
> > If that is not the case, then the MODULE_DEVICE_TABLE() magic is
> > broken and will need fixing.  Removing boiler-plate is good, but not
> > at the expense of obfuscation.
>
> Okay, I'm satisfied.  This test build is without __maybe_unused:
>
> # All configs built as modules (MODULE) - the compiler knows to use the tables
>
>  $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86 allmodconfig
>  $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86  W=1  drivers/cpufreq/
>  [...]
>    CC [M]  drivers/cpufreq/pcc-cpufreq.o
>
> # All configs built-in (!MODULE) - the compiler sees that they are unused
>
>  $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86 allyesconfig
>  $ ccache make -f Makefile -j24 KBUILD_OUTPUT=../builds/build-x86  W=1  drivers/cpufreq/
>    CC      drivers/cpufreq/pcc-cpufreq.o
>   drivers/cpufreq/pcc-cpufreq.c:619:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
>   619 | static const struct acpi_device_id processor_device_ids[] = {
>       |                                    ^~~~~~~~~~~~~~~~~~~~
>

OK

I thought that this would be the case. :-)

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Rafael J. Wysocki]

On Wed, Jul 15, 2020 at 1:50 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
>
> > On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > >
> > > > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > >
> > > > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > > > >
> > > > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > > > that they're missing, so ...
> > > > > > > >
> > > > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > > > >
> > > > > > > Any idea how that works, or where the code is for that?
> > > > > >
> > > > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > > > product-id that the kernel keeps after boot (and so there is no static
> > > > > > reference of it for the compiler), later when a device is hotplugged
> > > > > > into the kernel it refers to the map to find the related driver for it
> > > > > > and loads it if it isn't already loaded.
> > > > > >
> > > > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > > > Documentation/driver-api/usb/hotplug.rst
> > > > >
> > > > > And you just need to add __maybe_unused to them to suppress the
> > > > > warning.
> > > >
> > > > Wouldn't that cause the compiler to optimize them away if it doesn't
> > > > see any users?
> > >
> > > It looks like they're only unused when !MODULE,
> >
> > OK
> >
> > > in which case optimising them away would be the correct thing to do, no?
>
> It would be good if someone with a little more knowledge could provide
> a second opinion though.  I would think (hope) that the compiler would
> be smart enough to see when its actually in use.  After all, it is the
> compiler that places the information into the device table.
>
> If that is not the case, then the MODULE_DEVICE_TABLE() magic is
> broken and will need fixing.

I'm not sure why that would be the case?

> Removing boiler-plate is good, but not at the expense of obfuscation.

I'm not following you here to be honest.

BTW, I'm wondering if removing the "static" modifier from the
definitions of the structures in question makes the warnings you want
to get rid of go away.

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:

> On Wed, Jul 15, 2020 at 1:50 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> >
> > > On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > >
> > > > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > > >
> > > > > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > > >
> > > > > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > > > > >
> > > > > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > > > > that they're missing, so ...
> > > > > > > > >
> > > > > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > > > > >
> > > > > > > > Any idea how that works, or where the code is for that?
> > > > > > >
> > > > > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > > > > product-id that the kernel keeps after boot (and so there is no static
> > > > > > > reference of it for the compiler), later when a device is hotplugged
> > > > > > > into the kernel it refers to the map to find the related driver for it
> > > > > > > and loads it if it isn't already loaded.
> > > > > > >
> > > > > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > > > > Documentation/driver-api/usb/hotplug.rst
> > > > > >
> > > > > > And you just need to add __maybe_unused to them to suppress the
> > > > > > warning.
> > > > >
> > > > > Wouldn't that cause the compiler to optimize them away if it doesn't
> > > > > see any users?
> > > >
> > > > It looks like they're only unused when !MODULE,
> > >
> > > OK
> > >
> > > > in which case optimising them away would be the correct thing to do, no?
> >
> > It would be good if someone with a little more knowledge could provide
> > a second opinion though.  I would think (hope) that the compiler would
> > be smart enough to see when its actually in use.  After all, it is the
> > compiler that places the information into the device table.
> >
> If that is not the case, then the MODULE_DEVICE_TABLE() magic is
> > broken and will need fixing.
> 
> I'm not sure why that would be the case?

Nor me.  In fact, take a look at my latest email.  I think I just
proved out that it's not broken.  The warning is valid and
MODULE_DEVICE_TABLE() appears to work just as it should.

> > Removing boiler-plate is good, but not at the expense of obfuscation.
> 
> I'm not following you here to be honest.

Never mind.  It's no longer important.

> BTW, I'm wondering if removing the "static" modifier from the
> definitions of the structures in question makes the warnings you want
> to get rid of go away.

I'm sure that it would.  But that just alludes to the fact that the
tables may be in use elsewhere, which in the case of !MODULE is
untrue.  That's probably more of a hack than using __maybe_unused.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Robin Murphy]

On 2020-07-15 13:16, Lee Jones wrote:
> On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> 
>> On Wed, Jul 15, 2020 at 1:50 PM Lee Jones <lee.jones@linaro.org> wrote:
>>>
>>> On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
>>>
>>>> On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
>>>>>
>>>>> On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
>>>>>
>>>>>> On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>>>>>
>>>>>>> On 15-07-20, 08:54, Viresh Kumar wrote:
>>>>>>>> On 14-07-20, 22:03, Lee Jones wrote:
>>>>>>>>> On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
>>>>>>>>>
>>>>>>>>>> On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Can't see them being used anywhere and the compiler doesn't complain
>>>>>>>>>>> that they're missing, so ...
>>>>>>>>>>
>>>>>>>>>> Aren't they needed for automatic module loading in certain configurations?
>>>>>>>>>
>>>>>>>>> Any idea how that works, or where the code is for that?
>>>>>>>>
>>>>>>>> The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
>>>>>>>> product-id that the kernel keeps after boot (and so there is no static
>>>>>>>> reference of it for the compiler), later when a device is hotplugged
>>>>>>>> into the kernel it refers to the map to find the related driver for it
>>>>>>>> and loads it if it isn't already loaded.
>>>>>>>>
>>>>>>>> This has some of it, search for MODULE_DEVICE_TABLE() in it.
>>>>>>>> Documentation/driver-api/usb/hotplug.rst
>>>>>>>
>>>>>>> And you just need to add __maybe_unused to them to suppress the
>>>>>>> warning.
>>>>>>
>>>>>> Wouldn't that cause the compiler to optimize them away if it doesn't
>>>>>> see any users?
>>>>>
>>>>> It looks like they're only unused when !MODULE,
>>>>
>>>> OK
>>>>
>>>>> in which case optimising them away would be the correct thing to do, no?
>>>
>>> It would be good if someone with a little more knowledge could provide
>>> a second opinion though.  I would think (hope) that the compiler would
>>> be smart enough to see when its actually in use.  After all, it is the
>>> compiler that places the information into the device table.
>>>
>> If that is not the case, then the MODULE_DEVICE_TABLE() magic is
>>> broken and will need fixing.
>>
>> I'm not sure why that would be the case?
> 
> Nor me.  In fact, take a look at my latest email.  I think I just
> proved out that it's not broken.  The warning is valid and
> MODULE_DEVICE_TABLE() appears to work just as it should.

I won't claim to be an expert at all, but...

For !MODULE, MODULE_DEVICE_TABLE() expands to nothing, so after 
preprocessing the static variable is literally unreferenced.

Otherwise, MODULE_DEVICE_TABLE() emits an extern declaration of another 
variable which is defined via the "alias" attribute to refer to the 
symbol of the static variable. Thus the compiler presumably has to treat 
it as potentially accessible from other compilation units such that it 
can't be optimised away.

>>> Removing boiler-plate is good, but not at the expense of obfuscation.
>>
>> I'm not following you here to be honest.
> 
> Never mind.  It's no longer important.
> 
>> BTW, I'm wondering if removing the "static" modifier from the
>> definitions of the structures in question makes the warnings you want
>> to get rid of go away.
> 
> I'm sure that it would.  But that just alludes to the fact that the
> tables may be in use elsewhere, which in the case of !MODULE is
> untrue.  That's probably more of a hack than using __maybe_unused.

Right, that just ends up with someone sending another patch changing it 
back to shut up "variable foo was not declared, should it be static?" 
warnings from Sparse ;)

Robin.

Re: [PATCH 09/13] cpufreq: acpi-cpufreq: Remove unused ID structs

[Lee Jones]

On Wed, 15 Jul 2020, Robin Murphy wrote:

> On 2020-07-15 13:16, Lee Jones wrote:
> > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > 
> > > On Wed, Jul 15, 2020 at 1:50 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > 
> > > > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > > > 
> > > > > On Wed, Jul 15, 2020 at 1:34 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > 
> > > > > > On Wed, 15 Jul 2020, Rafael J. Wysocki wrote:
> > > > > > 
> > > > > > > On Wed, Jul 15, 2020 at 5:27 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > > > > > 
> > > > > > > > On 15-07-20, 08:54, Viresh Kumar wrote:
> > > > > > > > > On 14-07-20, 22:03, Lee Jones wrote:
> > > > > > > > > > On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
> > > > > > > > > > 
> > > > > > > > > > > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > Can't see them being used anywhere and the compiler doesn't complain
> > > > > > > > > > > > that they're missing, so ...
> > > > > > > > > > > 
> > > > > > > > > > > Aren't they needed for automatic module loading in certain configurations?
> > > > > > > > > > 
> > > > > > > > > > Any idea how that works, or where the code is for that?
> > > > > > > > > 
> > > > > > > > > The MODULE_DEVICE_TABLE() thingy creates a map of vendor-id,
> > > > > > > > > product-id that the kernel keeps after boot (and so there is no static
> > > > > > > > > reference of it for the compiler), later when a device is hotplugged
> > > > > > > > > into the kernel it refers to the map to find the related driver for it
> > > > > > > > > and loads it if it isn't already loaded.
> > > > > > > > > 
> > > > > > > > > This has some of it, search for MODULE_DEVICE_TABLE() in it.
> > > > > > > > > Documentation/driver-api/usb/hotplug.rst
> > > > > > > > 
> > > > > > > > And you just need to add __maybe_unused to them to suppress the
> > > > > > > > warning.
> > > > > > > 
> > > > > > > Wouldn't that cause the compiler to optimize them away if it doesn't
> > > > > > > see any users?
> > > > > > 
> > > > > > It looks like they're only unused when !MODULE,
> > > > > 
> > > > > OK
> > > > > 
> > > > > > in which case optimising them away would be the correct thing to do, no?
> > > > 
> > > > It would be good if someone with a little more knowledge could provide
> > > > a second opinion though.  I would think (hope) that the compiler would
> > > > be smart enough to see when its actually in use.  After all, it is the
> > > > compiler that places the information into the device table.
> > > > 
> > > If that is not the case, then the MODULE_DEVICE_TABLE() magic is
> > > > broken and will need fixing.
> > > 
> > > I'm not sure why that would be the case?
> > 
> > Nor me.  In fact, take a look at my latest email.  I think I just
> > proved out that it's not broken.  The warning is valid and
> > MODULE_DEVICE_TABLE() appears to work just as it should.
> 
> I won't claim to be an expert at all, but...
> 
> For !MODULE, MODULE_DEVICE_TABLE() expands to nothing, so after
> preprocessing the static variable is literally unreferenced.
> 
> Otherwise, MODULE_DEVICE_TABLE() emits an extern declaration of another
> variable which is defined via the "alias" attribute to refer to the symbol
> of the static variable. Thus the compiler presumably has to treat it as
> potentially accessible from other compilation units such that it can't be
> optimised away.

Right.  That's essentially how I'm reading it.

> > > > Removing boiler-plate is good, but not at the expense of obfuscation.
> > > 
> > > I'm not following you here to be honest.
> > 
> > Never mind.  It's no longer important.
> > 
> > > BTW, I'm wondering if removing the "static" modifier from the
> > > definitions of the structures in question makes the warnings you want
> > > to get rid of go away.
> > 
> > I'm sure that it would.  But that just alludes to the fact that the
> > tables may be in use elsewhere, which in the case of !MODULE is
> > untrue.  That's probably more of a hack than using __maybe_unused.
> 
> Right, that just ends up with someone sending another patch changing it back
> to shut up "variable foo was not declared, should it be static?" warnings
> from Sparse ;)

Exactly.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 10/13] cpufreq: powernow-k8: Make use of known set but not used variables

[Lee Jones]

If we fail to use a variable, even a 'dummy' one, then the compiler
complains that it is set but not used.  We know this is fine, so we
set it to its own value here.

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/powernow-k8.c: In function ‘pending_bit_stuck’:
 drivers/cpufreq/powernow-k8.c:89:10: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
 89 | u32 lo, hi;
 | ^~
 drivers/cpufreq/powernow-k8.c: In function ‘core_voltage_pre_transition’:
 drivers/cpufreq/powernow-k8.c:285:14: warning: variable ‘lo’ set but not used [-Wunused-but-set-variable]
 285 | u32 maxvid, lo, rvomult = 1;
 | ^~

Cc: Andreas Herrmann <herrmann.der.user@googlemail.com>
Cc: Dominik Brodowski <linux@brodo.de>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Paul Devriendt <paul.devriendt@amd.com>
Cc: Mark Langsdorf <mark.langsdorf@amd.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/powernow-k8.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 3984959eed1d5..b76df81044c4f 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -89,6 +89,7 @@ static int pending_bit_stuck(void)
 	u32 lo, hi;
 
 	rdmsr(MSR_FIDVID_STATUS, lo, hi);
+	hi &= hi; /* Silence set but not used warning */
 	return lo & MSR_S_LO_CHANGE_PENDING ? 1 : 0;
 }
 
@@ -292,6 +293,7 @@ static int core_voltage_pre_transition(struct powernow_k8_data *data,
 		rvomult = 2;
 	rvosteps *= rvomult;
 	rdmsr(MSR_FIDVID_STATUS, lo, maxvid);
+	lo &= lo; /* Silence set but not used warning */
 	maxvid = 0x1f & (maxvid >> 16);
 	pr_debug("ph1 maxvid=0x%x\n", maxvid);
 	if (reqvid < maxvid) /* lower numbers are higher voltages */
-- 
2.25.1

Re: [PATCH 10/13] cpufreq: powernow-k8: Make use of known set but not used variables

[Rafael J. Wysocki]

On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> If we fail to use a variable, even a 'dummy' one, then the compiler
> complains that it is set but not used.  We know this is fine, so we
> set it to its own value here.
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/cpufreq/powernow-k8.c: In function ‘pending_bit_stuck’:
>  drivers/cpufreq/powernow-k8.c:89:10: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
>  89 | u32 lo, hi;
>  | ^~
>  drivers/cpufreq/powernow-k8.c: In function ‘core_voltage_pre_transition’:
>  drivers/cpufreq/powernow-k8.c:285:14: warning: variable ‘lo’ set but not used [-Wunused-but-set-variable]
>  285 | u32 maxvid, lo, rvomult = 1;
>  | ^~
>
> Cc: Andreas Herrmann <herrmann.der.user@googlemail.com>
> Cc: Dominik Brodowski <linux@brodo.de>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Paul Devriendt <paul.devriendt@amd.com>
> Cc: Mark Langsdorf <mark.langsdorf@amd.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/powernow-k8.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
> index 3984959eed1d5..b76df81044c4f 100644
> --- a/drivers/cpufreq/powernow-k8.c
> +++ b/drivers/cpufreq/powernow-k8.c
> @@ -89,6 +89,7 @@ static int pending_bit_stuck(void)
>         u32 lo, hi;
>
>         rdmsr(MSR_FIDVID_STATUS, lo, hi);
> +       hi &= hi; /* Silence set but not used warning */

Hmm.

What about using __always_unused for this?

>         return lo & MSR_S_LO_CHANGE_PENDING ? 1 : 0;
>  }
>
> @@ -292,6 +293,7 @@ static int core_voltage_pre_transition(struct powernow_k8_data *data,
>                 rvomult = 2;
>         rvosteps *= rvomult;
>         rdmsr(MSR_FIDVID_STATUS, lo, maxvid);
> +       lo &= lo; /* Silence set but not used warning */
>         maxvid = 0x1f & (maxvid >> 16);
>         pr_debug("ph1 maxvid=0x%x\n", maxvid);
>         if (reqvid < maxvid) /* lower numbers are higher voltages */
> --
> 2.25.1
>

Re: [PATCH 10/13] cpufreq: powernow-k8: Make use of known set but not used variables

[Lee Jones]

On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:

> On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > If we fail to use a variable, even a 'dummy' one, then the compiler
> > complains that it is set but not used.  We know this is fine, so we
> > set it to its own value here.
> >
> > Fixes the following W=1 kernel build warning(s):
> >
> >  drivers/cpufreq/powernow-k8.c: In function ‘pending_bit_stuck’:
> >  drivers/cpufreq/powernow-k8.c:89:10: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
> >  89 | u32 lo, hi;
> >  | ^~
> >  drivers/cpufreq/powernow-k8.c: In function ‘core_voltage_pre_transition’:
> >  drivers/cpufreq/powernow-k8.c:285:14: warning: variable ‘lo’ set but not used [-Wunused-but-set-variable]
> >  285 | u32 maxvid, lo, rvomult = 1;
> >  | ^~
> >
> > Cc: Andreas Herrmann <herrmann.der.user@googlemail.com>
> > Cc: Dominik Brodowski <linux@brodo.de>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Paul Devriendt <paul.devriendt@amd.com>
> > Cc: Mark Langsdorf <mark.langsdorf@amd.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/cpufreq/powernow-k8.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
> > index 3984959eed1d5..b76df81044c4f 100644
> > --- a/drivers/cpufreq/powernow-k8.c
> > +++ b/drivers/cpufreq/powernow-k8.c
> > @@ -89,6 +89,7 @@ static int pending_bit_stuck(void)
> >         u32 lo, hi;
> >
> >         rdmsr(MSR_FIDVID_STATUS, lo, hi);
> > +       hi &= hi; /* Silence set but not used warning */
> 
> Hmm.
> 
> What about using __always_unused for this?

I didn't know you could use that for standard variables.

Will fix.

> >         return lo & MSR_S_LO_CHANGE_PENDING ? 1 : 0;
> >  }
> >
> > @@ -292,6 +293,7 @@ static int core_voltage_pre_transition(struct powernow_k8_data *data,
> >                 rvomult = 2;
> >         rvosteps *= rvomult;
> >         rdmsr(MSR_FIDVID_STATUS, lo, maxvid);
> > +       lo &= lo; /* Silence set but not used warning */
> >         maxvid = 0x1f & (maxvid >> 16);
> >         pr_debug("ph1 maxvid=0x%x\n", maxvid);
> >         if (reqvid < maxvid) /* lower numbers are higher voltages */
> >

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 11/13] cpufreq: pcc-cpufreq: Remove unused ID structs

[Lee Jones]

Can't see them being used anywhere and the compiler doesn't complain
that they're missing, so ...

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/pcc-cpufreq.c:619:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
 619 | static const struct acpi_device_id processor_device_ids[] = {
 | ^~~~~~~~~~~~~~~~~~~~

Cc: Matthew Garrett <mjg@redhat.com>
Cc: Nagananda Chumbalkar <nagananda.chumbalkar@hp.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/pcc-cpufreq.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index 5789fe7a94bd3..99cf8b78c032a 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -616,13 +616,6 @@ static void __exit pcc_cpufreq_exit(void)
 	free_percpu(pcc_cpu_info);
 }
 
-static const struct acpi_device_id processor_device_ids[] = {
-	{ACPI_PROCESSOR_OBJECT_HID, },
-	{ACPI_PROCESSOR_DEVICE_HID, },
-	{},
-};
-MODULE_DEVICE_TABLE(acpi, processor_device_ids);
-
 MODULE_AUTHOR("Matthew Garrett, Naga Chumbalkar");
 MODULE_VERSION(PCC_VERSION);
 MODULE_DESCRIPTION("Processor Clocking Control interface driver");
-- 
2.25.1

Re: [PATCH 11/13] cpufreq: pcc-cpufreq: Remove unused ID structs

[Rafael J. Wysocki]

On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> Can't see them being used anywhere and the compiler doesn't complain
> that they're missing, so ...

Autoloading of the module too.

> Fixes the following W=1 kernel build warning(s):
>
>  drivers/cpufreq/pcc-cpufreq.c:619:36: warning: ‘processor_device_ids’ defined but not used [-Wunused-const-variable=]
>  619 | static const struct acpi_device_id processor_device_ids[] = {
>  | ^~~~~~~~~~~~~~~~~~~~
>
> Cc: Matthew Garrett <mjg@redhat.com>
> Cc: Nagananda Chumbalkar <nagananda.chumbalkar@hp.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/pcc-cpufreq.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> index 5789fe7a94bd3..99cf8b78c032a 100644
> --- a/drivers/cpufreq/pcc-cpufreq.c
> +++ b/drivers/cpufreq/pcc-cpufreq.c
> @@ -616,13 +616,6 @@ static void __exit pcc_cpufreq_exit(void)
>         free_percpu(pcc_cpu_info);
>  }
>
> -static const struct acpi_device_id processor_device_ids[] = {
> -       {ACPI_PROCESSOR_OBJECT_HID, },
> -       {ACPI_PROCESSOR_DEVICE_HID, },
> -       {},
> -};
> -MODULE_DEVICE_TABLE(acpi, processor_device_ids);
> -
>  MODULE_AUTHOR("Matthew Garrett, Naga Chumbalkar");
>  MODULE_VERSION(PCC_VERSION);
>  MODULE_DESCRIPTION("Processor Clocking Control interface driver");
> --
> 2.25.1
>

[PATCH 12/13] cpufreq: intel_pstate: Supply struct attribute description for get_aperf_mperf_shift()

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/intel_pstate.c:293: warning: Function parameter or member 'get_aperf_mperf_shift' not described in 'pstate_funcs'

Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Dirk Brandewie <dirk.j.brandewie@intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/intel_pstate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 44c7b4677675d..f92fc69c87269 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -275,6 +275,8 @@ static struct cpudata **all_cpu_data;
  * @get_min:		Callback to get minimum P state
  * @get_turbo:		Callback to get turbo P state
  * @get_scaling:	Callback to get frequency scaling factor
+ * @get_aperf_mperf_shift: Callback to get the number of clock cycles after
+ *			aperf, merf is incremented
  * @get_val:		Callback to convert P state to actual MSR write value
  * @get_vid:		Callback to get VID data for Atom platforms
  *
-- 
2.25.1

Re: [PATCH 12/13] cpufreq: intel_pstate: Supply struct attribute description for get_aperf_mperf_shift()

[Rafael J. Wysocki]

On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/cpufreq/intel_pstate.c:293: warning: Function parameter or member 'get_aperf_mperf_shift' not described in 'pstate_funcs'
>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Dirk Brandewie <dirk.j.brandewie@intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/cpufreq/intel_pstate.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 44c7b4677675d..f92fc69c87269 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -275,6 +275,8 @@ static struct cpudata **all_cpu_data;
>   * @get_min:           Callback to get minimum P state
>   * @get_turbo:         Callback to get turbo P state
>   * @get_scaling:       Callback to get frequency scaling factor
> + * @get_aperf_mperf_shift: Callback to get the number of clock cycles after
> + *                     aperf, merf is incremented

This added description is inaccurate.  It should be something like
"Callback to get the APERF vs MPERF frequency difference".

>   * @get_val:           Callback to convert P state to actual MSR write value
>   * @get_vid:           Callback to get VID data for Atom platforms
>   *
> --

Re: [PATCH 12/13] cpufreq: intel_pstate: Supply struct attribute description for get_aperf_mperf_shift()

[Lee Jones]

On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:

> On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > Fixes the following W=1 kernel build warning(s):
> >
> >  drivers/cpufreq/intel_pstate.c:293: warning: Function parameter or member 'get_aperf_mperf_shift' not described in 'pstate_funcs'
> >
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/cpufreq/intel_pstate.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index 44c7b4677675d..f92fc69c87269 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -275,6 +275,8 @@ static struct cpudata **all_cpu_data;
> >   * @get_min:           Callback to get minimum P state
> >   * @get_turbo:         Callback to get turbo P state
> >   * @get_scaling:       Callback to get frequency scaling factor
> > + * @get_aperf_mperf_shift: Callback to get the number of clock cycles after
> > + *                     aperf, merf is incremented
> 
> This added description is inaccurate.  It should be something like
> "Callback to get the APERF vs MPERF frequency difference".

Does that mean the description of @aperf_mperf_shift above is also
incorrect?

> >   * @get_val:           Callback to convert P state to actual MSR write value
> >   * @get_vid:           Callback to get VID data for Atom platforms
> >   *
> > --

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 12/13] cpufreq: intel_pstate: Supply struct attribute description for get_aperf_mperf_shift()

[Rafael J. Wysocki]

On Tue, Jul 14, 2020 at 11:28 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Tue, 14 Jul 2020, Rafael J. Wysocki wrote:
>
> > On Tue, Jul 14, 2020 at 4:51 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > Fixes the following W=1 kernel build warning(s):
> > >
> > >  drivers/cpufreq/intel_pstate.c:293: warning: Function parameter or member 'get_aperf_mperf_shift' not described in 'pstate_funcs'
> > >
> > > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Cc: Len Brown <lenb@kernel.org>
> > > Cc: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  drivers/cpufreq/intel_pstate.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index 44c7b4677675d..f92fc69c87269 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -275,6 +275,8 @@ static struct cpudata **all_cpu_data;
> > >   * @get_min:           Callback to get minimum P state
> > >   * @get_turbo:         Callback to get turbo P state
> > >   * @get_scaling:       Callback to get frequency scaling factor
> > > + * @get_aperf_mperf_shift: Callback to get the number of clock cycles after
> > > + *                     aperf, merf is incremented
> >
> > This added description is inaccurate.  It should be something like
> > "Callback to get the APERF vs MPERF frequency difference".
>
> Does that mean the description of @aperf_mperf_shift above is also
> incorrect?

It is slightly unclear, let me improve it.

Thanks!

[PATCH 13/13] cpufreq: amd_freq_sensitivity: Remove unused ID structs

[Lee Jones]

Can't see them being used anywhere and the compiler doesn't complain
that they're missing, so ...

Fixes the following W=1 kernel build warning(s):

 drivers/cpufreq/amd_freq_sensitivity.c:147:32: warning: ‘amd_freq_sensitivity_ids’ defined but not used [-Wunused-const-variable=]
 147 | static const struct x86_cpu_id amd_freq_sensitivity_ids[] = {
 | ^~~~~~~~~~~~~~~~~~~~~~~~

Cc: Jacob Shin <jacob.shin@amd.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/cpufreq/amd_freq_sensitivity.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
index f7c4206d4c90b..a17093729511b 100644
--- a/drivers/cpufreq/amd_freq_sensitivity.c
+++ b/drivers/cpufreq/amd_freq_sensitivity.c
@@ -144,12 +144,6 @@ static void __exit amd_freq_sensitivity_exit(void)
 }
 module_exit(amd_freq_sensitivity_exit);
 
-static const struct x86_cpu_id amd_freq_sensitivity_ids[] = {
-	X86_MATCH_FEATURE(X86_FEATURE_PROC_FEEDBACK, NULL),
-	{}
-};
-MODULE_DEVICE_TABLE(x86cpu, amd_freq_sensitivity_ids);
-
 MODULE_AUTHOR("Jacob Shin <jacob.shin@amd.com>");
 MODULE_DESCRIPTION("AMD frequency sensitivity feedback powersave bias for "
 		"the ondemand governor.");
-- 
2.25.1

Re: [PATCH 13/13] cpufreq: amd_freq_sensitivity: Remove unused ID structs

[Kim Phillips]

On 7/14/20 9:50 AM, Lee Jones wrote:
> Can't see them being used anywhere and the compiler doesn't complain
> that they're missing, so ...
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/cpufreq/amd_freq_sensitivity.c:147:32: warning: ‘amd_freq_sensitivity_ids’ defined but not used [-Wunused-const-variable=]
>  147 | static const struct x86_cpu_id amd_freq_sensitivity_ids[] = {
>  | ^~~~~~~~~~~~~~~~~~~~~~~~
> 
> Cc: Jacob Shin <jacob.shin@amd.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---

Same comment as for patch 9/13: these are used automatic module loading.

Kim

Re: [PATCH 13/13] cpufreq: amd_freq_sensitivity: Remove unused ID structs

[Lee Jones]

On Tue, 14 Jul 2020, Kim Phillips wrote:

> On 7/14/20 9:50 AM, Lee Jones wrote:
> > Can't see them being used anywhere and the compiler doesn't complain
> > that they're missing, so ...
> > 
> > Fixes the following W=1 kernel build warning(s):
> > 
> >  drivers/cpufreq/amd_freq_sensitivity.c:147:32: warning: ‘amd_freq_sensitivity_ids’ defined but not used [-Wunused-const-variable=]
> >  147 | static const struct x86_cpu_id amd_freq_sensitivity_ids[] = {
> >  | ^~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Cc: Jacob Shin <jacob.shin@amd.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> 
> Same comment as for patch 9/13: these are used automatic module loading.

How does that work?

Could you point me to the code which handles that please?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 13/13] cpufreq: amd_freq_sensitivity: Remove unused ID structs

[Kim Phillips]

On 7/14/20 4:02 PM, Lee Jones wrote:
> On Tue, 14 Jul 2020, Kim Phillips wrote:
>> On 7/14/20 9:50 AM, Lee Jones wrote:
>>> Can't see them being used anywhere and the compiler doesn't complain
>>> that they're missing, so ...
>>>
>>> Fixes the following W=1 kernel build warning(s):
>>>
>>>  drivers/cpufreq/amd_freq_sensitivity.c:147:32: warning: ‘amd_freq_sensitivity_ids’ defined but not used [-Wunused-const-variable=]
>>>  147 | static const struct x86_cpu_id amd_freq_sensitivity_ids[] = {
>>>  | ^~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Cc: Jacob Shin <jacob.shin@amd.com>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>> ---
>>
>> Same comment as for patch 9/13: these are used automatic module loading.
> 
> How does that work?
> 
> Could you point me to the code which handles that please?

It's MODULE_DEVICE_TABLE magic.

Kim

Re: [PATCH 13/13] cpufreq: amd_freq_sensitivity: Remove unused ID structs

[Lee Jones]

On Tue, 14 Jul 2020, Kim Phillips wrote:

> On 7/14/20 4:02 PM, Lee Jones wrote:
> > On Tue, 14 Jul 2020, Kim Phillips wrote:
> >> On 7/14/20 9:50 AM, Lee Jones wrote:
> >>> Can't see them being used anywhere and the compiler doesn't complain
> >>> that they're missing, so ...
> >>>
> >>> Fixes the following W=1 kernel build warning(s):
> >>>
> >>>  drivers/cpufreq/amd_freq_sensitivity.c:147:32: warning: ‘amd_freq_sensitivity_ids’ defined but not used [-Wunused-const-variable=]
> >>>  147 | static const struct x86_cpu_id amd_freq_sensitivity_ids[] = {
> >>>  | ^~~~~~~~~~~~~~~~~~~~~~~~
> >>>
> >>> Cc: Jacob Shin <jacob.shin@amd.com>
> >>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >>> ---
> >>
> >> Same comment as for patch 9/13: these are used automatic module loading.
> > 
> > How does that work?
> > 
> > Could you point me to the code which handles that please?
> 
> It's MODULE_DEVICE_TABLE magic.

I'll look into that.  Thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 00/13] Rid W=1 warnings in CPUFreq

[Viresh Kumar]

On 14-07-20, 15:50, Lee Jones wrote:
> This set is part of a larger effort attempting to clean-up W=1
> kernel builds, which are currently overwhelmingly riddled with
> niggly little warnings.
> 
> After these patches are applied, the build system no longer
> complains about any W=0 nor W=1 level warnings in drivers/cpufreq.

And you need to rebase this stuff of pm/linux-next, as there are some
changes in cpufreq.c there.

-- 
viresh

Re: [PATCH 00/13] Rid W=1 warnings in CPUFreq

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 14-07-20, 15:50, Lee Jones wrote:
> > This set is part of a larger effort attempting to clean-up W=1
> > kernel builds, which are currently overwhelmingly riddled with
> > niggly little warnings.
> > 
> > After these patches are applied, the build system no longer
> > complains about any W=0 nor W=1 level warnings in drivers/cpufreq.
> 
> And you need to rebase this stuff of pm/linux-next, as there are some
> changes in cpufreq.c there.

It's based on the latest -next.  Is pm/linux-next in -next?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 00/13] Rid W=1 warnings in CPUFreq

[Viresh Kumar]

On 15-07-20, 07:32, Lee Jones wrote:
> On Wed, 15 Jul 2020, Viresh Kumar wrote:
> 
> > On 14-07-20, 15:50, Lee Jones wrote:
> > > This set is part of a larger effort attempting to clean-up W=1
> > > kernel builds, which are currently overwhelmingly riddled with
> > > niggly little warnings.
> > > 
> > > After these patches are applied, the build system no longer
> > > complains about any W=0 nor W=1 level warnings in drivers/cpufreq.
> > 
> > And you need to rebase this stuff of pm/linux-next, as there are some
> > changes in cpufreq.c there.
> 
> It's based on the latest -next.  Is pm/linux-next in -next?

Yes it is. Actually my bad, I based it on my next which didn't had
pm/linux-next :)

-- 
viresh

Re: [PATCH 00/13] Rid W=1 warnings in CPUFreq

[Lee Jones]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 15-07-20, 07:32, Lee Jones wrote:
> > On Wed, 15 Jul 2020, Viresh Kumar wrote:
> > 
> > > On 14-07-20, 15:50, Lee Jones wrote:
> > > > This set is part of a larger effort attempting to clean-up W=1
> > > > kernel builds, which are currently overwhelmingly riddled with
> > > > niggly little warnings.
> > > > 
> > > > After these patches are applied, the build system no longer
> > > > complains about any W=0 nor W=1 level warnings in drivers/cpufreq.
> > > 
> > > And you need to rebase this stuff of pm/linux-next, as there are some
> > > changes in cpufreq.c there.
> > 
> > It's based on the latest -next.  Is pm/linux-next in -next?
> 
> Yes it is. Actually my bad, I based it on my next which didn't had
> pm/linux-next :)

No problem.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog