Re: [PATCH v8 2/2] hwmon: ibmpowernv: Add attributes to enable/disable sensor groups

2018-07-25 Thread Guenter Roeck
On Tue, Jul 24, 2018 at 02:43:09PM +0530, Shilpasri G Bhat wrote:
> OPAL firmware provides the facility for some groups of sensors to be
> enabled/disabled at runtime to give the user the option of using the
> system resources for collecting these sensors or not.
> 
> For example, on POWER9 systems, the On Chip Controller (OCC) gathers
> various system and chip level sensors and maintains their values in
> main memory.
> 
> This patch provides support for enabling/disabling the sensor groups
> like power, temperature, current and voltage.
> 
> Signed-off-by: Shilpasri G Bhat 
> [stew...@linux.vnet.ibm.com: Commit message]

Acked-by: Guenter Roeck 

I would appreciate if subsequent changes were handled as additional
patches instead of forcing me to re-review everything again from
scratch.

I would also appreciate if those requesting changes would have the
courtesy of reviewing the changes triggered by those requests.

Thanks,
Guenter

> ---
> Changes from v7:
> - Use of_for_each_phandle() and of_count_phandle_with_args() to parse
>   through the phandle array
> 
>  Documentation/hwmon/ibmpowernv |  43 +++-
>  drivers/hwmon/ibmpowernv.c | 238 
> +++--
>  2 files changed, 247 insertions(+), 34 deletions(-)
> 
> diff --git a/Documentation/hwmon/ibmpowernv b/Documentation/hwmon/ibmpowernv
> index 8826ba2..5646825 100644
> --- a/Documentation/hwmon/ibmpowernv
> +++ b/Documentation/hwmon/ibmpowernv
> @@ -33,9 +33,48 @@ fanX_input Measured RPM value.
>  fanX_min Threshold RPM for alert generation.
>  fanX_fault   0: No fail condition
>   1: Failing fan
> +
>  tempX_input  Measured ambient temperature.
>  tempX_maxThreshold ambient temperature for alert generation.
> -inX_inputMeasured power supply voltage
> +tempX_highestHistorical maximum temperature
> +tempX_lowest Historical minimum temperature
> +tempX_enable Enable/disable all temperature sensors belonging to the
> + sub-group. In POWER9, this attribute corresponds to
> + each OCC. Using this attribute each OCC can be asked to
> + disable/enable all of its temperature sensors.
> + 1: Enable
> + 0: Disable
> +
> +inX_inputMeasured power supply voltage (millivolt)
>  inX_fault0: No fail condition.
>   1: Failing power supply.
> -power1_input System power consumption (microWatt)
> +inX_highest  Historical maximum voltage
> +inX_lowest   Historical minimum voltage
> +inX_enable   Enable/disable all voltage sensors belonging to the
> + sub-group. In POWER9, this attribute corresponds to
> + each OCC. Using this attribute each OCC can be asked to
> + disable/enable all of its voltage sensors.
> + 1: Enable
> + 0: Disable
> +
> +powerX_input Power consumption (microWatt)
> +powerX_input_highest Historical maximum power
> +powerX_input_lowest  Historical minimum power
> +powerX_enableEnable/disable all power sensors belonging to 
> the
> + sub-group. In POWER9, this attribute corresponds to
> + each OCC. Using this attribute each OCC can be asked to
> + disable/enable all of its power sensors.
> + 1: Enable
> + 0: Disable
> +
> +currX_input  Measured current (milliampere)
> +currX_highestHistorical maximum current
> +currX_lowest Historical minimum current
> +currX_enable Enable/disable all current sensors belonging to the
> + sub-group. In POWER9, this attribute corresponds to
> + each OCC. Using this attribute each OCC can be asked to
> + disable/enable all of its current sensors.
> + 1: Enable
> + 0: Disable
> +
> +energyX_inputCumulative energy (microJoule)
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index f829dad..8347280 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -90,11 +90,20 @@ struct sensor_data {
>   char label[MAX_LABEL_LEN];
>   char name[MAX_ATTR_LEN];
>   struct device_attribute dev_attr;
> + struct sensor_group_data *sgrp_data;
> +};
> +
> +struct sensor_group_data {
> + struct mutex mutex;
> + u32 gid;
> + bool enable;
>  };
>  
>  struct platform_data {
>   const struct attribute_group *attr_groups[MAX_SENSOR_TYPE + 1];
> + struct sensor_group_data *sgrp_data;
>   u32 sensors_count; /* Total count of sensors from each group */
> + u32 nr_sensor_groups; /* Total number of sensor groups */
>  };
>  
>  static ssize_t show_sensor(struct device 

[PATCH v8 2/2] hwmon: ibmpowernv: Add attributes to enable/disable sensor groups

2018-07-24 Thread Shilpasri G Bhat
OPAL firmware provides the facility for some groups of sensors to be
enabled/disabled at runtime to give the user the option of using the
system resources for collecting these sensors or not.

For example, on POWER9 systems, the On Chip Controller (OCC) gathers
various system and chip level sensors and maintains their values in
main memory.

This patch provides support for enabling/disabling the sensor groups
like power, temperature, current and voltage.

Signed-off-by: Shilpasri G Bhat 
[stew...@linux.vnet.ibm.com: Commit message]
---
Changes from v7:
- Use of_for_each_phandle() and of_count_phandle_with_args() to parse
  through the phandle array

 Documentation/hwmon/ibmpowernv |  43 +++-
 drivers/hwmon/ibmpowernv.c | 238 +++--
 2 files changed, 247 insertions(+), 34 deletions(-)

diff --git a/Documentation/hwmon/ibmpowernv b/Documentation/hwmon/ibmpowernv
index 8826ba2..5646825 100644
--- a/Documentation/hwmon/ibmpowernv
+++ b/Documentation/hwmon/ibmpowernv
@@ -33,9 +33,48 @@ fanX_input   Measured RPM value.
 fanX_min   Threshold RPM for alert generation.
 fanX_fault 0: No fail condition
1: Failing fan
+
 tempX_inputMeasured ambient temperature.
 tempX_max  Threshold ambient temperature for alert generation.
-inX_input  Measured power supply voltage
+tempX_highest  Historical maximum temperature
+tempX_lowest   Historical minimum temperature
+tempX_enable   Enable/disable all temperature sensors belonging to the
+   sub-group. In POWER9, this attribute corresponds to
+   each OCC. Using this attribute each OCC can be asked to
+   disable/enable all of its temperature sensors.
+   1: Enable
+   0: Disable
+
+inX_input  Measured power supply voltage (millivolt)
 inX_fault  0: No fail condition.
1: Failing power supply.
-power1_input   System power consumption (microWatt)
+inX_highestHistorical maximum voltage
+inX_lowest Historical minimum voltage
+inX_enable Enable/disable all voltage sensors belonging to the
+   sub-group. In POWER9, this attribute corresponds to
+   each OCC. Using this attribute each OCC can be asked to
+   disable/enable all of its voltage sensors.
+   1: Enable
+   0: Disable
+
+powerX_input   Power consumption (microWatt)
+powerX_input_highest   Historical maximum power
+powerX_input_lowestHistorical minimum power
+powerX_enable  Enable/disable all power sensors belonging to the
+   sub-group. In POWER9, this attribute corresponds to
+   each OCC. Using this attribute each OCC can be asked to
+   disable/enable all of its power sensors.
+   1: Enable
+   0: Disable
+
+currX_inputMeasured current (milliampere)
+currX_highest  Historical maximum current
+currX_lowest   Historical minimum current
+currX_enable   Enable/disable all current sensors belonging to the
+   sub-group. In POWER9, this attribute corresponds to
+   each OCC. Using this attribute each OCC can be asked to
+   disable/enable all of its current sensors.
+   1: Enable
+   0: Disable
+
+energyX_input  Cumulative energy (microJoule)
diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
index f829dad..8347280 100644
--- a/drivers/hwmon/ibmpowernv.c
+++ b/drivers/hwmon/ibmpowernv.c
@@ -90,11 +90,20 @@ struct sensor_data {
char label[MAX_LABEL_LEN];
char name[MAX_ATTR_LEN];
struct device_attribute dev_attr;
+   struct sensor_group_data *sgrp_data;
+};
+
+struct sensor_group_data {
+   struct mutex mutex;
+   u32 gid;
+   bool enable;
 };
 
 struct platform_data {
const struct attribute_group *attr_groups[MAX_SENSOR_TYPE + 1];
+   struct sensor_group_data *sgrp_data;
u32 sensors_count; /* Total count of sensors from each group */
+   u32 nr_sensor_groups; /* Total number of sensor groups */
 };
 
 static ssize_t show_sensor(struct device *dev, struct device_attribute 
*devattr,
@@ -105,6 +114,9 @@ static ssize_t show_sensor(struct device *dev, struct 
device_attribute *devattr,
ssize_t ret;
u64 x;
 
+   if (sdata->sgrp_data && !sdata->sgrp_data->enable)
+   return -ENODATA;
+
ret =  opal_get_sensor_data_u64(sdata->id, );
 
if (ret)
@@ -120,6 +132,46 @@ static ssize_t show_sensor(struct device *dev, struct 
device_attribute *devattr,
return sprintf(buf, "%llu\n", x);
 }
 
+static