Re: [lm-sensors] [PATCH 2/4] hwmon: (ibmpowernv) add support for the new device tree

[Guenter Roeck <linux@roeck-us.net>]

On Wed, Apr 01, 2015 at 12:15:04PM +0200, Cédric Le Goater wrote:
> The new OPAL device tree for sensors has a different layout and uses new
> property names, for the type and for the handler used to capture the
> sensor data.
> 
> This patch modifies the ibmpowernv driver to support such a tree in a
> way preserving compatibility with older OPAL firmwares.
> 
> This is achieved by changing the error path of the routine parsing
> an OPAL node name. The node is simply considered being from the new
> device tree layout and fallback values are used.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>

Hi Cedric,

I was about to apply the series, but then I found the following problem.

> ---
>  drivers/hwmon/ibmpowernv.c |   47 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 38 insertions(+), 9 deletions(-)
> 
[ ... ]
>  
> @@ -189,11 +204,16 @@ static u32 get_sensor_hwmon_index(struct sensor_data *sdata,
>  {
>  	int i;
>  
> -	for (i = 0; i < count; i++)
> -		if (sdata_table[i].opal_index == sdata->opal_index &&
> -		    sdata_table[i].type == sdata->type)
> -			return sdata_table[i].hwmon_index;
> +	/*
> +	 * We don't use the OPAL index on newer device trees
> +	 */
> +	if (sdata->opal_index != -1) {

opal_index is u32, so this won't work (or at least the result is
unpredictable).

Also, in patch 4/4 (v4), get_logical_cpu() takes unsigned int as parameter,
but get_hard_smp_processor_id() returns an int, causing gcc to complain
if the code is built with W=1.

Please fix and resubmit the entire series.

When you do that, please also ensure that continuation lines
are aligned (in patch 3/4).

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors