Re: [PATCH v2 1/2] powernv/cpuidle: Parse dt idle properties into global structure

2018-07-02 Thread kbuild test robot 
Hi Akshay,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.18-rc3 next-20180702]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Akshay-Adiga/powernv-cpuidle-Device-tree-parsing-cleanup/20180703-024607
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

Note: the 
linux-review/Akshay-Adiga/powernv-cpuidle-Device-tree-parsing-cleanup/20180703-024607
 HEAD 4beae9263d77036dae7f43905823867c6d982690 builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/cpuidle/cpuidle-powernv.c: In function 'powernv_add_idle_states':
>> drivers/cpuidle/cpuidle-powernv.c:417:28: error: 'state' undeclared (first 
>> use in this function); did you mean 'statx'?
  if (has_stop_states && !(state->valid))
   ^
   statx
   drivers/cpuidle/cpuidle-powernv.c:417:28: note: each undeclared identifier 
is reported only once for each function it appears in

vim +417 drivers/cpuidle/cpuidle-powernv.c

   262  
   263  extern u32 pnv_get_supported_cpuidle_states(void);
   264  static int powernv_add_idle_states(void)
   265  {
   266  struct device_node *power_mgt;
   267  int nr_idle_states = 1; /* Snooze */
   268  int dt_idle_states, count;
   269  u32 latency_ns[CPUIDLE_STATE_MAX];
   270  u32 residency_ns[CPUIDLE_STATE_MAX];
   271  u32 flags[CPUIDLE_STATE_MAX];
   272  u64 psscr_val[CPUIDLE_STATE_MAX];
   273  u64 psscr_mask[CPUIDLE_STATE_MAX];
   274  const char *names[CPUIDLE_STATE_MAX];
   275  u32 has_stop_states = 0;
   276  int i, rc;
   277  u32 supported_flags = pnv_get_supported_cpuidle_states();
   278  
   279  
   280  /* Currently we have snooze statically defined */
   281  
   282  power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
   283  if (!power_mgt) {
   284  pr_warn("opal: PowerMgmt Node not found\n");
   285  goto out;
   286  }
   287  
   288  /* Read values of any property to determine the num of idle 
states */
   289  dt_idle_states = of_property_count_u32_elems(power_mgt, 
"ibm,cpu-idle-state-flags");
   290  if (dt_idle_states < 0) {
   291  pr_warn("cpuidle-powernv: no idle states found in the 
DT\n");
   292  goto out;
   293  }
   294  
   295  count = of_property_count_u32_elems(power_mgt,
   296  
"ibm,cpu-idle-state-latencies-ns");
   297  
   298  if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", 
dt_idle_states,
   299 "ibm,cpu-idle-state-latencies-ns",
   300 count) != 0)
   301  goto out;
   302  
   303  count = of_property_count_strings(power_mgt,
   304"ibm,cpu-idle-state-names");
   305  if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", 
dt_idle_states,
   306 "ibm,cpu-idle-state-names",
   307 count) != 0)
   308  goto out;
   309  
   310  /*
   311   * Since snooze is used as first idle state, max idle states 
allowed is
   312   * CPUIDLE_STATE_MAX -1
   313   */
   314  if (dt_idle_states > CPUIDLE_STATE_MAX - 1) {
   315  pr_warn("cpuidle-powernv: discovered idle states more 
than allowed");
   316  dt_idle_states = CPUIDLE_STATE_MAX - 1;
   317  }
   318  
   319  if (of_property_read_u32_array(power_mgt,
   320  "ibm,cpu-idle-state-flags", flags, 
dt_idle_states)) {
   321  pr_warn("cpuidle-powernv : missing 
ibm,cpu-idle-state-flags in DT\n");
   322  goto out;
   323  }
   324  
   325  if (of_property_read_u32_array(power_mgt,
   326  "ibm,cpu-idle-state-latencies-ns", latency_ns,
   327  dt_idle_states)) {
   328  pr_warn("cpuidle-powernv: missing 
ibm,cpu-idle-state-latencies-ns in DT\n");
   329  goto out;
   330  }
   331  if (of_property_read_string_array(power_mgt,
   332  "ibm,cpu-idle-state-names", names, dt_idle_states) < 0) 
{

Re: [PATCH v2 1/2] powernv/cpuidle: Parse dt idle properties into global structure

2018-07-02 Thread Nicholas Piggin 
On Mon,  2 Jul 2018 19:53:20 +0530
Akshay Adiga  wrote:

> Device-tree parsing happens twice, once while deciding idle state to be
> used for hotplug and once during cpuidle init. Hence, parsing the device
> tree and caching it will reduce code duplication. Parsing code has been
> moved to pnv_parse_cpuidle_dt() from pnv_probe_idle_states(). In addition
> to the properties in the device tree the number of available states is
> also required.
> 
> Signed-off-by: Akshay Adiga 
> ---
>  arch/powerpc/include/asm/cpuidle.h|  11 ++
>  arch/powerpc/platforms/powernv/idle.c | 216 --
>  drivers/cpuidle/cpuidle-powernv.c |  11 +-
>  3 files changed, 151 insertions(+), 87 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h 
> b/arch/powerpc/include/asm/cpuidle.h
> index e210a83eb196..574b0ce1d671 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -79,6 +79,17 @@ struct stop_sprs {
>   u64 mmcra;
>  };
>  
> +#define PNV_IDLE_NAME_LEN16
> +struct pnv_idle_states_t {
> + char name[PNV_IDLE_NAME_LEN];
> + u32 latency_ns;
> + u32 residency_ns;
> + u64 psscr_val;
> + u64 psscr_mask;
> + u32 flags;
> + bool valid;
> +};


This is a nice looking cleanup.

Reviewed-by: Nicholas Piggin