[PATCH v2 00/10] Platform support for thermal management on Juno

[PATCH v2 00/10] Platform support for thermal management on Juno

[Punit Agrawal]

Hi,

The series adds support for thermal management on ARM Juno development
platform. As part of this development, common infrastructure is added
to support registering cpu cooling devices that work with the power
allocator thermal governor.

Patch 1 extends the CPU nodes binding to provide an optional dynamic
power coefficient which can be used to create a dynamic power model
for the CPUs. This model is used to constrain device power consumption
(using power_allocator governor) when the system is thermally
constrained.

Patches 2-3 extends the cpufreq-dt and arm_big_little driver to
register cpu cooling devices with the dynamic coefficient when
provided.

Patch 4 is a fix that aligns the behaviour of
thermal_zone_of_sensor_register with it's documented return value when
THERMAL_OF is configured off.

Patches 5-8 create a hwmon sensor driver for sensors provided by SCPI
firmware. Patch 7 adds support for the temperature sensors to register
with the thermal framework. This allows setting up platform thermals
using OF thermal bindings.

The last two patches add support for the sensors and the thermal zones
in the Juno device tree.

The Juno specific patches depend on SCPI and cpufreq patches[0] from
Sudeep. The SCPI protocol document with details of the sensor
interface can be found at [1], [2].

Thanks,
Punit

Changes v1->v2
  - added documentation for the scpi-hwmon driver
  - replaced static allocation of platform structure in scpi-hwmon.c
    with dynamic allocation
  - re-structured registering of thermal zones to better handle
    error conditions
  - reduced chattiness in scpi-hwmon.c
  - corrected sensor numbering to align with hwmon documentation
  - dynamic allocation of attributes and attribute groups

[0] http://thread.gmane.org/gmane.linux.kernel/2009735
[1] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
[2] https://wiki.linaro.org/ARM/Juno?action=AttachFile&do=get&target=DUI0922B_scp_message_interface.pdf
v1: http://thread.gmane.org/gmane.linux.kernel/2002152

Punit Agrawal (10):
  devicetree: bindings: Add optional dynamic-power-coefficient property
  cpufreq-dt: Supply power coefficient when registering cooling devices
  cpufreq: arm_big_little: Add support to register a cpufreq cooling
    device
  thermal: Fix thermal_zone_of_sensor_register to match documentation
  Documentation: add DT bindings for ARM SCPI sensors
  firmware: arm_scpi: Extend to support sensors
  hwmon: Support sensors exported via ARM SCP interface
  hwmon: Support registration of thermal zones for SCP temperature
    sensors
  arm64: dts: Add sensor node to Juno dt
  arm64: dts: Create SoC thermal zone for Juno

 Documentation/devicetree/bindings/arm/arm,scpi.txt |  15 ++
 Documentation/devicetree/bindings/arm/cpus.txt     |  15 ++
 Documentation/hwmon/scpi-hwmon                     |  33 +++
 arch/arm64/boot/dts/arm/juno-base.dtsi             |   5 +
 arch/arm64/boot/dts/arm/juno.dts                   |  50 ++++
 drivers/cpufreq/arm_big_little.c                   |  52 +++-
 drivers/cpufreq/cpufreq-dt.c                       |   9 +-
 drivers/firmware/arm_scpi.c                        |  60 +++++
 drivers/hwmon/Kconfig                              |   8 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/scpi-hwmon.c                         | 284 +++++++++++++++++++++
 include/linux/scpi_protocol.h                      |  17 ++
 include/linux/thermal.h                            |   2 +-
 13 files changed, 546 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/hwmon/scpi-hwmon
 create mode 100644 drivers/hwmon/scpi-hwmon.c

-- 
2.1.4

[PATCH v2 01/10] devicetree: bindings: Add optional dynamic-power-coefficient property

[Punit Agrawal]

The dynamic power consumption of a device is proportional to the
square of voltage (V) and the clock frequency (f). It can be expressed as

Pdyn = dynamic-power-coefficient * V^2 * f.

The dynamic-power-coefficient property represents an indicative
running time dynamic power coefficient in fundamental units of
mw/MHz/uVolt^2 and can be used in the above formula to calculate the
dynamic power in mW.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/devicetree/bindings/arm/cpus.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index d6b794c..c32bfac 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -239,6 +239,21 @@ nodes to be present and contain the properties described below.
 		Definition: Specifies the syscon node controlling the cpu core
 			    power domains.
 
+	- dynamic-power-coefficient
+		Usage: optional
+		Value type: <prop-encoded-array>
+		Definition: A u32 value that represents an indicative
+			    running time dynamic power coefficient in
+			    fundamental units of mW/MHz/uVolt^2.
+
+			    The dynamic energy consumption of the CPU
+			    is proportional to the square of the
+			    Voltage (V) and the clock frequency (f).
+
+			    Pdyn = dynamic-power-coefficient * V^2 * f
+
+			    where voltage is in uV, frequency is in MHz.
+
 Example 1 (dual-cluster big.LITTLE system 32-bit):
 
 	cpus {
-- 
2.1.4

[PATCH v2 02/10] cpufreq-dt: Supply power coefficient when registering cooling devices

[Punit Agrawal]

Support registering cooling devices with dynamic power coefficient
where provided by the device tree. This allows OF registered cooling
devices driver to be used with the power_allocator thermal governor.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/cpufreq/cpufreq-dt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 528a82bf..1d1f61a 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -344,8 +344,13 @@ static void cpufreq_ready(struct cpufreq_policy *policy)
 	 * thermal DT code takes care of matching them.
 	 */
 	if (of_find_property(np, "#cooling-cells", NULL)) {
-		priv->cdev = of_cpufreq_cooling_register(np,
-							 policy->related_cpus);
+		u32 power_coefficient = 0;
+
+		of_property_read_u32(np, "dynamic-power-coefficient",
+				     &power_coefficient);
+
+		priv->cdev = of_cpufreq_power_cooling_register(np,
+				policy->related_cpus, power_coefficient, NULL);
 		if (IS_ERR(priv->cdev)) {
 			dev_err(priv->cpu_dev,
 				"running cpufreq without cooling device: %ld\n",
-- 
2.1.4

[PATCH v2 03/10] cpufreq: arm_big_little: Add support to register a cpufreq cooling device

[Punit Agrawal]

Register passive cooling devices when initialising cpufreq on
big.LITTLE systems. If the device tree provides a dynamic power
coefficient for the CPUs then the bound cooling device will support
the extensions that allow it to be used with all the existing thermal
governors including the power allocator governor.

A cooling device will be created per individual frequency domain and
can be bound to thermal zones via the thermal DT bindings.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/cpufreq/arm_big_little.c | 52 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index f1e42f8..72a2777 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -23,6 +23,7 @@
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
+#include <linux/cpu_cooling.h>
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -55,6 +56,10 @@ static bool bL_switching_enabled;
 #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
 #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
 
+struct private_data {
+	struct thermal_cooling_device *cdev;
+};
+
 static struct cpufreq_arm_bL_ops *arm_bL_ops;
 static struct clk *clk[MAX_CLUSTERS];
 static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -440,6 +445,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 {
 	u32 cur_cluster = cpu_to_cluster(policy->cpu);
 	struct device *cpu_dev;
+	struct private_data *priv;
 	int ret;
 
 	cpu_dev = get_cpu_device(policy->cpu);
@@ -457,9 +463,15 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 	if (ret) {
 		dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
 				policy->cpu, cur_cluster);
-		put_cluster_clk_and_freq_table(cpu_dev);
-		return ret;
+		goto out_free_cpufreq_table;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out_free_cpufreq_table;
 	}
+	policy->driver_data = priv;
 
 	if (cur_cluster < MAX_CLUSTERS) {
 		int cpu;
@@ -484,12 +496,19 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 
 	dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);
 	return 0;
+
+out_free_cpufreq_table:
+	put_cluster_clk_and_freq_table(cpu_dev);
+
+	return ret;
 }
 
 static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 {
 	struct device *cpu_dev;
+	struct private_data *priv = policy->driver_data;
 
+	cpufreq_cooling_unregister(priv->cdev);
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
 		pr_err("%s: failed to get cpu%d device\n", __func__,
@@ -498,11 +517,39 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 	}
 
 	put_cluster_clk_and_freq_table(cpu_dev);
+	kfree(priv);
 	dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
 
 	return 0;
 }
 
+static void bL_cpufreq_ready(struct cpufreq_policy *policy)
+{
+	struct device *cpu_dev = get_cpu_device(policy->cpu);
+	struct device_node *np = of_node_get(cpu_dev->of_node);
+	struct private_data *priv = policy->driver_data;
+
+	if (WARN_ON(!np))
+		return;
+
+	if (of_find_property(np, "#cooling-cells", NULL)) {
+		u32 power_coefficient = 0;
+
+		of_property_read_u32(np, "dynamic-power-coefficient",
+				     &power_coefficient);
+
+		priv->cdev = of_cpufreq_power_cooling_register(np,
+				policy->related_cpus, power_coefficient, NULL);
+		if (IS_ERR(priv->cdev)) {
+			dev_err(cpu_dev,
+				"running cpufreq without cooling device: %ld\n",
+				PTR_ERR(priv->cdev));
+			priv->cdev = NULL;
+		}
+	}
+	of_node_put(np);
+}
+
 static struct cpufreq_driver bL_cpufreq_driver = {
 	.name			= "arm-big-little",
 	.flags			= CPUFREQ_STICKY |
@@ -513,6 +560,7 @@ static struct cpufreq_driver bL_cpufreq_driver = {
 	.get			= bL_cpufreq_get_rate,
 	.init			= bL_cpufreq_init,
 	.exit			= bL_cpufreq_exit,
+	.ready			= bL_cpufreq_ready,
 	.attr			= cpufreq_generic_attr,
 };
 
-- 
2.1.4

[PATCH v2 04/10] thermal: Fix thermal_zone_of_sensor_register to match documentation

[Punit Agrawal]

thermal_zone_of_sensor_register is documented as returning a pointer
to either a valid thermal_zone_device on success, or a corresponding
ERR_PTR() value.

In contrast, the function returns NULL when THERMAL_OF is configured
off. Fix this.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Zhang Rui <rui.zhang@intel.com>
---
 include/linux/thermal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 037e9df..a47b34a 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -364,7 +364,7 @@ static inline struct thermal_zone_device *
 thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 				const struct thermal_zone_of_device_ops *ops)
 {
-	return NULL;
+	return ERR_PTR(-ENOSYS);
 }
 
 static inline
-- 
2.1.4

[PATCH v2 05/10] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

The System Control Processor (SCP) provides access to SoC sensors via
the System Control and Power Interface (SCPI) Message Protocol. Add
bindings to allow probing of these sensors. Also support referencing
of the sensors for setting up thermal zones via the thermal DT
bindings.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
index e309249..8ead68a 100644
--- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -72,8 +72,18 @@ Required sub-node properties:
 - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
 	       shared memory on Juno platforms
 
+Sensor bindings for the sensors based on SCPI Message Protocol
+--------------------------------------------------------------
+SCPI provides an API to access the various sensors on the SoC.
+
+Required properties:
+- compatible : should be "arm,scpi-sensors".
+- #thermal-sensor-cells: should be set to 1. This property follows the
+			 thermal device tree bindings[2].
+
 [0] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/thermal/thermal.txt
 
 Example:
 
@@ -122,6 +132,11 @@ scpi_protocol: scpi@2e000000 {
 			clock-output-names = "pxlclk0", "pxlclk1";
 		};
 	};
+
+	scpi_sensors0: sensors {
+		compatible = "arm,scpi-sensors";
+		#thermal-sensor-cells = <1>;
+	};
 };
 
 cpu@0 {
-- 
2.1.4

[PATCH v2 06/10] firmware: arm_scpi: Extend to support sensors

[Punit Agrawal]

ARM System Control Processor (SCP) provides an API to query and use
the sensors available in the system. Extend the SCPI driver to support
 sensor messages.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scpi.c   | 60 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/scpi_protocol.h | 17 ++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index cb75c75..321680e 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -219,6 +219,21 @@ struct dvfs_set {
 	u8 index;
 } __packed;
 
+struct sensor_capabilities {
+	__le16 sensors;
+} __packed;
+
+struct _scpi_sensor_info {
+	__le16 sensor_id;
+	u8 class;
+	u8 trigger_type;
+	char name[20];
+};
+
+struct sensor_value {
+	__le32 val;
+} __packed;
+
 static struct scpi_drvinfo *scpi_info;
 
 static int scpi_linux_errmap[SCPI_ERR_MAX] = {
@@ -481,6 +496,48 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
 	return info;
 }
 
+static int scpi_sensor_get_capability(u16 *sensors)
+{
+	struct sensor_capabilities cap_buf;
+	int ret;
+
+	ret = scpi_send_message(SCPI_CMD_SENSOR_CAPABILITIES, NULL, 0, &cap_buf,
+				sizeof(cap_buf));
+	if (!ret)
+		*sensors = le16_to_cpu(cap_buf.sensors);
+
+	return ret;
+}
+
+static int scpi_sensor_get_info(u16 sensor_id, struct scpi_sensor_info *info)
+{
+	__le16 id = cpu_to_le16(sensor_id);
+	struct _scpi_sensor_info _info;
+	int ret;
+
+	ret = scpi_send_message(SCPI_CMD_SENSOR_INFO, &id,
+				 sizeof(id), &_info, sizeof(_info));
+	if (!ret) {
+		memcpy(info, &_info, sizeof(*info));
+		info->sensor_id = le16_to_cpu(_info.sensor_id);
+	}
+
+	return ret;
+}
+
+int scpi_sensor_get_value(u16 sensor, u32 *val)
+{
+	struct sensor_value buf;
+	int ret;
+
+	ret = scpi_send_message(SCPI_CMD_SENSOR_VALUE, &sensor, sizeof(sensor),
+				&buf, sizeof(buf));
+	if (!ret)
+		*val = le32_to_cpu(buf.val);
+
+	return ret;
+}
+
 static struct scpi_ops scpi_ops = {
 	.get_version = scpi_get_version,
 	.clk_get_range = scpi_clk_get_range,
@@ -489,6 +546,9 @@ static struct scpi_ops scpi_ops = {
 	.dvfs_get_idx = scpi_dvfs_get_idx,
 	.dvfs_set_idx = scpi_dvfs_set_idx,
 	.dvfs_get_info = scpi_dvfs_get_info,
+	.sensor_get_capability = scpi_sensor_get_capability,
+	.sensor_get_info = scpi_sensor_get_info,
+	.sensor_get_value = scpi_sensor_get_value,
 };
 
 struct scpi_ops *get_scpi_ops(void)
diff --git a/include/linux/scpi_protocol.h b/include/linux/scpi_protocol.h
index e7169cd..80af3cd 100644
--- a/include/linux/scpi_protocol.h
+++ b/include/linux/scpi_protocol.h
@@ -28,6 +28,20 @@ struct scpi_dvfs_info {
 	struct scpi_opp *opps;
 };
 
+enum scpi_sensor_class {
+	TEMPERATURE,
+	VOLTAGE,
+	CURRENT,
+	POWER,
+};
+
+struct scpi_sensor_info {
+	u16 sensor_id;
+	u8 class;
+	u8 trigger_type;
+	char name[20];
+} __packed;
+
 /**
  * struct scpi_ops - represents the various operations provided
  *	by SCP through SCPI message protocol
@@ -52,6 +66,9 @@ struct scpi_ops {
 	int (*dvfs_get_idx)(u8);
 	int (*dvfs_set_idx)(u8, u8);
 	struct scpi_dvfs_info *(*dvfs_get_info)(u8);
+	int (*sensor_get_capability)(u16 *sensors);
+	int (*sensor_get_info)(u16 sensor_id, struct scpi_sensor_info *);
+	int (*sensor_get_value)(u16, u32 *);
 };
 
 #if IS_ENABLED(CONFIG_ARM_SCPI_PROTOCOL)
-- 
2.1.4

[PATCH v2 07/10] hwmon: Support sensors exported via ARM SCP interface

[Punit Agrawal]

Create a driver to add support for SoC sensors exported by the System
Control Processor (SCP) via the System Control and Power Interface
(SCPI). The supported sensor types is one of voltage, temperature,
current, and power.

The sensor labels and values provided by the SCP are exported via the
hwmon sysfs interface.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Jean Delvare <jdelvare@suse.de>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/hwmon/scpi-hwmon |  33 ++++++++
 drivers/hwmon/Kconfig          |   8 ++
 drivers/hwmon/Makefile         |   1 +
 drivers/hwmon/scpi-hwmon.c     | 183 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 225 insertions(+)
 create mode 100644 Documentation/hwmon/scpi-hwmon
 create mode 100644 drivers/hwmon/scpi-hwmon.c

diff --git a/Documentation/hwmon/scpi-hwmon b/Documentation/hwmon/scpi-hwmon
new file mode 100644
index 0000000..4cfcdf2d
--- /dev/null
+++ b/Documentation/hwmon/scpi-hwmon
@@ -0,0 +1,33 @@
+Kernel driver scpi-hwmon
+========================
+
+Supported chips:
+ * Chips based on ARM System Control Processor Interface
+   Addresses scanned: -
+   Datasheet: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/index.html
+
+Author: Punit Agrawal <punit.agrawal@arm.com>
+
+Description
+-----------
+
+This driver supports hardware monitoring for SoC's based on the ARM
+System Control Processor (SCP) implementing the System Control
+Processor Interface (SCPI). The following sensor types are supported
+by the SCP -
+
+  * temperature
+  * voltage
+  * current
+  * power
+
+The SCP interface provides an API to query the available sensors and
+their values which are then exported to userspace by this driver.
+
+Usage Notes
+-----------
+
+The driver relies on device tree node to indicate the presence of SCPI
+support in the kernel. See
+Documentation/devicetree/bindings/arm/arm,scpi.txt for details of the
+devicetree node.
\ No newline at end of file
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 4943c3c..c9714b0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1551,6 +1551,14 @@ config SENSORS_VEXPRESS
 	  the ARM Ltd's Versatile Express platform. It can provide wide
 	  range of information like temperature, power, energy.
 
+config SENSORS_ARM_SCPI
+	tristate "ARM SCPI Sensors"
+	depends on ARM_SCPI_PROTOCOL
+	help
+	  This driver provides support for temperature, voltage, current
+	  and power sensors available on ARM Ltd's SCP based platforms. The
+	  actual number and type of sensors exported depend the platform.
+
 config SENSORS_VIA_CPUTEMP
 	tristate "VIA CPU temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 8aba87f..4961710 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -150,6 +150,7 @@ obj-$(CONFIG_SENSORS_TMP421)	+= tmp421.o
 obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o
 obj-$(CONFIG_SENSORS_V2M_JUNO)	+= v2m-juno.o
 obj-$(CONFIG_SENSORS_VEXPRESS)	+= vexpress.o
+obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
 obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
 obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
 obj-$(CONFIG_SENSORS_VT1211)	+= vt1211.o
diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
new file mode 100644
index 0000000..7e7e06b
--- /dev/null
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -0,0 +1,183 @@
+/*
+ * System Control and Power Interface(SCPI) based hwmon sensor driver
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ * Punit Agrawal <punit.agrawal@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/hwmon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/scpi_protocol.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+struct sensor_data {
+	struct scpi_sensor_info info;
+	struct device_attribute dev_attr_input;
+	struct device_attribute dev_attr_label;
+	char input[20];
+	char label[20];
+};
+
+struct scpi_sensors {
+	struct scpi_ops *scpi_ops;
+	struct sensor_data *data;
+	struct attribute **attrs;
+	struct attribute_group group;
+	const struct attribute_group *groups[2];
+};
+
+/* hwmon callback functions */
+static ssize_t
+scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct scpi_sensors *scpi_sensors = dev_get_drvdata(dev);
+	struct scpi_ops *scpi_ops = scpi_sensors->scpi_ops;
+	struct sensor_data *sensor;
+	u32 value;
+	int ret;
+
+	sensor = container_of(attr, struct sensor_data, dev_attr_input);
+
+	ret = scpi_ops->sensor_get_value(sensor->info.sensor_id, &value);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%u\n", value);
+}
+
+static ssize_t
+scpi_show_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct sensor_data *sensor;
+
+	sensor = container_of(attr, struct sensor_data, dev_attr_label);
+
+	return sprintf(buf, "%s\n", sensor->info.name);
+}
+
+static int scpi_hwmon_probe(struct platform_device *pdev)
+{
+	u16 nr_sensors, i;
+	int num_temp = 0, num_volt = 0, num_current = 0, num_power = 0;
+	struct scpi_ops *scpi_ops;
+	struct device *hwdev, *dev = &pdev->dev;
+	struct scpi_sensors *scpi_sensors;
+	int ret;
+
+	scpi_ops = get_scpi_ops();
+	if (!scpi_ops)
+		return -EPROBE_DEFER;
+
+	ret = scpi_ops->sensor_get_capability(&nr_sensors);
+	if (ret || !nr_sensors)
+		return ret;
+
+	scpi_sensors = devm_kzalloc(dev, sizeof(*scpi_sensors), GFP_KERNEL);
+	if (!scpi_sensors)
+		return -ENOMEM;
+
+	scpi_sensors->data = devm_kcalloc(dev, nr_sensors,
+				   sizeof(*scpi_sensors->data), GFP_KERNEL);
+	if (!scpi_sensors->data)
+		return -ENOMEM;
+
+	scpi_sensors->attrs = devm_kcalloc(dev, (nr_sensors * 2) + 1,
+				   sizeof(*scpi_sensors->attrs), GFP_KERNEL);
+	if (!scpi_sensors->attrs)
+		return -ENOMEM;
+
+	scpi_sensors->scpi_ops = scpi_ops;
+
+	for (i = 0; i < nr_sensors; i++) {
+		struct sensor_data *sensor = &scpi_sensors->data[i];
+
+		ret = scpi_ops->sensor_get_info(i, &sensor->info);
+		if (ret)
+			return ret;
+
+		switch (sensor->info.class) {
+		case TEMPERATURE:
+			snprintf(sensor->input, sizeof(sensor->input),
+				 "temp%d_input", num_temp + 1);
+			snprintf(sensor->label, sizeof(sensor->input),
+				 "temp%d_label", num_temp + 1);
+			num_temp++;
+			break;
+		case VOLTAGE:
+			snprintf(sensor->input, sizeof(sensor->input),
+				 "in%d_input", num_volt);
+			snprintf(sensor->label, sizeof(sensor->input),
+				 "in%d_label", num_volt);
+			num_volt++;
+			break;
+		case CURRENT:
+			snprintf(sensor->input, sizeof(sensor->input),
+				 "curr%d_input", num_current + 1);
+			snprintf(sensor->label, sizeof(sensor->input),
+				 "curr%d_label", num_current + 1);
+			num_current++;
+			break;
+		case POWER:
+			snprintf(sensor->input, sizeof(sensor->input),
+				 "power%d_input", num_power + 1);
+			snprintf(sensor->label, sizeof(sensor->input),
+				 "power%d_label", num_power + 1);
+			num_power++;
+			break;
+		default:
+			break;
+		}
+
+		sensor->dev_attr_input.attr.mode = S_IRUGO;
+		sensor->dev_attr_input.show = scpi_show_sensor;
+		sensor->dev_attr_input.attr.name = sensor->input;
+
+		sensor->dev_attr_label.attr.mode = S_IRUGO;
+		sensor->dev_attr_label.show = scpi_show_label;
+		sensor->dev_attr_label.attr.name = sensor->label;
+
+		scpi_sensors->attrs[i << 1] = &sensor->dev_attr_input.attr;
+		scpi_sensors->attrs[(i << 1) + 1] = &sensor->dev_attr_label.attr;
+
+		sysfs_attr_init(scpi_sensors->attrs[i << 1]);
+		sysfs_attr_init(scpi_sensors->attrs[(i << 1) + 1]);
+	}
+
+	scpi_sensors->group.attrs = scpi_sensors->attrs;
+	scpi_sensors->groups[0] = &scpi_sensors->group;
+
+	hwdev = devm_hwmon_device_register_with_groups(dev,
+			"scpi_sensors", scpi_sensors, scpi_sensors->groups);
+
+	return PTR_ERR_OR_ZERO(hwdev);
+}
+
+static const struct of_device_id scpi_of_match[] = {
+	{.compatible = "arm,scpi-sensors"},
+	{},
+};
+
+static struct platform_driver scpi_hwmon_platdrv = {
+	.driver = {
+		.name	= "scpi-hwmon",
+		.owner	= THIS_MODULE,
+		.of_match_table = scpi_of_match,
+	},
+	.probe		= scpi_hwmon_probe,
+};
+module_platform_driver(scpi_hwmon_platdrv);
+
+MODULE_AUTHOR("Punit Agrawal <punit.agrawal@arm.com>");
+MODULE_DESCRIPTION("ARM SCPI HWMON interface driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

[PATCH v2 08/10] hwmon: Support registration of thermal zones for SCP temperature sensors

[Punit Agrawal]

Add support to create thermal zones based on the temperature sensors
provided by the SCP. The thermal zones can be defined using the
thermal DT bindings and should refer to the SCP sensor id to select
the sensor.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Jean Delvare <jdelvare@suse.de>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/hwmon/scpi-hwmon.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index 7e7e06b..d96a3e5 100644
--- a/drivers/hwmon/scpi-hwmon.c
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -20,6 +20,7 @@
 #include <linux/scpi_protocol.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
+#include <linux/thermal.h>
 
 struct sensor_data {
 	struct scpi_sensor_info info;
@@ -29,14 +30,39 @@ struct sensor_data {
 	char label[20];
 };
 
+struct scpi_thermal_zone {
+	struct list_head list;
+	int sensor_id;
+	struct scpi_sensors *scpi_sensors;
+	struct thermal_zone_device *tzd;
+};
+
 struct scpi_sensors {
 	struct scpi_ops *scpi_ops;
 	struct sensor_data *data;
+	struct list_head thermal_zones;
 	struct attribute **attrs;
 	struct attribute_group group;
 	const struct attribute_group *groups[2];
 };
 
+static int scpi_read_temp(void *dev, long *temp)
+{
+	struct scpi_thermal_zone *zone = dev;
+	struct scpi_sensors *scpi_sensors = zone->scpi_sensors;
+	struct scpi_ops *scpi_ops = scpi_sensors->scpi_ops;
+	struct sensor_data *sensor = &scpi_sensors->data[zone->sensor_id];
+	u32 value;
+	int ret;
+
+	ret = scpi_ops->sensor_get_value(sensor->info.sensor_id, &value);
+	if (ret)
+		return ret;
+
+	*temp = value;
+	return 0;
+}
+
 /* hwmon callback functions */
 static ssize_t
 scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
@@ -66,6 +92,24 @@ scpi_show_label(struct device *dev, struct device_attribute *attr, char *buf)
 	return sprintf(buf, "%s\n", sensor->info.name);
 }
 
+static void
+unregister_thermal_zones(struct platform_device *pdev,
+			 struct scpi_sensors *scpi_sensors)
+{
+	struct list_head *pos;
+
+	list_for_each(pos, &scpi_sensors->thermal_zones) {
+		struct scpi_thermal_zone *zone;
+
+		zone = list_entry(pos, struct scpi_thermal_zone, list);
+		thermal_zone_of_sensor_unregister(&pdev->dev, zone->tzd);
+	}
+}
+
+static struct thermal_zone_of_device_ops scpi_sensor_ops = {
+	.get_temp = scpi_read_temp,
+};
+
 static int scpi_hwmon_probe(struct platform_device *pdev)
 {
 	u16 nr_sensors, i;
@@ -157,10 +201,66 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 	scpi_sensors->group.attrs = scpi_sensors->attrs;
 	scpi_sensors->groups[0] = &scpi_sensors->group;
 
+	platform_set_drvdata(pdev, scpi_sensors);
+
 	hwdev = devm_hwmon_device_register_with_groups(dev,
 			"scpi_sensors", scpi_sensors, scpi_sensors->groups);
 
-	return PTR_ERR_OR_ZERO(hwdev);
+	if (IS_ERR(hwdev))
+		return PTR_ERR(hwdev);
+
+	/*
+	 * Register the temperature sensors with the thermal framework
+	 * to allow their usage in setting up the thermal zones from
+	 * device tree.
+	 *
+	 * NOTE: Not all temperature sensors maybe used for thermal
+	 * control
+	 */
+	INIT_LIST_HEAD(&scpi_sensors->thermal_zones);
+	for (i = 0; i < nr_sensors; i++) {
+		struct sensor_data *sensor = &scpi_sensors->data[i];
+		struct scpi_thermal_zone *zone;
+
+		if (sensor->info.class != TEMPERATURE)
+			continue;
+
+		zone = devm_kzalloc(&pdev->dev, sizeof(*zone), GFP_KERNEL);
+		if (!zone) {
+			ret = -ENOMEM;
+			goto unregister_tzd;
+		}
+
+		zone->sensor_id = i;
+		zone->scpi_sensors = scpi_sensors;
+		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
+							    &scpi_sensor_ops);
+		/*
+		 * The call to thermal_zone_of_sensor_register returns
+		 * an error for sensors that are not associated with
+		 * any thermal zones.
+		 */
+		if (IS_ERR(zone->tzd)) {
+			devm_kfree(dev, zone);
+			continue;
+		}
+		list_add(&zone->list, &scpi_sensors->thermal_zones);
+	}
+
+	return 0;
+
+unregister_tzd:
+	unregister_thermal_zones(pdev, scpi_sensors);
+	return ret;
+}
+
+static int scpi_hwmon_remove(struct platform_device *pdev)
+{
+	struct scpi_sensors *scpi_sensors = platform_get_drvdata(pdev);
+
+	unregister_thermal_zones(pdev, scpi_sensors);
+
+	return 0;
 }
 
 static const struct of_device_id scpi_of_match[] = {
@@ -175,6 +275,7 @@ static struct platform_driver scpi_hwmon_platdrv = {
 		.of_match_table = scpi_of_match,
 	},
 	.probe		= scpi_hwmon_probe,
+	.remove		= scpi_hwmon_remove,
 };
 module_platform_driver(scpi_hwmon_platdrv);
 
-- 
2.1.4

[PATCH v2 09/10] arm64: dts: Add sensor node to Juno dt

[Punit Agrawal]

The SCP firmware on Juno provides access to SoC sensors via the
SCPI. Add the sensor nodes to the device tree to enable this support.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index c624208..dd5158e 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -96,6 +96,11 @@
 				clock-output-names = "pxlclk0", "pxlclk1";
 			};
 		};
+
+		scpi_sensors0: sensors {
+			compatible = "arm,scpi-sensors";
+			#thermal-sensor-cells = <1>;
+		};
 	};
 
 	/include/ "juno-clocks.dtsi"
-- 
2.1.4

[PATCH v2 10/10] arm64: dts: Create SoC thermal zone for Juno

[Punit Agrawal]

Setup a thermal zone driven by the SoC temperature sensor on Juno
r0. Create passive trip points and bind them to cpufreq cooling
devices that support the power extensions.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno.dts | 50 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index d2e67f3..664961c 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -9,6 +9,7 @@
 /dts-v1/;
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	model = "ARM Juno development board (r0)";
@@ -90,6 +91,8 @@
 			next-level-cache = <&A57_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <530>;
 		};
 
 		A57_1: cpu@1 {
@@ -100,6 +103,8 @@
 			next-level-cache = <&A57_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <530>;
 		};
 
 		A53_0: cpu@100 {
@@ -110,6 +115,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A53_1: cpu@101 {
@@ -120,6 +127,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A53_2: cpu@102 {
@@ -130,6 +139,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A53_3: cpu@103 {
@@ -140,6 +151,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A57_L2: l2-cache0 {
@@ -167,5 +180,42 @@
 				     <&A53_3>;
 	};
 
+	thermal-zones {
+		soc_thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <100>;
+			sustainable-power = <2500>;
+
+			thermal-sensors = <&scpi_sensors0 3>;
+
+			trips {
+				threshold: trip-point@0 {
+					temperature = <55000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+				target: trip-point@1 {
+					temperature = <65000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&target>;
+					cooling-device = <&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <2048>;
+				};
+				map1 {
+					trip = <&target>;
+					cooling-device = <&A57_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <1024>;
+				};
+
+			};
+		};
+	};
+
 	#include "juno-base.dtsi"
 };
-- 
2.1.4

Re: [PATCH v2 04/10] thermal: Fix thermal_zone_of_sensor_register to match documentation

[Guenter Roeck]

On 08/03/2015 08:22 AM, Punit Agrawal wrote:
> thermal_zone_of_sensor_register is documented as returning a pointer
> to either a valid thermal_zone_device on success, or a corresponding
> ERR_PTR() value.
>
> In contrast, the function returns NULL when THERMAL_OF is configured
> off. Fix this.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Zhang Rui <rui.zhang@intel.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   include/linux/thermal.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 037e9df..a47b34a 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -364,7 +364,7 @@ static inline struct thermal_zone_device *
>   thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
>   				const struct thermal_zone_of_device_ops *ops)
>   {
> -	return NULL;
> +	return ERR_PTR(-ENOSYS);
>   }
>
>   static inline
>

Re: [PATCH v2 04/10] thermal: Fix thermal_zone_of_sensor_register to match documentation

[Punit Agrawal]

Guenter Roeck <linux@roeck-us.net> writes:

> On 08/03/2015 08:22 AM, Punit Agrawal wrote:
>> thermal_zone_of_sensor_register is documented as returning a pointer
>> to either a valid thermal_zone_device on success, or a corresponding
>> ERR_PTR() value.
>>
>> In contrast, the function returns NULL when THERMAL_OF is configured
>> off. Fix this.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
>

Thanks, Guenter.

>> ---
>>   include/linux/thermal.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 037e9df..a47b34a 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -364,7 +364,7 @@ static inline struct thermal_zone_device *
>>   thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
>>   				const struct thermal_zone_of_device_ops *ops)
>>   {
>> -	return NULL;
>> +	return ERR_PTR(-ENOSYS);
>>   }
>>
>>   static inline
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 07/10] hwmon: Support sensors exported via ARM SCP interface

[Guenter Roeck]

On 08/03/2015 08:22 AM, Punit Agrawal wrote:
> Create a driver to add support for SoC sensors exported by the System
> Control Processor (SCP) via the System Control and Power Interface
> (SCPI). The supported sensor types is one of voltage, temperature,
> current, and power.
>
> The sensor labels and values provided by the SCP are exported via the
> hwmon sysfs interface.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Jean Delvare <jdelvare@suse.de>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Looks good for the most part. Single comment below.
Assuming you fix it up, feel free to add

Acked-by: Guenter Roeck <linux@roeck-us.net>

for v3.

Thanks,
Guenter

> ---
>   Documentation/hwmon/scpi-hwmon |  33 ++++++++
>   drivers/hwmon/Kconfig          |   8 ++
>   drivers/hwmon/Makefile         |   1 +
>   drivers/hwmon/scpi-hwmon.c     | 183 +++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 225 insertions(+)
>   create mode 100644 Documentation/hwmon/scpi-hwmon
>   create mode 100644 drivers/hwmon/scpi-hwmon.c
>
> diff --git a/Documentation/hwmon/scpi-hwmon b/Documentation/hwmon/scpi-hwmon
> new file mode 100644
> index 0000000..4cfcdf2d
> --- /dev/null
> +++ b/Documentation/hwmon/scpi-hwmon
> @@ -0,0 +1,33 @@
> +Kernel driver scpi-hwmon
> +========================
> +
> +Supported chips:
> + * Chips based on ARM System Control Processor Interface
> +   Addresses scanned: -
> +   Datasheet: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/index.html
> +
> +Author: Punit Agrawal <punit.agrawal@arm.com>
> +
> +Description
> +-----------
> +
> +This driver supports hardware monitoring for SoC's based on the ARM
> +System Control Processor (SCP) implementing the System Control
> +Processor Interface (SCPI). The following sensor types are supported
> +by the SCP -
> +
> +  * temperature
> +  * voltage
> +  * current
> +  * power
> +
> +The SCP interface provides an API to query the available sensors and
> +their values which are then exported to userspace by this driver.
> +
> +Usage Notes
> +-----------
> +
> +The driver relies on device tree node to indicate the presence of SCPI
> +support in the kernel. See
> +Documentation/devicetree/bindings/arm/arm,scpi.txt for details of the
> +devicetree node.
> \ No newline at end of file
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 4943c3c..c9714b0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1551,6 +1551,14 @@ config SENSORS_VEXPRESS
>   	  the ARM Ltd's Versatile Express platform. It can provide wide
>   	  range of information like temperature, power, energy.
>
> +config SENSORS_ARM_SCPI
> +	tristate "ARM SCPI Sensors"
> +	depends on ARM_SCPI_PROTOCOL
> +	help
> +	  This driver provides support for temperature, voltage, current
> +	  and power sensors available on ARM Ltd's SCP based platforms. The
> +	  actual number and type of sensors exported depend the platform.
> +
>   config SENSORS_VIA_CPUTEMP
>   	tristate "VIA CPU temperature sensor"
>   	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8aba87f..4961710 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -150,6 +150,7 @@ obj-$(CONFIG_SENSORS_TMP421)	+= tmp421.o
>   obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o
>   obj-$(CONFIG_SENSORS_V2M_JUNO)	+= v2m-juno.o
>   obj-$(CONFIG_SENSORS_VEXPRESS)	+= vexpress.o
> +obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
>   obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
>   obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
>   obj-$(CONFIG_SENSORS_VT1211)	+= vt1211.o
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> new file mode 100644
> index 0000000..7e7e06b
> --- /dev/null
> +++ b/drivers/hwmon/scpi-hwmon.c
> @@ -0,0 +1,183 @@
> +/*
> + * System Control and Power Interface(SCPI) based hwmon sensor driver
> + *
> + * Copyright (C) 2015 ARM Ltd.
> + * Punit Agrawal <punit.agrawal@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/scpi_protocol.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +struct sensor_data {
> +	struct scpi_sensor_info info;
> +	struct device_attribute dev_attr_input;
> +	struct device_attribute dev_attr_label;
> +	char input[20];
> +	char label[20];
> +};
> +
> +struct scpi_sensors {
> +	struct scpi_ops *scpi_ops;
> +	struct sensor_data *data;
> +	struct attribute **attrs;
> +	struct attribute_group group;
> +	const struct attribute_group *groups[2];
> +};
> +
> +/* hwmon callback functions */
> +static ssize_t
> +scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct scpi_sensors *scpi_sensors = dev_get_drvdata(dev);
> +	struct scpi_ops *scpi_ops = scpi_sensors->scpi_ops;
> +	struct sensor_data *sensor;
> +	u32 value;
> +	int ret;
> +
> +	sensor = container_of(attr, struct sensor_data, dev_attr_input);
> +
> +	ret = scpi_ops->sensor_get_value(sensor->info.sensor_id, &value);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%u\n", value);
> +}
> +
> +static ssize_t
> +scpi_show_label(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct sensor_data *sensor;
> +
> +	sensor = container_of(attr, struct sensor_data, dev_attr_label);
> +
> +	return sprintf(buf, "%s\n", sensor->info.name);
> +}
> +
> +static int scpi_hwmon_probe(struct platform_device *pdev)
> +{
> +	u16 nr_sensors, i;
> +	int num_temp = 0, num_volt = 0, num_current = 0, num_power = 0;
> +	struct scpi_ops *scpi_ops;
> +	struct device *hwdev, *dev = &pdev->dev;
> +	struct scpi_sensors *scpi_sensors;
> +	int ret;
> +
> +	scpi_ops = get_scpi_ops();
> +	if (!scpi_ops)
> +		return -EPROBE_DEFER;
> +
> +	ret = scpi_ops->sensor_get_capability(&nr_sensors);
> +	if (ret || !nr_sensors)
> +		return ret;
> +
This will return 0 if nr_sensors == 0. Should it be -ENODEV ?

> +	scpi_sensors = devm_kzalloc(dev, sizeof(*scpi_sensors), GFP_KERNEL);
> +	if (!scpi_sensors)
> +		return -ENOMEM;
> +
> +	scpi_sensors->data = devm_kcalloc(dev, nr_sensors,
> +				   sizeof(*scpi_sensors->data), GFP_KERNEL);
> +	if (!scpi_sensors->data)
> +		return -ENOMEM;
> +
> +	scpi_sensors->attrs = devm_kcalloc(dev, (nr_sensors * 2) + 1,
> +				   sizeof(*scpi_sensors->attrs), GFP_KERNEL);
> +	if (!scpi_sensors->attrs)
> +		return -ENOMEM;
> +
> +	scpi_sensors->scpi_ops = scpi_ops;
> +
> +	for (i = 0; i < nr_sensors; i++) {
> +		struct sensor_data *sensor = &scpi_sensors->data[i];
> +
> +		ret = scpi_ops->sensor_get_info(i, &sensor->info);
> +		if (ret)
> +			return ret;
> +
> +		switch (sensor->info.class) {
> +		case TEMPERATURE:
> +			snprintf(sensor->input, sizeof(sensor->input),
> +				 "temp%d_input", num_temp + 1);
> +			snprintf(sensor->label, sizeof(sensor->input),
> +				 "temp%d_label", num_temp + 1);
> +			num_temp++;
> +			break;
> +		case VOLTAGE:
> +			snprintf(sensor->input, sizeof(sensor->input),
> +				 "in%d_input", num_volt);
> +			snprintf(sensor->label, sizeof(sensor->input),
> +				 "in%d_label", num_volt);
> +			num_volt++;
> +			break;
> +		case CURRENT:
> +			snprintf(sensor->input, sizeof(sensor->input),
> +				 "curr%d_input", num_current + 1);
> +			snprintf(sensor->label, sizeof(sensor->input),
> +				 "curr%d_label", num_current + 1);
> +			num_current++;
> +			break;
> +		case POWER:
> +			snprintf(sensor->input, sizeof(sensor->input),
> +				 "power%d_input", num_power + 1);
> +			snprintf(sensor->label, sizeof(sensor->input),
> +				 "power%d_label", num_power + 1);
> +			num_power++;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		sensor->dev_attr_input.attr.mode = S_IRUGO;
> +		sensor->dev_attr_input.show = scpi_show_sensor;
> +		sensor->dev_attr_input.attr.name = sensor->input;
> +
> +		sensor->dev_attr_label.attr.mode = S_IRUGO;
> +		sensor->dev_attr_label.show = scpi_show_label;
> +		sensor->dev_attr_label.attr.name = sensor->label;
> +
> +		scpi_sensors->attrs[i << 1] = &sensor->dev_attr_input.attr;
> +		scpi_sensors->attrs[(i << 1) + 1] = &sensor->dev_attr_label.attr;
> +
> +		sysfs_attr_init(scpi_sensors->attrs[i << 1]);
> +		sysfs_attr_init(scpi_sensors->attrs[(i << 1) + 1]);
> +	}
> +
> +	scpi_sensors->group.attrs = scpi_sensors->attrs;
> +	scpi_sensors->groups[0] = &scpi_sensors->group;
> +
> +	hwdev = devm_hwmon_device_register_with_groups(dev,
> +			"scpi_sensors", scpi_sensors, scpi_sensors->groups);
> +
> +	return PTR_ERR_OR_ZERO(hwdev);
> +}
> +
> +static const struct of_device_id scpi_of_match[] = {
> +	{.compatible = "arm,scpi-sensors"},
> +	{},
> +};
> +
> +static struct platform_driver scpi_hwmon_platdrv = {
> +	.driver = {
> +		.name	= "scpi-hwmon",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = scpi_of_match,
> +	},
> +	.probe		= scpi_hwmon_probe,
> +};
> +module_platform_driver(scpi_hwmon_platdrv);
> +
> +MODULE_AUTHOR("Punit Agrawal <punit.agrawal@arm.com>");
> +MODULE_DESCRIPTION("ARM SCPI HWMON interface driver");
> +MODULE_LICENSE("GPL v2");
>

Re: [PATCH v2 08/10] hwmon: Support registration of thermal zones for SCP temperature sensors

[Guenter Roeck]

On 08/03/2015 08:22 AM, Punit Agrawal wrote:
> Add support to create thermal zones based on the temperature sensors
> provided by the SCP. The thermal zones can be defined using the
> thermal DT bindings and should refer to the SCP sensor id to select
> the sensor.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Jean Delvare <jdelvare@suse.de>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Eduardo Valentin <edubezval@gmail.com>

Hi,

assuming you fix up the nitpicks below,

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/hwmon/scpi-hwmon.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index 7e7e06b..d96a3e5 100644
> --- a/drivers/hwmon/scpi-hwmon.c
> +++ b/drivers/hwmon/scpi-hwmon.c
> @@ -20,6 +20,7 @@
>   #include <linux/scpi_protocol.h>
>   #include <linux/slab.h>
>   #include <linux/sysfs.h>
> +#include <linux/thermal.h>
>
>   struct sensor_data {
>   	struct scpi_sensor_info info;
> @@ -29,14 +30,39 @@ struct sensor_data {
>   	char label[20];
>   };
>
> +struct scpi_thermal_zone {
> +	struct list_head list;
> +	int sensor_id;
> +	struct scpi_sensors *scpi_sensors;
> +	struct thermal_zone_device *tzd;
> +};
> +
>   struct scpi_sensors {
>   	struct scpi_ops *scpi_ops;
>   	struct sensor_data *data;
> +	struct list_head thermal_zones;
>   	struct attribute **attrs;
>   	struct attribute_group group;
>   	const struct attribute_group *groups[2];
>   };
>
> +static int scpi_read_temp(void *dev, long *temp)
> +{
> +	struct scpi_thermal_zone *zone = dev;
> +	struct scpi_sensors *scpi_sensors = zone->scpi_sensors;
> +	struct scpi_ops *scpi_ops = scpi_sensors->scpi_ops;
> +	struct sensor_data *sensor = &scpi_sensors->data[zone->sensor_id];
> +	u32 value;
> +	int ret;
> +
> +	ret = scpi_ops->sensor_get_value(sensor->info.sensor_id, &value);
> +	if (ret)
> +		return ret;
> +
> +	*temp = value;
> +	return 0;
> +}
> +
>   /* hwmon callback functions */
>   static ssize_t
>   scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
> @@ -66,6 +92,24 @@ scpi_show_label(struct device *dev, struct device_attribute *attr, char *buf)
>   	return sprintf(buf, "%s\n", sensor->info.name);
>   }
>
> +static void
> +unregister_thermal_zones(struct platform_device *pdev,
> +			 struct scpi_sensors *scpi_sensors)
> +{
> +	struct list_head *pos;
> +
> +	list_for_each(pos, &scpi_sensors->thermal_zones) {
> +		struct scpi_thermal_zone *zone;
> +
> +		zone = list_entry(pos, struct scpi_thermal_zone, list);
> +		thermal_zone_of_sensor_unregister(&pdev->dev, zone->tzd);
> +	}
> +}
> +
> +static struct thermal_zone_of_device_ops scpi_sensor_ops = {
> +	.get_temp = scpi_read_temp,
> +};
> +
>   static int scpi_hwmon_probe(struct platform_device *pdev)
>   {
>   	u16 nr_sensors, i;
> @@ -157,10 +201,66 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>   	scpi_sensors->group.attrs = scpi_sensors->attrs;
>   	scpi_sensors->groups[0] = &scpi_sensors->group;
>
> +	platform_set_drvdata(pdev, scpi_sensors);
> +
>   	hwdev = devm_hwmon_device_register_with_groups(dev,
>   			"scpi_sensors", scpi_sensors, scpi_sensors->groups);
>
> -	return PTR_ERR_OR_ZERO(hwdev);
> +	if (IS_ERR(hwdev))
> +		return PTR_ERR(hwdev);
> +
> +	/*
> +	 * Register the temperature sensors with the thermal framework
> +	 * to allow their usage in setting up the thermal zones from
> +	 * device tree.
> +	 *
> +	 * NOTE: Not all temperature sensors maybe used for thermal
> +	 * control
> +	 */
> +	INIT_LIST_HEAD(&scpi_sensors->thermal_zones);
> +	for (i = 0; i < nr_sensors; i++) {
> +		struct sensor_data *sensor = &scpi_sensors->data[i];
> +		struct scpi_thermal_zone *zone;
> +
> +		if (sensor->info.class != TEMPERATURE)
> +			continue;
> +
> +		zone = devm_kzalloc(&pdev->dev, sizeof(*zone), GFP_KERNEL);

			devm_kzalloc(dev, ...);

for consistency.

> +		if (!zone) {
> +			ret = -ENOMEM;
> +			goto unregister_tzd;
> +		}
> +
> +		zone->sensor_id = i;
> +		zone->scpi_sensors = scpi_sensors;
> +		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
> +							    &scpi_sensor_ops);
> +		/*
> +		 * The call to thermal_zone_of_sensor_register returns
> +		 * an error for sensors that are not associated with
> +		 * any thermal zones.

... or if the thermal subsystem is not configured.

> +		 */
> +		if (IS_ERR(zone->tzd)) {
> +			devm_kfree(dev, zone);
> +			continue;
> +		}
> +		list_add(&zone->list, &scpi_sensors->thermal_zones);
> +	}
> +
> +	return 0;
> +
> +unregister_tzd:
> +	unregister_thermal_zones(pdev, scpi_sensors);
> +	return ret;
> +}
> +
> +static int scpi_hwmon_remove(struct platform_device *pdev)
> +{
> +	struct scpi_sensors *scpi_sensors = platform_get_drvdata(pdev);
> +
> +	unregister_thermal_zones(pdev, scpi_sensors);
> +
> +	return 0;
>   }
>
>   static const struct of_device_id scpi_of_match[] = {
> @@ -175,6 +275,7 @@ static struct platform_driver scpi_hwmon_platdrv = {
>   		.of_match_table = scpi_of_match,
>   	},
>   	.probe		= scpi_hwmon_probe,
> +	.remove		= scpi_hwmon_remove,
>   };
>   module_platform_driver(scpi_hwmon_platdrv);
>
>

[PATCH v3] thermal: Fix thermal_zone_of_sensor_register to match documentation

[Punit Agrawal]

thermal_zone_of_sensor_register is documented as returning a pointer
to either a valid thermal_zone_device on success, or a corresponding
ERR_PTR() value.

In contrast, the function returns NULL when THERMAL_OF is configured
off. Fix this.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Zhang Rui <rui.zhang@intel.com>
---
Hi Guenter,

It was pointed out that ENOSYS is frowned upon for anything other than
indicating lack of support for a syscall. The rest of the functions in
the file use ENODEV. Updating thermal_zone_of_sensor_register to do
the same.

Could you please re-ack if you're ok with this change?

Thanks,
Punit

 include/linux/thermal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 037e9df..f344e51 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -364,7 +364,7 @@ static inline struct thermal_zone_device *
 thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 				const struct thermal_zone_of_device_ops *ops)
 {
-	return NULL;
+	return ERR_PTR(-ENODEV);
 }
 
 static inline
-- 
2.1.4

Re: [PATCH v2 08/10] hwmon: Support registration of thermal zones for SCP temperature sensors

[Punit Agrawal]

Guenter Roeck <linux@roeck-us.net> writes:

> On 08/03/2015 08:22 AM, Punit Agrawal wrote:
>> Add support to create thermal zones based on the temperature sensors
>> provided by the SCP. The thermal zones can be defined using the
>> thermal DT bindings and should refer to the SCP sensor id to select
>> the sensor.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Jean Delvare <jdelvare@suse.de>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>
> Hi,
>
> assuming you fix up the nitpicks below,
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
>
>> ---
>>   drivers/hwmon/scpi-hwmon.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 102 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
>> index 7e7e06b..d96a3e5 100644
>> --- a/drivers/hwmon/scpi-hwmon.c
>> +++ b/drivers/hwmon/scpi-hwmon.c
>> @@ -20,6 +20,7 @@
>>   #include <linux/scpi_protocol.h>
>>   #include <linux/slab.h>
>>   #include <linux/sysfs.h>
>> +#include <linux/thermal.h>
>>
>>   struct sensor_data {
>>   	struct scpi_sensor_info info;
>> @@ -29,14 +30,39 @@ struct sensor_data {
>>   	char label[20];
>>   };
>>
>> +struct scpi_thermal_zone {
>> +	struct list_head list;
>> +	int sensor_id;
>> +	struct scpi_sensors *scpi_sensors;
>> +	struct thermal_zone_device *tzd;
>> +};
>> +
>>   struct scpi_sensors {
>>   	struct scpi_ops *scpi_ops;
>>   	struct sensor_data *data;
>> +	struct list_head thermal_zones;
>>   	struct attribute **attrs;
>>   	struct attribute_group group;
>>   	const struct attribute_group *groups[2];
>>   };
>>
>> +static int scpi_read_temp(void *dev, long *temp)
>> +{
>> +	struct scpi_thermal_zone *zone = dev;
>> +	struct scpi_sensors *scpi_sensors = zone->scpi_sensors;
>> +	struct scpi_ops *scpi_ops = scpi_sensors->scpi_ops;
>> +	struct sensor_data *sensor = &scpi_sensors->data[zone->sensor_id];
>> +	u32 value;
>> +	int ret;
>> +
>> +	ret = scpi_ops->sensor_get_value(sensor->info.sensor_id, &value);
>> +	if (ret)
>> +		return ret;
>> +
>> +	*temp = value;
>> +	return 0;
>> +}
>> +
>>   /* hwmon callback functions */
>>   static ssize_t
>>   scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
>> @@ -66,6 +92,24 @@ scpi_show_label(struct device *dev, struct device_attribute *attr, char *buf)
>>   	return sprintf(buf, "%s\n", sensor->info.name);
>>   }
>>
>> +static void
>> +unregister_thermal_zones(struct platform_device *pdev,
>> +			 struct scpi_sensors *scpi_sensors)
>> +{
>> +	struct list_head *pos;
>> +
>> +	list_for_each(pos, &scpi_sensors->thermal_zones) {
>> +		struct scpi_thermal_zone *zone;
>> +
>> +		zone = list_entry(pos, struct scpi_thermal_zone, list);
>> +		thermal_zone_of_sensor_unregister(&pdev->dev, zone->tzd);
>> +	}
>> +}
>> +
>> +static struct thermal_zone_of_device_ops scpi_sensor_ops = {
>> +	.get_temp = scpi_read_temp,
>> +};
>> +
>>   static int scpi_hwmon_probe(struct platform_device *pdev)
>>   {
>>   	u16 nr_sensors, i;
>> @@ -157,10 +201,66 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>>   	scpi_sensors->group.attrs = scpi_sensors->attrs;
>>   	scpi_sensors->groups[0] = &scpi_sensors->group;
>>
>> +	platform_set_drvdata(pdev, scpi_sensors);
>> +
>>   	hwdev = devm_hwmon_device_register_with_groups(dev,
>>   			"scpi_sensors", scpi_sensors, scpi_sensors->groups);
>>
>> -	return PTR_ERR_OR_ZERO(hwdev);
>> +	if (IS_ERR(hwdev))
>> +		return PTR_ERR(hwdev);
>> +
>> +	/*
>> +	 * Register the temperature sensors with the thermal framework
>> +	 * to allow their usage in setting up the thermal zones from
>> +	 * device tree.
>> +	 *
>> +	 * NOTE: Not all temperature sensors maybe used for thermal
>> +	 * control
>> +	 */
>> +	INIT_LIST_HEAD(&scpi_sensors->thermal_zones);
>> +	for (i = 0; i < nr_sensors; i++) {
>> +		struct sensor_data *sensor = &scpi_sensors->data[i];
>> +		struct scpi_thermal_zone *zone;
>> +
>> +		if (sensor->info.class != TEMPERATURE)
>> +			continue;
>> +
>> +		zone = devm_kzalloc(&pdev->dev, sizeof(*zone), GFP_KERNEL);
>
> 			devm_kzalloc(dev, ...);
>
> for consistency.
>

Thought I'd fixed this up. Done now.

>> +		if (!zone) {
>> +			ret = -ENOMEM;
>> +			goto unregister_tzd;
>> +		}
>> +
>> +		zone->sensor_id = i;
>> +		zone->scpi_sensors = scpi_sensors;
>> +		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
>> +							    &scpi_sensor_ops);
>> +		/*
>> +		 * The call to thermal_zone_of_sensor_register returns
>> +		 * an error for sensors that are not associated with
>> +		 * any thermal zones.
>
> ... or if the thermal subsystem is not configured.
>

Updated the comment.

Thanks for the ack.

I'll send out an updated version once you respond to Patch 4.

Thanks,
Punit

>> +		 */
>> +		if (IS_ERR(zone->tzd)) {
>> +			devm_kfree(dev, zone);
>> +			continue;
>> +		}
>> +		list_add(&zone->list, &scpi_sensors->thermal_zones);
>> +	}
>> +
>> +	return 0;
>> +
>> +unregister_tzd:
>> +	unregister_thermal_zones(pdev, scpi_sensors);
>> +	return ret;
>> +}
>> +
>> +static int scpi_hwmon_remove(struct platform_device *pdev)
>> +{
>> +	struct scpi_sensors *scpi_sensors = platform_get_drvdata(pdev);
>> +
>> +	unregister_thermal_zones(pdev, scpi_sensors);
>> +
>> +	return 0;
>>   }
>>
>>   static const struct of_device_id scpi_of_match[] = {
>> @@ -175,6 +275,7 @@ static struct platform_driver scpi_hwmon_platdrv = {
>>   		.of_match_table = scpi_of_match,
>>   	},
>>   	.probe		= scpi_hwmon_probe,
>> +	.remove		= scpi_hwmon_remove,
>>   };
>>   module_platform_driver(scpi_hwmon_platdrv);
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 07/10] hwmon: Support sensors exported via ARM SCP interface

[Punit Agrawal]

Guenter Roeck <linux@roeck-us.net> writes:

> On 08/03/2015 08:22 AM, Punit Agrawal wrote:
>> Create a driver to add support for SoC sensors exported by the System
>> Control Processor (SCP) via the System Control and Power Interface
>> (SCPI). The supported sensor types is one of voltage, temperature,
>> current, and power.
>>
>> The sensor labels and values provided by the SCP are exported via the
>> hwmon sysfs interface.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Jean Delvare <jdelvare@suse.de>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>
> Looks good for the most part. Single comment below.
> Assuming you fix it up, feel free to add
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
>
> for v3.
>
> Thanks,
> Guenter
>
>> ---
>>   Documentation/hwmon/scpi-hwmon |  33 ++++++++
>>   drivers/hwmon/Kconfig          |   8 ++
>>   drivers/hwmon/Makefile         |   1 +
>>   drivers/hwmon/scpi-hwmon.c     | 183 +++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 225 insertions(+)
>>   create mode 100644 Documentation/hwmon/scpi-hwmon
>>   create mode 100644 drivers/hwmon/scpi-hwmon.c
>>
>> diff --git a/Documentation/hwmon/scpi-hwmon b/Documentation/hwmon/scpi-hwmon
>> new file mode 100644
>> index 0000000..4cfcdf2d
>> --- /dev/null
>> +++ b/Documentation/hwmon/scpi-hwmon
>> @@ -0,0 +1,33 @@
>> +Kernel driver scpi-hwmon
>> +========================
>> +
>> +Supported chips:
>> + * Chips based on ARM System Control Processor Interface
>> +   Addresses scanned: -
>> +   Datasheet: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/index.html
>> +
>> +Author: Punit Agrawal <punit.agrawal@arm.com>
>> +
>> +Description
>> +-----------
>> +
>> +This driver supports hardware monitoring for SoC's based on the ARM
>> +System Control Processor (SCP) implementing the System Control
>> +Processor Interface (SCPI). The following sensor types are supported
>> +by the SCP -
>> +
>> +  * temperature
>> +  * voltage
>> +  * current
>> +  * power
>> +
>> +The SCP interface provides an API to query the available sensors and
>> +their values which are then exported to userspace by this driver.
>> +
>> +Usage Notes
>> +-----------
>> +
>> +The driver relies on device tree node to indicate the presence of SCPI
>> +support in the kernel. See
>> +Documentation/devicetree/bindings/arm/arm,scpi.txt for details of the
>> +devicetree node.
>> \ No newline at end of file
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 4943c3c..c9714b0 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1551,6 +1551,14 @@ config SENSORS_VEXPRESS
>>   	  the ARM Ltd's Versatile Express platform. It can provide wide
>>   	  range of information like temperature, power, energy.
>>
>> +config SENSORS_ARM_SCPI
>> +	tristate "ARM SCPI Sensors"
>> +	depends on ARM_SCPI_PROTOCOL
>> +	help
>> +	  This driver provides support for temperature, voltage, current
>> +	  and power sensors available on ARM Ltd's SCP based platforms. The
>> +	  actual number and type of sensors exported depend the platform.
>> +
>>   config SENSORS_VIA_CPUTEMP
>>   	tristate "VIA CPU temperature sensor"
>>   	depends on X86
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 8aba87f..4961710 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -150,6 +150,7 @@ obj-$(CONFIG_SENSORS_TMP421)	+= tmp421.o
>>   obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o
>>   obj-$(CONFIG_SENSORS_V2M_JUNO)	+= v2m-juno.o
>>   obj-$(CONFIG_SENSORS_VEXPRESS)	+= vexpress.o
>> +obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
>>   obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
>>   obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
>>   obj-$(CONFIG_SENSORS_VT1211)	+= vt1211.o
>> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
>> new file mode 100644
>> index 0000000..7e7e06b
>> --- /dev/null
>> +++ b/drivers/hwmon/scpi-hwmon.c
>> @@ -0,0 +1,183 @@
>> +/*
>> + * System Control and Power Interface(SCPI) based hwmon sensor driver
>> + *
>> + * Copyright (C) 2015 ARM Ltd.
>> + * Punit Agrawal <punit.agrawal@arm.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/hwmon.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/scpi_protocol.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +
>> +struct sensor_data {
>> +	struct scpi_sensor_info info;
>> +	struct device_attribute dev_attr_input;
>> +	struct device_attribute dev_attr_label;
>> +	char input[20];
>> +	char label[20];
>> +};
>> +
>> +struct scpi_sensors {
>> +	struct scpi_ops *scpi_ops;
>> +	struct sensor_data *data;
>> +	struct attribute **attrs;
>> +	struct attribute_group group;
>> +	const struct attribute_group *groups[2];
>> +};
>> +
>> +/* hwmon callback functions */
>> +static ssize_t
>> +scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +	struct scpi_sensors *scpi_sensors = dev_get_drvdata(dev);
>> +	struct scpi_ops *scpi_ops = scpi_sensors->scpi_ops;
>> +	struct sensor_data *sensor;
>> +	u32 value;
>> +	int ret;
>> +
>> +	sensor = container_of(attr, struct sensor_data, dev_attr_input);
>> +
>> +	ret = scpi_ops->sensor_get_value(sensor->info.sensor_id, &value);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sprintf(buf, "%u\n", value);
>> +}
>> +
>> +static ssize_t
>> +scpi_show_label(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +	struct sensor_data *sensor;
>> +
>> +	sensor = container_of(attr, struct sensor_data, dev_attr_label);
>> +
>> +	return sprintf(buf, "%s\n", sensor->info.name);
>> +}
>> +
>> +static int scpi_hwmon_probe(struct platform_device *pdev)
>> +{
>> +	u16 nr_sensors, i;
>> +	int num_temp = 0, num_volt = 0, num_current = 0, num_power = 0;
>> +	struct scpi_ops *scpi_ops;
>> +	struct device *hwdev, *dev = &pdev->dev;
>> +	struct scpi_sensors *scpi_sensors;
>> +	int ret;
>> +
>> +	scpi_ops = get_scpi_ops();
>> +	if (!scpi_ops)
>> +		return -EPROBE_DEFER;
>> +
>> +	ret = scpi_ops->sensor_get_capability(&nr_sensors);
>> +	if (ret || !nr_sensors)
>> +		return ret;
>> +
> This will return 0 if nr_sensors == 0. Should it be -ENODEV ?
>

Good catch. I've fixed this up to return -ENODEV.

Thanks for the ack.

Punit

>> +	scpi_sensors = devm_kzalloc(dev, sizeof(*scpi_sensors), GFP_KERNEL);
>> +	if (!scpi_sensors)
>> +		return -ENOMEM;
>> +
>> +	scpi_sensors->data = devm_kcalloc(dev, nr_sensors,
>> +				   sizeof(*scpi_sensors->data), GFP_KERNEL);
>> +	if (!scpi_sensors->data)
>> +		return -ENOMEM;
>> +
>> +	scpi_sensors->attrs = devm_kcalloc(dev, (nr_sensors * 2) + 1,
>> +				   sizeof(*scpi_sensors->attrs), GFP_KERNEL);
>> +	if (!scpi_sensors->attrs)
>> +		return -ENOMEM;
>> +
>> +	scpi_sensors->scpi_ops = scpi_ops;
>> +
>> +	for (i = 0; i < nr_sensors; i++) {
>> +		struct sensor_data *sensor = &scpi_sensors->data[i];
>> +
>> +		ret = scpi_ops->sensor_get_info(i, &sensor->info);
>> +		if (ret)
>> +			return ret;
>> +
>> +		switch (sensor->info.class) {
>> +		case TEMPERATURE:
>> +			snprintf(sensor->input, sizeof(sensor->input),
>> +				 "temp%d_input", num_temp + 1);
>> +			snprintf(sensor->label, sizeof(sensor->input),
>> +				 "temp%d_label", num_temp + 1);
>> +			num_temp++;
>> +			break;
>> +		case VOLTAGE:
>> +			snprintf(sensor->input, sizeof(sensor->input),
>> +				 "in%d_input", num_volt);
>> +			snprintf(sensor->label, sizeof(sensor->input),
>> +				 "in%d_label", num_volt);
>> +			num_volt++;
>> +			break;
>> +		case CURRENT:
>> +			snprintf(sensor->input, sizeof(sensor->input),
>> +				 "curr%d_input", num_current + 1);
>> +			snprintf(sensor->label, sizeof(sensor->input),
>> +				 "curr%d_label", num_current + 1);
>> +			num_current++;
>> +			break;
>> +		case POWER:
>> +			snprintf(sensor->input, sizeof(sensor->input),
>> +				 "power%d_input", num_power + 1);
>> +			snprintf(sensor->label, sizeof(sensor->input),
>> +				 "power%d_label", num_power + 1);
>> +			num_power++;
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +
>> +		sensor->dev_attr_input.attr.mode = S_IRUGO;
>> +		sensor->dev_attr_input.show = scpi_show_sensor;
>> +		sensor->dev_attr_input.attr.name = sensor->input;
>> +
>> +		sensor->dev_attr_label.attr.mode = S_IRUGO;
>> +		sensor->dev_attr_label.show = scpi_show_label;
>> +		sensor->dev_attr_label.attr.name = sensor->label;
>> +
>> +		scpi_sensors->attrs[i << 1] = &sensor->dev_attr_input.attr;
>> +		scpi_sensors->attrs[(i << 1) + 1] = &sensor->dev_attr_label.attr;
>> +
>> +		sysfs_attr_init(scpi_sensors->attrs[i << 1]);
>> +		sysfs_attr_init(scpi_sensors->attrs[(i << 1) + 1]);
>> +	}
>> +
>> +	scpi_sensors->group.attrs = scpi_sensors->attrs;
>> +	scpi_sensors->groups[0] = &scpi_sensors->group;
>> +
>> +	hwdev = devm_hwmon_device_register_with_groups(dev,
>> +			"scpi_sensors", scpi_sensors, scpi_sensors->groups);
>> +
>> +	return PTR_ERR_OR_ZERO(hwdev);
>> +}
>> +
>> +static const struct of_device_id scpi_of_match[] = {
>> +	{.compatible = "arm,scpi-sensors"},
>> +	{},
>> +};
>> +
>> +static struct platform_driver scpi_hwmon_platdrv = {
>> +	.driver = {
>> +		.name	= "scpi-hwmon",
>> +		.owner	= THIS_MODULE,
>> +		.of_match_table = scpi_of_match,
>> +	},
>> +	.probe		= scpi_hwmon_probe,
>> +};
>> +module_platform_driver(scpi_hwmon_platdrv);
>> +
>> +MODULE_AUTHOR("Punit Agrawal <punit.agrawal@arm.com>");
>> +MODULE_DESCRIPTION("ARM SCPI HWMON interface driver");
>> +MODULE_LICENSE("GPL v2");
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] thermal: Fix thermal_zone_of_sensor_register to match documentation

[Guenter Roeck]

On 08/05/2015 02:57 AM, Punit Agrawal wrote:
> thermal_zone_of_sensor_register is documented as returning a pointer
> to either a valid thermal_zone_device on success, or a corresponding
> ERR_PTR() value.
>
> In contrast, the function returns NULL when THERMAL_OF is configured
> off. Fix this.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Zhang Rui <rui.zhang@intel.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Hi Guenter,
>
> It was pointed out that ENOSYS is frowned upon for anything other than
> indicating lack of support for a syscall. The rest of the functions in
> the file use ENODEV. Updating thermal_zone_of_sensor_register to do
> the same.
>
> Could you please re-ack if you're ok with this change?
>
> Thanks,
> Punit
>
>   include/linux/thermal.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 037e9df..f344e51 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -364,7 +364,7 @@ static inline struct thermal_zone_device *
>   thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
>   				const struct thermal_zone_of_device_ops *ops)
>   {
> -	return NULL;
> +	return ERR_PTR(-ENODEV);
>   }
>
>   static inline
>

Re: [PATCH v2 10/10] arm64: dts: Create SoC thermal zone for Juno

[Liviu Dudau]

On Mon, Aug 03, 2015 at 04:22:58PM +0100, Punit Agrawal wrote:
> Setup a thermal zone driven by the SoC temperature sensor on Juno
> r0. Create passive trip points and bind them to cpufreq cooling
> devices that support the power extensions.

Hi Punit,

> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/boot/dts/arm/juno.dts | 50 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
> index d2e67f3..664961c 100644
> --- a/arch/arm64/boot/dts/arm/juno.dts
> +++ b/arch/arm64/boot/dts/arm/juno.dts
> @@ -9,6 +9,7 @@
>  /dts-v1/;
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/thermal/thermal.h>
>  
>  / {
>  	model = "ARM Juno development board (r0)";
> @@ -90,6 +91,8 @@
>  			next-level-cache = <&A57_L2>;
>  			clocks = <&scpi_dvfs 0>;
>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <530>;

The Documentation/devicetree/bindings/thermal/thermal.txt document describes two additional
required properties, cooling-min-state and cooling-max-state which are missing here.

>  		};
>  
>  		A57_1: cpu@1 {
> @@ -100,6 +103,8 @@
>  			next-level-cache = <&A57_L2>;
>  			clocks = <&scpi_dvfs 0>;
>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <530>;
>  		};
>  
>  		A53_0: cpu@100 {
> @@ -110,6 +115,8 @@
>  			next-level-cache = <&A53_L2>;
>  			clocks = <&scpi_dvfs 1>;
>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <140>;
>  		};
>  
>  		A53_1: cpu@101 {
> @@ -120,6 +127,8 @@
>  			next-level-cache = <&A53_L2>;
>  			clocks = <&scpi_dvfs 1>;
>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <140>;
>  		};
>  
>  		A53_2: cpu@102 {
> @@ -130,6 +139,8 @@
>  			next-level-cache = <&A53_L2>;
>  			clocks = <&scpi_dvfs 1>;
>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <140>;
>  		};
>  
>  		A53_3: cpu@103 {
> @@ -140,6 +151,8 @@
>  			next-level-cache = <&A53_L2>;
>  			clocks = <&scpi_dvfs 1>;
>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <140>;
>  		};
>  
>  		A57_L2: l2-cache0 {
> @@ -167,5 +180,42 @@
>  				     <&A53_3>;
>  	};
>  
> +	thermal-zones {
> +		soc_thermal {
> +			polling-delay = <1000>;
> +			polling-delay-passive = <100>;
> +			sustainable-power = <2500>;
> +
> +			thermal-sensors = <&scpi_sensors0 3>;
> +
> +			trips {
> +				threshold: trip-point@0 {

You also need a reg= property if you have the @index thing.

Best regards,
Liviu

> +					temperature = <55000>;
> +					hysteresis = <1000>;
> +					type = "passive";
> +				};
> +				target: trip-point@1 {
> +					temperature = <65000>;
> +					hysteresis = <1000>;
> +					type = "passive";
> +				};
> +			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&target>;
> +					cooling-device = <&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +					contribution = <2048>;
> +				};
> +				map1 {
> +					trip = <&target>;
> +					cooling-device = <&A57_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +					contribution = <1024>;
> +				};
> +
> +			};
> +		};
> +	};
> +
>  	#include "juno-base.dtsi"
>  };
> -- 
> 2.1.4
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 10/10] arm64: dts: Create SoC thermal zone for Juno

[Punit Agrawal]

Hi Liviu,

Thanks for having a look.

Liviu Dudau <Liviu.Dudau@arm.com> writes:

> On Mon, Aug 03, 2015 at 04:22:58PM +0100, Punit Agrawal wrote:
>> Setup a thermal zone driven by the SoC temperature sensor on Juno
>> r0. Create passive trip points and bind them to cpufreq cooling
>> devices that support the power extensions.
>
> Hi Punit,
>
>> 
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm64/boot/dts/arm/juno.dts | 50 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
>> index d2e67f3..664961c 100644
>> --- a/arch/arm64/boot/dts/arm/juno.dts
>> +++ b/arch/arm64/boot/dts/arm/juno.dts
>> @@ -9,6 +9,7 @@
>>  /dts-v1/;
>>  
>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/thermal/thermal.h>
>>  
>>  / {
>>  	model = "ARM Juno development board (r0)";
>> @@ -90,6 +91,8 @@
>>  			next-level-cache = <&A57_L2>;
>>  			clocks = <&scpi_dvfs 0>;
>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <530>;
>
> The Documentation/devicetree/bindings/thermal/thermal.txt document describes two additional
> required properties, cooling-min-state and cooling-max-state which are missing here.
>

The bindings do suggest that the cooling-*-state are required but I
couldn't find any code making use of this property.

I'll send a patch with the next version making those properties
optional.

>>  		};
>>  
>>  		A57_1: cpu@1 {
>> @@ -100,6 +103,8 @@
>>  			next-level-cache = <&A57_L2>;
>>  			clocks = <&scpi_dvfs 0>;
>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <530>;
>>  		};
>>  
>>  		A53_0: cpu@100 {
>> @@ -110,6 +115,8 @@
>>  			next-level-cache = <&A53_L2>;
>>  			clocks = <&scpi_dvfs 1>;
>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <140>;
>>  		};
>>  
>>  		A53_1: cpu@101 {
>> @@ -120,6 +127,8 @@
>>  			next-level-cache = <&A53_L2>;
>>  			clocks = <&scpi_dvfs 1>;
>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <140>;
>>  		};
>>  
>>  		A53_2: cpu@102 {
>> @@ -130,6 +139,8 @@
>>  			next-level-cache = <&A53_L2>;
>>  			clocks = <&scpi_dvfs 1>;
>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <140>;
>>  		};
>>  
>>  		A53_3: cpu@103 {
>> @@ -140,6 +151,8 @@
>>  			next-level-cache = <&A53_L2>;
>>  			clocks = <&scpi_dvfs 1>;
>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <140>;
>>  		};
>>  
>>  		A57_L2: l2-cache0 {
>> @@ -167,5 +180,42 @@
>>  				     <&A53_3>;
>>  	};
>>  
>> +	thermal-zones {
>> +		soc_thermal {
>> +			polling-delay = <1000>;
>> +			polling-delay-passive = <100>;
>> +			sustainable-power = <2500>;
>> +
>> +			thermal-sensors = <&scpi_sensors0 3>;
>> +
>> +			trips {
>> +				threshold: trip-point@0 {
>
> You also need a reg= property if you have the @index thing.
>

Having looked at the examples, I don't need to have the @index for the
trip points. I'll update in the next version.

Thanks,
Punit

> Best regards,
> Liviu
>
>> +					temperature = <55000>;
>> +					hysteresis = <1000>;
>> +					type = "passive";
>> +				};
>> +				target: trip-point@1 {
>> +					temperature = <65000>;
>> +					hysteresis = <1000>;
>> +					type = "passive";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				map0 {
>> +					trip = <&target>;
>> +					cooling-device = <&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +					contribution = <2048>;
>> +				};
>> +				map1 {
>> +					trip = <&target>;
>> +					cooling-device = <&A57_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +					contribution = <1024>;
>> +				};
>> +
>> +			};
>> +		};
>> +	};
>> +
>>  	#include "juno-base.dtsi"
>>  };
>> -- 
>> 2.1.4
>> 

Re: [PATCH v2 10/10] arm64: dts: Create SoC thermal zone for Juno

[Punit Agrawal]

[ adding Eduardo, Rui for their take on OF-thermal bindings ]

Punit Agrawal <punit.agrawal@arm.com> writes:

> Hi Liviu,
>
> Thanks for having a look.
>
> Liviu Dudau <Liviu.Dudau@arm.com> writes:
>
>> On Mon, Aug 03, 2015 at 04:22:58PM +0100, Punit Agrawal wrote:
>>> Setup a thermal zone driven by the SoC temperature sensor on Juno
>>> r0. Create passive trip points and bind them to cpufreq cooling
>>> devices that support the power extensions.
>>
>> Hi Punit,
>>
>>> 
>>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>  arch/arm64/boot/dts/arm/juno.dts | 50 ++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 50 insertions(+)
>>> 
>>> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
>>> index d2e67f3..664961c 100644
>>> --- a/arch/arm64/boot/dts/arm/juno.dts
>>> +++ b/arch/arm64/boot/dts/arm/juno.dts
>>> @@ -9,6 +9,7 @@
>>>  /dts-v1/;
>>>  
>>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +#include <dt-bindings/thermal/thermal.h>
>>>  
>>>  / {
>>>  	model = "ARM Juno development board (r0)";
>>> @@ -90,6 +91,8 @@
>>>  			next-level-cache = <&A57_L2>;
>>>  			clocks = <&scpi_dvfs 0>;
>>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>>> +			#cooling-cells = <2>;
>>> +			dynamic-power-coefficient = <530>;
>>
>> The Documentation/devicetree/bindings/thermal/thermal.txt document describes two additional
>> required properties, cooling-min-state and cooling-max-state which are missing here.
>>
>
> The bindings do suggest that the cooling-*-state are required but I
> couldn't find any code making use of this property.
>
> I'll send a patch with the next version making those properties
> optional.
>

On second thoughts...

The question about cooling-*-state has come up in another thread as
well [0]. I'll re-iterate the question for the thermal maintainers to
clarify.

The cooling-{max,min}-state properties are marked mandatory in the
bindings but aren't used in the code anywhere. Can we drop those
properties from the binding documentation? Or at the least, make them optional?

[...]


> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/10] arm64: dts: Create SoC thermal zone for Juno

[Punit Agrawal]

Apologies for the duplicate, I accidentally Hit send before I finished
the reply.

Punit Agrawal <punit.agrawal@arm.com> writes:

> [ adding Eduardo, Rui for their take on OF-thermal bindings ]
>
> Punit Agrawal <punit.agrawal@arm.com> writes:
>
>> Hi Liviu,
>>
>> Thanks for having a look.
>>
>> Liviu Dudau <Liviu.Dudau@arm.com> writes:
>>
>>> On Mon, Aug 03, 2015 at 04:22:58PM +0100, Punit Agrawal wrote:
>>>> Setup a thermal zone driven by the SoC temperature sensor on Juno
>>>> r0. Create passive trip points and bind them to cpufreq cooling
>>>> devices that support the power extensions.
>>>
>>> Hi Punit,
>>>
>>>> 
>>>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>>  arch/arm64/boot/dts/arm/juno.dts | 50 ++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 50 insertions(+)
>>>> 
>>>> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
>>>> index d2e67f3..664961c 100644
>>>> --- a/arch/arm64/boot/dts/arm/juno.dts
>>>> +++ b/arch/arm64/boot/dts/arm/juno.dts
>>>> @@ -9,6 +9,7 @@
>>>>  /dts-v1/;
>>>>  
>>>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> +#include <dt-bindings/thermal/thermal.h>
>>>>  
>>>>  / {
>>>>  	model = "ARM Juno development board (r0)";
>>>> @@ -90,6 +91,8 @@
>>>>  			next-level-cache = <&A57_L2>;
>>>>  			clocks = <&scpi_dvfs 0>;
>>>>  			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
>>>> +			#cooling-cells = <2>;
>>>> +			dynamic-power-coefficient = <530>;
>>>
>>> The Documentation/devicetree/bindings/thermal/thermal.txt document describes two additional
>>> required properties, cooling-min-state and cooling-max-state which are missing here.
>>>
>>
>> The bindings do suggest that the cooling-*-state are required but I
>> couldn't find any code making use of this property.
>>
>> I'll send a patch with the next version making those properties
>> optional.
>>
>
> On second thoughts...
>
> The question about cooling-*-state has come up in another thread as
> well [0]. I'll re-iterate the question for the thermal maintainers to
> clarify.
>
> The cooling-{max,min}-state properties are marked mandatory in the
> bindings but aren't used in the code anywhere. Can we drop those
> properties from the binding documentation? Or at the least, make them optional?
>

Let me know your preferred approach. I can include a fix in the next version.

[0] http://thread.gmane.org/gmane.linux.ports.ppc.embedded/84512/focus=63339

> [...]
>
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 05/10] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Punit Agrawal <punit.agrawal@arm.com> writes:

> The System Control Processor (SCP) provides access to SoC sensors via
> the System Control and Power Interface (SCPI) Message Protocol. Add
> bindings to allow probing of these sensors. Also support referencing
> of the sensors for setting up thermal zones via the thermal DT
> bindings.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Gentle ping for the bindings review.

> ---
>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> index e309249..8ead68a 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -72,8 +72,18 @@ Required sub-node properties:
>  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>  	       shared memory on Juno platforms
>  
> +Sensor bindings for the sensors based on SCPI Message Protocol
> +--------------------------------------------------------------
> +SCPI provides an API to access the various sensors on the SoC.
> +
> +Required properties:
> +- compatible : should be "arm,scpi-sensors".
> +- #thermal-sensor-cells: should be set to 1. This property follows the
> +			 thermal device tree bindings[2].
> +
>  [0] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>  
>  Example:
>  
> @@ -122,6 +132,11 @@ scpi_protocol: scpi@2e000000 {
>  			clock-output-names = "pxlclk0", "pxlclk1";
>  		};
>  	};
> +
> +	scpi_sensors0: sensors {
> +		compatible = "arm,scpi-sensors";
> +		#thermal-sensor-cells = <1>;
> +	};
>  };
>  
>  cpu@0 {