[RFC 0/2] Add support for OCC inband sensors in P9

[RFC 0/2] Add support for OCC inband sensors in P9

[Shilpasri G Bhat]

On-Chip-Controller(OCC) is an embedded chip in POWER processors which
maintains the thermal and power safety of the chip. OCC measures
power and temperature sensors and various performance counters to
maintain healthy activity of the chip. In POWER9, OCC copies these
measured sensors periodically to main memory which can be consumend by
the host.

This patch-set provides support to export power and temperature
sensors in OCC inband sensors as standard hwmon sensors.

The skiboot patch for to add device-tree properties for this feature
is posted here:
https://lists.ozlabs.org/pipermail/skiboot/2017-March/006652.html

Shilpasri G Bhat (2):
  powerpc/powernv: Enable support for OCC inband platform sensors
  hwmon: powernv: Hwmon driver for OCC inband power and temperature
    sensors

 .../devicetree/bindings/hwmon/ibmpowernv-occ.txt   |   4 +
 Documentation/hwmon/ibmpowernv-occ                 |  24 ++
 arch/powerpc/include/asm/opal-api.h                |  36 +++
 arch/powerpc/platforms/powernv/Makefile            |   2 +-
 arch/powerpc/platforms/powernv/opal-occ-sensors.c  | 302 +++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c              |   3 +
 drivers/hwmon/Kconfig                              |  11 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/ibmpowernv-occ.c                     | 302 +++++++++++++++++++++
 9 files changed, 684 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
 create mode 100644 Documentation/hwmon/ibmpowernv-occ
 create mode 100644 arch/powerpc/platforms/powernv/opal-occ-sensors.c
 create mode 100644 drivers/hwmon/ibmpowernv-occ.c

-- 
1.8.3.1

[RFC 1/2] powerpc/powernv: Enable support for OCC inband platform sensors

[Shilpasri G Bhat]

In P9, OCC writes the platform sensors to main memory. This patch
provides support to read the sensors from main memory.

OCC writes three buffers which includes one names buffer for sensor
meta data and two buffers for sensor readings. The sensor names
buffers is written once and contains information like name, units,
frequency, sensor type, location, offset relative to reading buffer
and scale factor for each sensor. There are two buffers to store
sensor readings. While OCC writes to one buffer the sensor values can
be read from the other buffer. The sensor records stored in sensor
buffer can be of two types like counter sensor and full sensor.
Counter sensor record contains just the counter value. Full sensor
contains sample value, min, max, accumulator. It also maintains
min/max for three different clients which can be reset at runtime.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal-api.h               |  36 +++
 arch/powerpc/platforms/powernv/Makefile           |   2 +-
 arch/powerpc/platforms/powernv/opal-occ-sensors.c | 302 ++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c             |   3 +
 4 files changed, 342 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/opal-occ-sensors.c

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index a0aa285..0ed9f79a 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -928,6 +928,42 @@ enum {
 	OPAL_PCI_TCE_KILL_ALL,
 };
 
+/* PowerNV OPAL-OCC Inband sensor definitions */
+enum occ_sensor_type {
+	OCC_SENSOR_TYPE_INVALID		= 0x0000,
+	OCC_SENSOR_TYPE_GENERIC		= 0x0001,
+	OCC_SENSOR_TYPE_CURRENT		= 0x0002,
+	OCC_SENSOR_TYPE_VOLTAGE		= 0x0004,
+	OCC_SENSOR_TYPE_TEMPERATURE	= 0x0008,
+	OCC_SENSOR_TYPE_UTILIZATION	= 0x0010,
+	OCC_SENSOR_TYPE_TIME		= 0x0020,
+	OCC_SENSOR_TYPE_FREQUENCY	= 0x0040,
+	OCC_SENSOR_TYPE_POWER		= 0x0080,
+	OCC_SENSOR_TYPE_PERFORMANCE	= 0x0200,
+};
+
+#define MAX_OCC_SENSOR_NAME_LEN         16
+#define MAX_OCC_SENSOR_UNITS_LEN        4
+
+struct occ_hwmon_sensor {
+	int type;
+	int occ_id;
+	u64 offset;
+	char name[MAX_OCC_SENSOR_NAME_LEN * 2];
+};
+
+int opal_occ_sensors_init(void);
+struct occ_hwmon_sensor *opal_occ_sensor_get_hwmon_list(int *nr_sensors);
+int opal_occ_sensor_get_sample(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_min(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_max(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_csm_min(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_csm_max(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_js_min(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_js_max(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_prof_min(int occ_id, u64 offset, u64 *val);
+int opal_occ_sensor_get_prof_max(int occ_id, u64 offset, u64 *val);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __OPAL_API_H */
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index b5d98cb..940849d 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -2,7 +2,7 @@ obj-y			+= setup.o opal-wrappers.o opal.o opal-async.o idle.o
 obj-y			+= opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
 obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
 obj-y			+= opal-msglog.o opal-hmi.o opal-power.o opal-irqchip.o
-obj-y			+= opal-kmsg.o
+obj-y			+= opal-kmsg.o opal-occ-sensors.o
 
 obj-$(CONFIG_SMP)	+= smp.o subcore.o subcore-asm.o
 obj-$(CONFIG_PCI)	+= pci.o pci-ioda.o npu-dma.o
diff --git a/arch/powerpc/platforms/powernv/opal-occ-sensors.c b/arch/powerpc/platforms/powernv/opal-occ-sensors.c
new file mode 100644
index 0000000..b789925
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-occ-sensors.c
@@ -0,0 +1,302 @@
+/*
+ * Copyright IBM Corporation 2017
+ *
+ * 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 in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) "opal-occ-sensors: " fmt
+
+#include <linux/of_platform.h>
+#include <linux/miscdevice.h>
+#include <linux/highmem.h>
+#include <linux/slab.h>
+
+#include <asm/opal.h>
+
+enum sensor_structure_type {
+	OCC_SENSOR_STRUCTURE_TYPE_FULL		= 0x01,
+	OCC_SENSOR_STRUCTURE_TYPE_COUNTER	= 0x02,
+};
+
+enum occ_sensor_location {
+	OCC_SENSOR_LOC_SYSTEM		= 0x0001,
+	OCC_SENSOR_LOC_PROCESSOR	= 0x0002,
+	OCC_SENSOR_LOC_PARTITION	= 0x0004,
+	OCC_SENSOR_LOC_MEMORY		= 0x0008,
+	OCC_SENSOR_LOC_VRM		= 0x0010,
+	OCC_SENSOR_LOC_OCC		= 0x0020,
+	OCC_SENSOR_LOC_CORE		= 0x0040,
+	OCC_SENSOR_LOC_QUAD		= 0x0080,
+	OCC_SENSOR_LOC_GPU		= 0x0100,
+};
+
+struct occ_sensor_name {
+	char name[MAX_OCC_SENSOR_NAME_LEN];
+	char units[MAX_OCC_SENSOR_UNITS_LEN];
+	u16 gsid;
+	u32 freq;
+	u32 scale_factor;
+	u16 type;
+	u16 location;
+	u8 structure_type;
+	u32 reading_offset;
+	u8 sensor_specific_info;
+	u8 pad[];
+} __packed;
+
+struct occ_sensor_record {
+	u16 gsid;
+	u64 timestamp;
+	u16 sample;
+	u16 min;
+	u16 max;
+	u16 csm_min;
+	u16 csm_max;
+	u16 prof_min;
+	u16 prof_max;
+	u16 js_min;
+	u16 js_max;
+	u64 accumulator;
+	u32 update_tag;
+	u8 pad[];
+} __packed;
+
+struct occ_sensor_counter {
+	u16 gsid;
+	u64 timestamp;
+	u64 accumulator;
+	u8 sample;
+	u8 pad[];
+} __packed;
+
+static struct occ_data {
+	int id;
+	int nr_sensors;
+	u64 pbase;
+	void *base;
+	int names_offset;
+	int ping_offset;
+	int pong_offset;
+} *occs;
+
+static int nr_occs;
+static int name_len;
+
+static int opal_occ_sensor_get_count(enum occ_sensor_type type)
+{
+	struct occ_sensor_name *sensor;
+	int count = 0;
+	int i, j;
+
+	for (i = 0; i < nr_occs; i++)
+		for (j = 0; j < occs[i].nr_sensors; j++) {
+			sensor = (struct occ_sensor_name *)(occs[i].base +
+				 occs[i].names_offset + j * name_len);
+			if (type == be16_to_cpu(sensor->type))
+				count++;
+		}
+
+	return count;
+}
+
+struct occ_hwmon_sensor *opal_occ_sensor_get_hwmon_list(int *nr_sensors)
+{
+	struct occ_sensor_name *sensor;
+	struct occ_hwmon_sensor *slist;
+	int i, j, count = 0;
+
+	*nr_sensors = opal_occ_sensor_get_count(OCC_SENSOR_TYPE_POWER);
+	*nr_sensors += opal_occ_sensor_get_count(OCC_SENSOR_TYPE_TEMPERATURE);
+	slist = kcalloc(*nr_sensors, sizeof(*slist), GFP_KERNEL);
+	if (!slist)
+		return NULL;
+
+	for (i = 0; i < nr_occs; i++)
+		for (j = 0; j < occs[i].nr_sensors; j++) {
+			enum occ_sensor_type type;
+			enum occ_sensor_location loc;
+
+			sensor = (struct occ_sensor_name *)(occs[i].base +
+				 occs[i].names_offset + j * name_len);
+			type = be16_to_cpu(sensor->type);
+			loc = be16_to_cpu(sensor->location);
+
+			if (type != OCC_SENSOR_TYPE_POWER &&
+			    type != OCC_SENSOR_TYPE_TEMPERATURE)
+				continue;
+
+			if (loc == OCC_SENSOR_LOC_SYSTEM)
+				strncpy(slist[count].name, sensor->name,
+					strlen(sensor->name));
+			else
+				snprintf(slist[count].name,
+					 sizeof(slist[count].name), "P%d_%s",
+					 occs[i].id, sensor->name);
+
+			slist[count].type = type;
+			slist[count].occ_id = occs[i].id;
+			slist[count].offset =
+					be32_to_cpu(sensor->reading_offset);
+			count++;
+		}
+
+	return slist;
+}
+EXPORT_SYMBOL_GPL(opal_occ_sensor_get_hwmon_list);
+
+static inline struct occ_data *get_occ(int occ_id)
+{
+	int i;
+
+	for (i = 0; i < nr_occs; i++)
+		if (occ_id == occs[i].id)
+			return &occs[i];
+
+	return NULL;
+}
+
+static struct occ_sensor_record *opal_occ_sensor_read_rec(int occ_id,
+							  u64 offset)
+{
+	struct occ_sensor_record *sping, *spong;
+	struct occ_data *occ;
+	u8 *ping, *pong;
+
+	occ = get_occ(occ_id);
+	if (!occ)
+		return NULL;
+
+	ping = (u8 *)(occ->base + occ->ping_offset);
+	pong = (u8 *)(occ->base + occ->pong_offset);
+	sping = (struct occ_sensor_record *)((u64)ping + offset);
+	spong = (struct occ_sensor_record *)((u64)pong + offset);
+
+	if (*ping && *pong) {
+		if (be64_to_cpu(sping->timestamp) >
+		    be64_to_cpu(spong->timestamp))
+			return sping;
+		else
+			return spong;
+	} else if (*ping && !*pong) {
+		return sping;
+	} else if (*pong && !*pong) {
+		return spong;
+	} else if (!*ping && !*pong) {
+		return NULL;
+	}
+
+	return NULL;
+}
+
+#define get(name)							\
+int opal_occ_sensor_get_##name(int occ_id, u64 offset, u64 *val)	\
+{									\
+	struct occ_sensor_record *sensor;				\
+	sensor = opal_occ_sensor_read_rec(occ_id, offset);		\
+	if (!sensor)							\
+		return -EIO;						\
+	*val = be16_to_cpu(sensor->name);				\
+	return 0;							\
+}									\
+EXPORT_SYMBOL_GPL(opal_occ_sensor_get_##name)
+
+get(sample);
+get(min);
+get(max);
+get(csm_min);
+get(csm_max);
+get(js_min);
+get(js_max);
+get(prof_min);
+get(prof_max);
+
+int __init opal_occ_sensors_init(void)
+{
+	struct platform_device *pdev;
+	struct device_node *sensor, *node;
+	int i, ret = -ENODEV;
+
+	sensor = of_find_compatible_node(NULL, NULL,
+					 "ibm,p9-occ-inband-sensor");
+	if (!sensor) {
+		pr_info("OCC inband sensors node not found\n");
+		return ret;
+	}
+
+	for_each_child_of_node(sensor, node)
+		if (strcmp(node->name, "occ") == 0)
+			nr_occs++;
+
+	if (of_property_read_u32(sensor, "sensor-names-size", &name_len)) {
+		pr_info("Missing sensor-names-size DT property\n");
+		return ret;
+	}
+
+	occs = kcalloc(nr_occs, sizeof(*occs), GFP_KERNEL);
+	if (!occs)
+		return -ENOMEM;
+
+	i = 0;
+	for_each_child_of_node(sensor, node) {
+		const __be32 *reg;
+		int reg_len = 0;
+
+		if (strcmp(node->name, "occ") != 0)
+			continue;
+
+		if (of_property_read_u32(node, "ibm,occ-id", &occs[i].id)) {
+			pr_info("Missing ibm,occ-id DT property\n");
+			goto out;
+		}
+
+		if (of_property_read_u32(node, "nr-sensors",
+					 &occs[i].nr_sensors)) {
+			pr_info("Missing nr_sensors DT property\n");
+			goto out;
+		}
+
+		if (of_property_read_u32(node, "ping-offset",
+					 &occs[i].ping_offset)) {
+			pr_info("Missing ping_offset DT property\n");
+			goto out;
+		}
+
+		if (of_property_read_u32(node, "pong-offset",
+					 &occs[i].pong_offset)) {
+			pr_info("Missing pong_offset DT property\n");
+			goto out;
+		}
+
+		if (of_property_read_u32(node, "names-offset",
+					 &occs[i].names_offset)) {
+			pr_info("Missing names_offset DT property\n");
+			goto out;
+		}
+
+		reg = of_get_property(node, "reg", &reg_len);
+		if (!reg_len) {
+			pr_info("Missing reg DT property\n");
+			goto out;
+		}
+
+		occs[i].pbase = be32_to_cpu(reg[0]);
+		occs[i].pbase = be32_to_cpu(reg[1]) | (occs[i].pbase << 32);
+		occs[i].base = phys_to_virt(occs[i].pbase);
+		i++;
+	}
+
+	pdev = of_platform_device_create(sensor, "occ-inband-sensor", NULL);
+	ret = PTR_ERR_OR_ZERO(pdev);
+	if (!ret)
+		return 0;
+out:
+	kfree(occs);
+	return ret;
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 86d9fde..750d181 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -704,6 +704,9 @@ static int __init opal_init(void)
 	/* Initialise OPAL sensor interface */
 	opal_sensor_init();
 
+	/* Initialize OPAL OCC Inband sensor interface */
+	opal_occ_sensors_init();
+
 	/* Initialise OPAL hypervisor maintainence interrupt handling */
 	opal_hmi_handler_init();
 
-- 
1.8.3.1

[RFC 2/2] hwmon: powernv: Hwmon driver for OCC inband power and temperature sensors

[Shilpasri G Bhat]

Add support to read power and temperature sensors from OCC inband
sensors which are copied to main memory by OCC.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Jean Delvare <jdelvare@suse.com>
CC: Guenter Roeck <linux@roeck-us.net>
CC: Jonathan Corbet <corbet@lwn.net>
CC: devicetree@vger.kernel.org
CC: linux-hwmon@vger.kernel.org
CC: linux-doc@vger.kernel.org
---
 .../devicetree/bindings/hwmon/ibmpowernv-occ.txt   |   4 +
 Documentation/hwmon/ibmpowernv-occ                 |  24 ++
 drivers/hwmon/Kconfig                              |  11 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/ibmpowernv-occ.c                     | 302 +++++++++++++++++++++
 5 files changed, 342 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
 create mode 100644 Documentation/hwmon/ibmpowernv-occ
 create mode 100644 drivers/hwmon/ibmpowernv-occ.c

diff --git a/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
new file mode 100644
index 0000000..d03f744
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
@@ -0,0 +1,4 @@
+IBM POWERNV OCC inband platform sensors
+
+Required device-tree property:
+- compatible: "ibm,p9-occ-inband-sensor"
diff --git a/Documentation/hwmon/ibmpowernv-occ b/Documentation/hwmon/ibmpowernv-occ
new file mode 100644
index 0000000..151028b
--- /dev/null
+++ b/Documentation/hwmon/ibmpowernv-occ
@@ -0,0 +1,24 @@
+Kernel driver ibmpowernv-occ
+=============================
+
+Supported systems:
+ * P9 server based on POWERNV platform
+
+Description
+------------
+
+This driver exports the power and temperature sensors from OCC inband
+sensors on P9 POWERNV platforms.
+
+Sysfs attributes
+----------------
+
+powerX_input		Latest power reading
+powerX_input_highest	Minimum power
+powerX_input_lowest	Maximum power
+powerX_label		Sensor name
+
+tempX_input		Latest temperature reading
+tempX_max		Minimum temperature
+tempX_min		Maximum temperature
+tempX_label		Sensor name
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0649d53f3..3b1dbb9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -598,6 +598,17 @@ config SENSORS_IBMPOWERNV
 	  This driver can also be built as a module. If so, the module
 	  will be called ibmpowernv.
 
+config SENSORS_IBMPOWERNV_OCC
+	tristate "IBM POWERNV OCC Inband platform sensors"
+	depends on PPC_POWERNV
+	default y
+	help
+	  If you say yes here you get support for the temperature/power
+	  OCC inband sensors on your PowerNV platform.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called ibmpowernv-occ.
+
 config SENSORS_IIO_HWMON
 	tristate "Hwmon driver that uses channels specified via iio maps"
 	depends on IIO
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5509edf..0da2207 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
+obj-$(CONFIG_SENSORS_IBMPOWERNV_OCC)+= ibmpowernv-occ.o
 obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
 obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
 obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
diff --git a/drivers/hwmon/ibmpowernv-occ.c b/drivers/hwmon/ibmpowernv-occ.c
new file mode 100644
index 0000000..97b1bbe
--- /dev/null
+++ b/drivers/hwmon/ibmpowernv-occ.c
@@ -0,0 +1,302 @@
+/*
+ * IBM PowerNV platform OCC inband sensors for temperature/power
+ * Copyright (C) 2017 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
+ */
+
+#define DRVNAME		"ibmpowernv_occ"
+#define pr_fmt(fmt)	DRVNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/hwmon.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+
+#include <asm/opal.h>
+
+#define MAX_HWMON_ATTR_LEN	32
+#define MAX_HWMON_LABEL_LEN	(MAX_OCC_SENSOR_NAME_LEN * 2)
+#define HWMON_ATTRS_PER_SENSOR	16
+#define TO_MILLI_UNITS(x)	((x) * 1000)
+#define TO_MICRO_UNITS(x)	((x) * 1000000)
+
+enum sensors {
+	TEMP,
+	POWER,
+	MAX_SENSOR_TYPE,
+};
+
+static struct sensor_type {
+	const char *name;
+	int hwmon_id;
+} sensor_types[] = {
+	{ "temp"},
+	{ "power"},
+};
+
+static struct sensor_data {
+	u32 occ_id;
+	u64 offset;  /* Offset to ping/pong reading buffer */
+	enum sensors type;
+	char label[MAX_HWMON_LABEL_LEN];
+	char name[MAX_HWMON_ATTR_LEN];
+	struct device_attribute attr;
+} *sdata;
+
+static struct attribute_group sensor_attrs_group;
+__ATTRIBUTE_GROUPS(sensor_attrs);
+
+#define show(file_name)							\
+static ssize_t ibmpowernv_occ_show_##file_name				\
+(struct device *dev, struct device_attribute *dattr, char *buf)		\
+{									\
+	struct sensor_data *sdata = container_of(dattr,			\
+						 struct sensor_data,	\
+						 attr);			\
+	u64 val;							\
+	int ret;							\
+	ret = opal_occ_sensor_get_##file_name(sdata->occ_id,		\
+					      sdata->offset,		\
+					      &val);			\
+	if (ret)							\
+		return ret;						\
+	if (sdata->type == TEMP)					\
+		val = TO_MILLI_UNITS(val);				\
+	else if (sdata->type == POWER)					\
+		val = TO_MICRO_UNITS(val);				\
+	return sprintf(buf, "%llu\n", val);				\
+}
+
+show(sample);
+show(max);
+show(min);
+show(js_min);
+show(js_max);
+show(csm_min);
+show(csm_max);
+show(prof_min);
+show(prof_max);
+
+static struct sensor_view_groups {
+	const char *name;
+	ssize_t (*show_sample)(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buf);
+	ssize_t (*show_min)(struct device *dev,
+			    struct device_attribute *attr,
+			    char *buf);
+	ssize_t (*show_max)(struct device *dev,
+			    struct device_attribute *attr,
+			    char *buf);
+} sensor_views[] = {
+	{
+		.name		= "",
+		.show_sample	= ibmpowernv_occ_show_sample,
+		.show_min	= ibmpowernv_occ_show_min,
+		.show_max	= ibmpowernv_occ_show_max
+	},
+	{
+		.name		= "_JS",
+		.show_sample	= ibmpowernv_occ_show_sample,
+		.show_min	= ibmpowernv_occ_show_js_min,
+		.show_max	= ibmpowernv_occ_show_js_max
+	},
+	{	.name		= "_CSM",
+		.show_sample	= ibmpowernv_occ_show_sample,
+		.show_min	= ibmpowernv_occ_show_csm_min,
+		.show_max	= ibmpowernv_occ_show_csm_max
+	},
+	{	.name		= "_Prof",
+		.show_sample	= ibmpowernv_occ_show_sample,
+		.show_min	= ibmpowernv_occ_show_prof_min,
+		.show_max	= ibmpowernv_occ_show_prof_max
+	},
+};
+
+static ssize_t ibmpowernv_occ_show_label(struct device *dev,
+					 struct device_attribute *dattr,
+					 char *buf)
+{
+	struct sensor_data *sdata = container_of(dattr, struct sensor_data,
+						 attr);
+
+	return sprintf(buf, "%s\n", sdata->label);
+}
+
+static int ibmpowernv_occ_get_sensor_type(enum occ_sensor_type type)
+{
+	switch (type) {
+	case OCC_SENSOR_TYPE_POWER:
+		return POWER;
+	case OCC_SENSOR_TYPE_TEMPERATURE:
+		return TEMP;
+	default:
+		return MAX_SENSOR_TYPE;
+	}
+
+	return MAX_SENSOR_TYPE;
+}
+
+static void ibmpowernv_occ_add_sdata(struct occ_hwmon_sensor sensor,
+				     struct sensor_data *sdata, char *name,
+				     int hwmon_id, enum sensors type,
+				     ssize_t (*show)(struct device *dev,
+						struct device_attribute *attr,
+						char *buf))
+{
+	sdata->type = type;
+	sdata->occ_id = sensor.occ_id;
+	sdata->offset = sensor.offset;
+	snprintf(sdata->name, MAX_HWMON_ATTR_LEN, "%s%d_%s",
+		 sensor_types[type].name, hwmon_id, name);
+	sysfs_attr_init(&sdata->attr.attr);
+	sdata->attr.attr.name = sdata->name;
+	sdata->attr.attr.mode = 0444;
+	sdata->attr.show = show;
+}
+
+static void ibmpowernv_occ_add_sensor_attrs(struct occ_hwmon_sensor sensor,
+					    int index)
+{
+	struct attribute **attrs = sensor_attrs_group.attrs;
+	char attr_str[MAX_HWMON_ATTR_LEN];
+	enum sensors type = ibmpowernv_occ_get_sensor_type(sensor.type);
+	int i;
+
+	index *= HWMON_ATTRS_PER_SENSOR;
+	for (i = 0; i < ARRAY_SIZE(sensor_views); i++) {
+		int hid = ++sensor_types[type].hwmon_id;
+
+		/* input */
+		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "input", hid,
+					 type, sensor_views[i].show_sample);
+		attrs[index] = &sdata[index].attr.attr;
+		index++;
+
+		/* min */
+		if (type == POWER)
+			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
+				 "input_lowest");
+		else
+			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "min");
+
+		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
+					 type, sensor_views[i].show_min);
+		attrs[index] = &sdata[index].attr.attr;
+		index++;
+
+		/* max */
+		if (type == POWER)
+			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
+				 "input_highest");
+		else
+			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "max");
+
+		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
+					 type, sensor_views[i].show_max);
+		attrs[index] = &sdata[index].attr.attr;
+		index++;
+
+		/* label */
+		snprintf(sdata[index].label, MAX_HWMON_LABEL_LEN, "%s%s",
+			 sensor.name, sensor_views[i].name);
+		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "label", hid,
+					 type, ibmpowernv_occ_show_label);
+		attrs[index] = &sdata[index].attr.attr;
+		index++;
+	}
+}
+
+static int ibmpowernv_occ_add_device_attrs(struct platform_device *pdev)
+{
+	struct attribute **attrs;
+	struct occ_hwmon_sensor *slist = NULL;
+	int nr_sensors = 0, i;
+	int ret = -ENOMEM;
+
+	slist = opal_occ_sensor_get_hwmon_list(&nr_sensors);
+	if (!nr_sensors)
+		return -ENODEV;
+
+	if (!slist)
+		return ret;
+
+	sdata = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*sdata) *
+			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
+	if (!sdata)
+		goto out;
+
+	attrs = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*attrs) *
+			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
+	if (!attrs)
+		goto out;
+
+	sensor_attrs_group.attrs = attrs;
+	for (i = 0; i < nr_sensors; i++)
+		ibmpowernv_occ_add_sensor_attrs(slist[i], i);
+
+	ret = 0;
+out:
+	kfree(slist);
+	return ret;
+}
+
+static int ibmpowernv_occ_probe(struct platform_device *pdev)
+{
+	struct device *hwmon_dev;
+	int err;
+
+	err = ibmpowernv_occ_add_device_attrs(pdev);
+	if (err)
+		goto out;
+
+	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev, DRVNAME,
+							   NULL,
+							   sensor_attrs_groups);
+
+	err = PTR_ERR_OR_ZERO(hwmon_dev);
+out:
+	if (err)
+		pr_warn("Failed to initialize Hwmon OCC inband sensors\n");
+
+	return err;
+}
+
+static const struct platform_device_id occ_sensor_ids[] = {
+	{ .name = "occ-inband-sensor" },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, occ_sensor_ids);
+
+static const struct of_device_id occ_sensor_of_ids[] = {
+	{ .compatible	= "ibm,p9-occ-inband-sensor" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, occ_sensor_of_ids);
+
+static struct platform_driver ibmpowernv_occ_driver = {
+	.probe		= ibmpowernv_occ_probe,
+	.id_table	= occ_sensor_ids,
+	.driver		= {
+		.name	= DRVNAME,
+		.of_match_table	= occ_sensor_of_ids,
+	},
+};
+
+module_platform_driver(ibmpowernv_occ_driver);
+
+MODULE_AUTHOR("Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>");
+MODULE_DESCRIPTION("IBM POWERNV platform OCC inband sensors");
+MODULE_LICENSE("GPL");
-- 
1.8.3.1

Re: [RFC 2/2] hwmon: powernv: Hwmon driver for OCC inband power and temperature sensors

[Guenter Roeck]

On Thu, Mar 09, 2017 at 05:19:15PM +0530, Shilpasri G Bhat wrote:
> Add support to read power and temperature sensors from OCC inband
> sensors which are copied to main memory by OCC.
>

Is this supposed to be an alternative to the submission from 
Eddie James ? If so, is there a reason to consider such an
alternative ?

Thanks,
Guenter

> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Jean Delvare <jdelvare@suse.com>
> CC: Guenter Roeck <linux@roeck-us.net>
> CC: Jonathan Corbet <corbet@lwn.net>
> CC: devicetree@vger.kernel.org
> CC: linux-hwmon@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> ---
>  .../devicetree/bindings/hwmon/ibmpowernv-occ.txt   |   4 +
>  Documentation/hwmon/ibmpowernv-occ                 |  24 ++
>  drivers/hwmon/Kconfig                              |  11 +
>  drivers/hwmon/Makefile                             |   1 +
>  drivers/hwmon/ibmpowernv-occ.c                     | 302 +++++++++++++++++++++
>  5 files changed, 342 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>  create mode 100644 Documentation/hwmon/ibmpowernv-occ
>  create mode 100644 drivers/hwmon/ibmpowernv-occ.c
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
> new file mode 100644
> index 0000000..d03f744
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
> @@ -0,0 +1,4 @@
> +IBM POWERNV OCC inband platform sensors
> +
> +Required device-tree property:
> +- compatible: "ibm,p9-occ-inband-sensor"
> diff --git a/Documentation/hwmon/ibmpowernv-occ b/Documentation/hwmon/ibmpowernv-occ
> new file mode 100644
> index 0000000..151028b
> --- /dev/null
> +++ b/Documentation/hwmon/ibmpowernv-occ
> @@ -0,0 +1,24 @@
> +Kernel driver ibmpowernv-occ
> +=============================
> +
> +Supported systems:
> + * P9 server based on POWERNV platform
> +
> +Description
> +------------
> +
> +This driver exports the power and temperature sensors from OCC inband
> +sensors on P9 POWERNV platforms.
> +
> +Sysfs attributes
> +----------------
> +
> +powerX_input		Latest power reading
> +powerX_input_highest	Minimum power
> +powerX_input_lowest	Maximum power
> +powerX_label		Sensor name
> +
> +tempX_input		Latest temperature reading
> +tempX_max		Minimum temperature
> +tempX_min		Maximum temperature
> +tempX_label		Sensor name
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 0649d53f3..3b1dbb9 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -598,6 +598,17 @@ config SENSORS_IBMPOWERNV
>  	  This driver can also be built as a module. If so, the module
>  	  will be called ibmpowernv.
>  
> +config SENSORS_IBMPOWERNV_OCC
> +	tristate "IBM POWERNV OCC Inband platform sensors"
> +	depends on PPC_POWERNV
> +	default y
> +	help
> +	  If you say yes here you get support for the temperature/power
> +	  OCC inband sensors on your PowerNV platform.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called ibmpowernv-occ.
> +
>  config SENSORS_IIO_HWMON
>  	tristate "Hwmon driver that uses channels specified via iio maps"
>  	depends on IIO
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 5509edf..0da2207 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -75,6 +75,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
>  obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
>  obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
> +obj-$(CONFIG_SENSORS_IBMPOWERNV_OCC)+= ibmpowernv-occ.o
>  obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
>  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
>  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
> diff --git a/drivers/hwmon/ibmpowernv-occ.c b/drivers/hwmon/ibmpowernv-occ.c
> new file mode 100644
> index 0000000..97b1bbe
> --- /dev/null
> +++ b/drivers/hwmon/ibmpowernv-occ.c
> @@ -0,0 +1,302 @@
> +/*
> + * IBM PowerNV platform OCC inband sensors for temperature/power
> + * Copyright (C) 2017 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.
> + */
> +
> +#define DRVNAME		"ibmpowernv_occ"
> +#define pr_fmt(fmt)	DRVNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/hwmon.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/opal.h>
> +
> +#define MAX_HWMON_ATTR_LEN	32
> +#define MAX_HWMON_LABEL_LEN	(MAX_OCC_SENSOR_NAME_LEN * 2)
> +#define HWMON_ATTRS_PER_SENSOR	16
> +#define TO_MILLI_UNITS(x)	((x) * 1000)
> +#define TO_MICRO_UNITS(x)	((x) * 1000000)
> +
> +enum sensors {
> +	TEMP,
> +	POWER,
> +	MAX_SENSOR_TYPE,
> +};
> +
> +static struct sensor_type {
> +	const char *name;
> +	int hwmon_id;
> +} sensor_types[] = {
> +	{ "temp"},
> +	{ "power"},
> +};
> +
> +static struct sensor_data {
> +	u32 occ_id;
> +	u64 offset;  /* Offset to ping/pong reading buffer */
> +	enum sensors type;
> +	char label[MAX_HWMON_LABEL_LEN];
> +	char name[MAX_HWMON_ATTR_LEN];
> +	struct device_attribute attr;
> +} *sdata;
> +
> +static struct attribute_group sensor_attrs_group;
> +__ATTRIBUTE_GROUPS(sensor_attrs);
> +
> +#define show(file_name)							\
> +static ssize_t ibmpowernv_occ_show_##file_name				\
> +(struct device *dev, struct device_attribute *dattr, char *buf)		\
> +{									\
> +	struct sensor_data *sdata = container_of(dattr,			\
> +						 struct sensor_data,	\
> +						 attr);			\
> +	u64 val;							\
> +	int ret;							\
> +	ret = opal_occ_sensor_get_##file_name(sdata->occ_id,		\
> +					      sdata->offset,		\
> +					      &val);			\
> +	if (ret)							\
> +		return ret;						\
> +	if (sdata->type == TEMP)					\
> +		val = TO_MILLI_UNITS(val);				\
> +	else if (sdata->type == POWER)					\
> +		val = TO_MICRO_UNITS(val);				\
> +	return sprintf(buf, "%llu\n", val);				\
> +}
> +
> +show(sample);
> +show(max);
> +show(min);
> +show(js_min);
> +show(js_max);
> +show(csm_min);
> +show(csm_max);
> +show(prof_min);
> +show(prof_max);
> +
> +static struct sensor_view_groups {
> +	const char *name;
> +	ssize_t (*show_sample)(struct device *dev,
> +			       struct device_attribute *attr,
> +			       char *buf);
> +	ssize_t (*show_min)(struct device *dev,
> +			    struct device_attribute *attr,
> +			    char *buf);
> +	ssize_t (*show_max)(struct device *dev,
> +			    struct device_attribute *attr,
> +			    char *buf);
> +} sensor_views[] = {
> +	{
> +		.name		= "",
> +		.show_sample	= ibmpowernv_occ_show_sample,
> +		.show_min	= ibmpowernv_occ_show_min,
> +		.show_max	= ibmpowernv_occ_show_max
> +	},
> +	{
> +		.name		= "_JS",
> +		.show_sample	= ibmpowernv_occ_show_sample,
> +		.show_min	= ibmpowernv_occ_show_js_min,
> +		.show_max	= ibmpowernv_occ_show_js_max
> +	},
> +	{	.name		= "_CSM",
> +		.show_sample	= ibmpowernv_occ_show_sample,
> +		.show_min	= ibmpowernv_occ_show_csm_min,
> +		.show_max	= ibmpowernv_occ_show_csm_max
> +	},
> +	{	.name		= "_Prof",
> +		.show_sample	= ibmpowernv_occ_show_sample,
> +		.show_min	= ibmpowernv_occ_show_prof_min,
> +		.show_max	= ibmpowernv_occ_show_prof_max
> +	},
> +};
> +
> +static ssize_t ibmpowernv_occ_show_label(struct device *dev,
> +					 struct device_attribute *dattr,
> +					 char *buf)
> +{
> +	struct sensor_data *sdata = container_of(dattr, struct sensor_data,
> +						 attr);
> +
> +	return sprintf(buf, "%s\n", sdata->label);
> +}
> +
> +static int ibmpowernv_occ_get_sensor_type(enum occ_sensor_type type)
> +{
> +	switch (type) {
> +	case OCC_SENSOR_TYPE_POWER:
> +		return POWER;
> +	case OCC_SENSOR_TYPE_TEMPERATURE:
> +		return TEMP;
> +	default:
> +		return MAX_SENSOR_TYPE;
> +	}
> +
> +	return MAX_SENSOR_TYPE;
> +}
> +
> +static void ibmpowernv_occ_add_sdata(struct occ_hwmon_sensor sensor,
> +				     struct sensor_data *sdata, char *name,
> +				     int hwmon_id, enum sensors type,
> +				     ssize_t (*show)(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf))
> +{
> +	sdata->type = type;
> +	sdata->occ_id = sensor.occ_id;
> +	sdata->offset = sensor.offset;
> +	snprintf(sdata->name, MAX_HWMON_ATTR_LEN, "%s%d_%s",
> +		 sensor_types[type].name, hwmon_id, name);
> +	sysfs_attr_init(&sdata->attr.attr);
> +	sdata->attr.attr.name = sdata->name;
> +	sdata->attr.attr.mode = 0444;
> +	sdata->attr.show = show;
> +}
> +
> +static void ibmpowernv_occ_add_sensor_attrs(struct occ_hwmon_sensor sensor,
> +					    int index)
> +{
> +	struct attribute **attrs = sensor_attrs_group.attrs;
> +	char attr_str[MAX_HWMON_ATTR_LEN];
> +	enum sensors type = ibmpowernv_occ_get_sensor_type(sensor.type);
> +	int i;
> +
> +	index *= HWMON_ATTRS_PER_SENSOR;
> +	for (i = 0; i < ARRAY_SIZE(sensor_views); i++) {
> +		int hid = ++sensor_types[type].hwmon_id;
> +
> +		/* input */
> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "input", hid,
> +					 type, sensor_views[i].show_sample);
> +		attrs[index] = &sdata[index].attr.attr;
> +		index++;
> +
> +		/* min */
> +		if (type == POWER)
> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
> +				 "input_lowest");
> +		else
> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "min");
> +
> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
> +					 type, sensor_views[i].show_min);
> +		attrs[index] = &sdata[index].attr.attr;
> +		index++;
> +
> +		/* max */
> +		if (type == POWER)
> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
> +				 "input_highest");
> +		else
> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "max");
> +
> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
> +					 type, sensor_views[i].show_max);
> +		attrs[index] = &sdata[index].attr.attr;
> +		index++;
> +
> +		/* label */
> +		snprintf(sdata[index].label, MAX_HWMON_LABEL_LEN, "%s%s",
> +			 sensor.name, sensor_views[i].name);
> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "label", hid,
> +					 type, ibmpowernv_occ_show_label);
> +		attrs[index] = &sdata[index].attr.attr;
> +		index++;
> +	}
> +}
> +
> +static int ibmpowernv_occ_add_device_attrs(struct platform_device *pdev)
> +{
> +	struct attribute **attrs;
> +	struct occ_hwmon_sensor *slist = NULL;
> +	int nr_sensors = 0, i;
> +	int ret = -ENOMEM;
> +
> +	slist = opal_occ_sensor_get_hwmon_list(&nr_sensors);
> +	if (!nr_sensors)
> +		return -ENODEV;
> +
> +	if (!slist)
> +		return ret;
> +
> +	sdata = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*sdata) *
> +			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
> +	if (!sdata)
> +		goto out;
> +
> +	attrs = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*attrs) *
> +			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
> +	if (!attrs)
> +		goto out;
> +
> +	sensor_attrs_group.attrs = attrs;
> +	for (i = 0; i < nr_sensors; i++)
> +		ibmpowernv_occ_add_sensor_attrs(slist[i], i);
> +
> +	ret = 0;
> +out:
> +	kfree(slist);
> +	return ret;
> +}
> +
> +static int ibmpowernv_occ_probe(struct platform_device *pdev)
> +{
> +	struct device *hwmon_dev;
> +	int err;
> +
> +	err = ibmpowernv_occ_add_device_attrs(pdev);
> +	if (err)
> +		goto out;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev, DRVNAME,
> +							   NULL,
> +							   sensor_attrs_groups);
> +
> +	err = PTR_ERR_OR_ZERO(hwmon_dev);
> +out:
> +	if (err)
> +		pr_warn("Failed to initialize Hwmon OCC inband sensors\n");
> +
> +	return err;
> +}
> +
> +static const struct platform_device_id occ_sensor_ids[] = {
> +	{ .name = "occ-inband-sensor" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, occ_sensor_ids);
> +
> +static const struct of_device_id occ_sensor_of_ids[] = {
> +	{ .compatible	= "ibm,p9-occ-inband-sensor" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, occ_sensor_of_ids);
> +
> +static struct platform_driver ibmpowernv_occ_driver = {
> +	.probe		= ibmpowernv_occ_probe,
> +	.id_table	= occ_sensor_ids,
> +	.driver		= {
> +		.name	= DRVNAME,
> +		.of_match_table	= occ_sensor_of_ids,
> +	},
> +};
> +
> +module_platform_driver(ibmpowernv_occ_driver);
> +
> +MODULE_AUTHOR("Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>");
> +MODULE_DESCRIPTION("IBM POWERNV platform OCC inband sensors");
> +MODULE_LICENSE("GPL");
> -- 
> 1.8.3.1
> 

Re: [RFC 2/2] hwmon: powernv: Hwmon driver for OCC inband power and temperature sensors

[Shilpasri G Bhat]

Hi Guenter,

On 03/09/2017 05:40 PM, Guenter Roeck wrote:
> On Thu, Mar 09, 2017 at 05:19:15PM +0530, Shilpasri G Bhat wrote:
>> Add support to read power and temperature sensors from OCC inband
>> sensors which are copied to main memory by OCC.
>>
> 
> Is this supposed to be an alternative to the submission from 
> Eddie James ? If so, is there a reason to consider such an
> alternative ?
> 

This is not an alternative submission. Eddie James' hwmon OCC driver is to
export the sensors in BMC which is a service processor for POWER{8,9} servers.
This patch adds a hwmon driver in the POWER9 server.

Thanks and Regards,
Shilpa

> Thanks,
> Guenter
> 
>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Mark Rutland <mark.rutland@arm.com>
>> CC: Jean Delvare <jdelvare@suse.com>
>> CC: Guenter Roeck <linux@roeck-us.net>
>> CC: Jonathan Corbet <corbet@lwn.net>
>> CC: devicetree@vger.kernel.org
>> CC: linux-hwmon@vger.kernel.org
>> CC: linux-doc@vger.kernel.org
>> ---
>>  .../devicetree/bindings/hwmon/ibmpowernv-occ.txt   |   4 +
>>  Documentation/hwmon/ibmpowernv-occ                 |  24 ++
>>  drivers/hwmon/Kconfig                              |  11 +
>>  drivers/hwmon/Makefile                             |   1 +
>>  drivers/hwmon/ibmpowernv-occ.c                     | 302 +++++++++++++++++++++
>>  5 files changed, 342 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>>  create mode 100644 Documentation/hwmon/ibmpowernv-occ
>>  create mode 100644 drivers/hwmon/ibmpowernv-occ.c
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>> new file mode 100644
>> index 0000000..d03f744
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>> @@ -0,0 +1,4 @@
>> +IBM POWERNV OCC inband platform sensors
>> +
>> +Required device-tree property:
>> +- compatible: "ibm,p9-occ-inband-sensor"
>> diff --git a/Documentation/hwmon/ibmpowernv-occ b/Documentation/hwmon/ibmpowernv-occ
>> new file mode 100644
>> index 0000000..151028b
>> --- /dev/null
>> +++ b/Documentation/hwmon/ibmpowernv-occ
>> @@ -0,0 +1,24 @@
>> +Kernel driver ibmpowernv-occ
>> +=============================
>> +
>> +Supported systems:
>> + * P9 server based on POWERNV platform
>> +
>> +Description
>> +------------
>> +
>> +This driver exports the power and temperature sensors from OCC inband
>> +sensors on P9 POWERNV platforms.
>> +
>> +Sysfs attributes
>> +----------------
>> +
>> +powerX_input		Latest power reading
>> +powerX_input_highest	Minimum power
>> +powerX_input_lowest	Maximum power
>> +powerX_label		Sensor name
>> +
>> +tempX_input		Latest temperature reading
>> +tempX_max		Minimum temperature
>> +tempX_min		Maximum temperature
>> +tempX_label		Sensor name
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 0649d53f3..3b1dbb9 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -598,6 +598,17 @@ config SENSORS_IBMPOWERNV
>>  	  This driver can also be built as a module. If so, the module
>>  	  will be called ibmpowernv.
>>  
>> +config SENSORS_IBMPOWERNV_OCC
>> +	tristate "IBM POWERNV OCC Inband platform sensors"
>> +	depends on PPC_POWERNV
>> +	default y
>> +	help
>> +	  If you say yes here you get support for the temperature/power
>> +	  OCC inband sensors on your PowerNV platform.
>> +
>> +	  This driver can also be built as a module. If so, the module
>> +	  will be called ibmpowernv-occ.
>> +
>>  config SENSORS_IIO_HWMON
>>  	tristate "Hwmon driver that uses channels specified via iio maps"
>>  	depends on IIO
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 5509edf..0da2207 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -75,6 +75,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
>>  obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
>>  obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
>> +obj-$(CONFIG_SENSORS_IBMPOWERNV_OCC)+= ibmpowernv-occ.o
>>  obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
>>  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
>>  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
>> diff --git a/drivers/hwmon/ibmpowernv-occ.c b/drivers/hwmon/ibmpowernv-occ.c
>> new file mode 100644
>> index 0000000..97b1bbe
>> --- /dev/null
>> +++ b/drivers/hwmon/ibmpowernv-occ.c
>> @@ -0,0 +1,302 @@
>> +/*
>> + * IBM PowerNV platform OCC inband sensors for temperature/power
>> + * Copyright (C) 2017 IBM
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.
>> + */
>> +
>> +#define DRVNAME		"ibmpowernv_occ"
>> +#define pr_fmt(fmt)	DRVNAME ": " fmt
>> +
>> +#include <linux/module.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include <asm/opal.h>
>> +
>> +#define MAX_HWMON_ATTR_LEN	32
>> +#define MAX_HWMON_LABEL_LEN	(MAX_OCC_SENSOR_NAME_LEN * 2)
>> +#define HWMON_ATTRS_PER_SENSOR	16
>> +#define TO_MILLI_UNITS(x)	((x) * 1000)
>> +#define TO_MICRO_UNITS(x)	((x) * 1000000)
>> +
>> +enum sensors {
>> +	TEMP,
>> +	POWER,
>> +	MAX_SENSOR_TYPE,
>> +};
>> +
>> +static struct sensor_type {
>> +	const char *name;
>> +	int hwmon_id;
>> +} sensor_types[] = {
>> +	{ "temp"},
>> +	{ "power"},
>> +};
>> +
>> +static struct sensor_data {
>> +	u32 occ_id;
>> +	u64 offset;  /* Offset to ping/pong reading buffer */
>> +	enum sensors type;
>> +	char label[MAX_HWMON_LABEL_LEN];
>> +	char name[MAX_HWMON_ATTR_LEN];
>> +	struct device_attribute attr;
>> +} *sdata;
>> +
>> +static struct attribute_group sensor_attrs_group;
>> +__ATTRIBUTE_GROUPS(sensor_attrs);
>> +
>> +#define show(file_name)							\
>> +static ssize_t ibmpowernv_occ_show_##file_name				\
>> +(struct device *dev, struct device_attribute *dattr, char *buf)		\
>> +{									\
>> +	struct sensor_data *sdata = container_of(dattr,			\
>> +						 struct sensor_data,	\
>> +						 attr);			\
>> +	u64 val;							\
>> +	int ret;							\
>> +	ret = opal_occ_sensor_get_##file_name(sdata->occ_id,		\
>> +					      sdata->offset,		\
>> +					      &val);			\
>> +	if (ret)							\
>> +		return ret;						\
>> +	if (sdata->type == TEMP)					\
>> +		val = TO_MILLI_UNITS(val);				\
>> +	else if (sdata->type == POWER)					\
>> +		val = TO_MICRO_UNITS(val);				\
>> +	return sprintf(buf, "%llu\n", val);				\
>> +}
>> +
>> +show(sample);
>> +show(max);
>> +show(min);
>> +show(js_min);
>> +show(js_max);
>> +show(csm_min);
>> +show(csm_max);
>> +show(prof_min);
>> +show(prof_max);
>> +
>> +static struct sensor_view_groups {
>> +	const char *name;
>> +	ssize_t (*show_sample)(struct device *dev,
>> +			       struct device_attribute *attr,
>> +			       char *buf);
>> +	ssize_t (*show_min)(struct device *dev,
>> +			    struct device_attribute *attr,
>> +			    char *buf);
>> +	ssize_t (*show_max)(struct device *dev,
>> +			    struct device_attribute *attr,
>> +			    char *buf);
>> +} sensor_views[] = {
>> +	{
>> +		.name		= "",
>> +		.show_sample	= ibmpowernv_occ_show_sample,
>> +		.show_min	= ibmpowernv_occ_show_min,
>> +		.show_max	= ibmpowernv_occ_show_max
>> +	},
>> +	{
>> +		.name		= "_JS",
>> +		.show_sample	= ibmpowernv_occ_show_sample,
>> +		.show_min	= ibmpowernv_occ_show_js_min,
>> +		.show_max	= ibmpowernv_occ_show_js_max
>> +	},
>> +	{	.name		= "_CSM",
>> +		.show_sample	= ibmpowernv_occ_show_sample,
>> +		.show_min	= ibmpowernv_occ_show_csm_min,
>> +		.show_max	= ibmpowernv_occ_show_csm_max
>> +	},
>> +	{	.name		= "_Prof",
>> +		.show_sample	= ibmpowernv_occ_show_sample,
>> +		.show_min	= ibmpowernv_occ_show_prof_min,
>> +		.show_max	= ibmpowernv_occ_show_prof_max
>> +	},
>> +};
>> +
>> +static ssize_t ibmpowernv_occ_show_label(struct device *dev,
>> +					 struct device_attribute *dattr,
>> +					 char *buf)
>> +{
>> +	struct sensor_data *sdata = container_of(dattr, struct sensor_data,
>> +						 attr);
>> +
>> +	return sprintf(buf, "%s\n", sdata->label);
>> +}
>> +
>> +static int ibmpowernv_occ_get_sensor_type(enum occ_sensor_type type)
>> +{
>> +	switch (type) {
>> +	case OCC_SENSOR_TYPE_POWER:
>> +		return POWER;
>> +	case OCC_SENSOR_TYPE_TEMPERATURE:
>> +		return TEMP;
>> +	default:
>> +		return MAX_SENSOR_TYPE;
>> +	}
>> +
>> +	return MAX_SENSOR_TYPE;
>> +}
>> +
>> +static void ibmpowernv_occ_add_sdata(struct occ_hwmon_sensor sensor,
>> +				     struct sensor_data *sdata, char *name,
>> +				     int hwmon_id, enum sensors type,
>> +				     ssize_t (*show)(struct device *dev,
>> +						struct device_attribute *attr,
>> +						char *buf))
>> +{
>> +	sdata->type = type;
>> +	sdata->occ_id = sensor.occ_id;
>> +	sdata->offset = sensor.offset;
>> +	snprintf(sdata->name, MAX_HWMON_ATTR_LEN, "%s%d_%s",
>> +		 sensor_types[type].name, hwmon_id, name);
>> +	sysfs_attr_init(&sdata->attr.attr);
>> +	sdata->attr.attr.name = sdata->name;
>> +	sdata->attr.attr.mode = 0444;
>> +	sdata->attr.show = show;
>> +}
>> +
>> +static void ibmpowernv_occ_add_sensor_attrs(struct occ_hwmon_sensor sensor,
>> +					    int index)
>> +{
>> +	struct attribute **attrs = sensor_attrs_group.attrs;
>> +	char attr_str[MAX_HWMON_ATTR_LEN];
>> +	enum sensors type = ibmpowernv_occ_get_sensor_type(sensor.type);
>> +	int i;
>> +
>> +	index *= HWMON_ATTRS_PER_SENSOR;
>> +	for (i = 0; i < ARRAY_SIZE(sensor_views); i++) {
>> +		int hid = ++sensor_types[type].hwmon_id;
>> +
>> +		/* input */
>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "input", hid,
>> +					 type, sensor_views[i].show_sample);
>> +		attrs[index] = &sdata[index].attr.attr;
>> +		index++;
>> +
>> +		/* min */
>> +		if (type == POWER)
>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
>> +				 "input_lowest");
>> +		else
>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "min");
>> +
>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
>> +					 type, sensor_views[i].show_min);
>> +		attrs[index] = &sdata[index].attr.attr;
>> +		index++;
>> +
>> +		/* max */
>> +		if (type == POWER)
>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
>> +				 "input_highest");
>> +		else
>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "max");
>> +
>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
>> +					 type, sensor_views[i].show_max);
>> +		attrs[index] = &sdata[index].attr.attr;
>> +		index++;
>> +
>> +		/* label */
>> +		snprintf(sdata[index].label, MAX_HWMON_LABEL_LEN, "%s%s",
>> +			 sensor.name, sensor_views[i].name);
>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "label", hid,
>> +					 type, ibmpowernv_occ_show_label);
>> +		attrs[index] = &sdata[index].attr.attr;
>> +		index++;
>> +	}
>> +}
>> +
>> +static int ibmpowernv_occ_add_device_attrs(struct platform_device *pdev)
>> +{
>> +	struct attribute **attrs;
>> +	struct occ_hwmon_sensor *slist = NULL;
>> +	int nr_sensors = 0, i;
>> +	int ret = -ENOMEM;
>> +
>> +	slist = opal_occ_sensor_get_hwmon_list(&nr_sensors);
>> +	if (!nr_sensors)
>> +		return -ENODEV;
>> +
>> +	if (!slist)
>> +		return ret;
>> +
>> +	sdata = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*sdata) *
>> +			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
>> +	if (!sdata)
>> +		goto out;
>> +
>> +	attrs = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*attrs) *
>> +			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
>> +	if (!attrs)
>> +		goto out;
>> +
>> +	sensor_attrs_group.attrs = attrs;
>> +	for (i = 0; i < nr_sensors; i++)
>> +		ibmpowernv_occ_add_sensor_attrs(slist[i], i);
>> +
>> +	ret = 0;
>> +out:
>> +	kfree(slist);
>> +	return ret;
>> +}
>> +
>> +static int ibmpowernv_occ_probe(struct platform_device *pdev)
>> +{
>> +	struct device *hwmon_dev;
>> +	int err;
>> +
>> +	err = ibmpowernv_occ_add_device_attrs(pdev);
>> +	if (err)
>> +		goto out;
>> +
>> +	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev, DRVNAME,
>> +							   NULL,
>> +							   sensor_attrs_groups);
>> +
>> +	err = PTR_ERR_OR_ZERO(hwmon_dev);
>> +out:
>> +	if (err)
>> +		pr_warn("Failed to initialize Hwmon OCC inband sensors\n");
>> +
>> +	return err;
>> +}
>> +
>> +static const struct platform_device_id occ_sensor_ids[] = {
>> +	{ .name = "occ-inband-sensor" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(platform, occ_sensor_ids);
>> +
>> +static const struct of_device_id occ_sensor_of_ids[] = {
>> +	{ .compatible	= "ibm,p9-occ-inband-sensor" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, occ_sensor_of_ids);
>> +
>> +static struct platform_driver ibmpowernv_occ_driver = {
>> +	.probe		= ibmpowernv_occ_probe,
>> +	.id_table	= occ_sensor_ids,
>> +	.driver		= {
>> +		.name	= DRVNAME,
>> +		.of_match_table	= occ_sensor_of_ids,
>> +	},
>> +};
>> +
>> +module_platform_driver(ibmpowernv_occ_driver);
>> +
>> +MODULE_AUTHOR("Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>");
>> +MODULE_DESCRIPTION("IBM POWERNV platform OCC inband sensors");
>> +MODULE_LICENSE("GPL");
>> -- 
>> 1.8.3.1
>>
> 

Re: [RFC 2/2] hwmon: powernv: Hwmon driver for OCC inband power and temperature sensors

[Guenter Roeck]

On 03/09/2017 05:30 AM, Shilpasri G Bhat wrote:
> Hi Guenter,
>
> On 03/09/2017 05:40 PM, Guenter Roeck wrote:
>> On Thu, Mar 09, 2017 at 05:19:15PM +0530, Shilpasri G Bhat wrote:
>>> Add support to read power and temperature sensors from OCC inband
>>> sensors which are copied to main memory by OCC.
>>>
>>
>> Is this supposed to be an alternative to the submission from
>> Eddie James ? If so, is there a reason to consider such an
>> alternative ?
>>
>
> This is not an alternative submission. Eddie James' hwmon OCC driver is to
> export the sensors in BMC which is a service processor for POWER{8,9} servers.
> This patch adds a hwmon driver in the POWER9 server.
>

Both are names ...occ... kind of difficult to understand what is what.

Either case, it appears that there are some dependencies to other code
which I was not copied on. that is not very helpful.

Please use the new hwmon API (devm_hwmon_device_register_with_info); this driver
seems to be a perfect candidate. Also, please do not use function macros;
those tend to obfuscate the code and make it hard to understand.
With the new API, such macros should not be necessary.
Couple of additional comments inline.

Thanks,
Guenter

> Thanks and Regards,
> Shilpa
>
>> Thanks,
>> Guenter
>>
>>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>>> CC: Rob Herring <robh+dt@kernel.org>
>>> CC: Mark Rutland <mark.rutland@arm.com>
>>> CC: Jean Delvare <jdelvare@suse.com>
>>> CC: Guenter Roeck <linux@roeck-us.net>
>>> CC: Jonathan Corbet <corbet@lwn.net>
>>> CC: devicetree@vger.kernel.org
>>> CC: linux-hwmon@vger.kernel.orgThat
>>> CC: linux-doc@vger.kernel.org
>>> ---
>>>  .../devicetree/bindings/hwmon/ibmpowernv-occ.txt   |   4 +
>>>  Documentation/hwmon/ibmpowernv-occ                 |  24 ++
>>>  drivers/hwmon/Kconfig                              |  11 +
>>>  drivers/hwmon/Makefile                             |   1 +
>>>  drivers/hwmon/ibmpowernv-occ.c                     | 302 +++++++++++++++++++++
>>>  5 files changed, 342 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>>>  create mode 100644 Documentation/hwmon/ibmpowernv-occ
>>>  create mode 100644 drivers/hwmon/ibmpowernv-occ.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>>> new file mode 100644
>>> index 0000000..d03f744
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/ibmpowernv-occ.txt
>>> @@ -0,0 +1,4 @@
>>> +IBM POWERNV OCC inband platform sensors
>>> +
>>> +Required device-tree property:
>>> +- compatible: "ibm,p9-occ-inband-sensor"
>>> diff --git a/Documentation/hwmon/ibmpowernv-occ b/Documentation/hwmon/ibmpowernv-occ
>>> new file mode 100644
>>> index 0000000..151028b
>>> --- /dev/null
>>> +++ b/Documentation/hwmon/ibmpowernv-occ
>>> @@ -0,0 +1,24 @@
>>> +Kernel driver ibmpowernv-occ
>>> +=============================
>>> +
>>> +Supported systems:
>>> + * P9 server based on POWERNV platform
>>> +
>>> +Description
>>> +------------
>>> +
>>> +This driver exports the power and temperature sensors from OCC inband
>>> +sensors on P9 POWERNV platforms.
>>> +
>>> +Sysfs attributes
>>> +----------------
>>> +
>>> +powerX_input		Latest power reading
>>> +powerX_input_highest	Minimum power
>>> +powerX_input_lowest	Maximum power
>>> +powerX_label		Sensor name
>>> +
>>> +tempX_input		Latest temperature reading
>>> +tempX_max		Minimum temperature
>>> +tempX_min		Maximum temperature
>>> +tempX_label		Sensor name
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 0649d53f3..3b1dbb9 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -598,6 +598,17 @@ config SENSORS_IBMPOWERNV
>>>  	  This driver can also be built as a module. If so, the module
>>>  	  will be called ibmpowernv.
>>>
>>> +config SENSORS_IBMPOWERNV_OCC
>>> +	tristate "IBM POWERNV OCC Inband platform sensors"
>>> +	depends on PPC_POWERNV
>>> +	default y
>>> +	help
>>> +	  If you say yes here you get support for the temperature/power
>>> +	  OCC inband sensors on your PowerNV platform.
>>> +
>>> +	  This driver can also be built as a module. If so, the module
>>> +	  will be called ibmpowernv-occ.
>>> +
>>>  config SENSORS_IIO_HWMON
>>>  	tristate "Hwmon driver that uses channels specified via iio maps"
>>>  	depends on IIO
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index 5509edf..0da2207 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -75,6 +75,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>>>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
>>>  obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
>>>  obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
>>> +obj-$(CONFIG_SENSORS_IBMPOWERNV_OCC)+= ibmpowernv-occ.o
>>>  obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
>>>  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
>>>  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
>>> diff --git a/drivers/hwmon/ibmpowernv-occ.c b/drivers/hwmon/ibmpowernv-occ.c
>>> new file mode 100644
>>> index 0000000..97b1bbe
>>> --- /dev/null
>>> +++ b/drivers/hwmon/ibmpowernv-occ.c
>>> @@ -0,0 +1,302 @@
>>> +/*
>>> + * IBM PowerNV platform OCC inband sensors for temperature/power
>>> + * Copyright (C) 2017 IBM
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program.

Unnecessary.

>>> + */
>>> +
>>> +#define DRVNAME		"ibmpowernv_occ"
>>> +#define pr_fmt(fmt)	DRVNAME ": " fmt
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/platform_device.h>

alphabetic order please

>>> +
>>> +#include <asm/opal.h>
>>> +
>>> +#define MAX_HWMON_ATTR_LEN	32
>>> +#define MAX_HWMON_LABEL_LEN	(MAX_OCC_SENSOR_NAME_LEN * 2)
>>> +#define HWMON_ATTRS_PER_SENSOR	16
>>> +#define TO_MILLI_UNITS(x)	((x) * 1000)
>>> +#define TO_MICRO_UNITS(x)	((x) * 1000000)
>>> +
>>> +enum sensors {
>>> +	TEMP,
>>> +	POWER,
>>> +	MAX_SENSOR_TYPE,
>>> +};
>>> +
>>> +static struct sensor_type {
>>> +	const char *name;
>>> +	int hwmon_id;
>>> +} sensor_types[] = {
>>> +	{ "temp"},
>>> +	{ "power"},
>>> +};
>>> +
>>> +static struct sensor_data {
>>> +	u32 occ_id;
>>> +	u64 offset;  /* Offset to ping/pong reading buffer */
>>> +	enum sensors type;
>>> +	char label[MAX_HWMON_LABEL_LEN];
>>> +	char name[MAX_HWMON_ATTR_LEN];
>>> +	struct device_attribute attr;
>>> +} *sdata;
>>> +
>>> +static struct attribute_group sensor_attrs_group;
>>> +__ATTRIBUTE_GROUPS(sensor_attrs);
>>> +
>>> +#define show(file_name)							\
>>> +static ssize_t ibmpowernv_occ_show_##file_name				\
>>> +(struct device *dev, struct device_attribute *dattr, char *buf)		\
>>> +{									\
>>> +	struct sensor_data *sdata = container_of(dattr,			\
>>> +						 struct sensor_data,	\
>>> +						 attr);			\
>>> +	u64 val;							\
>>> +	int ret;							\

this does or should generate a checkpatch warning.

>>> +	ret = opal_occ_sensor_get_##file_name(sdata->occ_id,		\
>>> +					      sdata->offset,		\
>>> +					      &val);			\
>>> +	if (ret)							\
>>> +		return ret;						\
>>> +	if (sdata->type == TEMP)					\
>>> +		val = TO_MILLI_UNITS(val);				\
>>> +	else if (sdata->type == POWER)					\
>>> +		val = TO_MICRO_UNITS(val);				\
>>> +	return sprintf(buf, "%llu\n", val);				\
>>> +}
>>> +
>>> +show(sample);
>>> +show(max);
>>> +show(min);
>>> +show(js_min);
>>> +show(js_max);
>>> +show(csm_min);
>>> +show(csm_max);
>>> +show(prof_min);
>>> +show(prof_max);
>>> +
>>> +static struct sensor_view_groups {
>>> +	const char *name;
>>> +	ssize_t (*show_sample)(struct device *dev,
>>> +			       struct device_attribute *attr,
>>> +			       char *buf);
>>> +	ssize_t (*show_min)(struct device *dev,
>>> +			    struct device_attribute *attr,
>>> +			    char *buf);
>>> +	ssize_t (*show_max)(struct device *dev,
>>> +			    struct device_attribute *attr,
>>> +			    char *buf);
>>> +} sensor_views[] = {
>>> +	{
>>> +		.name		= "",
>>> +		.show_sample	= ibmpowernv_occ_show_sample,
>>> +		.show_min	= ibmpowernv_occ_show_min,
>>> +		.show_max	= ibmpowernv_occ_show_max
>>> +	},
>>> +	{
>>> +		.name		= "_JS",
>>> +		.show_sample	= ibmpowernv_occ_show_sample,
>>> +		.show_min	= ibmpowernv_occ_show_js_min,
>>> +		.show_max	= ibmpowernv_occ_show_js_max
>>> +	},
>>> +	{	.name		= "_CSM",
>>> +		.show_sample	= ibmpowernv_occ_show_sample,
>>> +		.show_min	= ibmpowernv_occ_show_csm_min,
>>> +		.show_max	= ibmpowernv_occ_show_csm_max
>>> +	},
>>> +	{	.name		= "_Prof",
>>> +		.show_sample	= ibmpowernv_occ_show_sample,
>>> +		.show_min	= ibmpowernv_occ_show_prof_min,
>>> +		.show_max	= ibmpowernv_occ_show_prof_max
>>> +	},
>>> +};
>>> +
>>> +static ssize_t ibmpowernv_occ_show_label(struct device *dev,
>>> +					 struct device_attribute *dattr,
>>> +					 char *buf)
>>> +{
>>> +	struct sensor_data *sdata = container_of(dattr, struct sensor_data,
>>> +						 attr);
>>> +
>>> +	return sprintf(buf, "%s\n", sdata->label);
>>> +}
>>> +
>>> +static int ibmpowernv_occ_get_sensor_type(enum occ_sensor_type type)
>>> +{
>>> +	switch (type) {
>>> +	case OCC_SENSOR_TYPE_POWER:
>>> +		return POWER;
>>> +	case OCC_SENSOR_TYPE_TEMPERATURE:
>>> +		return TEMP;
>>> +	default:
>>> +		return MAX_SENSOR_TYPE;
>>> +	}
>>> +
>>> +	return MAX_SENSOR_TYPE;

Never reached.

>>> +}
>>> +
>>> +static void ibmpowernv_occ_add_sdata(struct occ_hwmon_sensor sensor,

Passing a data structure (instead of a pointer to it) as parameter is quite
unusual. Is this really needed ? I don't really see the point.

>>> +				     struct sensor_data *sdata, char *name,
>>> +				     int hwmon_id, enum sensors type,
>>> +				     ssize_t (*show)(struct device *dev,
>>> +						struct device_attribute *attr,
>>> +						char *buf))
>>> +{
>>> +	sdata->type = type;
>>> +	sdata->occ_id = sensor.occ_id;
>>> +	sdata->offset = sensor.offset;
>>> +	snprintf(sdata->name, MAX_HWMON_ATTR_LEN, "%s%d_%s",
>>> +		 sensor_types[type].name, hwmon_id, name);
>>> +	sysfs_attr_init(&sdata->attr.attr);
>>> +	sdata->attr.attr.name = sdata->name;
>>> +	sdata->attr.attr.mode = 0444;
>>> +	sdata->attr.show = show;
>>> +}
>>> +
>>> +static void ibmpowernv_occ_add_sensor_attrs(struct occ_hwmon_sensor sensor,
>>> +					    int index)
>>> +{
>>> +	struct attribute **attrs = sensor_attrs_group.attrs;
>>> +	char attr_str[MAX_HWMON_ATTR_LEN];
>>> +	enum sensors type = ibmpowernv_occ_get_sensor_type(sensor.type);
>>> +	int i;
>>> +
>>> +	index *= HWMON_ATTRS_PER_SENSOR;
>>> +	for (i = 0; i < ARRAY_SIZE(sensor_views); i++) {
>>> +		int hid = ++sensor_types[type].hwmon_id;
>>> +
>>> +		/* input */
>>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "input", hid,
>>> +					 type, sensor_views[i].show_sample);
>>> +		attrs[index] = &sdata[index].attr.attr;
>>> +		index++;
>>> +
>>> +		/* min */
>>> +		if (type == POWER)
>>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
>>> +				 "input_lowest");
>>> +		else
>>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "min");
>>> +
>>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
>>> +					 type, sensor_views[i].show_min);
>>> +		attrs[index] = &sdata[index].attr.attr;
>>> +		index++;
>>> +
>>> +		/* max */
>>> +		if (type == POWER)
>>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s",
>>> +				 "input_highest");
>>> +		else
>>> +			snprintf(attr_str, MAX_HWMON_ATTR_LEN, "%s", "max");

I don't really see the point of those snprintf()s and for having attr_str[]
(instead of char *attr_str).

>>> +
>>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], attr_str, hid,
>>> +					 type, sensor_views[i].show_max);
>>> +		attrs[index] = &sdata[index].attr.attr;
>>> +		index++;
>>> +
>>> +		/* label */
>>> +		snprintf(sdata[index].label, MAX_HWMON_LABEL_LEN, "%s%s",
>>> +			 sensor.name, sensor_views[i].name);
>>> +		ibmpowernv_occ_add_sdata(sensor, &sdata[index], "label", hid,
>>> +					 type, ibmpowernv_occ_show_label);
>>> +		attrs[index] = &sdata[index].attr.attr;
>>> +		index++;
>>> +	}
>>> +}
>>> +
>>> +static int ibmpowernv_occ_add_device_attrs(struct platform_device *pdev)
>>> +{
>>> +	struct attribute **attrs;
>>> +	struct occ_hwmon_sensor *slist = NULL;

Unnecessary initialization.

>>> +	int nr_sensors = 0, i;
>>> +	int ret = -ENOMEM;
>>> +
>>> +	slist = opal_occ_sensor_get_hwmon_list(&nr_sensors);
>>> +	if (!nr_sensors)
>>> +		return -ENODEV;
>>> +
>>> +	if (!slist)
>>> +		return ret;
>>> +

is nr_sensors guaranteed to be valid if slist is NULL ?

>>> +	sdata = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*sdata) *
>>> +			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
>>> +	if (!sdata)
>>> +		goto out;
>>> +
>>> +	attrs = devm_kzalloc(&pdev->dev, nr_sensors * sizeof(*attrs) *
>>> +			     HWMON_ATTRS_PER_SENSOR, GFP_KERNEL);
>>> +	if (!attrs)
>>> +		goto out;
>>> +
>>> +	sensor_attrs_group.attrs = attrs;
>>> +	for (i = 0; i < nr_sensors; i++)
>>> +		ibmpowernv_occ_add_sensor_attrs(slist[i], i);
>>> +
>>> +	ret = 0;
>>> +out:
>>> +	kfree(slist);
>>> +	return ret;
>>> +}
>>> +
>>> +static int ibmpowernv_occ_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *hwmon_dev;
>>> +	int err;
>>> +
>>> +	err = ibmpowernv_occ_add_device_attrs(pdev);
>>> +	if (err)
>>> +		goto out;
>>> +

For -ENODEV and -ENOMEM you don't really want/need an error message.

>>> +	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev, DRVNAME,
>>> +							   NULL,
>>> +							   sensor_attrs_groups);
>>> +
>>> +	err = PTR_ERR_OR_ZERO(hwmon_dev);
>>> +out:
>>> +	if (err)
>>> +		pr_warn("Failed to initialize Hwmon OCC inband sensors\n");

dev_warn() if this is really neceessary. I would suggest a more specific
error message, though. This doesn't tell if ibmpowernv_occ_add_device_attrs()
or devm_hwmon_device_register_with_groups() failed.

>>> +
>>> +	return err;
>>> +}
>>> +
>>> +static const struct platform_device_id occ_sensor_ids[] = {
>>> +	{ .name = "occ-inband-sensor" },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(platform, occ_sensor_ids);
>>> +
>>> +static const struct of_device_id occ_sensor_of_ids[] = {
>>> +	{ .compatible	= "ibm,p9-occ-inband-sensor" },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, occ_sensor_of_ids);
>>> +
>>> +static struct platform_driver ibmpowernv_occ_driver = {
>>> +	.probe		= ibmpowernv_occ_probe,
>>> +	.id_table	= occ_sensor_ids,
>>> +	.driver		= {
>>> +		.name	= DRVNAME,
>>> +		.of_match_table	= occ_sensor_of_ids,
>>> +	},
>>> +};
>>> +
>>> +module_platform_driver(ibmpowernv_occ_driver);
>>> +
>>> +MODULE_AUTHOR("Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>");
>>> +MODULE_DESCRIPTION("IBM POWERNV platform OCC inband sensors");
>>> +MODULE_LICENSE("GPL");
>>> --
>>> 1.8.3.1
>>>
>>
>
>