[PATCH V2 0/2] hwmon: (ibmpowernv) Add support for current(A) sensors

[PATCH V2 0/2] hwmon: (ibmpowernv) Add support for current(A) sensors

[Shilpasri G Bhat]

The first patch from Cedric in the patchset cleans up the driver to
provide a neater way to define new sensor types. The second patch adds
current sensor.

Cédric Le Goater (1):
  hwmon: (ibmpowernv) introduce a legacy_compatibles array

Shilpasri G Bhat (1):
  hwmon: (ibmpowernv) Add current(A) sensor

 drivers/hwmon/ibmpowernv.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[PATCH V2 1/2] hwmon: (ibmpowernv) introduce a legacy_compatibles array

[Shilpasri G Bhat]

From: Cédric Le Goater <clg@kaod.org>

Today, the type of a PowerNV sensor system is determined with the
"compatible" property for legacy Firmwares and with the "sensor-type"
for newer ones. The same array of strings is used for both to do the
matching and this raises some issue to introduce new sensor types.

Let's introduce two different arrays (legacy and current) to make
things easier for new sensor types.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 drivers/hwmon/ibmpowernv.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
index 862b832..6d8909c 100644
--- a/drivers/hwmon/ibmpowernv.c
+++ b/drivers/hwmon/ibmpowernv.c
@@ -55,17 +55,27 @@ enum sensors {
 
 #define INVALID_INDEX (-1U)
 
+/*
+ * 'compatible' string properties for sensor types as defined in old
+ * PowerNV firmware (skiboot). These are ordered as 'enum sensors'.
+ */
+static const char * const legacy_compatibles[] = {
+	"ibm,opal-sensor-cooling-fan",
+	"ibm,opal-sensor-amb-temp",
+	"ibm,opal-sensor-power-supply",
+	"ibm,opal-sensor-power"
+};
+
 static struct sensor_group {
-	const char *name;
-	const char *compatible;
+	const char *name; /* matches property 'sensor-type' */
 	struct attribute_group group;
 	u32 attr_count;
 	u32 hwmon_index;
 } sensor_groups[] = {
-	{"fan", "ibm,opal-sensor-cooling-fan"},
-	{"temp", "ibm,opal-sensor-amb-temp"},
-	{"in", "ibm,opal-sensor-power-supply"},
-	{"power", "ibm,opal-sensor-power"}
+	{ "fan"   },
+	{ "temp"  },
+	{ "in"    },
+	{ "power" }
 };
 
 struct sensor_data {
@@ -239,8 +249,8 @@ static int get_sensor_type(struct device_node *np)
 	enum sensors type;
 	const char *str;
 
-	for (type = 0; type < MAX_SENSOR_TYPE; type++) {
-		if (of_device_is_compatible(np, sensor_groups[type].compatible))
+	for (type = 0; type < ARRAY_SIZE(legacy_compatibles); type++) {
+		if (of_device_is_compatible(np, legacy_compatibles[type]))
 			return type;
 	}
 
-- 
1.8.3.1

Re: [PATCH V2 1/2] hwmon: (ibmpowernv) introduce a legacy_compatibles array

[Cédric Le Goater]

On 06/20/2017 07:08 AM, Shilpasri G Bhat wrote:
> From: Cédric Le Goater <clg@kaod.org>
> 
> Today, the type of a PowerNV sensor system is determined with the
> "compatible" property for legacy Firmwares and with the "sensor-type"
> for newer ones. The same array of strings is used for both to do the
> matching and this raises some issue to introduce new sensor types.
> 
> Let's introduce two different arrays (legacy and current) to make
> things easier for new sensor types.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Tested-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>

Did you test on a Tuleta (IBM Power) system ? 

Thanks,

C. 

> ---
>  drivers/hwmon/ibmpowernv.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index 862b832..6d8909c 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -55,17 +55,27 @@ enum sensors {
>  
>  #define INVALID_INDEX (-1U)
>  
> +/*
> + * 'compatible' string properties for sensor types as defined in old
> + * PowerNV firmware (skiboot). These are ordered as 'enum sensors'.
> + */
> +static const char * const legacy_compatibles[] = {
> +	"ibm,opal-sensor-cooling-fan",
> +	"ibm,opal-sensor-amb-temp",
> +	"ibm,opal-sensor-power-supply",
> +	"ibm,opal-sensor-power"
> +};
> +
>  static struct sensor_group {
> -	const char *name;
> -	const char *compatible;
> +	const char *name; /* matches property 'sensor-type' */
>  	struct attribute_group group;
>  	u32 attr_count;
>  	u32 hwmon_index;
>  } sensor_groups[] = {
> -	{"fan", "ibm,opal-sensor-cooling-fan"},
> -	{"temp", "ibm,opal-sensor-amb-temp"},
> -	{"in", "ibm,opal-sensor-power-supply"},
> -	{"power", "ibm,opal-sensor-power"}
> +	{ "fan"   },
> +	{ "temp"  },
> +	{ "in"    },
> +	{ "power" }
>  };
>  
>  struct sensor_data {
> @@ -239,8 +249,8 @@ static int get_sensor_type(struct device_node *np)
>  	enum sensors type;
>  	const char *str;
>  
> -	for (type = 0; type < MAX_SENSOR_TYPE; type++) {
> -		if (of_device_is_compatible(np, sensor_groups[type].compatible))
> +	for (type = 0; type < ARRAY_SIZE(legacy_compatibles); type++) {
> +		if (of_device_is_compatible(np, legacy_compatibles[type]))
>  			return type;
>  	}
>  
> 

Re: [PATCH V2 1/2] hwmon: (ibmpowernv) introduce a legacy_compatibles array

[Shilpasri G Bhat]

On 06/20/2017 11:36 AM, Cédric Le Goater wrote:
> On 06/20/2017 07:08 AM, Shilpasri G Bhat wrote:
>> From: Cédric Le Goater <clg@kaod.org>
>>
>> Today, the type of a PowerNV sensor system is determined with the
>> "compatible" property for legacy Firmwares and with the "sensor-type"
>> for newer ones. The same array of strings is used for both to do the
>> matching and this raises some issue to introduce new sensor types.
>>
>> Let's introduce two different arrays (legacy and current) to make
>> things easier for new sensor types.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Tested-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> 
> Did you test on a Tuleta (IBM Power) system ? 

I have tested this patch on P9 FSP and Firestone.

> 
> Thanks,
> 
> C. 
> 
>> ---
>>  drivers/hwmon/ibmpowernv.c | 26 ++++++++++++++++++--------
>>  1 file changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
>> index 862b832..6d8909c 100644
>> --- a/drivers/hwmon/ibmpowernv.c
>> +++ b/drivers/hwmon/ibmpowernv.c
>> @@ -55,17 +55,27 @@ enum sensors {
>>  
>>  #define INVALID_INDEX (-1U)
>>  
>> +/*
>> + * 'compatible' string properties for sensor types as defined in old
>> + * PowerNV firmware (skiboot). These are ordered as 'enum sensors'.
>> + */
>> +static const char * const legacy_compatibles[] = {
>> +	"ibm,opal-sensor-cooling-fan",
>> +	"ibm,opal-sensor-amb-temp",
>> +	"ibm,opal-sensor-power-supply",
>> +	"ibm,opal-sensor-power"
>> +};
>> +
>>  static struct sensor_group {
>> -	const char *name;
>> -	const char *compatible;
>> +	const char *name; /* matches property 'sensor-type' */
>>  	struct attribute_group group;
>>  	u32 attr_count;
>>  	u32 hwmon_index;
>>  } sensor_groups[] = {
>> -	{"fan", "ibm,opal-sensor-cooling-fan"},
>> -	{"temp", "ibm,opal-sensor-amb-temp"},
>> -	{"in", "ibm,opal-sensor-power-supply"},
>> -	{"power", "ibm,opal-sensor-power"}
>> +	{ "fan"   },
>> +	{ "temp"  },
>> +	{ "in"    },
>> +	{ "power" }
>>  };
>>  
>>  struct sensor_data {
>> @@ -239,8 +249,8 @@ static int get_sensor_type(struct device_node *np)
>>  	enum sensors type;
>>  	const char *str;
>>  
>> -	for (type = 0; type < MAX_SENSOR_TYPE; type++) {
>> -		if (of_device_is_compatible(np, sensor_groups[type].compatible))
>> +	for (type = 0; type < ARRAY_SIZE(legacy_compatibles); type++) {
>> +		if (of_device_is_compatible(np, legacy_compatibles[type]))
>>  			return type;
>>  	}
>>  
>>
> 

Re: [PATCH V2 1/2] hwmon: (ibmpowernv) introduce a legacy_compatibles array

[Cédric Le Goater]

On 06/20/2017 09:15 AM, Shilpasri G Bhat wrote:
> 
> 
> On 06/20/2017 11:36 AM, Cédric Le Goater wrote:
>> On 06/20/2017 07:08 AM, Shilpasri G Bhat wrote:
>>> From: Cédric Le Goater <clg@kaod.org>
>>>
>>> Today, the type of a PowerNV sensor system is determined with the
>>> "compatible" property for legacy Firmwares and with the "sensor-type"
>>> for newer ones. The same array of strings is used for both to do the
>>> matching and this raises some issue to introduce new sensor types.
>>>
>>> Let's introduce two different arrays (legacy and current) to make
>>> things easier for new sensor types.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> Tested-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>>
>> Did you test on a Tuleta (IBM Power) system ? 
> 
> I have tested this patch on P9 FSP and Firestone.

OK. I just gave it a try on a Tuleta, P8 FSP, IBM Power system
Looks good.

Thanks,

C.

> 
>>
>> Thanks,
>>
>> C. 
>>
>>> ---
>>>  drivers/hwmon/ibmpowernv.c | 26 ++++++++++++++++++--------
>>>  1 file changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
>>> index 862b832..6d8909c 100644
>>> --- a/drivers/hwmon/ibmpowernv.c
>>> +++ b/drivers/hwmon/ibmpowernv.c
>>> @@ -55,17 +55,27 @@ enum sensors {
>>>  
>>>  #define INVALID_INDEX (-1U)
>>>  
>>> +/*
>>> + * 'compatible' string properties for sensor types as defined in old
>>> + * PowerNV firmware (skiboot). These are ordered as 'enum sensors'.
>>> + */
>>> +static const char * const legacy_compatibles[] = {
>>> +	"ibm,opal-sensor-cooling-fan",
>>> +	"ibm,opal-sensor-amb-temp",
>>> +	"ibm,opal-sensor-power-supply",
>>> +	"ibm,opal-sensor-power"
>>> +};
>>> +
>>>  static struct sensor_group {
>>> -	const char *name;
>>> -	const char *compatible;
>>> +	const char *name; /* matches property 'sensor-type' */
>>>  	struct attribute_group group;
>>>  	u32 attr_count;
>>>  	u32 hwmon_index;
>>>  } sensor_groups[] = {
>>> -	{"fan", "ibm,opal-sensor-cooling-fan"},
>>> -	{"temp", "ibm,opal-sensor-amb-temp"},
>>> -	{"in", "ibm,opal-sensor-power-supply"},
>>> -	{"power", "ibm,opal-sensor-power"}
>>> +	{ "fan"   },
>>> +	{ "temp"  },
>>> +	{ "in"    },
>>> +	{ "power" }
>>>  };
>>>  
>>>  struct sensor_data {
>>> @@ -239,8 +249,8 @@ static int get_sensor_type(struct device_node *np)
>>>  	enum sensors type;
>>>  	const char *str;
>>>  
>>> -	for (type = 0; type < MAX_SENSOR_TYPE; type++) {
>>> -		if (of_device_is_compatible(np, sensor_groups[type].compatible))
>>> +	for (type = 0; type < ARRAY_SIZE(legacy_compatibles); type++) {
>>> +		if (of_device_is_compatible(np, legacy_compatibles[type]))
>>>  			return type;
>>>  	}
>>>  
>>>
>>
> 

[PATCH V2 2/2] hwmon: (ibmpowernv) Add current(A) sensor

[Shilpasri G Bhat]

This patch exports current(A) sensors in inband sensors copied to
main memory by OCC.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
Changes from V1:
- Rebased on top of Cedric's patch to remove legay-compatible type for
  the current(A) sensor.

 drivers/hwmon/ibmpowernv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
index 6d8909c..9b11b13 100644
--- a/drivers/hwmon/ibmpowernv.c
+++ b/drivers/hwmon/ibmpowernv.c
@@ -50,6 +50,7 @@ enum sensors {
 	TEMP,
 	POWER_SUPPLY,
 	POWER_INPUT,
+	CURRENT,
 	MAX_SENSOR_TYPE,
 };
 
@@ -75,7 +76,8 @@ enum sensors {
 	{ "fan"   },
 	{ "temp"  },
 	{ "in"    },
-	{ "power" }
+	{ "power" },
+	{ "curr"  },
 };
 
 struct sensor_data {
-- 
1.8.3.1

Re: [PATCH V2 2/2] hwmon: (ibmpowernv) Add current(A) sensor

[Cédric Le Goater]

On 06/20/2017 07:08 AM, Shilpasri G Bhat wrote:
> This patch exports current(A) sensors in inband sensors copied to
> main memory by OCC.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
> Changes from V1:
> - Rebased on top of Cedric's patch to remove legay-compatible type for
>   the current(A) sensor.
> 
>  drivers/hwmon/ibmpowernv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index 6d8909c..9b11b13 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -50,6 +50,7 @@ enum sensors {
>  	TEMP,
>  	POWER_SUPPLY,
>  	POWER_INPUT,
> +	CURRENT,
>  	MAX_SENSOR_TYPE,
>  };
>  
> @@ -75,7 +76,8 @@ enum sensors {
>  	{ "fan"   },
>  	{ "temp"  },
>  	{ "in"    },
> -	{ "power" }
> +	{ "power" },
> +	{ "curr"  },
>  };
>  
>  struct sensor_data {
> 

Re: [PATCH V2 0/2] hwmon: (ibmpowernv) Add support for current(A) sensors

[Guenter Roeck]

On Tue, Jun 20, 2017 at 10:38:11AM +0530, Shilpasri G Bhat wrote:
> The first patch from Cedric in the patchset cleans up the driver to
> provide a neater way to define new sensor types. The second patch adds
> current sensor.
> 
> Cédric Le Goater (1):
>   hwmon: (ibmpowernv) introduce a legacy_compatibles array
> 
> Shilpasri G Bhat (1):
>   hwmon: (ibmpowernv) Add current(A) sensor
> 

Series applied to hwmon-next.

Thanks,
Guenter