[PATCH v2 0/2] scpi-sensors: Fix SCP sensor readings scale

[PATCH v2 0/2] scpi-sensors: Fix SCP sensor readings scale

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

The implementation details for SCPI seems to suggest that the sensor
readings must be reported by SCP using a well defined scale
(millidegree Celsius for temperature, millivolts for voltage,
milliamperes for current, microwatts for power and microjoules for
energy).

This is also important for the interaction with other subsystems: for
example both the thermal sub-system and the hwmon sysfs interface expect
the temperature expressed in millidegree Celsius.

Unfortunately since this behaviour is dependent on the firmware
implementation there are cases where the sensor readings are reported
using a different scale. For example in the Amlogic SoCs the
temperature is reported in degree and not millidegree Celsius.

In this patchset we introduce a new DT property `scpi,sensors-scale` that is
used by the scpi-hwmon driver to convert the sensor readings to the expected
scale.

v2:
 - Added data validation (scale != 0)
 - s/unsigned int/u32/ for the array
 - Switched to array initialiser notation
 - Introduced scpi_scale_reading()

Carlo Caione (2):
  Documentation: bindings: Introduce scpi,sensors-scale
  hwmon: (scpi) Fix the scale of SCP sensor readings

 Documentation/devicetree/bindings/arm/arm,scpi.txt | 18 ++++++++++++
 drivers/hwmon/scpi-hwmon.c                         | 34 ++++++++++++++++++++++
 2 files changed, 52 insertions(+)

-- 
2.12.0

[PATCH v2 1/2] Documentation: bindings: Introduce scpi,sensors-scale

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

Document the new property `scpi,sensors-scale` used by the hwmon-scpi
driver to convert the sensor readings to the correct / expected scale.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
index 401831973638..35c621ce52c5 100644
--- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -84,6 +84,24 @@ Required properties:
 			 as used by the firmware. Refer to  platform details
 			 for your implementation for the IDs to use.
 
+Optional properties:
+- scpi,sensors-scale: array of 5 elements defining the unit scale used by the
+                      SCP for the 5 different sensor classes (temperature,
+                      voltage, current, power, energy) such that 1 == unit,
+                      1000 == milli, 1000000 == micro, ...
+
+                      Example:
+                         scpi,sensors-scale = <1 1000 1000 1000000 1000000>
+                      In this case the values reported by SCP are:
+                      -> temperature in degree Celsius
+                      -> voltage in millivolts
+                      -> current in milliamperes
+                      -> power in microwatts
+                      -> energy in microjoules
+
+                      The default value is:
+                         scpi,sensors-scale = <1000 1000 1000 1000000 1000000>
+
 Power domain bindings for the power domains based on SCPI Message Protocol
 ------------------------------------------------------------
 
-- 
2.12.0

[PATCH v2 2/2] hwmon: (scpi) Fix the scale of SCP sensor readings

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

The implementation details for SCPI seems to suggest that the sensor
readings must be reported by SCP using a well defined scale
(millidegree Celsius for temperature, millivolts for voltage,
milliamperes for current, microwatts for power and microjoules for
energy).

This is also important for the interaction with other subsystems: for
example both the thermal sub-system and the hwmon sysfs interface expect
the temperature expressed in millidegree Celsius.

Unfortunately since this behaviour is dependent on the firmware
implementation there are cases where the sensor readings are reported
using a different scale. For example in the Amlogic SoCs the
temperature is reported in degree and not millidegree Celsius.

To take into account this discrepancy and fixup the values reported by
SCP a new DT property `scpi,sensors-scale' is introduced and used in
this patch by the scpi-hwmon driver to convert the sensor readings to
the expected scale.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 drivers/hwmon/scpi-hwmon.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index 094f948f99ff..b64fe7d10742 100644
--- a/drivers/hwmon/scpi-hwmon.c
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -23,6 +23,7 @@
 #include <linux/thermal.h>
 
 struct sensor_data {
+	unsigned int scale;
 	struct scpi_sensor_info info;
 	struct device_attribute dev_attr_input;
 	struct device_attribute dev_attr_label;
@@ -44,6 +45,22 @@ struct scpi_sensors {
 	const struct attribute_group *groups[2];
 };
 
+static const u32 scpi_scale[] = {
+	[TEMPERATURE]	= 1000,		/* (millicelsius)	*/
+	[VOLTAGE]	= 1000,		/* (millivolts)		*/
+	[CURRENT]	= 1000,		/* (milliamperes)	*/
+	[POWER]		= 1000000,	/* (microwatts)		*/
+	[ENERGY]	= 1000000,	/* (microjoules)	*/
+};
+
+void scpi_scale_reading(u64 *value, struct sensor_data *sensor)
+{
+	if (scpi_scale[sensor->info.class] != sensor->scale) {
+		*value *= scpi_scale[sensor->info.class];
+		do_div(*value, sensor->scale);
+	}
+}
+
 static int scpi_read_temp(void *dev, int *temp)
 {
 	struct scpi_thermal_zone *zone = dev;
@@ -57,6 +74,8 @@ static int scpi_read_temp(void *dev, int *temp)
 	if (ret)
 		return ret;
 
+	scpi_scale_reading(&value, sensor);
+
 	*temp = value;
 	return 0;
 }
@@ -77,6 +96,8 @@ scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
 	if (ret)
 		return ret;
 
+	scpi_scale_reading(&value, sensor);
+
 	return sprintf(buf, "%llu\n", value);
 }
 
@@ -97,6 +118,7 @@ static struct thermal_zone_of_device_ops scpi_sensor_ops = {
 static int scpi_hwmon_probe(struct platform_device *pdev)
 {
 	u16 nr_sensors, i;
+	u32 scale[] = { 1000, 1000, 1000, 1000000, 1000000 };
 	int num_temp = 0, num_volt = 0, num_current = 0, num_power = 0;
 	int num_energy = 0;
 	struct scpi_ops *scpi_ops;
@@ -131,6 +153,16 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 
 	scpi_sensors->scpi_ops = scpi_ops;
 
+	of_property_read_u32_array(dev->of_node, "scpi,sensors-scale",
+				   scale, ARRAY_SIZE(scale));
+
+	for (i = 0; i < ARRAY_SIZE(scale); i++) {
+		if (!scale[i]) {
+			dev_err(dev, "%s: scale cannot be zero (%d)\n", __func__, i);
+			return -EINVAL;
+		}
+	}
+
 	for (i = 0, idx = 0; i < nr_sensors; i++) {
 		struct sensor_data *sensor = &scpi_sensors->data[idx];
 
@@ -178,6 +210,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 			continue;
 		}
 
+		sensor->scale = scale[sensor->info.class];
+
 		sensor->dev_attr_input.attr.mode = S_IRUGO;
 		sensor->dev_attr_input.show = scpi_show_sensor;
 		sensor->dev_attr_input.attr.name = sensor->input;
-- 
2.12.0

[PATCH v2 0/2] scpi-sensors: Fix SCP sensor readings scale

[Carlo Caione]

On Fri, Mar 3, 2017 at 10:14 AM, Carlo Caione <carlo@caione.org> wrote:
> From: Carlo Caione <carlo@endlessm.com>
>
> The implementation details for SCPI seems to suggest that the sensor
> readings must be reported by SCP using a well defined scale
> (millidegree Celsius for temperature, millivolts for voltage,
> milliamperes for current, microwatts for power and microjoules for
> energy).
>
> This is also important for the interaction with other subsystems: for
> example both the thermal sub-system and the hwmon sysfs interface expect
> the temperature expressed in millidegree Celsius.
>
> Unfortunately since this behaviour is dependent on the firmware
> implementation there are cases where the sensor readings are reported
> using a different scale. For example in the Amlogic SoCs the
> temperature is reported in degree and not millidegree Celsius.
>
> In this patchset we introduce a new DT property `scpi,sensors-scale` that is
> used by the scpi-hwmon driver to convert the sensor readings to the expected
> scale.
>
> v2:
>  - Added data validation (scale != 0)
>  - s/unsigned int/u32/ for the array
>  - Switched to array initialiser notation
>  - Introduced scpi_scale_reading()

gentle ping



-- 
Carlo Caione

[PATCH v2 2/2] hwmon: (scpi) Fix the scale of SCP sensor readings

[Punit Agrawal]

Carlo Caione <carlo@caione.org> writes:

> From: Carlo Caione <carlo@endlessm.com>
>
> The implementation details for SCPI seems to suggest that the sensor
> readings must be reported by SCP using a well defined scale
> (millidegree Celsius for temperature, millivolts for voltage,
> milliamperes for current, microwatts for power and microjoules for
> energy).
>
> This is also important for the interaction with other subsystems: for
> example both the thermal sub-system and the hwmon sysfs interface expect
> the temperature expressed in millidegree Celsius.
>
> Unfortunately since this behaviour is dependent on the firmware
> implementation there are cases where the sensor readings are reported
> using a different scale. For example in the Amlogic SoCs the
> temperature is reported in degree and not millidegree Celsius.
>
> To take into account this discrepancy and fixup the values reported by
> SCP a new DT property `scpi,sensors-scale' is introduced and used in
> this patch by the scpi-hwmon driver to convert the sensor readings to
> the expected scale.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  drivers/hwmon/scpi-hwmon.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index 094f948f99ff..b64fe7d10742 100644
> --- a/drivers/hwmon/scpi-hwmon.c
> +++ b/drivers/hwmon/scpi-hwmon.c
> @@ -23,6 +23,7 @@
>  #include <linux/thermal.h>
>  
>  struct sensor_data {
> +	unsigned int scale;
>  	struct scpi_sensor_info info;
>  	struct device_attribute dev_attr_input;
>  	struct device_attribute dev_attr_label;
> @@ -44,6 +45,22 @@ struct scpi_sensors {
>  	const struct attribute_group *groups[2];
>  };
>  
> +static const u32 scpi_scale[] = {
> +	[TEMPERATURE]	= 1000,		/* (millicelsius)	*/
> +	[VOLTAGE]	= 1000,		/* (millivolts)		*/
> +	[CURRENT]	= 1000,		/* (milliamperes)	*/
> +	[POWER]		= 1000000,	/* (microwatts)		*/
> +	[ENERGY]	= 1000000,	/* (microjoules)	*/
> +};
> +
> +void scpi_scale_reading(u64 *value, struct sensor_data *sensor)

This function is not intended for use outside this file. Please mark as
static.

> +{
> +	if (scpi_scale[sensor->info.class] != sensor->scale) {
> +		*value *= scpi_scale[sensor->info.class];
> +		do_div(*value, sensor->scale);
> +	}
> +}
> +
>  static int scpi_read_temp(void *dev, int *temp)
>  {
>  	struct scpi_thermal_zone *zone = dev;
> @@ -57,6 +74,8 @@ static int scpi_read_temp(void *dev, int *temp)
>  	if (ret)
>  		return ret;
>  
> +	scpi_scale_reading(&value, sensor);
> +
>  	*temp = value;
>  	return 0;
>  }
> @@ -77,6 +96,8 @@ scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
>  	if (ret)
>  		return ret;
>  
> +	scpi_scale_reading(&value, sensor);
> +
>  	return sprintf(buf, "%llu\n", value);
>  }
>  
> @@ -97,6 +118,7 @@ static struct thermal_zone_of_device_ops scpi_sensor_ops = {
>  static int scpi_hwmon_probe(struct platform_device *pdev)
>  {
>  	u16 nr_sensors, i;
> +	u32 scale[] = { 1000, 1000, 1000, 1000000, 1000000 };
>  	int num_temp = 0, num_volt = 0, num_current = 0, num_power = 0;
>  	int num_energy = 0;
>  	struct scpi_ops *scpi_ops;
> @@ -131,6 +153,16 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>  
>  	scpi_sensors->scpi_ops = scpi_ops;
>  
> +	of_property_read_u32_array(dev->of_node, "scpi,sensors-scale",
> +				   scale, ARRAY_SIZE(scale));
> +
> +	for (i = 0; i < ARRAY_SIZE(scale); i++) {
> +		if (!scale[i]) {
> +			dev_err(dev, "%s: scale cannot be zero (%d)\n", __func__, i);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	for (i = 0, idx = 0; i < nr_sensors; i++) {
>  		struct sensor_data *sensor = &scpi_sensors->data[idx];
>  
> @@ -178,6 +210,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>  			continue;
>  		}
>  
> +		sensor->scale = scale[sensor->info.class];
> +
>  		sensor->dev_attr_input.attr.mode = S_IRUGO;
>  		sensor->dev_attr_input.show = scpi_show_sensor;
>  		sensor->dev_attr_input.attr.name = sensor->input;

So with the function marked as static,

Acked-by: Punit Agrawal <punit.agrawal@arm.com>

[PATCH v2 0/2] scpi-sensors: Fix SCP sensor readings scale

[Guenter Roeck]

On Wed, Mar 08, 2017 at 09:24:38AM +0100, Carlo Caione wrote:
> On Fri, Mar 3, 2017 at 10:14 AM, Carlo Caione <carlo@caione.org> wrote:
> > From: Carlo Caione <carlo@endlessm.com>
> >
> > The implementation details for SCPI seems to suggest that the sensor
> > readings must be reported by SCP using a well defined scale
> > (millidegree Celsius for temperature, millivolts for voltage,
> > milliamperes for current, microwatts for power and microjoules for
> > energy).
> >
> > This is also important for the interaction with other subsystems: for
> > example both the thermal sub-system and the hwmon sysfs interface expect
> > the temperature expressed in millidegree Celsius.
> >
> > Unfortunately since this behaviour is dependent on the firmware
> > implementation there are cases where the sensor readings are reported
> > using a different scale. For example in the Amlogic SoCs the
> > temperature is reported in degree and not millidegree Celsius.
> >
> > In this patchset we introduce a new DT property `scpi,sensors-scale` that is
> > used by the scpi-hwmon driver to convert the sensor readings to the expected
> > scale.
> >
> > v2:
> >  - Added data validation (scale != 0)
> >  - s/unsigned int/u32/ for the array
> >  - Switched to array initialiser notation
> >  - Introduced scpi_scale_reading()
> 
> gentle ping
> 

Waiting for Rob to provide feedback on the devicetree property.

Guenter