All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] SCPI Sensor support
@ 2015-09-14 13:00 ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+dt, Punit Agrawal

Hi

The series adds support for SCPI based sensors for ARM development
platform. In addition to supporting sensors, when a temperature sensor
is discovered the driver registers the sensor with the thermal
framework.

The sensor patches here have been split out from a longer series
adding support for registering power model based cooling devices for
the CPU via device tree. Earliers postings can be seen at [0], [1].

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

The last patch adds support for the sensors in the Juno device tree.

The patches depend on SCPI and mailbox patches[2] from Sudeep. The
SCPI protocol document with details of the sensor interface can be
found at [3].

I'd like to aim for merging the patches with the SCPI changes if
possible. The hwmon patches have been acked by Guenter. If there
aren't any objections, I would like to request acks for the binding
updates in Patch 1.

Thanks,
Punit

Changes v2->v3
  - split out the sensor patches (posted here)
  - rebase onto v4.3-rc1
  
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/2002152
[1] http://thread.gmane.org/gmane.linux.kernel/2011466
[2] http://thread.gmane.org/gmane.linux.kernel/2009735
[3] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html

Punit Agrawal (5):
  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

 Documentation/devicetree/bindings/arm/arm,scpi.txt |  15 ++
 Documentation/hwmon/scpi-hwmon                     |  33 +++
 arch/arm64/boot/dts/arm/juno-base.dtsi             |   5 +
 drivers/firmware/arm_scpi.c                        |  60 +++++
 drivers/hwmon/Kconfig                              |   8 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/scpi-hwmon.c                         | 288 +++++++++++++++++++++
 include/linux/scpi_protocol.h                      |  17 ++
 8 files changed, 427 insertions(+)
 create mode 100644 Documentation/hwmon/scpi-hwmon
 create mode 100644 drivers/hwmon/scpi-hwmon.c

-- 
2.5.0


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 0/5] SCPI Sensor support
@ 2015-09-14 13:00 ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi

The series adds support for SCPI based sensors for ARM development
platform. In addition to supporting sensors, when a temperature sensor
is discovered the driver registers the sensor with the thermal
framework.

The sensor patches here have been split out from a longer series
adding support for registering power model based cooling devices for
the CPU via device tree. Earliers postings can be seen at [0], [1].

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

The last patch adds support for the sensors in the Juno device tree.

The patches depend on SCPI and mailbox patches[2] from Sudeep. The
SCPI protocol document with details of the sensor interface can be
found at [3].

I'd like to aim for merging the patches with the SCPI changes if
possible. The hwmon patches have been acked by Guenter. If there
aren't any objections, I would like to request acks for the binding
updates in Patch 1.

Thanks,
Punit

Changes v2->v3
  - split out the sensor patches (posted here)
  - rebase onto v4.3-rc1
  
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/2002152
[1] http://thread.gmane.org/gmane.linux.kernel/2011466
[2] http://thread.gmane.org/gmane.linux.kernel/2009735
[3] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html

Punit Agrawal (5):
  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

 Documentation/devicetree/bindings/arm/arm,scpi.txt |  15 ++
 Documentation/hwmon/scpi-hwmon                     |  33 +++
 arch/arm64/boot/dts/arm/juno-base.dtsi             |   5 +
 drivers/firmware/arm_scpi.c                        |  60 +++++
 drivers/hwmon/Kconfig                              |   8 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/scpi-hwmon.c                         | 288 +++++++++++++++++++++
 include/linux/scpi_protocol.h                      |  17 ++
 8 files changed, 427 insertions(+)
 create mode 100644 Documentation/hwmon/scpi-hwmon
 create mode 100644 drivers/hwmon/scpi-hwmon.c

-- 
2.5.0

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+dt, 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 f002460..af66f7f 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://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
 [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.5.0


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: sudeep.holla-5wv7dgnIgG8, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, liviu.dudau-5wv7dgnIgG8,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
	linux-0h96xk9xTtrk1uMJSBkQmQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, 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-5wv7dgnIgG8@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
---
 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 f002460..af66f7f 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://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
 [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.5.0

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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 f002460..af66f7f 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://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/thermal/thermal.txt
 
 Example:
 
@@ -122,6 +132,11 @@ scpi_protocol: scpi at 2e000000 {
 			clock-output-names = "pxlclk0", "pxlclk1";
 		};
 	};
+
+	scpi_sensors0: sensors {
+		compatible = "arm,scpi-sensors";
+		#thermal-sensor-cells = <1>;
+	};
 };
 
 cpu at 0 {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 2/5] firmware: arm_scpi: Extend to support sensors
  2015-09-14 13:00 ` Punit Agrawal
@ 2015-09-14 13:00   ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+dt, 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.5.0


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 2/5] firmware: arm_scpi: Extend to support sensors
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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.5.0

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 3/5] hwmon: Support sensors exported via ARM SCP interface
  2015-09-14 13:00 ` Punit Agrawal
@ 2015-09-14 13:00   ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+dt, 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>
Acked-by: 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     | 186 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 228 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 c7a4f09..4b94327 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..c7d1d14
--- /dev/null
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -0,0 +1,186 @@
+/*
+ * 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)
+		return ret;
+
+	if (!nr_sensors)
+		return -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");
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 3/5] hwmon: Support sensors exported via ARM SCP interface
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: 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     | 186 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 228 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 c7a4f09..4b94327 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..c7d1d14
--- /dev/null
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -0,0 +1,186 @@
+/*
+ * 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)
+		return ret;
+
+	if (!nr_sensors)
+		return -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");
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [Patch v3 4/5] hwmon: Support registration of thermal zones for SCP temperature sensors
  2015-09-14 13:00 ` Punit Agrawal
@ 2015-09-14 13:00   ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+dt, 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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index c7d1d14..2c1241b 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, int *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;
@@ -160,10 +204,67 @@ 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(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 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[] = {
@@ -178,6 +279,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.5.0


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [Patch v3 4/5] hwmon: Support registration of thermal zones for SCP temperature sensors
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index c7d1d14..2c1241b 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, int *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;
@@ -160,10 +204,67 @@ 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(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 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[] = {
@@ -178,6 +279,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.5.0

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 4/5] hwmon: Support thermal zones registration for SCP temperature sensors
  2015-09-14 13:00 ` Punit Agrawal
@ 2015-09-14 13:00   ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+dt, 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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index c7d1d14..2c1241b 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, int *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;
@@ -160,10 +204,67 @@ 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(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 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[] = {
@@ -178,6 +279,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.5.0


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 4/5] hwmon: Support thermal zones registration for SCP temperature sensors
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Cc: Eduardo Valentin <edubezval@gmail.com>
---
 drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index c7d1d14..2c1241b 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, int *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;
@@ -160,10 +204,67 @@ 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(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 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[] = {
@@ -178,6 +279,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.5.0

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 5/5] arm64: dts: Add sensor node to Juno dt
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: sudeep.holla, linux-pm, lm-sensors, liviu.dudau, edubezval,
	mark.rutland, linux, linux-kernel, robh+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.5.0


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 5/5] arm64: dts: Add sensor node to Juno dt
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: sudeep.holla-5wv7dgnIgG8, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, liviu.dudau-5wv7dgnIgG8,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
	linux-0h96xk9xTtrk1uMJSBkQmQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, 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-5wv7dgnIgG8@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Liviu Dudau <liviu.dudau-5wv7dgnIgG8@public.gmane.org>
Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
---
 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.5.0

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 5/5] arm64: dts: Add sensor node to Juno dt
@ 2015-09-14 13:00   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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.5.0

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:14     ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 13:14 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
> 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 f002460..af66f7f 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].

You need to specify what the valid values for this cell are.

Mark.

> +
>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>  [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.5.0
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:14     ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 13:14 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sudeep Holla,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org, Liviu Dudau,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
> 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-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> ---
>  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 f002460..af66f7f 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].

You need to specify what the valid values for this cell are.

Mark.

> +
>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>  [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.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:14     ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
> 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 f002460..af66f7f 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].

You need to specify what the valid values for this cell are.

Mark.

> +
>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>  
>  Example:
>  
> @@ -122,6 +132,11 @@ scpi_protocol: scpi at 2e000000 {
>  			clock-output-names = "pxlclk0", "pxlclk1";
>  		};
>  	};
> +
> +	scpi_sensors0: sensors {
> +		compatible = "arm,scpi-sensors";
> +		#thermal-sensor-cells = <1>;
> +	};
>  };
>  
>  cpu at 0 {
> -- 
> 2.5.0
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [Patch v3 4/5] hwmon: Support registration of thermal zones for SCP temperature sensors
@ 2015-09-14 13:19     ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:19 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, sudeep.holla, linux-pm, lm-sensors, liviu.dudau,
	edubezval, mark.rutland, linux, linux-kernel, robh+dt

Please ignore this particular patch. I had an earlier one lying around
when I used git send-email *.

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

> 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>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> ---
>  drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index c7d1d14..2c1241b 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, int *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;
> @@ -160,10 +204,67 @@ 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(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 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[] = {
> @@ -178,6 +279,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);

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [Patch v3 4/5] hwmon: Support registration of thermal zones for SCP temperature sensors
@ 2015-09-14 13:19     ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:19 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, liviu.dudau-5wv7dgnIgG8,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
	linux-0h96xk9xTtrk1uMJSBkQmQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A

Please ignore this particular patch. I had an earlier one lying around
when I used git send-email *.

Punit Agrawal <punit.agrawal-5wv7dgnIgG8@public.gmane.org> writes:

> 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-5wv7dgnIgG8@public.gmane.org>
> Acked-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> Cc: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index c7d1d14..2c1241b 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, int *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;
> @@ -160,10 +204,67 @@ 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(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 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[] = {
> @@ -178,6 +279,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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [Patch v3 4/5] hwmon: Support registration of thermal zones for SCP temperature sensors
@ 2015-09-14 13:19     ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

Please ignore this particular patch. I had an earlier one lying around
when I used git send-email *.

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

> 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>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> ---
>  drivers/hwmon/scpi-hwmon.c | 104 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index c7d1d14..2c1241b 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, int *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;
> @@ -160,10 +204,67 @@ 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(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 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[] = {
> @@ -178,6 +279,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);

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 13:14     ` Mark Rutland
  (?)
@ 2015-09-14 13:34       ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:34 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Hi Mark,

Thanks for taking a look.

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
>> 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 f002460..af66f7f 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].
>
> You need to specify what the valid values for this cell are.

The enumeration depends on the number of sensors exported by SCP
firmware - which is platform dependent. I could add add something like
if you think that is helpful -

"Valid cell value is a number between 0..n-1, where n is the number
of sensors exported by SCP firmware."

Thanks,

Punit

>
> Mark.
>
>> +
>>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>>  [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.5.0
>> 
> --
> 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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:34       ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:34 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Hi Mark,

Thanks for taking a look.

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
>> 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 f002460..af66f7f 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].
>
> You need to specify what the valid values for this cell are.

The enumeration depends on the number of sensors exported by SCP
firmware - which is platform dependent. I could add add something like
if you think that is helpful -

"Valid cell value is a number between 0..n-1, where n is the number
of sensors exported by SCP firmware."

Thanks,

Punit

>
> Mark.
>
>> +
>>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>>  [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.5.0
>> 
> --
> 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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:34       ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mark,

Thanks for taking a look.

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
>> 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 f002460..af66f7f 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].
>
> You need to specify what the valid values for this cell are.

The enumeration depends on the number of sensors exported by SCP
firmware - which is platform dependent. I could add add something like
if you think that is helpful -

"Valid cell value is a number between 0..n-1, where n is the number
of sensors exported by SCP firmware."

Thanks,

Punit

>
> Mark.
>
>> +
>>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>>  
>>  Example:
>>  
>> @@ -122,6 +132,11 @@ scpi_protocol: scpi at 2e000000 {
>>  			clock-output-names = "pxlclk0", "pxlclk1";
>>  		};
>>  	};
>> +
>> +	scpi_sensors0: sensors {
>> +		compatible = "arm,scpi-sensors";
>> +		#thermal-sensor-cells = <1>;
>> +	};
>>  };
>>  
>>  cpu at 0 {
>> -- 
>> 2.5.0
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 13:34       ` Punit Agrawal
@ 2015-09-14 13:49         ` Mark Rutland
  -1 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 13:49 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

> >> +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].
> >
> > You need to specify what the valid values for this cell are.
> 
> The enumeration depends on the number of sensors exported by SCP
> firmware - which is platform dependent. I could add add something like
> if you think that is helpful -
> 
> "Valid cell value is a number between 0..n-1, where n is the number
> of sensors exported by SCP firmware."

Can the FW identifer space have holes? Or are they always contiguous?

If this is the same as the raw FW identifer value, specify that.
Otherwise, you need to specify the mapping.

There needs to be enough information for a dts author to figure out
which values to place in the DT.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 13:49         ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 13:49 UTC (permalink / raw)
  To: linux-arm-kernel

> >> +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].
> >
> > You need to specify what the valid values for this cell are.
> 
> The enumeration depends on the number of sensors exported by SCP
> firmware - which is platform dependent. I could add add something like
> if you think that is helpful -
> 
> "Valid cell value is a number between 0..n-1, where n is the number
> of sensors exported by SCP firmware."

Can the FW identifer space have holes? Or are they always contiguous?

If this is the same as the raw FW identifer value, specify that.
Otherwise, you need to specify the mapping.

There needs to be enough information for a dts author to figure out
which values to place in the DT.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 13:49         ` Mark Rutland
  (?)
@ 2015-09-14 14:38           ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 14:38 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Mark Rutland <mark.rutland@arm.com> writes:

>> >> +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].
>> >
>> > You need to specify what the valid values for this cell are.
>> 
>> The enumeration depends on the number of sensors exported by SCP
>> firmware - which is platform dependent. I could add add something like
>> if you think that is helpful -
>> 
>> "Valid cell value is a number between 0..n-1, where n is the number
>> of sensors exported by SCP firmware."
>
> Can the FW identifer space have holes? Or are they always contiguous?

The way the SCP interface is defined, the sensor identifiers are
contiguous, but not all are temperature sensors.

>
> If this is the same as the raw FW identifer value, specify that.
> Otherwise, you need to specify the mapping.

I'll update the patch to add mappings for Juno r0 (and r1 if I can get
my hands on one).

>
> There needs to be enough information for a dts author to figure out
> which values to place in the DT.

I understand. Except sometimes it is hard to get the firmware to commit to not
modify the ordering - discoverability and all that. :)

>
> Thanks,
> Mark.
> --
> 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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 14:38           ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 14:38 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:

>> >> +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].
>> >
>> > You need to specify what the valid values for this cell are.
>> 
>> The enumeration depends on the number of sensors exported by SCP
>> firmware - which is platform dependent. I could add add something like
>> if you think that is helpful -
>> 
>> "Valid cell value is a number between 0..n-1, where n is the number
>> of sensors exported by SCP firmware."
>
> Can the FW identifer space have holes? Or are they always contiguous?

The way the SCP interface is defined, the sensor identifiers are
contiguous, but not all are temperature sensors.

>
> If this is the same as the raw FW identifer value, specify that.
> Otherwise, you need to specify the mapping.

I'll update the patch to add mappings for Juno r0 (and r1 if I can get
my hands on one).

>
> There needs to be enough information for a dts author to figure out
> which values to place in the DT.

I understand. Except sometimes it is hard to get the firmware to commit to not
modify the ordering - discoverability and all that. :)

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 14:38           ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 14:38 UTC (permalink / raw)
  To: linux-arm-kernel

Mark Rutland <mark.rutland@arm.com> writes:

>> >> +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].
>> >
>> > You need to specify what the valid values for this cell are.
>> 
>> The enumeration depends on the number of sensors exported by SCP
>> firmware - which is platform dependent. I could add add something like
>> if you think that is helpful -
>> 
>> "Valid cell value is a number between 0..n-1, where n is the number
>> of sensors exported by SCP firmware."
>
> Can the FW identifer space have holes? Or are they always contiguous?

The way the SCP interface is defined, the sensor identifiers are
contiguous, but not all are temperature sensors.

>
> If this is the same as the raw FW identifer value, specify that.
> Otherwise, you need to specify the mapping.

I'll update the patch to add mappings for Juno r0 (and r1 if I can get
my hands on one).

>
> There needs to be enough information for a dts author to figure out
> which values to place in the DT.

I understand. Except sometimes it is hard to get the firmware to commit to not
modify the ordering - discoverability and all that. :)

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 14:43             ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 14:43 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> >> >> +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].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous, but not all are temperature sensors.

Ok. So how exactly are they enumerated for this binding?

> > If this is the same as the raw FW identifer value, specify that.
> > Otherwise, you need to specify the mapping.
> 
> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> my hands on one).

If there's identical logic mapping the two, we might just be able to
describe that rather than having to add tables all the time.

> > There needs to be enough information for a dts author to figure out
> > which values to place in the DT.
> 
> I understand. Except sometimes it is hard to get the firmware to commit to not
> modify the ordering - discoverability and all that. :)

If they do that, then things are broken regardless, no? I guess that'll
be clear if/when I see how the mapping works.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 14:43             ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 14:43 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sudeep Holla,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org, Liviu Dudau,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:
> 
> >> >> +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].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous, but not all are temperature sensors.

Ok. So how exactly are they enumerated for this binding?

> > If this is the same as the raw FW identifer value, specify that.
> > Otherwise, you need to specify the mapping.
> 
> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> my hands on one).

If there's identical logic mapping the two, we might just be able to
describe that rather than having to add tables all the time.

> > There needs to be enough information for a dts author to figure out
> > which values to place in the DT.
> 
> I understand. Except sometimes it is hard to get the firmware to commit to not
> modify the ordering - discoverability and all that. :)

If they do that, then things are broken regardless, no? I guess that'll
be clear if/when I see how the mapping works.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 14:43             ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> >> >> +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].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous, but not all are temperature sensors.

Ok. So how exactly are they enumerated for this binding?

> > If this is the same as the raw FW identifer value, specify that.
> > Otherwise, you need to specify the mapping.
> 
> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> my hands on one).

If there's identical logic mapping the two, we might just be able to
describe that rather than having to add tables all the time.

> > There needs to be enough information for a dts author to figure out
> > which values to place in the DT.
> 
> I understand. Except sometimes it is hard to get the firmware to commit to not
> modify the ordering - discoverability and all that. :)

If they do that, then things are broken regardless, no? I guess that'll
be clear if/when I see how the mapping works.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 14:43             ` Mark Rutland
  (?)
@ 2015-09-14 15:01               ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 15:01 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 
>> >> >> +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].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous, but not all are temperature sensors.
>
> Ok. So how exactly are they enumerated for this binding?

The sensor enumeration for r0 (which I've verified) and r1 can be found
at [0]. The valid cell values are identifiers for the temperature sensors.

>
>> > If this is the same as the raw FW identifer value, specify that.
>> > Otherwise, you need to specify the mapping.
>> 
>> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
>> my hands on one).
>
> If there's identical logic mapping the two, we might just be able to
> describe that rather than having to add tables all the time.
>

After seeing the mapping already published, I am wondering if there is
any value in duplicating the information. If there are no objections,
I'll update this patch to add pointers instead.

>> > There needs to be enough information for a dts author to figure out
>> > which values to place in the DT.
>> 
>> I understand. Except sometimes it is hard to get the firmware to commit to not
>> modify the ordering - discoverability and all that. :)
>
> If they do that, then things are broken regardless, no? I guess that'll
> be clear if/when I see how the mapping works.

[0] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html

>
> Thanks,
> Mark.
> --
> 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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 15:01               ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 15:01 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:

> On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:
>> 
>> >> >> +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].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous, but not all are temperature sensors.
>
> Ok. So how exactly are they enumerated for this binding?

The sensor enumeration for r0 (which I've verified) and r1 can be found
at [0]. The valid cell values are identifiers for the temperature sensors.

>
>> > If this is the same as the raw FW identifer value, specify that.
>> > Otherwise, you need to specify the mapping.
>> 
>> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
>> my hands on one).
>
> If there's identical logic mapping the two, we might just be able to
> describe that rather than having to add tables all the time.
>

After seeing the mapping already published, I am wondering if there is
any value in duplicating the information. If there are no objections,
I'll update this patch to add pointers instead.

>> > There needs to be enough information for a dts author to figure out
>> > which values to place in the DT.
>> 
>> I understand. Except sometimes it is hard to get the firmware to commit to not
>> modify the ordering - discoverability and all that. :)
>
> If they do that, then things are broken regardless, no? I guess that'll
> be clear if/when I see how the mapping works.

[0] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 15:01               ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 
>> >> >> +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].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous, but not all are temperature sensors.
>
> Ok. So how exactly are they enumerated for this binding?

The sensor enumeration for r0 (which I've verified) and r1 can be found
at [0]. The valid cell values are identifiers for the temperature sensors.

>
>> > If this is the same as the raw FW identifer value, specify that.
>> > Otherwise, you need to specify the mapping.
>> 
>> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
>> my hands on one).
>
> If there's identical logic mapping the two, we might just be able to
> describe that rather than having to add tables all the time.
>

After seeing the mapping already published, I am wondering if there is
any value in duplicating the information. If there are no objections,
I'll update this patch to add pointers instead.

>> > There needs to be enough information for a dts author to figure out
>> > which values to place in the DT.
>> 
>> I understand. Except sometimes it is hard to get the firmware to commit to not
>> modify the ordering - discoverability and all that. :)
>
> If they do that, then things are broken regardless, no? I guess that'll
> be clear if/when I see how the mapping works.

[0] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 15:01               ` Punit Agrawal
@ 2015-09-14 15:15                 ` Mark Rutland
  -1 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 15:15 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> > On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> >> Mark Rutland <mark.rutland@arm.com> writes:
> >> 
> >> >> >> +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].
> >> >> >
> >> >> > You need to specify what the valid values for this cell are.
> >> >> 
> >> >> The enumeration depends on the number of sensors exported by SCP
> >> >> firmware - which is platform dependent. I could add add something like
> >> >> if you think that is helpful -
> >> >> 
> >> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> >> of sensors exported by SCP firmware."
> >> >
> >> > Can the FW identifer space have holes? Or are they always contiguous?
> >> 
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous, but not all are temperature sensors.
> >
> > Ok. So how exactly are they enumerated for this binding?
> 
> The sensor enumeration for r0 (which I've verified) and r1 can be found
> at [0]. The valid cell values are identifiers for the temperature sensors.

Ok. That's fine by me; I was confused and thought that there was some
internal mapping.

> >> > If this is the same as the raw FW identifer value, specify that.
> >> > Otherwise, you need to specify the mapping.
> >> 
> >> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> >> my hands on one).
> >
> > If there's identical logic mapping the two, we might just be able to
> > describe that rather than having to add tables all the time.
> >
> 
> After seeing the mapping already published, I am wondering if there is
> any value in duplicating the information. If there are no objections,
> I'll update this patch to add pointers instead.

That's fine by me. The important part is that the value is a raw Sensor
ID as the FW uses. So long as we state that, the IDs themselves can come
from whatever documentation is valid for a particular instance.

Thanks for the info!

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 15:15                 ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-14 15:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> > On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> >> Mark Rutland <mark.rutland@arm.com> writes:
> >> 
> >> >> >> +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].
> >> >> >
> >> >> > You need to specify what the valid values for this cell are.
> >> >> 
> >> >> The enumeration depends on the number of sensors exported by SCP
> >> >> firmware - which is platform dependent. I could add add something like
> >> >> if you think that is helpful -
> >> >> 
> >> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> >> of sensors exported by SCP firmware."
> >> >
> >> > Can the FW identifer space have holes? Or are they always contiguous?
> >> 
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous, but not all are temperature sensors.
> >
> > Ok. So how exactly are they enumerated for this binding?
> 
> The sensor enumeration for r0 (which I've verified) and r1 can be found
> at [0]. The valid cell values are identifiers for the temperature sensors.

Ok. That's fine by me; I was confused and thought that there was some
internal mapping.

> >> > If this is the same as the raw FW identifer value, specify that.
> >> > Otherwise, you need to specify the mapping.
> >> 
> >> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> >> my hands on one).
> >
> > If there's identical logic mapping the two, we might just be able to
> > describe that rather than having to add tables all the time.
> >
> 
> After seeing the mapping already published, I am wondering if there is
> any value in duplicating the information. If there are no objections,
> I'll update this patch to add pointers instead.

That's fine by me. The important part is that the value is a raw Sensor
ID as the FW uses. So long as we state that, the IDs themselves can come
from whatever documentation is valid for a particular instance.

Thanks for the info!

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 15:15                 ` Mark Rutland
  (?)
@ 2015-09-14 16:03                   ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 16:03 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 

[...]

>> 
>> After seeing the mapping already published, I am wondering if there is
>> any value in duplicating the information. If there are no objections,
>> I'll update this patch to add pointers instead.
>
> That's fine by me. The important part is that the value is a raw Sensor
> ID as the FW uses. So long as we state that, the IDs themselves can come
> from whatever documentation is valid for a particular instance.
>
> Thanks for the info!

Sure. I've updated the patch to explain what the valid cell values are
along with a pointer to platform documentation for Juno R0 and R1.

I've also added an example to show usage of the sensors to create a
thermal zone.

I'll post a new version shortly.

Thanks for your having a look.

>
> Thanks,
> Mark.
> --
> 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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 16:03                   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 16:03 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Sudeep Holla, linux-pm@vger.kernel.org, lm-sensors@lm-sensors.org,
	Liviu Dudau, edubezval@gmail.com, linux@roeck-us.net,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 

[...]

>> 
>> After seeing the mapping already published, I am wondering if there is
>> any value in duplicating the information. If there are no objections,
>> I'll update this patch to add pointers instead.
>
> That's fine by me. The important part is that the value is a raw Sensor
> ID as the FW uses. So long as we state that, the IDs themselves can come
> from whatever documentation is valid for a particular instance.
>
> Thanks for the info!

Sure. I've updated the patch to explain what the valid cell values are
along with a pointer to platform documentation for Juno R0 and R1.

I've also added an example to show usage of the sensors to create a
thermal zone.

I'll post a new version shortly.

Thanks for your having a look.

>
> Thanks,
> Mark.
> --
> 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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 16:03                   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-14 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

Mark Rutland <mark.rutland@arm.com> writes:

> On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 

[...]

>> 
>> After seeing the mapping already published, I am wondering if there is
>> any value in duplicating the information. If there are no objections,
>> I'll update this patch to add pointers instead.
>
> That's fine by me. The important part is that the value is a raw Sensor
> ID as the FW uses. So long as we state that, the IDs themselves can come
> from whatever documentation is valid for a particular instance.
>
> Thanks for the info!

Sure. I've updated the patch to explain what the valid cell values are
along with a pointer to platform documentation for Juno R0 and R1.

I've also added an example to show usage of the sensors to create a
thermal zone.

I'll post a new version shortly.

Thanks for your having a look.

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 17:18             ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-14 17:18 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> >> >> +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].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous,

Is there any documentation other than DUI0922A? [1] From what I can seen
that just says it's a 16-bit value and doesn't put any particular
constraints on its value.

[1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf

-- 
Tixy


^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 17:18             ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-14 17:18 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liviu Dudau,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org, Sudeep Holla,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org

On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:
> 
> >> >> +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].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous,

Is there any documentation other than DUI0922A? [1] From what I can seen
that just says it's a 16-bit value and doesn't put any particular
constraints on its value.

[1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf

-- 
Tixy

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-14 17:18             ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-14 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> >> >> +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].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous,

Is there any documentation other than DUI0922A? [1] From what I can seen
that just says it's a 16-bit value and doesn't put any particular
constraints on its value.

[1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf

-- 
Tixy

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-14 17:18             ` Jon Medhurst (Tixy)
  (?)
@ 2015-09-15  9:37               ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-15  9:37 UTC (permalink / raw)
  To: Jon Medhurst (Tixy)
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

"Jon Medhurst (Tixy)" <tixy@linaro.org> writes:

> On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 
>> >> >> +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].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous,
>
> Is there any documentation other than DUI0922A? [1] From what I can seen
> that just says it's a 16-bit value and doesn't put any particular
> constraints on its value.

Although not explicitly stated, if you look at the Get Sensor Capability
[2] and Get Sensor Info [3] commands you can indirectly infer that the
Sensor IDs are contiguous. Not the strongest guarantee I know.

All platforms currently using SCP (Juno R0 and R1) do indeed expose
contiguous identifiers. Maybe we can convince the authors to make it
explicit.

>
> [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
[2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15  9:37               ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-15  9:37 UTC (permalink / raw)
  To: Jon Medhurst (Tixy)
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

"Jon Medhurst (Tixy)" <tixy@linaro.org> writes:

> On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 
>> >> >> +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].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous,
>
> Is there any documentation other than DUI0922A? [1] From what I can seen
> that just says it's a 16-bit value and doesn't put any particular
> constraints on its value.

Although not explicitly stated, if you look at the Get Sensor Capability
[2] and Get Sensor Info [3] commands you can indirectly infer that the
Sensor IDs are contiguous. Not the strongest guarantee I know.

All platforms currently using SCP (Juno R0 and R1) do indeed expose
contiguous identifiers. Maybe we can convince the authors to make it
explicit.

>
> [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
[2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15  9:37               ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-15  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

"Jon Medhurst (Tixy)" <tixy@linaro.org> writes:

> On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>> 
>> >> >> +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].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous,
>
> Is there any documentation other than DUI0922A? [1] From what I can seen
> that just says it's a 16-bit value and doesn't put any particular
> constraints on its value.

Although not explicitly stated, if you look at the Get Sensor Capability
[2] and Get Sensor Info [3] commands you can indirectly infer that the
Sensor IDs are contiguous. Not the strongest guarantee I know.

All platforms currently using SCP (Juno R0 and R1) do indeed expose
contiguous identifiers. Maybe we can convince the authors to make it
explicit.

>
> [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
[2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-15  9:37               ` Punit Agrawal
@ 2015-09-15 10:46                 ` Jon Medhurst (Tixy)
  -1 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 10:46 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> 
> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> Mark Rutland <mark.rutland@arm.com> writes:
> >> 
[...]
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous,
> >
> > Is there any documentation other than DUI0922A? [1] From what I can seen
> > that just says it's a 16-bit value and doesn't put any particular
> > constraints on its value.
> 
> Although not explicitly stated, if you look at the Get Sensor Capability
> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> Sensor IDs are contiguous.

I personally wouldn't even indirectly infer they are contiguous from
what the document says. If I were implementing the firmware I would feel
quite in my rights to, for example, use the top 8 bits of the ID for a
sensor type and the bottom 8 for an index, if that made dispatching of
requests more efficient. Or if some optional hardware was detected as
missing, leaving some holes in ID space.

As a specification of a 'standard' the document seems to be rather
lacking. So, Sensor ID should be documented as being "an unsigned
integer less than then number of sensors returned by the Get Sensor
Capability command", or something like that. I guess clocks and other
devices suffer from similar lack of specificity.

>  Not the strongest guarantee I know.
> 
> All platforms currently using SCP (Juno R0 and R1) do indeed expose
> contiguous identifiers.

IMO, Linux drivers should be coded to the standard or written
specification (where they are available) not the particular
implementations available.

> >
> > [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
> [2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
> [3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

I think those links are on ARM's intranet, they return NXDOMAIN for me.

-- 
Tixy


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 10:46                 ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> 
> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> Mark Rutland <mark.rutland@arm.com> writes:
> >> 
[...]
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous,
> >
> > Is there any documentation other than DUI0922A? [1] From what I can seen
> > that just says it's a 16-bit value and doesn't put any particular
> > constraints on its value.
> 
> Although not explicitly stated, if you look at the Get Sensor Capability
> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> Sensor IDs are contiguous.

I personally wouldn't even indirectly infer they are contiguous from
what the document says. If I were implementing the firmware I would feel
quite in my rights to, for example, use the top 8 bits of the ID for a
sensor type and the bottom 8 for an index, if that made dispatching of
requests more efficient. Or if some optional hardware was detected as
missing, leaving some holes in ID space.

As a specification of a 'standard' the document seems to be rather
lacking. So, Sensor ID should be documented as being "an unsigned
integer less than then number of sensors returned by the Get Sensor
Capability command", or something like that. I guess clocks and other
devices suffer from similar lack of specificity.

>  Not the strongest guarantee I know.
> 
> All platforms currently using SCP (Juno R0 and R1) do indeed expose
> contiguous identifiers.

IMO, Linux drivers should be coded to the standard or written
specification (where they are available) not the particular
implementations available.

> >
> > [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
> [2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
> [3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

I think those links are on ARM's intranet, they return NXDOMAIN for me.

-- 
Tixy

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 11:03                   ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-15 11:03 UTC (permalink / raw)
  To: Jon Medhurst (Tixy)
  Cc: Punit Agrawal, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> > 
> > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > >> Mark Rutland <mark.rutland@arm.com> writes:
> > >> 
> [...]
> > >> The way the SCP interface is defined, the sensor identifiers are
> > >> contiguous,
> > >
> > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > that just says it's a 16-bit value and doesn't put any particular
> > > constraints on its value.
> > 
> > Although not explicitly stated, if you look at the Get Sensor Capability
> > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > Sensor IDs are contiguous.
> 
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.
> 
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I think from the PoV of the binding, this doesn't matter. The value is
just an arbitrary opaue token written down in a spec, that the FW
understands how to interpret.

I only asked about how the IDs were organised because I thought there
was additional translation in the kernel, but this is not the case.

The only potential problem is bit-width. Punit, I assume the value is
32-bit (or less) in the messages to the FW?

If for the moment we assume the values can index a small array, then we
can change that later (though ideally we'd handle the most geeneral case
we're aware of from the outset). We just need to not infer any semantics
from a value alone.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 11:03                   ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-15 11:03 UTC (permalink / raw)
  To: Jon Medhurst (Tixy)
  Cc: Punit Agrawal, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liviu Dudau,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org, Sudeep Holla,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org

On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > "Jon Medhurst (Tixy)" <tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
> > 
> > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > >> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:
> > >> 
> [...]
> > >> The way the SCP interface is defined, the sensor identifiers are
> > >> contiguous,
> > >
> > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > that just says it's a 16-bit value and doesn't put any particular
> > > constraints on its value.
> > 
> > Although not explicitly stated, if you look at the Get Sensor Capability
> > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > Sensor IDs are contiguous.
> 
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.
> 
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I think from the PoV of the binding, this doesn't matter. The value is
just an arbitrary opaue token written down in a spec, that the FW
understands how to interpret.

I only asked about how the IDs were organised because I thought there
was additional translation in the kernel, but this is not the case.

The only potential problem is bit-width. Punit, I assume the value is
32-bit (or less) in the messages to the FW?

If for the moment we assume the values can index a small array, then we
can change that later (though ideally we'd handle the most geeneral case
we're aware of from the outset). We just need to not infer any semantics
from a value alone.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 11:03                   ` Mark Rutland
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Rutland @ 2015-09-15 11:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> > 
> > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > >> Mark Rutland <mark.rutland@arm.com> writes:
> > >> 
> [...]
> > >> The way the SCP interface is defined, the sensor identifiers are
> > >> contiguous,
> > >
> > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > that just says it's a 16-bit value and doesn't put any particular
> > > constraints on its value.
> > 
> > Although not explicitly stated, if you look at the Get Sensor Capability
> > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > Sensor IDs are contiguous.
> 
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.
> 
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I think from the PoV of the binding, this doesn't matter. The value is
just an arbitrary opaue token written down in a spec, that the FW
understands how to interpret.

I only asked about how the IDs were organised because I thought there
was additional translation in the kernel, but this is not the case.

The only potential problem is bit-width. Punit, I assume the value is
32-bit (or less) in the messages to the FW?

If for the moment we assume the values can index a small array, then we
can change that later (though ideally we'd handle the most geeneral case
we're aware of from the outset). We just need to not infer any semantics
from a value alone.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-15 11:03                   ` Mark Rutland
  (?)
@ 2015-09-15 11:37                     ` Jon Medhurst (Tixy)
  -1 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 11:37 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Punit Agrawal, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

On Tue, 2015-09-15 at 12:03 +0100, Mark Rutland wrote:
> On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > > "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> > > 
> > > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > > >> Mark Rutland <mark.rutland@arm.com> writes:
> > > >> 
> > [...]
> > > >> The way the SCP interface is defined, the sensor identifiers are
> > > >> contiguous,
> > > >
> > > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > > that just says it's a 16-bit value and doesn't put any particular
> > > > constraints on its value.
> > > 
> > > Although not explicitly stated, if you look at the Get Sensor Capability
> > > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > > Sensor IDs are contiguous.
> > 
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> > 
> > As a specification of a 'standard' the document seems to be rather
> > lacking. So, Sensor ID should be documented as being "an unsigned
> > integer less than then number of sensors returned by the Get Sensor
> > Capability command", or something like that. I guess clocks and other
> > devices suffer from similar lack of specificity.
> 
> I think from the PoV of the binding, this doesn't matter. The value is
> just an arbitrary opaue token written down in a spec, that the FW
> understands how to interpret.

True, it's the Linux implementation in following patches that has
assumptions, e.g. for loops iterating over id's 0..N-1
 
> 
> I only asked about how the IDs were organised because I thought there
> was additional translation in the kernel, but this is not the case.
> 
> The only potential problem is bit-width. Punit, I assume the value is
> 32-bit (or less) in the messages to the FW?

It's 16 bit.

-- 
Tixy


^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 11:37                     ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 11:37 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Punit Agrawal, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liviu Dudau,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org,
	edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org, Sudeep Holla,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org

On Tue, 2015-09-15 at 12:03 +0100, Mark Rutland wrote:
> On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > > "Jon Medhurst (Tixy)" <tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
> > > 
> > > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > > >> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:
> > > >> 
> > [...]
> > > >> The way the SCP interface is defined, the sensor identifiers are
> > > >> contiguous,
> > > >
> > > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > > that just says it's a 16-bit value and doesn't put any particular
> > > > constraints on its value.
> > > 
> > > Although not explicitly stated, if you look at the Get Sensor Capability
> > > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > > Sensor IDs are contiguous.
> > 
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> > 
> > As a specification of a 'standard' the document seems to be rather
> > lacking. So, Sensor ID should be documented as being "an unsigned
> > integer less than then number of sensors returned by the Get Sensor
> > Capability command", or something like that. I guess clocks and other
> > devices suffer from similar lack of specificity.
> 
> I think from the PoV of the binding, this doesn't matter. The value is
> just an arbitrary opaue token written down in a spec, that the FW
> understands how to interpret.

True, it's the Linux implementation in following patches that has
assumptions, e.g. for loops iterating over id's 0..N-1
 
> 
> I only asked about how the IDs were organised because I thought there
> was additional translation in the kernel, but this is not the case.
> 
> The only potential problem is bit-width. Punit, I assume the value is
> 32-bit (or less) in the messages to the FW?

It's 16 bit.

-- 
Tixy

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 11:37                     ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 11:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2015-09-15 at 12:03 +0100, Mark Rutland wrote:
> On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > > "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> > > 
> > > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > > >> Mark Rutland <mark.rutland@arm.com> writes:
> > > >> 
> > [...]
> > > >> The way the SCP interface is defined, the sensor identifiers are
> > > >> contiguous,
> > > >
> > > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > > that just says it's a 16-bit value and doesn't put any particular
> > > > constraints on its value.
> > > 
> > > Although not explicitly stated, if you look at the Get Sensor Capability
> > > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > > Sensor IDs are contiguous.
> > 
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> > 
> > As a specification of a 'standard' the document seems to be rather
> > lacking. So, Sensor ID should be documented as being "an unsigned
> > integer less than then number of sensors returned by the Get Sensor
> > Capability command", or something like that. I guess clocks and other
> > devices suffer from similar lack of specificity.
> 
> I think from the PoV of the binding, this doesn't matter. The value is
> just an arbitrary opaue token written down in a spec, that the FW
> understands how to interpret.

True, it's the Linux implementation in following patches that has
assumptions, e.g. for loops iterating over id's 0..N-1
 
> 
> I only asked about how the IDs were organised because I thought there
> was additional translation in the kernel, but this is not the case.
> 
> The only potential problem is bit-width. Punit, I assume the value is
> 32-bit (or less) in the messages to the FW?

It's 16 bit.

-- 
Tixy

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-15 10:46                 ` Jon Medhurst (Tixy)
  (?)
@ 2015-09-15 16:04                   ` Punit Agrawal
  -1 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-15 16:04 UTC (permalink / raw)
  To: Jon Medhurst (Tixy)
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

"Jon Medhurst (Tixy)" <tixy@linaro.org> writes:

> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
>> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
>> 
>> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> >> Mark Rutland <mark.rutland@arm.com> writes:
>> >> 
> [...]
>> >> The way the SCP interface is defined, the sensor identifiers are
>> >> contiguous,
>> >
>> > Is there any documentation other than DUI0922A? [1] From what I can seen
>> > that just says it's a 16-bit value and doesn't put any particular
>> > constraints on its value.
>> 
>> Although not explicitly stated, if you look at the Get Sensor Capability
>> [2] and Get Sensor Info [3] commands you can indirectly infer that the
>> Sensor IDs are contiguous.
>
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.

True. And without a command to convey the list of valid IDs, the
consumer of the API would have to iterate over the entire 16bit space to
locate valid IDs.

>
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I was thinking of suggesting something similar as an update.

>
>>  Not the strongest guarantee I know.
>> 
>> All platforms currently using SCP (Juno R0 and R1) do indeed expose
>> contiguous identifiers.
>
> IMO, Linux drivers should be coded to the standard or written
> specification (where they are available) not the particular
> implementations available.
>
>> >
>> > [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
>> [2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
>> [3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
>
> I think those links are on ARM's intranet, they return NXDOMAIN for me.

Apologies, the below should work.

[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 16:04                   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-15 16:04 UTC (permalink / raw)
  To: Jon Medhurst (Tixy)
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

"Jon Medhurst (Tixy)" <tixy@linaro.org> writes:

> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
>> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
>> 
>> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> >> Mark Rutland <mark.rutland@arm.com> writes:
>> >> 
> [...]
>> >> The way the SCP interface is defined, the sensor identifiers are
>> >> contiguous,
>> >
>> > Is there any documentation other than DUI0922A? [1] From what I can seen
>> > that just says it's a 16-bit value and doesn't put any particular
>> > constraints on its value.
>> 
>> Although not explicitly stated, if you look at the Get Sensor Capability
>> [2] and Get Sensor Info [3] commands you can indirectly infer that the
>> Sensor IDs are contiguous.
>
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.

True. And without a command to convey the list of valid IDs, the
consumer of the API would have to iterate over the entire 16bit space to
locate valid IDs.

>
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I was thinking of suggesting something similar as an update.

>
>>  Not the strongest guarantee I know.
>> 
>> All platforms currently using SCP (Juno R0 and R1) do indeed expose
>> contiguous identifiers.
>
> IMO, Linux drivers should be coded to the standard or written
> specification (where they are available) not the particular
> implementations available.
>
>> >
>> > [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
>> [2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
>> [3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
>
> I think those links are on ARM's intranet, they return NXDOMAIN for me.

Apologies, the below should work.

[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 16:04                   ` Punit Agrawal
  0 siblings, 0 replies; 60+ messages in thread
From: Punit Agrawal @ 2015-09-15 16:04 UTC (permalink / raw)
  To: linux-arm-kernel

"Jon Medhurst (Tixy)" <tixy@linaro.org> writes:

> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
>> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
>> 
>> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> >> Mark Rutland <mark.rutland@arm.com> writes:
>> >> 
> [...]
>> >> The way the SCP interface is defined, the sensor identifiers are
>> >> contiguous,
>> >
>> > Is there any documentation other than DUI0922A? [1] From what I can seen
>> > that just says it's a 16-bit value and doesn't put any particular
>> > constraints on its value.
>> 
>> Although not explicitly stated, if you look at the Get Sensor Capability
>> [2] and Get Sensor Info [3] commands you can indirectly infer that the
>> Sensor IDs are contiguous.
>
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.

True. And without a command to convey the list of valid IDs, the
consumer of the API would have to iterate over the entire 16bit space to
locate valid IDs.

>
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I was thinking of suggesting something similar as an update.

>
>>  Not the strongest guarantee I know.
>> 
>> All platforms currently using SCP (Juno R0 and R1) do indeed expose
>> contiguous identifiers.
>
> IMO, Linux drivers should be coded to the standard or written
> specification (where they are available) not the particular
> implementations available.
>
>> >
>> > [1] http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
>> [2] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
>> [3] http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
>
> I think those links are on ARM's intranet, they return NXDOMAIN for me.

Apologies, the below should work.

[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
  2015-09-15 16:04                   ` Punit Agrawal
@ 2015-09-15 16:31                     ` Jon Medhurst (Tixy)
  -1 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 16:31 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, Liviu Dudau,
	linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org,
	edubezval@gmail.com, robh+dt@kernel.org, linux@roeck-us.net,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org

On Tue, 2015-09-15 at 17:04 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> 
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> >> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> >> 
> >> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> >> Mark Rutland <mark.rutland@arm.com> writes:
> >> >> 
> > [...]
> >> >> The way the SCP interface is defined, the sensor identifiers are
> >> >> contiguous,
> >> >
> >> > Is there any documentation other than DUI0922A? [1] From what I can seen
> >> > that just says it's a 16-bit value and doesn't put any particular
> >> > constraints on its value.
> >> 
> >> Although not explicitly stated, if you look at the Get Sensor Capability
> >> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> >> Sensor IDs are contiguous.
> >
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> 
> True. And without a command to convey the list of valid IDs, the
> consumer of the API would have to iterate over the entire 16bit space to
> locate valid IDs.

Or get IDs from device-tree :-) Anyway, I'm not arguing that the IDs
shouldn't be 0..N-1, just that it should explicitly documented in the
SCPI doc, which we're are in agreement on.

-- 
Tixy


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors
@ 2015-09-15 16:31                     ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 60+ messages in thread
From: Jon Medhurst (Tixy) @ 2015-09-15 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2015-09-15 at 17:04 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> 
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> >> "Jon Medhurst (Tixy)" <tixy@linaro.org> writes:
> >> 
> >> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> >> Mark Rutland <mark.rutland@arm.com> writes:
> >> >> 
> > [...]
> >> >> The way the SCP interface is defined, the sensor identifiers are
> >> >> contiguous,
> >> >
> >> > Is there any documentation other than DUI0922A? [1] From what I can seen
> >> > that just says it's a 16-bit value and doesn't put any particular
> >> > constraints on its value.
> >> 
> >> Although not explicitly stated, if you look at the Get Sensor Capability
> >> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> >> Sensor IDs are contiguous.
> >
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> 
> True. And without a command to convey the list of valid IDs, the
> consumer of the API would have to iterate over the entire 16bit space to
> locate valid IDs.

Or get IDs from device-tree :-) Anyway, I'm not arguing that the IDs
shouldn't be 0..N-1, just that it should explicitly documented in the
SCPI doc, which we're are in agreement on.

-- 
Tixy

^ permalink raw reply	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2015-09-15 16:32 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 13:00 [PATCH v3 0/5] SCPI Sensor support Punit Agrawal
2015-09-14 13:00 ` Punit Agrawal
2015-09-14 13:00 ` [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:14   ` Mark Rutland
2015-09-14 13:14     ` Mark Rutland
2015-09-14 13:14     ` Mark Rutland
2015-09-14 13:34     ` Punit Agrawal
2015-09-14 13:34       ` Punit Agrawal
2015-09-14 13:34       ` Punit Agrawal
2015-09-14 13:49       ` Mark Rutland
2015-09-14 13:49         ` Mark Rutland
2015-09-14 14:38         ` Punit Agrawal
2015-09-14 14:38           ` Punit Agrawal
2015-09-14 14:38           ` Punit Agrawal
2015-09-14 14:43           ` Mark Rutland
2015-09-14 14:43             ` Mark Rutland
2015-09-14 14:43             ` Mark Rutland
2015-09-14 15:01             ` Punit Agrawal
2015-09-14 15:01               ` Punit Agrawal
2015-09-14 15:01               ` Punit Agrawal
2015-09-14 15:15               ` Mark Rutland
2015-09-14 15:15                 ` Mark Rutland
2015-09-14 16:03                 ` Punit Agrawal
2015-09-14 16:03                   ` Punit Agrawal
2015-09-14 16:03                   ` Punit Agrawal
2015-09-14 17:18           ` Jon Medhurst (Tixy)
2015-09-14 17:18             ` Jon Medhurst (Tixy)
2015-09-14 17:18             ` Jon Medhurst (Tixy)
2015-09-15  9:37             ` Punit Agrawal
2015-09-15  9:37               ` Punit Agrawal
2015-09-15  9:37               ` Punit Agrawal
2015-09-15 10:46               ` Jon Medhurst (Tixy)
2015-09-15 10:46                 ` Jon Medhurst (Tixy)
2015-09-15 11:03                 ` Mark Rutland
2015-09-15 11:03                   ` Mark Rutland
2015-09-15 11:03                   ` Mark Rutland
2015-09-15 11:37                   ` Jon Medhurst (Tixy)
2015-09-15 11:37                     ` Jon Medhurst (Tixy)
2015-09-15 11:37                     ` Jon Medhurst (Tixy)
2015-09-15 16:04                 ` Punit Agrawal
2015-09-15 16:04                   ` Punit Agrawal
2015-09-15 16:04                   ` Punit Agrawal
2015-09-15 16:31                   ` Jon Medhurst (Tixy)
2015-09-15 16:31                     ` Jon Medhurst (Tixy)
2015-09-14 13:00 ` [PATCH v3 2/5] firmware: arm_scpi: Extend to support sensors Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:00 ` [PATCH v3 3/5] hwmon: Support sensors exported via ARM SCP interface Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:00 ` [Patch v3 4/5] hwmon: Support registration of thermal zones for SCP temperature sensors Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:19   ` Punit Agrawal
2015-09-14 13:19     ` Punit Agrawal
2015-09-14 13:19     ` Punit Agrawal
2015-09-14 13:00 ` [PATCH v3 4/5] hwmon: Support thermal zones registration " Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:00 ` [PATCH v3 5/5] arm64: dts: Add sensor node to Juno dt Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal
2015-09-14 13:00   ` Punit Agrawal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.