Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-22 Thread Gautham R Shenoy
Hi Ben, On Sat, Mar 22, 2014 at 09:56:30AM +1100, Benjamin Herrenschmidt wrote: On Fri, 2014-03-21 at 16:34 +0530, Gautham R Shenoy wrote: +/* + * Computes the current frequency on this cpu + * and stores the result in *ret_freq. + */ +static void

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Gautham R Shenoy
On Thu, Mar 20, 2014 at 05:41:00PM +0530, Gautham R. Shenoy wrote: From: Gautham R. Shenoy e...@linux.vnet.ibm.com The current frequency of a cpu is reported through the sysfs file cpuinfo_cur_freq. This requires the driver to implement a -get(unsigned int cpu) method which will return the

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Viresh Kumar
On Thu, Mar 20, 2014 at 5:41 PM, Gautham R. Shenoy e...@linux.vnet.ibm.com wrote: From: Gautham R. Shenoy e...@linux.vnet.ibm.com The current frequency of a cpu is reported through the sysfs file cpuinfo_cur_freq. This requires the driver to implement a -get(unsigned int cpu) method which

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Gautham R Shenoy
On Fri, Mar 21, 2014 at 03:01:29PM +0530, Viresh Kumar wrote: On Thu, Mar 20, 2014 at 5:41 PM, Gautham R. Shenoy e...@linux.vnet.ibm.com wrote: From: Gautham R. Shenoy e...@linux.vnet.ibm.com The current frequency of a cpu is reported through the sysfs file cpuinfo_cur_freq. This

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Viresh Kumar
On 21 March 2014 16:34, Gautham R Shenoy e...@linux.vnet.ibm.com wrote: Heh! Well, that wasn't the reason why this was sent out as a separate patch, but never mind. Though I don't understand why it would be difficult to review the patch though. Because the initial driver wasn't complete

RE: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread David Laight
From: Viresh Kumar On 21 March 2014 16:34, Gautham R Shenoy e...@linux.vnet.ibm.com wrote: Heh! Well, that wasn't the reason why this was sent out as a separate patch, but never mind. Though I don't understand why it would be difficult to review the patch though. Because the initial

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Gautham R Shenoy
On Fri, Mar 21, 2014 at 12:01:07PM +, David Laight wrote: From: Viresh Kumar On 21 March 2014 16:34, Gautham R Shenoy e...@linux.vnet.ibm.com wrote: Heh! Well, that wasn't the reason why this was sent out as a separate patch, but never mind. Though I don't understand why it would

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Viresh Kumar
On 21 March 2014 17:31, David Laight david.lai...@aculab.com wrote: *(int *)ret_freq = freq; Because it is very likely to be wrong. In general casts of pointers to integer types are dangerous. Where are we converting pointers to integers? We are doing a cast from 'void * ' to 'int *' and then

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Gautham R Shenoy
On Fri, Mar 21, 2014 at 05:15:34PM +0530, Viresh Kumar wrote: On 21 March 2014 16:34, Gautham R Shenoy e...@linux.vnet.ibm.com wrote: Heh! Well, that wasn't the reason why this was sent out as a separate patch, but never mind. Though I don't understand why it would be difficult to review

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Viresh Kumar
On 21 March 2014 18:34, Gautham R Shenoy e...@linux.vnet.ibm.com wrote: Consider the case when pmspr = 0x00fe; We are interested in extracting the value 'fe'. And ensure that when we store this value into an int, we get the sign extension right. So the following doesn't work:

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Gautham R Shenoy
On Fri, Mar 21, 2014 at 06:42:50PM +0530, Viresh Kumar wrote: On 21 March 2014 18:34, Gautham R Shenoy e...@linux.vnet.ibm.com wrote: Consider the case when pmspr = 0x00fe; We are interested in extracting the value 'fe'. And ensure that when we store this value into an int, we

RE: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread David Laight
From: Viresh Kumar [mailto:viresh.ku...@linaro.org] On 21 March 2014 17:31, David Laight david.lai...@aculab.com wrote: *(int *)ret_freq = freq; Because it is very likely to be wrong. In general casts of pointers to integer types are dangerous. Where are we converting pointers to

Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver-get() method

2014-03-21 Thread Benjamin Herrenschmidt
On Fri, 2014-03-21 at 16:34 +0530, Gautham R Shenoy wrote: +/* + * Computes the current frequency on this cpu + * and stores the result in *ret_freq. + */ +static void powernv_read_cpu_freq(void *ret_freq) +{ + unsigned long pmspr_val; + s8 local_pstate_id;