* [PATCH linux 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs
2016-12-30 17:56 [PATCH linux 0/6] drivers: hwmon: Add On-Chip Controller driver eajames.ibm
@ 2016-12-30 17:56 ` eajames.ibm
2016-12-30 19:18 ` kbuild test robot
2016-12-30 17:56 ` [PATCH linux 2/6] hwmon: occ: Add sysfs interface eajames.ibm
` (4 subsequent siblings)
5 siblings, 1 reply; 30+ messages in thread
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
Add core support for polling the OCC for it's sensor data and parsing that
data into sensor-specific information.
Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/hwmon/occ | 40 ++++
drivers/hwmon/Kconfig | 2 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/occ/Kconfig | 15 ++
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/occ.c | 544 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ.h | 86 +++++++
drivers/hwmon/occ/scom.h | 47 ++++
8 files changed, 736 insertions(+)
create mode 100644 Documentation/hwmon/occ
create mode 100644 drivers/hwmon/occ/Kconfig
create mode 100644 drivers/hwmon/occ/Makefile
create mode 100644 drivers/hwmon/occ/occ.c
create mode 100644 drivers/hwmon/occ/occ.h
create mode 100644 drivers/hwmon/occ/scom.h
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
new file mode 100644
index 0000000..79d1642
--- /dev/null
+++ b/Documentation/hwmon/occ
@@ -0,0 +1,40 @@
+Kernel driver occ
+=================
+
+Supported chips:
+ * ASPEED AST2400
+ * ASPEED AST2500
+
+Please note that the chip must be connected to a POWER8 or POWER9 processor
+(see the BMC - Host Communications section).
+
+Author: Eddie James <eajames@us.ibm.com>
+
+Description
+-----------
+
+This driver implements support for the OCC (On-Chip Controller) on the IBM
+POWER8 and POWER9 processors, from a BMC (Baseboard Management Controller). The
+OCC is an embedded processor that provides real time power and thermal
+monitoring.
+
+This driver provides an interface on a BMC to poll OCC sensor data, set user
+power caps, and perform some basic OCC error handling.
+
+Currently, all versions of the OCC support four types of sensor data: power,
+temperature, frequency, and "caps," which indicate limits and thresholds used
+internally on the OCC.
+
+BMC - Host Communications
+-------------------------
+
+For the POWER8 application, the BMC can communicate with the P8 over I2C bus.
+However, to access the OCC register space, any data transfer must use a SCOM
+operation. SCOM is a procedure to initiate a data transfer, typically of 8
+bytes. SCOMs consist of writing a 32-bit command register and then
+reading/writing two 32-bit data registers. This driver implements these
+SCOM operations over I2C bus in order to communicate with the OCC.
+
+For the POWER9 application, the BMC can communicate with the P9 over FSI bus
+and SBE engine. Once again, SCOM operations are required. This driver will
+implement SCOM ops over FSI/SBE. This will require the FSI driver.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 190d270..e80ca81 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1240,6 +1240,8 @@ config SENSORS_NSA320
This driver can also be built as a module. If so, the module
will be called nsa320-hwmon.
+source drivers/hwmon/occ/Kconfig
+
config SENSORS_PCF8591
tristate "Philips PCF8591 ADC/DAC"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d2cb7e8..c7ec5d4 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
+obj-$(CONFIG_SENSORS_PPC_OCC) += occ/
obj-$(CONFIG_PMBUS) += pmbus/
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
new file mode 100644
index 0000000..cdb64a7
--- /dev/null
+++ b/drivers/hwmon/occ/Kconfig
@@ -0,0 +1,15 @@
+#
+# On Chip Controller configuration
+#
+
+menuconfig SENSORS_PPC_OCC
+ bool "PPC On-Chip Controller"
+ help
+ If you say yes here you get support to monitor Power CPU
+ sensors via the On-Chip Controller (OCC).
+
+ Generally this is used by management controllers such as a BMC
+ on an OpenPower system.
+
+ This driver can also be built as a module. If so, the module
+ will be called occ.
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
new file mode 100644
index 0000000..93cb52f
--- /dev/null
+++ b/drivers/hwmon/occ/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
diff --git a/drivers/hwmon/occ/occ.c b/drivers/hwmon/occ/occ.c
new file mode 100644
index 0000000..9884ddd
--- /dev/null
+++ b/drivers/hwmon/occ/occ.c
@@ -0,0 +1,544 @@
+/*
+ * occ.c - OCC hwmon driver
+ *
+ * This file contains the methods and data structures for the OCC hwmon driver.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <asm/unaligned.h>
+
+#include "occ.h"
+
+#define OCC_DATA_MAX 4096
+#define OCC_BMC_TIMEOUT_MS 20000
+
+/* To generate attn to OCC */
+#define ATTN_DATA 0x0006B035
+
+/* For BMC to read/write SRAM */
+#define OCB_ADDRESS 0x0006B070
+#define OCB_DATA 0x0006B075
+#define OCB_STATUS_CONTROL_AND 0x0006B072
+#define OCB_STATUS_CONTROL_OR 0x0006B073
+
+/* To init OCB */
+#define OCB_AND_INIT0 0xFBFFFFFF
+#define OCB_AND_INIT1 0xFFFFFFFF
+#define OCB_OR_INIT0 0x08000000
+#define OCB_OR_INIT1 0x00000000
+
+/* To generate attention on OCC */
+#define ATTN0 0x01010000
+#define ATTN1 0x00000000
+
+/* OCC return status */
+#define RESP_RETURN_CMD_IN_PRG 0xFF
+#define RESP_RETURN_SUCCESS 0
+#define RESP_RETURN_CMD_INVAL 0x11
+#define RESP_RETURN_CMD_LEN 0x12
+#define RESP_RETURN_DATA_INVAL 0x13
+#define RESP_RETURN_CHKSUM 0x14
+#define RESP_RETURN_OCC_ERR 0x15
+#define RESP_RETURN_STATE 0x16
+
+/* time interval to retry on "command in progress" return status */
+#define CMD_IN_PRG_INT_MS 100
+#define CMD_IN_PRG_RETRIES (OCC_BMC_TIMEOUT_MS / CMD_IN_PRG_INT_MS)
+
+/* OCC command definitions */
+#define OCC_POLL 0
+#define OCC_SET_USER_POWR_CAP 0x22
+
+/* OCC poll command data */
+#define OCC_POLL_STAT_SENSOR 0x10
+
+/* OCC response data offsets */
+#define RESP_RETURN_STATUS 2
+#define RESP_DATA_LENGTH 3
+#define RESP_HEADER_OFFSET 5
+#define SENSOR_STR_OFFSET 37
+#define SENSOR_BLOCK_NUM_OFFSET 43
+#define SENSOR_BLOCK_OFFSET 45
+
+/* occ_poll_header
+ * structure to match the raw occ poll response data
+ */
+struct occ_poll_header {
+ u8 status;
+ u8 ext_status;
+ u8 occs_present;
+ u8 config;
+ u8 occ_state;
+ u8 mode;
+ u8 ips_status;
+ u8 error_log_id;
+ u32 error_log_addr_start;
+ u16 error_log_length;
+ u8 reserved2;
+ u8 reserved3;
+ u8 occ_code_level[16];
+ u8 sensor_eye_catcher[6];
+ u8 sensor_block_num;
+ u8 sensor_data_version;
+} __attribute__((packed, aligned(4)));
+
+struct occ_response {
+ struct occ_poll_header header;
+ struct occ_blocks data;
+};
+
+struct occ {
+ struct device *dev;
+ void *bus;
+ struct occ_bus_ops bus_ops;
+ struct occ_ops ops;
+ struct occ_config config;
+ unsigned long update_interval;
+ unsigned long last_updated;
+ struct mutex update_lock;
+ struct occ_response response;
+ bool valid;
+};
+
+static void deinit_occ_resp_buf(struct occ_response *resp)
+{
+ int i;
+
+ if (!resp)
+ return;
+
+ if (!resp->data.blocks)
+ return;
+
+ for (i = 0; i < resp->header.sensor_block_num; ++i)
+ kfree(resp->data.blocks[i].sensors);
+
+ kfree(resp->data.blocks);
+
+ memset(resp, 0, sizeof(struct occ_response));
+
+ for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
+ resp->data.sensor_block_id[i] = -1;
+}
+
+static void *occ_get_sensor_by_type(struct occ_response *resp,
+ enum sensor_type t)
+{
+ if (!resp->data.blocks)
+ return NULL;
+
+ if (resp->data.sensor_block_id[t] == -1)
+ return NULL;
+
+ return resp->data.blocks[resp->data.sensor_block_id[t]].sensors;
+}
+
+static int occ_check_sensor(struct occ *driver, u8 sensor_length,
+ u8 sensor_num, enum sensor_type t, int block)
+{
+ void *sensor;
+ int type_block_id;
+ struct occ_response *resp = &driver->response;
+
+ sensor = occ_get_sensor_by_type(resp, t);
+
+ /* empty sensor block, release older sensor data */
+ if (sensor_num == 0 || sensor_length == 0) {
+ kfree(sensor);
+ dev_err(driver->dev, "no sensor blocks available\n");
+ return -ENODATA;
+ }
+
+ type_block_id = resp->data.sensor_block_id[t];
+ if (!sensor || sensor_num !=
+ resp->data.blocks[type_block_id].header.sensor_num) {
+ kfree(sensor);
+ resp->data.blocks[block].sensors =
+ driver->ops.alloc_sensor(t, sensor_num);
+ if (!resp->data.blocks[block].sensors)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int parse_occ_response(struct occ *driver, u8 *data,
+ struct occ_response *resp)
+{
+ int b;
+ int s;
+ int rc;
+ int offset = SENSOR_BLOCK_OFFSET;
+ int sensor_type;
+ u8 sensor_block_num;
+ char sensor_type_string[5] = { 0 };
+ struct sensor_data_block_header *block;
+ struct device *dev = driver->dev;
+
+ /* check if the data is valid */
+ if (strncmp(&data[SENSOR_STR_OFFSET], "SENSOR", 6) != 0) {
+ dev_err(dev, "no SENSOR string in response\n");
+ rc = -ENODATA;
+ goto err;
+ }
+
+ sensor_block_num = data[SENSOR_BLOCK_NUM_OFFSET];
+ if (sensor_block_num == 0) {
+ dev_err(dev, "no sensor blocks available\n");
+ rc = -ENODATA;
+ goto err;
+ }
+
+ /* if number of sensor block has changed, re-malloc */
+ if (sensor_block_num != resp->header.sensor_block_num) {
+ deinit_occ_resp_buf(resp);
+ resp->data.blocks = kcalloc(sensor_block_num,
+ sizeof(struct sensor_data_block),
+ GFP_KERNEL);
+ if (!resp->data.blocks)
+ return -ENOMEM;
+ }
+
+ memcpy(&resp->header, &data[RESP_HEADER_OFFSET],
+ sizeof(struct occ_poll_header));
+ resp->header.error_log_addr_start =
+ be32_to_cpu(resp->header.error_log_addr_start);
+ resp->header.error_log_length =
+ be16_to_cpu(resp->header.error_log_length);
+
+ dev_dbg(dev, "Reading %d sensor blocks\n",
+ resp->header.sensor_block_num);
+ for (b = 0; b < sensor_block_num; b++) {
+ block = (struct sensor_data_block_header *)&data[offset];
+ /* copy to a null terminated string */
+ strncpy(sensor_type_string, block->sensor_type, 4);
+ offset += 8;
+
+ dev_dbg(dev, "sensor block[%d]: type: %s, sensor_num: %d\n", b,
+ sensor_type_string, block->sensor_num);
+
+ if (strncmp(block->sensor_type, "FREQ", 4) == 0)
+ sensor_type = FREQ;
+ else if (strncmp(block->sensor_type, "TEMP", 4) == 0)
+ sensor_type = TEMP;
+ else if (strncmp(block->sensor_type, "POWR", 4) == 0)
+ sensor_type = POWER;
+ else if (strncmp(block->sensor_type, "CAPS", 4) == 0)
+ sensor_type = CAPS;
+ else {
+ dev_err(dev, "sensor type not supported %s\n",
+ sensor_type_string);
+ continue;
+ }
+
+ rc = occ_check_sensor(driver, block->sensor_length,
+ block->sensor_num, sensor_type, b);
+ if (rc == -ENOMEM)
+ goto err;
+ else if (rc)
+ continue;
+
+ resp->data.sensor_block_id[sensor_type] = b;
+ for (s = 0; s < block->sensor_num; s++) {
+ driver->ops.parse_sensor(data,
+ resp->data.blocks[b].sensors,
+ sensor_type, offset, s);
+ offset += block->sensor_length;
+ }
+
+ /* copy block data over to response pointer */
+ resp->data.blocks[b].header = *block;
+ }
+
+ return 0;
+err:
+ deinit_occ_resp_buf(resp);
+ return rc;
+}
+
+static u8 occ_send_cmd(struct occ *driver, u8 seq, u8 type, u16 length,
+ u8 *data, u8 *resp)
+{
+ u32 cmd1, cmd2;
+ u16 checksum = 0;
+ u16 length_le = cpu_to_le16(length);
+ bool retry = 0;
+ int i, rc, tries = 0;
+
+ cmd1 = (seq << 24) | (type << 16) | length_le;
+ memcpy(&cmd2, data, length);
+ cmd2 <<= ((4 - length) * 8);
+
+ /* checksum: sum of every bytes of cmd1, cmd2 */
+ for (i = 0; i < 4; i++) {
+ checksum += (cmd1 >> (i * 8)) & 0xFF;
+ checksum += (cmd2 >> (i * 8)) & 0xFF;
+ }
+
+ cmd2 |= checksum << ((2 - length) * 8);
+
+ /* Init OCB */
+ rc = driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_OR,
+ OCB_OR_INIT0, OCB_OR_INIT1);
+ if (rc)
+ goto err;
+
+ rc = driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_AND,
+ OCB_AND_INIT0, OCB_AND_INIT1);
+ if (rc)
+ goto err;
+
+ /* Send command, 2nd half of the 64-bit addr is unused (write 0) */
+ rc = driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+ driver->config.command_addr, 0);
+ if (rc)
+ goto err;
+
+ rc = driver->bus_ops.putscom(driver->bus, OCB_DATA, cmd1, cmd2);
+ if (rc)
+ goto err;
+
+ /* Trigger attention */
+ rc = driver->bus_ops.putscom(driver->bus, ATTN_DATA, ATTN0, ATTN1);
+ if (rc)
+ goto err;
+
+ /* Get response data */
+ rc = driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+ driver->config.response_addr, 0);
+ if (rc)
+ goto err;
+
+ do {
+ if (retry) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(msecs_to_jiffies(CMD_IN_PRG_INT_MS));
+ }
+
+ rc = driver->bus_ops.getscom(driver->bus, OCB_DATA,
+ (u64 *)resp);
+ if (rc)
+ goto err;
+
+ /* retry if we get "command in progress" return status */
+ retry = (resp[RESP_RETURN_STATUS] == RESP_RETURN_CMD_IN_PRG) &&
+ (tries++ < CMD_IN_PRG_RETRIES);
+ } while (retry);
+
+ switch (resp[RESP_RETURN_STATUS]) {
+ case RESP_RETURN_CMD_IN_PRG:
+ rc = -EALREADY;
+ break;
+ case RESP_RETURN_SUCCESS:
+ rc = 0;
+ break;
+ case RESP_RETURN_CMD_INVAL:
+ case RESP_RETURN_CMD_LEN:
+ case RESP_RETURN_DATA_INVAL:
+ case RESP_RETURN_CHKSUM:
+ rc = -EINVAL;
+ break;
+ case RESP_RETURN_OCC_ERR:
+ rc = -EREMOTE;
+ break;
+ default:
+ rc = -EFAULT;
+ }
+
+ return rc;
+
+err:
+ dev_err(driver->dev, "scom op failed rc:%d\n", rc);
+ return rc;
+}
+
+static int occ_get_all(struct occ *driver)
+{
+ int i = 0, rc;
+ u8 *occ_data;
+ u16 num_bytes;
+ const u8 poll_cmd_data = OCC_POLL_STAT_SENSOR;
+ struct device *dev = driver->dev;
+ struct occ_response *resp = &driver->response;
+
+ occ_data = devm_kzalloc(dev, OCC_DATA_MAX, GFP_KERNEL);
+ if (!occ_data)
+ return -ENOMEM;
+
+ rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
+ if (rc) {
+ dev_err(dev, "OCC poll failed: %d\n", rc);
+ goto out;
+ }
+
+ num_bytes = get_unaligned((u16 *)&occ_data[RESP_DATA_LENGTH]);
+ num_bytes = be16_to_cpu(num_bytes);
+ dev_dbg(dev, "OCC data length: %d\n", num_bytes);
+
+ if (num_bytes > OCC_DATA_MAX) {
+ dev_err(dev, "OCC data length must be < 4KB\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (num_bytes <= 0) {
+ dev_err(dev, "OCC data length is zero\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* read remaining data */
+ for (i = 8; i < num_bytes + 8; i += 8) {
+ rc = driver->bus_ops.getscom(driver->bus, OCB_DATA,
+ (u64 *)&occ_data[i]);
+ if (rc) {
+ dev_err(dev, "scom op failed rc:%d\n", rc);
+ goto out;
+ }
+ }
+
+ /* don't need more sanity checks; buffer is alloc'd for max response
+ * size so we just check for valid data in parse_occ_response
+ */
+ rc = parse_occ_response(driver, occ_data, resp);
+
+out:
+ devm_kfree(dev, occ_data);
+ return rc;
+}
+
+int occ_update_device(struct occ *driver)
+{
+ int rc = 0;
+
+ mutex_lock(&driver->update_lock);
+
+ if (time_after(jiffies, driver->last_updated + driver->update_interval)
+ || !driver->valid) {
+ driver->valid = 1;
+
+ rc = occ_get_all(driver);
+ if (rc)
+ driver->valid = 0;
+
+ driver->last_updated = jiffies;
+ }
+
+ mutex_unlock(&driver->update_lock);
+
+ return rc;
+}
+EXPORT_SYMBOL(occ_update_device);
+
+void *occ_get_sensor(struct occ *driver, int sensor_type)
+{
+ int rc;
+
+ /* occ_update_device locks the update lock */
+ rc = occ_update_device(driver);
+ if (rc != 0) {
+ dev_err(driver->dev, "cannot get occ sensor data: %d\n",
+ rc);
+ return NULL;
+ }
+
+ return occ_get_sensor_by_type(&driver->response, sensor_type);
+}
+EXPORT_SYMBOL(occ_get_sensor);
+
+int occ_get_sensor_value(struct occ *occ, int sensor_type, int snum)
+{
+ return occ->ops.get_sensor_value(occ, sensor_type, snum);
+}
+EXPORT_SYMBOL(occ_get_sensor_value);
+
+int occ_get_sensor_id(struct occ *occ, int sensor_type, int snum)
+{
+ return occ->ops.get_sensor_id(occ, sensor_type, snum);
+}
+EXPORT_SYMBOL(occ_get_sensor_id);
+
+int occ_get_caps_value(struct occ *occ, void *sensor, int snum, int caps_field)
+{
+ return occ->ops.get_caps_value(sensor, snum, caps_field);
+}
+EXPORT_SYMBOL(occ_get_caps_value);
+
+void occ_get_response_blocks(struct occ *occ, struct occ_blocks **blocks)
+{
+ *blocks = &occ->response.data;
+}
+EXPORT_SYMBOL(occ_get_response_blocks);
+
+void occ_set_update_interval(struct occ *occ, unsigned long interval)
+{
+ occ->update_interval = msecs_to_jiffies(interval);
+}
+EXPORT_SYMBOL(occ_set_update_interval);
+
+int occ_set_user_powercap(struct occ *occ, u16 cap)
+{
+ u8 resp[8];
+
+ cap = cpu_to_be16(cap);
+
+ return occ_send_cmd(occ, 0, OCC_SET_USER_POWR_CAP, 2, (u8 *)&cap,
+ resp);
+}
+EXPORT_SYMBOL(occ_set_user_powercap);
+
+struct occ *occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops, const struct occ_ops *ops,
+ const struct occ_config *config)
+{
+ struct occ *driver = devm_kzalloc(dev, sizeof(struct occ), GFP_KERNEL);
+
+ if (!driver)
+ return ERR_PTR(-ENOMEM);
+
+ driver->dev = dev;
+ driver->bus = bus;
+ driver->bus_ops = *bus_ops;
+ driver->ops = *ops;
+ driver->config = *config;
+
+ driver->update_interval = HZ;
+ mutex_init(&driver->update_lock);
+
+ return driver;
+}
+EXPORT_SYMBOL(occ_start);
+
+int occ_stop(struct occ *occ)
+{
+ devm_kfree(occ->dev, occ);
+
+ return 0;
+}
+EXPORT_SYMBOL(occ_stop);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("OCC hwmon core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ.h b/drivers/hwmon/occ/occ.h
new file mode 100644
index 0000000..8c08607
--- /dev/null
+++ b/drivers/hwmon/occ/occ.h
@@ -0,0 +1,86 @@
+/*
+ * occ.h - hwmon OCC driver
+ *
+ * This file contains data structures and function prototypes for common access
+ * between different bus protocols and host systems.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_H__
+#define __OCC_H__
+
+#include "scom.h"
+
+struct device;
+struct occ;
+
+/* sensor_data_block_header
+ * structure to match the raw occ sensor block header
+ */
+struct sensor_data_block_header {
+ u8 sensor_type[4];
+ u8 reserved0;
+ u8 sensor_format;
+ u8 sensor_length;
+ u8 sensor_num;
+} __attribute__((packed, aligned(4)));
+
+struct sensor_data_block {
+ struct sensor_data_block_header header;
+ void *sensors;
+};
+
+enum sensor_type {
+ FREQ = 0,
+ TEMP,
+ POWER,
+ CAPS,
+ MAX_OCC_SENSOR_TYPE
+};
+
+struct occ_ops {
+ void (*parse_sensor)(u8 *data, void *sensor, int sensor_type, int off,
+ int snum);
+ void *(*alloc_sensor)(int sensor_type, int num_sensors);
+ int (*get_sensor_value)(struct occ *driver, int sensor_type, int snum);
+ int (*get_sensor_id)(struct occ *driver, int sensor_type, int snum);
+ int (*get_caps_value)(void *sensor, int snum, int caps_field);
+};
+
+struct occ_config {
+ u32 command_addr;
+ u32 response_addr;
+};
+
+struct occ_blocks {
+ int sensor_block_id[MAX_OCC_SENSOR_TYPE];
+ struct sensor_data_block *blocks;
+};
+
+struct occ *occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops, const struct occ_ops *ops,
+ const struct occ_config *config);
+int occ_stop(struct occ *occ);
+
+void *occ_get_sensor(struct occ *occ, int sensor_type);
+int occ_get_sensor_value(struct occ *occ, int sensor_type, int snum);
+int occ_get_sensor_id(struct occ *occ, int sensor_type, int snum);
+int occ_get_caps_value(struct occ *occ, void *sensor, int snum,
+ int caps_field);
+void occ_get_response_blocks(struct occ *occ, struct occ_blocks **blocks);
+int occ_update_device(struct occ *driver);
+void occ_set_update_interval(struct occ *occ, unsigned long interval);
+int occ_set_user_powercap(struct occ *occ, u16 cap);
+
+#endif /* __OCC_H__ */
diff --git a/drivers/hwmon/occ/scom.h b/drivers/hwmon/occ/scom.h
new file mode 100644
index 0000000..c1da645
--- /dev/null
+++ b/drivers/hwmon/occ/scom.h
@@ -0,0 +1,47 @@
+/*
+ * scom.h - hwmon OCC driver
+ *
+ * This file contains data structures for scom operations to the OCC
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __SCOM_H__
+#define __SCOM_H__
+
+/*
+ * occ_bus_ops - represent the low-level transfer methods to communicate with
+ * the OCC.
+ *
+ * getscom - OCC scom read
+ * @bus: handle to slave device
+ * @address: address
+ * @data: where to store data read from slave; buffer size must be at least
+ * eight bytes.
+ *
+ * Returns 0 on success or a negative errno on error
+ *
+ * putscom - OCC scom write
+ * @bus: handle to slave device
+ * @address: address
+ * @data0: first data byte to write
+ * @data1: second data byte to write
+ *
+ * Returns 0 on success or a negative errno on error
+ */
+struct occ_bus_ops {
+ int (*getscom)(void *bus, u32 address, u64 *data);
+ int (*putscom)(void *bus, u32 address, u32 data0, u32 data1);
+};
+
+#endif /* __SCOM_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH linux 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs
@ 2016-12-30 19:18 ` kbuild test robot
0 siblings, 0 replies; 30+ messages in thread
From: kbuild test robot @ 2016-12-30 19:18 UTC (permalink / raw)
To: eajames.ibm
Cc: kbuild-all, linux, jdelvare, corbet, mark.rutland, robh+dt, wsa,
andrew, joel, devicetree, linux-doc, linux-hwmon, linux-i2c,
linux-kernel, Edward A. James
[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]
Hi Edward,
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.10-rc1 next-20161224]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/eajames-ibm-gmail-com/drivers-hwmon-Add-On-Chip-Controller-driver/20161231-021324
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All warnings (new ones prefixed by >>):
drivers/hwmon/occ/occ.c: In function 'occ_get_all':
>> drivers/hwmon/occ/occ.c:390:44: warning: passing argument 5 of 'occ_send_cmd' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
^
drivers/hwmon/occ/occ.c:281:11: note: expected 'u8 * {aka unsigned char *}' but argument is of type 'const u8 * {aka const unsigned char *}'
static u8 occ_send_cmd(struct occ *driver, u8 seq, u8 type, u16 length,
^~~~~~~~~~~~
vim +390 drivers/hwmon/occ/occ.c
374 return rc;
375 }
376
377 static int occ_get_all(struct occ *driver)
378 {
379 int i = 0, rc;
380 u8 *occ_data;
381 u16 num_bytes;
382 const u8 poll_cmd_data = OCC_POLL_STAT_SENSOR;
383 struct device *dev = driver->dev;
384 struct occ_response *resp = &driver->response;
385
386 occ_data = devm_kzalloc(dev, OCC_DATA_MAX, GFP_KERNEL);
387 if (!occ_data)
388 return -ENOMEM;
389
> 390 rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
391 if (rc) {
392 dev_err(dev, "OCC poll failed: %d\n", rc);
393 goto out;
394 }
395
396 num_bytes = get_unaligned((u16 *)&occ_data[RESP_DATA_LENGTH]);
397 num_bytes = be16_to_cpu(num_bytes);
398 dev_dbg(dev, "OCC data length: %d\n", num_bytes);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45853 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs
@ 2016-12-30 19:18 ` kbuild test robot
0 siblings, 0 replies; 30+ messages in thread
From: kbuild test robot @ 2016-12-30 19:18 UTC (permalink / raw)
To: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
Cc: kbuild-all-JC7UmRfGjtg, linux-0h96xk9xTtrk1uMJSBkQmQ,
jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
wsa-z923LK4zBo2bacvFa/9K2g, andrew-zrmu5oMJ5Fs,
joel-U3u1mxZcP9KHXe+LvDLADg, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Edward A. James
[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]
Hi Edward,
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.10-rc1 next-20161224]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/eajames-ibm-gmail-com/drivers-hwmon-Add-On-Chip-Controller-driver/20161231-021324
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All warnings (new ones prefixed by >>):
drivers/hwmon/occ/occ.c: In function 'occ_get_all':
>> drivers/hwmon/occ/occ.c:390:44: warning: passing argument 5 of 'occ_send_cmd' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
^
drivers/hwmon/occ/occ.c:281:11: note: expected 'u8 * {aka unsigned char *}' but argument is of type 'const u8 * {aka const unsigned char *}'
static u8 occ_send_cmd(struct occ *driver, u8 seq, u8 type, u16 length,
^~~~~~~~~~~~
vim +390 drivers/hwmon/occ/occ.c
374 return rc;
375 }
376
377 static int occ_get_all(struct occ *driver)
378 {
379 int i = 0, rc;
380 u8 *occ_data;
381 u16 num_bytes;
382 const u8 poll_cmd_data = OCC_POLL_STAT_SENSOR;
383 struct device *dev = driver->dev;
384 struct occ_response *resp = &driver->response;
385
386 occ_data = devm_kzalloc(dev, OCC_DATA_MAX, GFP_KERNEL);
387 if (!occ_data)
388 return -ENOMEM;
389
> 390 rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
391 if (rc) {
392 dev_err(dev, "OCC poll failed: %d\n", rc);
393 goto out;
394 }
395
396 num_bytes = get_unaligned((u16 *)&occ_data[RESP_DATA_LENGTH]);
397 num_bytes = be16_to_cpu(num_bytes);
398 dev_dbg(dev, "OCC data length: %d\n", num_bytes);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45853 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2016-12-30 17:56 [PATCH linux 0/6] drivers: hwmon: Add On-Chip Controller driver eajames.ibm
2016-12-30 17:56 ` [PATCH linux 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs eajames.ibm
@ 2016-12-30 17:56 ` eajames.ibm
2016-12-30 19:34 ` Guenter Roeck
2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
` (3 subsequent siblings)
5 siblings, 1 reply; 30+ messages in thread
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
Add a generic mechanism to expose the sensors provided by the OCC in
sysfs.
Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/hwmon/occ | 48 +++++
drivers/hwmon/occ/Makefile | 2 +-
drivers/hwmon/occ/occ_sysfs.c | 492 ++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_sysfs.h | 52 +++++
4 files changed, 593 insertions(+), 1 deletion(-)
create mode 100644 drivers/hwmon/occ/occ_sysfs.c
create mode 100644 drivers/hwmon/occ/occ_sysfs.h
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
index 79d1642..1ee8689 100644
--- a/Documentation/hwmon/occ
+++ b/Documentation/hwmon/occ
@@ -25,6 +25,54 @@ Currently, all versions of the OCC support four types of sensor data: power,
temperature, frequency, and "caps," which indicate limits and thresholds used
internally on the OCC.
+sysfs Entries
+-------------
+
+The OCC driver uses the hwmon sysfs framework to provide data to userspace.
+
+The driver exports two sysfs files for each frequency, temperature, and power
+sensor. These are "input" and "label". The input file contains the value of the
+sensor. The label file contains the sensor id. The sensor id is the unique
+internal OCC identifier. Sensor ids may be provided by the OCC specification.
+The names of these files will be in the following format:
+ <sensor type><sensor index>_input
+ <sensor type><sensor index>_label
+Sensor types will be one of "temp", "freq", or "power". The sensor index is
+an index to differentiate different sensor files. For example, a single
+temperature sensor will have two sysfs files: temp1_input and temp1_label.
+
+Caps sensors are exported differently. For each caps sensor, the driver will
+export 6 entries:
+ curr_powercap - current power cap in watts
+ curr_powerreading - current power output in watts
+ norm_powercap - power cap without redundant power
+ max_powercap - maximum power cap that can be set in watts
+ min_powercap - minimum power cap that can be set in watts
+ user_powerlimit - power limit specified by the user in watts
+In addition, the OCC driver for P9 will export a 7th entry:
+ user_powerlimit_source - can be one of two values depending on who set
+ the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
+ in band from other source.
+The format for these files is caps<sensor index>_<entry type>. For example,
+caps1_curr_powercap.
+
+The driver also provides a number of sysfs entries through hwmon to better
+control the driver and monitor the OCC.
+ powercap - read or write the OCC user power limit in watts.
+ name - read the name of the driver
+ update_interval - read or write the minimum interval for polling the
+ OCC.
+
+The driver also exports a single sysfs file through the communication protocol
+device (see BMC - Host Communications). The filename is "online" and represents
+the status of the OCC with respect to the driver. The OCC can be in one of two
+states: OCC polling enabled or OCC polling disabled. The purpose of this file
+is to control the behavior of the driver and it's hwmon sysfs entries, not to
+infer any information about the state of the physical OCC. Reading the file
+returns either a 0 (polling disabled) or 1 (polling enabled). Writing 1 to the
+file enables OCC polling in the driver if communications can be established
+with the OCC. Writing a 0 to the driver disables OCC polling.
+
BMC - Host Communications
-------------------------
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index 93cb52f..a6881f9 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
+obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
new file mode 100644
index 0000000..b0e063da
--- /dev/null
+++ b/drivers/hwmon/occ/occ_sysfs.c
@@ -0,0 +1,492 @@
+/*
+ * occ_sysfs.c - OCC sysfs interface
+ *
+ * This file contains the methods and data structures for implementing the OCC
+ * hwmon sysfs entries.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+
+#include "occ_sysfs.h"
+
+#define MAX_SENSOR_ATTR_LEN 32
+
+#define RESP_RETURN_CMD_INVAL 0x13
+
+struct sensor_attr_data {
+ enum sensor_type type;
+ u32 hwmon_index;
+ u32 attr_id;
+ char name[MAX_SENSOR_ATTR_LEN];
+ struct device_attribute dev_attr;
+};
+
+static ssize_t show_input(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int val;
+ struct sensor_attr_data *sdata = container_of(attr,
+ struct sensor_attr_data,
+ dev_attr);
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+
+ val = occ_get_sensor_value(driver->occ, sdata->type,
+ sdata->hwmon_index - 1);
+ if (sdata->type == TEMP)
+ val *= 1000; /* in millidegree Celsius */
+
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
+}
+
+/* show_label provides the OCC sensor id. The sensor id will be either a
+ * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
+ * identify what each sensor represents, according to the OCC specification.
+ */
+static ssize_t show_label(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int val;
+ struct sensor_attr_data *sdata = container_of(attr,
+ struct sensor_attr_data,
+ dev_attr);
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+
+ val = occ_get_sensor_id(driver->occ, sdata->type,
+ sdata->hwmon_index - 1);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
+}
+
+static ssize_t show_caps(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int val;
+ struct caps_sensor *sensor;
+ struct sensor_attr_data *sdata = container_of(attr,
+ struct sensor_attr_data,
+ dev_attr);
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+
+ sensor = occ_get_sensor(driver->occ, CAPS);
+ if (!sensor) {
+ val = -1;
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
+ }
+
+ val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
+ sdata->attr_id);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
+}
+
+static ssize_t show_update_interval(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->update_interval);
+}
+
+static ssize_t store_update_interval(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+ unsigned long val;
+ int rc;
+
+ rc = kstrtoul(buf, 10, &val);
+ if (rc)
+ return rc;
+
+ driver->update_interval = val;
+ occ_set_update_interval(driver->occ, val);
+
+ return count;
+}
+
+static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_update_interval,
+ store_update_interval);
+
+static ssize_t show_name(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE - 1, "occ\n");
+}
+
+static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+
+static ssize_t show_user_powercap(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
+}
+
+static ssize_t store_user_powercap(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+ u16 val;
+ int rc;
+
+ rc = kstrtou16(buf, 10, &val);
+ if (rc)
+ return rc;
+
+ dev_dbg(dev, "set user powercap to: %d\n", val);
+ rc = occ_set_user_powercap(driver->occ, val);
+ if (rc) {
+ dev_err(dev, "set user powercap failed: 0x%x\n", rc);
+ if (rc == RESP_RETURN_CMD_INVAL) {
+ dev_err(dev, "set invalid powercap value: %d\n", val);
+ return -EINVAL;
+ }
+
+ return rc;
+ }
+
+ driver->user_powercap = val;
+
+ return count;
+}
+
+static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO, show_user_powercap,
+ store_user_powercap);
+
+static void deinit_sensor_groups(struct device *dev,
+ struct sensor_group *sensor_groups)
+{
+ int i;
+
+ for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
+ if (sensor_groups[i].group.attrs)
+ devm_kfree(dev, sensor_groups[i].group.attrs);
+ if (sensor_groups[i].sattr)
+ devm_kfree(dev, sensor_groups[i].sattr);
+ sensor_groups[i].group.attrs = NULL;
+ sensor_groups[i].sattr = NULL;
+ }
+}
+
+static void sensor_attr_init(struct sensor_attr_data *sdata,
+ char *sensor_group_name,
+ char *attr_name,
+ ssize_t (*show)(struct device *dev,
+ struct device_attribute *attr,
+ char *buf))
+{
+ sysfs_attr_init(&sdata->dev_attr.attr);
+
+ snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
+ sensor_group_name, sdata->hwmon_index, attr_name);
+ sdata->dev_attr.attr.name = sdata->name;
+ sdata->dev_attr.attr.mode = S_IRUGO;
+ sdata->dev_attr.show = show;
+}
+
+static int create_sensor_group(struct occ_sysfs *driver,
+ enum sensor_type type, int sensor_num)
+{
+ struct device *dev = driver->dev;
+ struct sensor_group *sensor_groups = driver->sensor_groups;
+ struct sensor_attr_data *sdata;
+ int rc, i;
+
+ /* each sensor has 'label' and 'input' attributes */
+ sensor_groups[type].group.attrs =
+ devm_kzalloc(dev, sizeof(struct attribute *) *
+ sensor_num * 2 + 1, GFP_KERNEL);
+ if (!sensor_groups[type].group.attrs) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ sensor_groups[type].sattr =
+ devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
+ sensor_num * 2, GFP_KERNEL);
+ if (!sensor_groups[type].sattr) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ for (i = 0; i < sensor_num; i++) {
+ sdata = &sensor_groups[type].sattr[i];
+ /* hwmon attributes index starts from 1 */
+ sdata->hwmon_index = i + 1;
+ sdata->type = type;
+ sensor_attr_init(sdata, sensor_groups[type].name, "input",
+ show_input);
+ sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
+
+ sdata = &sensor_groups[type].sattr[i + sensor_num];
+ sdata->hwmon_index = i + 1;
+ sdata->type = type;
+ sensor_attr_init(sdata, sensor_groups[type].name, "label",
+ show_label);
+ sensor_groups[type].group.attrs[i + sensor_num] =
+ &sdata->dev_attr.attr;
+ }
+
+ rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
+ if (rc)
+ goto err;
+
+ return 0;
+err:
+ deinit_sensor_groups(dev, sensor_groups);
+ return rc;
+}
+
+static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
+ char *attr_name, uint32_t hwmon_index,
+ uint32_t attr_id)
+{
+ sdata->type = CAPS;
+ sdata->hwmon_index = hwmon_index;
+ sdata->attr_id = attr_id;
+
+ snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
+ "caps", sdata->hwmon_index, attr_name);
+
+ sysfs_attr_init(&sdata->dev_attr.attr);
+ sdata->dev_attr.attr.name = sdata->name;
+ sdata->dev_attr.attr.mode = S_IRUGO;
+ sdata->dev_attr.show = show_caps;
+}
+
+static int create_caps_sensor_group(struct occ_sysfs *driver, int sensor_num)
+{
+ struct device *dev = driver->dev;
+ struct sensor_group *sensor_groups = driver->sensor_groups;
+ int field_num = driver->num_caps_fields;
+ struct sensor_attr_data *sdata;
+ int i, j, rc;
+
+ sensor_groups[CAPS].group.attrs =
+ devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
+ field_num + 1, GFP_KERNEL);
+ if (!sensor_groups[CAPS].group.attrs) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ sensor_groups[CAPS].sattr =
+ devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
+ sensor_num * field_num, GFP_KERNEL);
+ if (!sensor_groups[CAPS].sattr) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ for (j = 0; j < sensor_num; ++j) {
+ for (i = 0; i < field_num; ++i) {
+ sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
+ caps_sensor_attr_init(sdata,
+ driver->caps_names[i], j + 1, i);
+ sensor_groups[CAPS].group.attrs[j * field_num + i] =
+ &sdata->dev_attr.attr;
+ }
+ }
+
+ rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
+ if (rc)
+ goto err;
+
+ return rc;
+err:
+ deinit_sensor_groups(dev, sensor_groups);
+ return rc;
+}
+
+static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
+{
+ struct device *dev = driver->dev;
+
+ device_remove_file(dev, &dev_attr_user_powercap);
+ device_remove_file(dev, &dev_attr_update_interval);
+ device_remove_file(dev, &dev_attr_name);
+}
+
+static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
+{
+ int i, rc, id, sensor_num;
+ struct device *dev = driver->dev;
+ struct sensor_group *sensor_groups = driver->sensor_groups;
+ struct occ_blocks *resp = NULL;
+
+ occ_get_response_blocks(driver->occ, &resp);
+
+ for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
+ resp->sensor_block_id[i] = -1;
+
+ /* read sensor data from occ */
+ rc = occ_update_device(driver->occ);
+ if (rc) {
+ dev_err(dev, "cannot get occ sensor data: %d\n", rc);
+ return rc;
+ }
+ if (!resp->blocks)
+ return -ENOMEM;
+
+ rc = device_create_file(dev, &dev_attr_name);
+ if (rc)
+ goto error;
+
+ rc = device_create_file(dev, &dev_attr_update_interval);
+ if (rc)
+ goto error;
+
+ if (resp->sensor_block_id[CAPS] >= 0) {
+ /* user powercap: only for master OCC */
+ rc = device_create_file(dev, &dev_attr_user_powercap);
+ if (rc)
+ goto error;
+ }
+
+ sensor_groups[FREQ].name = "freq";
+ sensor_groups[TEMP].name = "temp";
+ sensor_groups[POWER].name = "power";
+ sensor_groups[CAPS].name = "caps";
+
+ for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
+ id = resp->sensor_block_id[i];
+ if (id < 0)
+ continue;
+
+ sensor_num = resp->blocks[id].header.sensor_num;
+ if (i == CAPS)
+ rc = create_caps_sensor_group(driver, sensor_num);
+ else
+ rc = create_sensor_group(driver, i, sensor_num);
+ if (rc)
+ goto error;
+ }
+
+ return 0;
+
+error:
+ dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
+ occ_remove_hwmon_attrs(driver);
+ return rc;
+}
+
+static ssize_t show_occ_online(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
+}
+
+static ssize_t store_occ_online(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
+ unsigned long val;
+ int rc;
+
+ rc = kstrtoul(buf, 10, &val);
+ if (rc)
+ return rc;
+
+ if (val == 1) {
+ if (driver->occ_online)
+ return count;
+
+ driver->dev = hwmon_device_register(dev);
+ if (IS_ERR(driver->dev))
+ return PTR_ERR(driver->dev);
+
+ dev_set_drvdata(driver->dev, driver);
+
+ rc = occ_create_hwmon_attrs(driver);
+ if (rc) {
+ hwmon_device_unregister(driver->dev);
+ driver->dev = NULL;
+ return rc;
+ }
+ } else if (val == 0) {
+ if (!driver->occ_online)
+ return count;
+
+ occ_remove_hwmon_attrs(driver);
+ hwmon_device_unregister(driver->dev);
+ driver->dev = NULL;
+ } else
+ return -EINVAL;
+
+ driver->occ_online = val;
+ return count;
+}
+
+static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
+ store_occ_online);
+
+struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
+ struct occ_sysfs_config *config)
+{
+ struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
+ GFP_KERNEL);
+ int rc;
+
+ if (!hwmon)
+ return ERR_PTR(-ENOMEM);
+
+ hwmon->occ = occ;
+ hwmon->num_caps_fields = config->num_caps_fields;
+ hwmon->caps_names = config->caps_names;
+
+ dev_set_drvdata(dev, hwmon);
+
+ rc = device_create_file(dev, &dev_attr_online);
+ if (rc)
+ return ERR_PTR(rc);
+
+ return hwmon;
+}
+EXPORT_SYMBOL(occ_sysfs_start);
+
+int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
+{
+ if (driver->dev) {
+ occ_remove_hwmon_attrs(driver);
+ hwmon_device_unregister(driver->dev);
+ }
+
+ device_remove_file(driver->dev, &dev_attr_online);
+
+ devm_kfree(dev, driver);
+
+ return 0;
+}
+EXPORT_SYMBOL(occ_sysfs_stop);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("OCC sysfs driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_sysfs.h b/drivers/hwmon/occ/occ_sysfs.h
new file mode 100644
index 0000000..2a8044f
--- /dev/null
+++ b/drivers/hwmon/occ/occ_sysfs.h
@@ -0,0 +1,52 @@
+/*
+ * occ_sysfs.h - OCC sysfs interface
+ *
+ * This file contains the data structures and function prototypes for the OCC
+ * hwmon sysfs entries.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_SYSFS_H__
+#define __OCC_SYSFS_H__
+
+#include "occ.h"
+
+struct sensor_group {
+ char *name;
+ struct sensor_attr_data *sattr;
+ struct attribute_group group;
+};
+
+struct occ_sysfs_config {
+ unsigned int num_caps_fields;
+ char **caps_names;
+};
+
+struct occ_sysfs {
+ struct device *dev;
+ struct occ *occ;
+
+ u16 user_powercap;
+ bool occ_online;
+ struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
+ unsigned long update_interval;
+ unsigned int num_caps_fields;
+ char **caps_names;
+};
+
+struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
+ struct occ_sysfs_config *config);
+int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
+
+#endif /* __OCC_SYSFS_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2016-12-30 17:56 ` [PATCH linux 2/6] hwmon: occ: Add sysfs interface eajames.ibm
@ 2016-12-30 19:34 ` Guenter Roeck
2017-01-03 14:30 ` Fwd: " Brad Bishop
2017-01-06 22:17 ` Edward James
0 siblings, 2 replies; 30+ messages in thread
From: Guenter Roeck @ 2016-12-30 19:34 UTC (permalink / raw)
To: eajames.ibm
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
On Fri, Dec 30, 2016 at 11:56:04AM -0600, eajames.ibm@gmail.com wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> Add a generic mechanism to expose the sensors provided by the OCC in
> sysfs.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Documentation/hwmon/occ | 48 +++++
> drivers/hwmon/occ/Makefile | 2 +-
> drivers/hwmon/occ/occ_sysfs.c | 492 ++++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/occ/occ_sysfs.h | 52 +++++
> 4 files changed, 593 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hwmon/occ/occ_sysfs.c
> create mode 100644 drivers/hwmon/occ/occ_sysfs.h
>
> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
> index 79d1642..1ee8689 100644
> --- a/Documentation/hwmon/occ
> +++ b/Documentation/hwmon/occ
> @@ -25,6 +25,54 @@ Currently, all versions of the OCC support four types of sensor data: power,
> temperature, frequency, and "caps," which indicate limits and thresholds used
> internally on the OCC.
>
> +sysfs Entries
> +-------------
> +
> +The OCC driver uses the hwmon sysfs framework to provide data to userspace.
> +
> +The driver exports two sysfs files for each frequency, temperature, and power
> +sensor. These are "input" and "label". The input file contains the value of the
> +sensor. The label file contains the sensor id. The sensor id is the unique
> +internal OCC identifier. Sensor ids may be provided by the OCC specification.
> +The names of these files will be in the following format:
> + <sensor type><sensor index>_input
> + <sensor type><sensor index>_label
> +Sensor types will be one of "temp", "freq", or "power". The sensor index is
> +an index to differentiate different sensor files. For example, a single
> +temperature sensor will have two sysfs files: temp1_input and temp1_label.
> +
> +Caps sensors are exported differently. For each caps sensor, the driver will
> +export 6 entries:
> + curr_powercap - current power cap in watts
> + curr_powerreading - current power output in watts
> + norm_powercap - power cap without redundant power
> + max_powercap - maximum power cap that can be set in watts
> + min_powercap - minimum power cap that can be set in watts
> + user_powerlimit - power limit specified by the user in watts
> +In addition, the OCC driver for P9 will export a 7th entry:
> + user_powerlimit_source - can be one of two values depending on who set
> + the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
> + in band from other source.
> +The format for these files is caps<sensor index>_<entry type>. For example,
> +caps1_curr_powercap.
> +
> +The driver also provides a number of sysfs entries through hwmon to better
> +control the driver and monitor the OCC.
> + powercap - read or write the OCC user power limit in watts.
> + name - read the name of the driver
> + update_interval - read or write the minimum interval for polling the
> + OCC.
> +
> +The driver also exports a single sysfs file through the communication protocol
> +device (see BMC - Host Communications). The filename is "online" and represents
> +the status of the OCC with respect to the driver. The OCC can be in one of two
> +states: OCC polling enabled or OCC polling disabled. The purpose of this file
> +is to control the behavior of the driver and it's hwmon sysfs entries, not to
> +infer any information about the state of the physical OCC. Reading the file
> +returns either a 0 (polling disabled) or 1 (polling enabled). Writing 1 to the
> +file enables OCC polling in the driver if communications can be established
> +with the OCC. Writing a 0 to the driver disables OCC polling.
> +
> BMC - Host Communications
> -------------------------
>
> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> index 93cb52f..a6881f9 100644
> --- a/drivers/hwmon/occ/Makefile
> +++ b/drivers/hwmon/occ/Makefile
> @@ -1 +1 @@
> -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
> +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
> diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
> new file mode 100644
> index 0000000..b0e063da
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_sysfs.c
> @@ -0,0 +1,492 @@
> +/*
> + * occ_sysfs.c - OCC sysfs interface
> + *
> + * This file contains the methods and data structures for implementing the OCC
> + * hwmon sysfs entries.
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +
> +#include "occ_sysfs.h"
> +
> +#define MAX_SENSOR_ATTR_LEN 32
> +
> +#define RESP_RETURN_CMD_INVAL 0x13
> +
> +struct sensor_attr_data {
> + enum sensor_type type;
> + u32 hwmon_index;
> + u32 attr_id;
> + char name[MAX_SENSOR_ATTR_LEN];
> + struct device_attribute dev_attr;
> +};
> +
> +static ssize_t show_input(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int val;
> + struct sensor_attr_data *sdata = container_of(attr,
> + struct sensor_attr_data,
> + dev_attr);
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + val = occ_get_sensor_value(driver->occ, sdata->type,
> + sdata->hwmon_index - 1);
> + if (sdata->type == TEMP)
> + val *= 1000; /* in millidegree Celsius */
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +}
> +
> +/* show_label provides the OCC sensor id. The sensor id will be either a
> + * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
> + * identify what each sensor represents, according to the OCC specification.
> + */
> +static ssize_t show_label(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int val;
> + struct sensor_attr_data *sdata = container_of(attr,
> + struct sensor_attr_data,
> + dev_attr);
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + val = occ_get_sensor_id(driver->occ, sdata->type,
> + sdata->hwmon_index - 1);
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +}
> +
> +static ssize_t show_caps(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int val;
> + struct caps_sensor *sensor;
> + struct sensor_attr_data *sdata = container_of(attr,
> + struct sensor_attr_data,
> + dev_attr);
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + sensor = occ_get_sensor(driver->occ, CAPS);
> + if (!sensor) {
> + val = -1;
> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> + }
> +
> + val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
> + sdata->attr_id);
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +}
> +
> +static ssize_t show_update_interval(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->update_interval);
> +}
> +
> +static ssize_t store_update_interval(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> + unsigned long val;
> + int rc;
> +
> + rc = kstrtoul(buf, 10, &val);
> + if (rc)
> + return rc;
> +
> + driver->update_interval = val;
> + occ_set_update_interval(driver->occ, val);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_update_interval,
> + store_update_interval);
> +
> +static ssize_t show_name(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE - 1, "occ\n");
> +}
> +
> +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
> +
> +static ssize_t show_user_powercap(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
> +}
> +
> +static ssize_t store_user_powercap(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> + u16 val;
> + int rc;
> +
> + rc = kstrtou16(buf, 10, &val);
> + if (rc)
> + return rc;
> +
> + dev_dbg(dev, "set user powercap to: %d\n", val);
> + rc = occ_set_user_powercap(driver->occ, val);
> + if (rc) {
> + dev_err(dev, "set user powercap failed: 0x%x\n", rc);
> + if (rc == RESP_RETURN_CMD_INVAL) {
> + dev_err(dev, "set invalid powercap value: %d\n", val);
> + return -EINVAL;
> + }
> +
> + return rc;
> + }
> +
> + driver->user_powercap = val;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO, show_user_powercap,
> + store_user_powercap);
> +
> +static void deinit_sensor_groups(struct device *dev,
> + struct sensor_group *sensor_groups)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
> + if (sensor_groups[i].group.attrs)
> + devm_kfree(dev, sensor_groups[i].group.attrs);
> + if (sensor_groups[i].sattr)
> + devm_kfree(dev, sensor_groups[i].sattr);
> + sensor_groups[i].group.attrs = NULL;
> + sensor_groups[i].sattr = NULL;
> + }
> +}
> +
> +static void sensor_attr_init(struct sensor_attr_data *sdata,
> + char *sensor_group_name,
> + char *attr_name,
> + ssize_t (*show)(struct device *dev,
> + struct device_attribute *attr,
> + char *buf))
> +{
> + sysfs_attr_init(&sdata->dev_attr.attr);
> +
> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
> + sensor_group_name, sdata->hwmon_index, attr_name);
> + sdata->dev_attr.attr.name = sdata->name;
> + sdata->dev_attr.attr.mode = S_IRUGO;
> + sdata->dev_attr.show = show;
> +}
> +
> +static int create_sensor_group(struct occ_sysfs *driver,
> + enum sensor_type type, int sensor_num)
> +{
> + struct device *dev = driver->dev;
> + struct sensor_group *sensor_groups = driver->sensor_groups;
> + struct sensor_attr_data *sdata;
> + int rc, i;
> +
> + /* each sensor has 'label' and 'input' attributes */
> + sensor_groups[type].group.attrs =
> + devm_kzalloc(dev, sizeof(struct attribute *) *
> + sensor_num * 2 + 1, GFP_KERNEL);
> + if (!sensor_groups[type].group.attrs) {
> + rc = -ENOMEM;
> + goto err;
> + }
> +
> + sensor_groups[type].sattr =
> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
> + sensor_num * 2, GFP_KERNEL);
> + if (!sensor_groups[type].sattr) {
> + rc = -ENOMEM;
> + goto err;
> + }
> +
> + for (i = 0; i < sensor_num; i++) {
> + sdata = &sensor_groups[type].sattr[i];
> + /* hwmon attributes index starts from 1 */
> + sdata->hwmon_index = i + 1;
> + sdata->type = type;
> + sensor_attr_init(sdata, sensor_groups[type].name, "input",
> + show_input);
> + sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
> +
> + sdata = &sensor_groups[type].sattr[i + sensor_num];
> + sdata->hwmon_index = i + 1;
> + sdata->type = type;
> + sensor_attr_init(sdata, sensor_groups[type].name, "label",
> + show_label);
> + sensor_groups[type].group.attrs[i + sensor_num] =
> + &sdata->dev_attr.attr;
> + }
> +
> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
> + if (rc)
> + goto err;
> +
> + return 0;
> +err:
> + deinit_sensor_groups(dev, sensor_groups);
> + return rc;
> +}
> +
> +static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
> + char *attr_name, uint32_t hwmon_index,
> + uint32_t attr_id)
> +{
> + sdata->type = CAPS;
> + sdata->hwmon_index = hwmon_index;
> + sdata->attr_id = attr_id;
> +
> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
> + "caps", sdata->hwmon_index, attr_name);
> +
> + sysfs_attr_init(&sdata->dev_attr.attr);
> + sdata->dev_attr.attr.name = sdata->name;
> + sdata->dev_attr.attr.mode = S_IRUGO;
> + sdata->dev_attr.show = show_caps;
> +}
> +
> +static int create_caps_sensor_group(struct occ_sysfs *driver, int sensor_num)
> +{
> + struct device *dev = driver->dev;
> + struct sensor_group *sensor_groups = driver->sensor_groups;
> + int field_num = driver->num_caps_fields;
> + struct sensor_attr_data *sdata;
> + int i, j, rc;
> +
> + sensor_groups[CAPS].group.attrs =
> + devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
> + field_num + 1, GFP_KERNEL);
> + if (!sensor_groups[CAPS].group.attrs) {
> + rc = -ENOMEM;
> + goto err;
> + }
> +
> + sensor_groups[CAPS].sattr =
> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
> + sensor_num * field_num, GFP_KERNEL);
> + if (!sensor_groups[CAPS].sattr) {
> + rc = -ENOMEM;
> + goto err;
> + }
> +
> + for (j = 0; j < sensor_num; ++j) {
> + for (i = 0; i < field_num; ++i) {
> + sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
> + caps_sensor_attr_init(sdata,
> + driver->caps_names[i], j + 1, i);
> + sensor_groups[CAPS].group.attrs[j * field_num + i] =
> + &sdata->dev_attr.attr;
> + }
> + }
> +
> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
> + if (rc)
> + goto err;
> +
> + return rc;
> +err:
> + deinit_sensor_groups(dev, sensor_groups);
> + return rc;
> +}
> +
> +static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
> +{
> + struct device *dev = driver->dev;
> +
> + device_remove_file(dev, &dev_attr_user_powercap);
> + device_remove_file(dev, &dev_attr_update_interval);
> + device_remove_file(dev, &dev_attr_name);
> +}
> +
> +static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
> +{
> + int i, rc, id, sensor_num;
> + struct device *dev = driver->dev;
> + struct sensor_group *sensor_groups = driver->sensor_groups;
> + struct occ_blocks *resp = NULL;
> +
> + occ_get_response_blocks(driver->occ, &resp);
> +
> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
> + resp->sensor_block_id[i] = -1;
> +
> + /* read sensor data from occ */
> + rc = occ_update_device(driver->occ);
> + if (rc) {
> + dev_err(dev, "cannot get occ sensor data: %d\n", rc);
> + return rc;
> + }
> + if (!resp->blocks)
> + return -ENOMEM;
> +
> + rc = device_create_file(dev, &dev_attr_name);
> + if (rc)
> + goto error;
> +
> + rc = device_create_file(dev, &dev_attr_update_interval);
> + if (rc)
> + goto error;
> +
> + if (resp->sensor_block_id[CAPS] >= 0) {
> + /* user powercap: only for master OCC */
> + rc = device_create_file(dev, &dev_attr_user_powercap);
> + if (rc)
> + goto error;
> + }
> +
> + sensor_groups[FREQ].name = "freq";
> + sensor_groups[TEMP].name = "temp";
> + sensor_groups[POWER].name = "power";
> + sensor_groups[CAPS].name = "caps";
> +
> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
> + id = resp->sensor_block_id[i];
> + if (id < 0)
> + continue;
> +
> + sensor_num = resp->blocks[id].header.sensor_num;
> + if (i == CAPS)
> + rc = create_caps_sensor_group(driver, sensor_num);
> + else
> + rc = create_sensor_group(driver, i, sensor_num);
> + if (rc)
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
> + occ_remove_hwmon_attrs(driver);
> + return rc;
> +}
> +
> +static ssize_t show_occ_online(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
> +}
> +
> +static ssize_t store_occ_online(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> + unsigned long val;
> + int rc;
> +
> + rc = kstrtoul(buf, 10, &val);
> + if (rc)
> + return rc;
> +
> + if (val == 1) {
> + if (driver->occ_online)
> + return count;
> +
> + driver->dev = hwmon_device_register(dev);
hwmon_device_register() is deprecated. Please consider using
devm_hwmon_device_register_with_info() or at least
hwmon_device_register_with_info().
Uuh ... and registering a hwmon device based on writing into a sysfs attribute
is completely out of the question.
Thanks,
Guenter
> + if (IS_ERR(driver->dev))
> + return PTR_ERR(driver->dev);
> +
> + dev_set_drvdata(driver->dev, driver);
> +
> + rc = occ_create_hwmon_attrs(driver);
> + if (rc) {
> + hwmon_device_unregister(driver->dev);
> + driver->dev = NULL;
> + return rc;
> + }
> + } else if (val == 0) {
> + if (!driver->occ_online)
> + return count;
> +
> + occ_remove_hwmon_attrs(driver);
> + hwmon_device_unregister(driver->dev);
> + driver->dev = NULL;
> + } else
> + return -EINVAL;
> +
> + driver->occ_online = val;
> + return count;
> +}
> +
> +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
> + store_occ_online);
> +
> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> + struct occ_sysfs_config *config)
> +{
> + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
> + GFP_KERNEL);
> + int rc;
> +
> + if (!hwmon)
> + return ERR_PTR(-ENOMEM);
> +
> + hwmon->occ = occ;
> + hwmon->num_caps_fields = config->num_caps_fields;
> + hwmon->caps_names = config->caps_names;
> +
> + dev_set_drvdata(dev, hwmon);
> +
> + rc = device_create_file(dev, &dev_attr_online);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + return hwmon;
> +}
> +EXPORT_SYMBOL(occ_sysfs_start);
> +
> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
> +{
> + if (driver->dev) {
> + occ_remove_hwmon_attrs(driver);
> + hwmon_device_unregister(driver->dev);
> + }
> +
> + device_remove_file(driver->dev, &dev_attr_online);
> +
> + devm_kfree(dev, driver);
Thw point of using devm_ functions is not to require remove/free functions.
Something is completely wrong here if you need that call.
Overall, this is architectually completely wrong. One does not register
or instantiate drivers based on writing into sysfs attributes. Please
reconsider your approach.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(occ_sysfs_stop);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("OCC sysfs driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/occ/occ_sysfs.h b/drivers/hwmon/occ/occ_sysfs.h
> new file mode 100644
> index 0000000..2a8044f
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_sysfs.h
> @@ -0,0 +1,52 @@
> +/*
> + * occ_sysfs.h - OCC sysfs interface
> + *
> + * This file contains the data structures and function prototypes for the OCC
> + * hwmon sysfs entries.
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * 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.
> + */
> +
> +#ifndef __OCC_SYSFS_H__
> +#define __OCC_SYSFS_H__
> +
> +#include "occ.h"
> +
> +struct sensor_group {
> + char *name;
> + struct sensor_attr_data *sattr;
> + struct attribute_group group;
> +};
> +
> +struct occ_sysfs_config {
> + unsigned int num_caps_fields;
> + char **caps_names;
> +};
> +
> +struct occ_sysfs {
> + struct device *dev;
> + struct occ *occ;
> +
> + u16 user_powercap;
> + bool occ_online;
> + struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
> + unsigned long update_interval;
> + unsigned int num_caps_fields;
> + char **caps_names;
> +};
> +
> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> + struct occ_sysfs_config *config);
> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
> +
> +#endif /* __OCC_SYSFS_H__ */
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Fwd: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2016-12-30 19:34 ` Guenter Roeck
@ 2017-01-03 14:30 ` Brad Bishop
2017-01-03 23:08 ` Eddie James
2017-01-06 22:17 ` Edward James
1 sibling, 1 reply; 30+ messages in thread
From: Brad Bishop @ 2017-01-03 14:30 UTC (permalink / raw)
To: Eddie James, Jeremy Kerr, Joel Stanley, Andrew Jeffery; +Cc: OpenBMC Maillist
The hwmon maintainer rejected Eddies OCC driver because of the online sysfs attribute. Any kernel hackers out there know what the architecturally correct way to make the occ hwmon device suspend/resume with the system power might be?
-brad
> Begin forwarded message:
>
> From: Guenter Roeck <linux@roeck-us.net>
> Subject: Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
> Date: December 30, 2016 at 2:34:04 PM EST
> To: eajames.ibm@gmail.com
> Cc: jdelvare@suse.com, corbet@lwn.net, mark.rutland@arm.com, robh+dt@kernel.org, wsa@the-dreams.de, andrew@aj.id.au, joel@jms.id.au, devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, "Edward A. James" <eajames@us.ibm.com>
>
> On Fri, Dec 30, 2016 at 11:56:04AM -0600, eajames.ibm@gmail.com wrote:
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> Add a generic mechanism to expose the sensors provided by the OCC in
>> sysfs.
>>
>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>> ---
>> Documentation/hwmon/occ | 48 +++++
>> drivers/hwmon/occ/Makefile | 2 +-
>> drivers/hwmon/occ/occ_sysfs.c | 492 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/hwmon/occ/occ_sysfs.h | 52 +++++
>> 4 files changed, 593 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/hwmon/occ/occ_sysfs.c
>> create mode 100644 drivers/hwmon/occ/occ_sysfs.h
>>
>> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
>> index 79d1642..1ee8689 100644
>> --- a/Documentation/hwmon/occ
>> +++ b/Documentation/hwmon/occ
>> @@ -25,6 +25,54 @@ Currently, all versions of the OCC support four types of sensor data: power,
>> temperature, frequency, and "caps," which indicate limits and thresholds used
>> internally on the OCC.
>>
>> +sysfs Entries
>> +-------------
>> +
>> +The OCC driver uses the hwmon sysfs framework to provide data to userspace.
>> +
>> +The driver exports two sysfs files for each frequency, temperature, and power
>> +sensor. These are "input" and "label". The input file contains the value of the
>> +sensor. The label file contains the sensor id. The sensor id is the unique
>> +internal OCC identifier. Sensor ids may be provided by the OCC specification.
>> +The names of these files will be in the following format:
>> + <sensor type><sensor index>_input
>> + <sensor type><sensor index>_label
>> +Sensor types will be one of "temp", "freq", or "power". The sensor index is
>> +an index to differentiate different sensor files. For example, a single
>> +temperature sensor will have two sysfs files: temp1_input and temp1_label.
>> +
>> +Caps sensors are exported differently. For each caps sensor, the driver will
>> +export 6 entries:
>> + curr_powercap - current power cap in watts
>> + curr_powerreading - current power output in watts
>> + norm_powercap - power cap without redundant power
>> + max_powercap - maximum power cap that can be set in watts
>> + min_powercap - minimum power cap that can be set in watts
>> + user_powerlimit - power limit specified by the user in watts
>> +In addition, the OCC driver for P9 will export a 7th entry:
>> + user_powerlimit_source - can be one of two values depending on who set
>> + the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
>> + in band from other source.
>> +The format for these files is caps<sensor index>_<entry type>. For example,
>> +caps1_curr_powercap.
>> +
>> +The driver also provides a number of sysfs entries through hwmon to better
>> +control the driver and monitor the OCC.
>> + powercap - read or write the OCC user power limit in watts.
>> + name - read the name of the driver
>> + update_interval - read or write the minimum interval for polling the
>> + OCC.
>> +
>> +The driver also exports a single sysfs file through the communication protocol
>> +device (see BMC - Host Communications). The filename is "online" and represents
>> +the status of the OCC with respect to the driver. The OCC can be in one of two
>> +states: OCC polling enabled or OCC polling disabled. The purpose of this file
>> +is to control the behavior of the driver and it's hwmon sysfs entries, not to
>> +infer any information about the state of the physical OCC. Reading the file
>> +returns either a 0 (polling disabled) or 1 (polling enabled). Writing 1 to the
>> +file enables OCC polling in the driver if communications can be established
>> +with the OCC. Writing a 0 to the driver disables OCC polling.
>> +
>> BMC - Host Communications
>> -------------------------
>>
>> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
>> index 93cb52f..a6881f9 100644
>> --- a/drivers/hwmon/occ/Makefile
>> +++ b/drivers/hwmon/occ/Makefile
>> @@ -1 +1 @@
>> -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
>> +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
>> diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
>> new file mode 100644
>> index 0000000..b0e063da
>> --- /dev/null
>> +++ b/drivers/hwmon/occ/occ_sysfs.c
>> @@ -0,0 +1,492 @@
>> +/*
>> + * occ_sysfs.c - OCC sysfs interface
>> + *
>> + * This file contains the methods and data structures for implementing the OCC
>> + * hwmon sysfs entries.
>> + *
>> + * Copyright 2016 IBM Corp.
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/slab.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/err.h>
>> +#include <linux/mutex.h>
>> +#include <linux/delay.h>
>> +#include <linux/kernel.h>
>> +#include <linux/device.h>
>> +
>> +#include "occ_sysfs.h"
>> +
>> +#define MAX_SENSOR_ATTR_LEN 32
>> +
>> +#define RESP_RETURN_CMD_INVAL 0x13
>> +
>> +struct sensor_attr_data {
>> + enum sensor_type type;
>> + u32 hwmon_index;
>> + u32 attr_id;
>> + char name[MAX_SENSOR_ATTR_LEN];
>> + struct device_attribute dev_attr;
>> +};
>> +
>> +static ssize_t show_input(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + int val;
>> + struct sensor_attr_data *sdata = container_of(attr,
>> + struct sensor_attr_data,
>> + dev_attr);
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> + val = occ_get_sensor_value(driver->occ, sdata->type,
>> + sdata->hwmon_index - 1);
>> + if (sdata->type == TEMP)
>> + val *= 1000; /* in millidegree Celsius */
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>> +}
>> +
>> +/* show_label provides the OCC sensor id. The sensor id will be either a
>> + * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
>> + * identify what each sensor represents, according to the OCC specification.
>> + */
>> +static ssize_t show_label(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + int val;
>> + struct sensor_attr_data *sdata = container_of(attr,
>> + struct sensor_attr_data,
>> + dev_attr);
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> + val = occ_get_sensor_id(driver->occ, sdata->type,
>> + sdata->hwmon_index - 1);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>> +}
>> +
>> +static ssize_t show_caps(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + int val;
>> + struct caps_sensor *sensor;
>> + struct sensor_attr_data *sdata = container_of(attr,
>> + struct sensor_attr_data,
>> + dev_attr);
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> + sensor = occ_get_sensor(driver->occ, CAPS);
>> + if (!sensor) {
>> + val = -1;
>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>> + }
>> +
>> + val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
>> + sdata->attr_id);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>> +}
>> +
>> +static ssize_t show_update_interval(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->update_interval);
>> +}
>> +
>> +static ssize_t store_update_interval(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> + unsigned long val;
>> + int rc;
>> +
>> + rc = kstrtoul(buf, 10, &val);
>> + if (rc)
>> + return rc;
>> +
>> + driver->update_interval = val;
>> + occ_set_update_interval(driver->occ, val);
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_update_interval,
>> + store_update_interval);
>> +
>> +static ssize_t show_name(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + return snprintf(buf, PAGE_SIZE - 1, "occ\n");
>> +}
>> +
>> +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
>> +
>> +static ssize_t show_user_powercap(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
>> +}
>> +
>> +static ssize_t store_user_powercap(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> + u16 val;
>> + int rc;
>> +
>> + rc = kstrtou16(buf, 10, &val);
>> + if (rc)
>> + return rc;
>> +
>> + dev_dbg(dev, "set user powercap to: %d\n", val);
>> + rc = occ_set_user_powercap(driver->occ, val);
>> + if (rc) {
>> + dev_err(dev, "set user powercap failed: 0x%x\n", rc);
>> + if (rc == RESP_RETURN_CMD_INVAL) {
>> + dev_err(dev, "set invalid powercap value: %d\n", val);
>> + return -EINVAL;
>> + }
>> +
>> + return rc;
>> + }
>> +
>> + driver->user_powercap = val;
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO, show_user_powercap,
>> + store_user_powercap);
>> +
>> +static void deinit_sensor_groups(struct device *dev,
>> + struct sensor_group *sensor_groups)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
>> + if (sensor_groups[i].group.attrs)
>> + devm_kfree(dev, sensor_groups[i].group.attrs);
>> + if (sensor_groups[i].sattr)
>> + devm_kfree(dev, sensor_groups[i].sattr);
>> + sensor_groups[i].group.attrs = NULL;
>> + sensor_groups[i].sattr = NULL;
>> + }
>> +}
>> +
>> +static void sensor_attr_init(struct sensor_attr_data *sdata,
>> + char *sensor_group_name,
>> + char *attr_name,
>> + ssize_t (*show)(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf))
>> +{
>> + sysfs_attr_init(&sdata->dev_attr.attr);
>> +
>> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
>> + sensor_group_name, sdata->hwmon_index, attr_name);
>> + sdata->dev_attr.attr.name = sdata->name;
>> + sdata->dev_attr.attr.mode = S_IRUGO;
>> + sdata->dev_attr.show = show;
>> +}
>> +
>> +static int create_sensor_group(struct occ_sysfs *driver,
>> + enum sensor_type type, int sensor_num)
>> +{
>> + struct device *dev = driver->dev;
>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>> + struct sensor_attr_data *sdata;
>> + int rc, i;
>> +
>> + /* each sensor has 'label' and 'input' attributes */
>> + sensor_groups[type].group.attrs =
>> + devm_kzalloc(dev, sizeof(struct attribute *) *
>> + sensor_num * 2 + 1, GFP_KERNEL);
>> + if (!sensor_groups[type].group.attrs) {
>> + rc = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + sensor_groups[type].sattr =
>> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
>> + sensor_num * 2, GFP_KERNEL);
>> + if (!sensor_groups[type].sattr) {
>> + rc = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + for (i = 0; i < sensor_num; i++) {
>> + sdata = &sensor_groups[type].sattr[i];
>> + /* hwmon attributes index starts from 1 */
>> + sdata->hwmon_index = i + 1;
>> + sdata->type = type;
>> + sensor_attr_init(sdata, sensor_groups[type].name, "input",
>> + show_input);
>> + sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
>> +
>> + sdata = &sensor_groups[type].sattr[i + sensor_num];
>> + sdata->hwmon_index = i + 1;
>> + sdata->type = type;
>> + sensor_attr_init(sdata, sensor_groups[type].name, "label",
>> + show_label);
>> + sensor_groups[type].group.attrs[i + sensor_num] =
>> + &sdata->dev_attr.attr;
>> + }
>> +
>> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
>> + if (rc)
>> + goto err;
>> +
>> + return 0;
>> +err:
>> + deinit_sensor_groups(dev, sensor_groups);
>> + return rc;
>> +}
>> +
>> +static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
>> + char *attr_name, uint32_t hwmon_index,
>> + uint32_t attr_id)
>> +{
>> + sdata->type = CAPS;
>> + sdata->hwmon_index = hwmon_index;
>> + sdata->attr_id = attr_id;
>> +
>> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
>> + "caps", sdata->hwmon_index, attr_name);
>> +
>> + sysfs_attr_init(&sdata->dev_attr.attr);
>> + sdata->dev_attr.attr.name = sdata->name;
>> + sdata->dev_attr.attr.mode = S_IRUGO;
>> + sdata->dev_attr.show = show_caps;
>> +}
>> +
>> +static int create_caps_sensor_group(struct occ_sysfs *driver, int sensor_num)
>> +{
>> + struct device *dev = driver->dev;
>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>> + int field_num = driver->num_caps_fields;
>> + struct sensor_attr_data *sdata;
>> + int i, j, rc;
>> +
>> + sensor_groups[CAPS].group.attrs =
>> + devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
>> + field_num + 1, GFP_KERNEL);
>> + if (!sensor_groups[CAPS].group.attrs) {
>> + rc = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + sensor_groups[CAPS].sattr =
>> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
>> + sensor_num * field_num, GFP_KERNEL);
>> + if (!sensor_groups[CAPS].sattr) {
>> + rc = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + for (j = 0; j < sensor_num; ++j) {
>> + for (i = 0; i < field_num; ++i) {
>> + sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
>> + caps_sensor_attr_init(sdata,
>> + driver->caps_names[i], j + 1, i);
>> + sensor_groups[CAPS].group.attrs[j * field_num + i] =
>> + &sdata->dev_attr.attr;
>> + }
>> + }
>> +
>> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
>> + if (rc)
>> + goto err;
>> +
>> + return rc;
>> +err:
>> + deinit_sensor_groups(dev, sensor_groups);
>> + return rc;
>> +}
>> +
>> +static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
>> +{
>> + struct device *dev = driver->dev;
>> +
>> + device_remove_file(dev, &dev_attr_user_powercap);
>> + device_remove_file(dev, &dev_attr_update_interval);
>> + device_remove_file(dev, &dev_attr_name);
>> +}
>> +
>> +static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
>> +{
>> + int i, rc, id, sensor_num;
>> + struct device *dev = driver->dev;
>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>> + struct occ_blocks *resp = NULL;
>> +
>> + occ_get_response_blocks(driver->occ, &resp);
>> +
>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
>> + resp->sensor_block_id[i] = -1;
>> +
>> + /* read sensor data from occ */
>> + rc = occ_update_device(driver->occ);
>> + if (rc) {
>> + dev_err(dev, "cannot get occ sensor data: %d\n", rc);
>> + return rc;
>> + }
>> + if (!resp->blocks)
>> + return -ENOMEM;
>> +
>> + rc = device_create_file(dev, &dev_attr_name);
>> + if (rc)
>> + goto error;
>> +
>> + rc = device_create_file(dev, &dev_attr_update_interval);
>> + if (rc)
>> + goto error;
>> +
>> + if (resp->sensor_block_id[CAPS] >= 0) {
>> + /* user powercap: only for master OCC */
>> + rc = device_create_file(dev, &dev_attr_user_powercap);
>> + if (rc)
>> + goto error;
>> + }
>> +
>> + sensor_groups[FREQ].name = "freq";
>> + sensor_groups[TEMP].name = "temp";
>> + sensor_groups[POWER].name = "power";
>> + sensor_groups[CAPS].name = "caps";
>> +
>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
>> + id = resp->sensor_block_id[i];
>> + if (id < 0)
>> + continue;
>> +
>> + sensor_num = resp->blocks[id].header.sensor_num;
>> + if (i == CAPS)
>> + rc = create_caps_sensor_group(driver, sensor_num);
>> + else
>> + rc = create_sensor_group(driver, i, sensor_num);
>> + if (rc)
>> + goto error;
>> + }
>> +
>> + return 0;
>> +
>> +error:
>> + dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
>> + occ_remove_hwmon_attrs(driver);
>> + return rc;
>> +}
>> +
>> +static ssize_t show_occ_online(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
>> +}
>> +
>> +static ssize_t store_occ_online(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>> + unsigned long val;
>> + int rc;
>> +
>> + rc = kstrtoul(buf, 10, &val);
>> + if (rc)
>> + return rc;
>> +
>> + if (val == 1) {
>> + if (driver->occ_online)
>> + return count;
>> +
>> + driver->dev = hwmon_device_register(dev);
>
> hwmon_device_register() is deprecated. Please consider using
> devm_hwmon_device_register_with_info() or at least
> hwmon_device_register_with_info().
>
> Uuh ... and registering a hwmon device based on writing into a sysfs attribute
> is completely out of the question.
>
> Thanks,
> Guenter
>
>> + if (IS_ERR(driver->dev))
>> + return PTR_ERR(driver->dev);
>> +
>> + dev_set_drvdata(driver->dev, driver);
>> +
>> + rc = occ_create_hwmon_attrs(driver);
>> + if (rc) {
>> + hwmon_device_unregister(driver->dev);
>> + driver->dev = NULL;
>> + return rc;
>> + }
>> + } else if (val == 0) {
>> + if (!driver->occ_online)
>> + return count;
>> +
>> + occ_remove_hwmon_attrs(driver);
>> + hwmon_device_unregister(driver->dev);
>> + driver->dev = NULL;
>> + } else
>> + return -EINVAL;
>> +
>> + driver->occ_online = val;
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
>> + store_occ_online);
>> +
>> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>> + struct occ_sysfs_config *config)
>> +{
>> + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
>> + GFP_KERNEL);
>> + int rc;
>> +
>> + if (!hwmon)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + hwmon->occ = occ;
>> + hwmon->num_caps_fields = config->num_caps_fields;
>> + hwmon->caps_names = config->caps_names;
>> +
>> + dev_set_drvdata(dev, hwmon);
>> +
>> + rc = device_create_file(dev, &dev_attr_online);
>> + if (rc)
>> + return ERR_PTR(rc);
>> +
>> + return hwmon;
>> +}
>> +EXPORT_SYMBOL(occ_sysfs_start);
>> +
>> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
>> +{
>> + if (driver->dev) {
>> + occ_remove_hwmon_attrs(driver);
>> + hwmon_device_unregister(driver->dev);
>> + }
>> +
>> + device_remove_file(driver->dev, &dev_attr_online);
>> +
>> + devm_kfree(dev, driver);
>
> Thw point of using devm_ functions is not to require remove/free functions.
> Something is completely wrong here if you need that call.
>
> Overall, this is architectually completely wrong. One does not register
> or instantiate drivers based on writing into sysfs attributes. Please
> reconsider your approach.
>
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(occ_sysfs_stop);
>> +
>> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
>> +MODULE_DESCRIPTION("OCC sysfs driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/hwmon/occ/occ_sysfs.h b/drivers/hwmon/occ/occ_sysfs.h
>> new file mode 100644
>> index 0000000..2a8044f
>> --- /dev/null
>> +++ b/drivers/hwmon/occ/occ_sysfs.h
>> @@ -0,0 +1,52 @@
>> +/*
>> + * occ_sysfs.h - OCC sysfs interface
>> + *
>> + * This file contains the data structures and function prototypes for the OCC
>> + * hwmon sysfs entries.
>> + *
>> + * Copyright 2016 IBM Corp.
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef __OCC_SYSFS_H__
>> +#define __OCC_SYSFS_H__
>> +
>> +#include "occ.h"
>> +
>> +struct sensor_group {
>> + char *name;
>> + struct sensor_attr_data *sattr;
>> + struct attribute_group group;
>> +};
>> +
>> +struct occ_sysfs_config {
>> + unsigned int num_caps_fields;
>> + char **caps_names;
>> +};
>> +
>> +struct occ_sysfs {
>> + struct device *dev;
>> + struct occ *occ;
>> +
>> + u16 user_powercap;
>> + bool occ_online;
>> + struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
>> + unsigned long update_interval;
>> + unsigned int num_caps_fields;
>> + char **caps_names;
>> +};
>> +
>> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>> + struct occ_sysfs_config *config);
>> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
>> +
>> +#endif /* __OCC_SYSFS_H__ */
>> --
>> 1.9.1
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" 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] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-03 14:30 ` Fwd: " Brad Bishop
@ 2017-01-03 23:08 ` Eddie James
2017-01-03 23:49 ` Brad Bishop
0 siblings, 1 reply; 30+ messages in thread
From: Eddie James @ 2017-01-03 23:08 UTC (permalink / raw)
To: Brad Bishop; +Cc: Jeremy Kerr, Joel Stanley, Andrew Jeffery, OpenBMC Maillist
Hi,
Just talked to Andrew. Can we just issue a modprobe command from
userspace once the system power is on? That should be pretty clean in
the driver and will make the "online" attribute unnecessary.
Eddie
On Tue, Jan 3, 2017 at 8:30 AM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
> The hwmon maintainer rejected Eddies OCC driver because of the online sysfs attribute. Any kernel hackers out there know what the architecturally correct way to make the occ hwmon device suspend/resume with the system power might be?
>
> -brad
>
>
>> Begin forwarded message:
>>
>> From: Guenter Roeck <linux@roeck-us.net>
>> Subject: Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
>> Date: December 30, 2016 at 2:34:04 PM EST
>> To: eajames.ibm@gmail.com
>> Cc: jdelvare@suse.com, corbet@lwn.net, mark.rutland@arm.com, robh+dt@kernel.org, wsa@the-dreams.de, andrew@aj.id.au, joel@jms.id.au, devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, "Edward A. James" <eajames@us.ibm.com>
>>
>> On Fri, Dec 30, 2016 at 11:56:04AM -0600, eajames.ibm@gmail.com wrote:
>>> From: "Edward A. James" <eajames@us.ibm.com>
>>>
>>> Add a generic mechanism to expose the sensors provided by the OCC in
>>> sysfs.
>>>
>>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>>> ---
>>> Documentation/hwmon/occ | 48 +++++
>>> drivers/hwmon/occ/Makefile | 2 +-
>>> drivers/hwmon/occ/occ_sysfs.c | 492 ++++++++++++++++++++++++++++++++++++++++++
>>> drivers/hwmon/occ/occ_sysfs.h | 52 +++++
>>> 4 files changed, 593 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/hwmon/occ/occ_sysfs.c
>>> create mode 100644 drivers/hwmon/occ/occ_sysfs.h
>>>
>>> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
>>> index 79d1642..1ee8689 100644
>>> --- a/Documentation/hwmon/occ
>>> +++ b/Documentation/hwmon/occ
>>> @@ -25,6 +25,54 @@ Currently, all versions of the OCC support four types of sensor data: power,
>>> temperature, frequency, and "caps," which indicate limits and thresholds used
>>> internally on the OCC.
>>>
>>> +sysfs Entries
>>> +-------------
>>> +
>>> +The OCC driver uses the hwmon sysfs framework to provide data to userspace.
>>> +
>>> +The driver exports two sysfs files for each frequency, temperature, and power
>>> +sensor. These are "input" and "label". The input file contains the value of the
>>> +sensor. The label file contains the sensor id. The sensor id is the unique
>>> +internal OCC identifier. Sensor ids may be provided by the OCC specification.
>>> +The names of these files will be in the following format:
>>> + <sensor type><sensor index>_input
>>> + <sensor type><sensor index>_label
>>> +Sensor types will be one of "temp", "freq", or "power". The sensor index is
>>> +an index to differentiate different sensor files. For example, a single
>>> +temperature sensor will have two sysfs files: temp1_input and temp1_label.
>>> +
>>> +Caps sensors are exported differently. For each caps sensor, the driver will
>>> +export 6 entries:
>>> + curr_powercap - current power cap in watts
>>> + curr_powerreading - current power output in watts
>>> + norm_powercap - power cap without redundant power
>>> + max_powercap - maximum power cap that can be set in watts
>>> + min_powercap - minimum power cap that can be set in watts
>>> + user_powerlimit - power limit specified by the user in watts
>>> +In addition, the OCC driver for P9 will export a 7th entry:
>>> + user_powerlimit_source - can be one of two values depending on who set
>>> + the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
>>> + in band from other source.
>>> +The format for these files is caps<sensor index>_<entry type>. For example,
>>> +caps1_curr_powercap.
>>> +
>>> +The driver also provides a number of sysfs entries through hwmon to better
>>> +control the driver and monitor the OCC.
>>> + powercap - read or write the OCC user power limit in watts.
>>> + name - read the name of the driver
>>> + update_interval - read or write the minimum interval for polling the
>>> + OCC.
>>> +
>>> +The driver also exports a single sysfs file through the communication protocol
>>> +device (see BMC - Host Communications). The filename is "online" and represents
>>> +the status of the OCC with respect to the driver. The OCC can be in one of two
>>> +states: OCC polling enabled or OCC polling disabled. The purpose of this file
>>> +is to control the behavior of the driver and it's hwmon sysfs entries, not to
>>> +infer any information about the state of the physical OCC. Reading the file
>>> +returns either a 0 (polling disabled) or 1 (polling enabled). Writing 1 to the
>>> +file enables OCC polling in the driver if communications can be established
>>> +with the OCC. Writing a 0 to the driver disables OCC polling.
>>> +
>>> BMC - Host Communications
>>> -------------------------
>>>
>>> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
>>> index 93cb52f..a6881f9 100644
>>> --- a/drivers/hwmon/occ/Makefile
>>> +++ b/drivers/hwmon/occ/Makefile
>>> @@ -1 +1 @@
>>> -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
>>> +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
>>> diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
>>> new file mode 100644
>>> index 0000000..b0e063da
>>> --- /dev/null
>>> +++ b/drivers/hwmon/occ/occ_sysfs.c
>>> @@ -0,0 +1,492 @@
>>> +/*
>>> + * occ_sysfs.c - OCC sysfs interface
>>> + *
>>> + * This file contains the methods and data structures for implementing the OCC
>>> + * hwmon sysfs entries.
>>> + *
>>> + * Copyright 2016 IBM Corp.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/jiffies.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +#include <linux/err.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/device.h>
>>> +
>>> +#include "occ_sysfs.h"
>>> +
>>> +#define MAX_SENSOR_ATTR_LEN 32
>>> +
>>> +#define RESP_RETURN_CMD_INVAL 0x13
>>> +
>>> +struct sensor_attr_data {
>>> + enum sensor_type type;
>>> + u32 hwmon_index;
>>> + u32 attr_id;
>>> + char name[MAX_SENSOR_ATTR_LEN];
>>> + struct device_attribute dev_attr;
>>> +};
>>> +
>>> +static ssize_t show_input(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + int val;
>>> + struct sensor_attr_data *sdata = container_of(attr,
>>> + struct sensor_attr_data,
>>> + dev_attr);
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> +
>>> + val = occ_get_sensor_value(driver->occ, sdata->type,
>>> + sdata->hwmon_index - 1);
>>> + if (sdata->type == TEMP)
>>> + val *= 1000; /* in millidegree Celsius */
>>> +
>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>> +}
>>> +
>>> +/* show_label provides the OCC sensor id. The sensor id will be either a
>>> + * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
>>> + * identify what each sensor represents, according to the OCC specification.
>>> + */
>>> +static ssize_t show_label(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + int val;
>>> + struct sensor_attr_data *sdata = container_of(attr,
>>> + struct sensor_attr_data,
>>> + dev_attr);
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> +
>>> + val = occ_get_sensor_id(driver->occ, sdata->type,
>>> + sdata->hwmon_index - 1);
>>> +
>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>> +}
>>> +
>>> +static ssize_t show_caps(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + int val;
>>> + struct caps_sensor *sensor;
>>> + struct sensor_attr_data *sdata = container_of(attr,
>>> + struct sensor_attr_data,
>>> + dev_attr);
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> +
>>> + sensor = occ_get_sensor(driver->occ, CAPS);
>>> + if (!sensor) {
>>> + val = -1;
>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>> + }
>>> +
>>> + val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
>>> + sdata->attr_id);
>>> +
>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>> +}
>>> +
>>> +static ssize_t show_update_interval(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> +
>>> + return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->update_interval);
>>> +}
>>> +
>>> +static ssize_t store_update_interval(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> + unsigned long val;
>>> + int rc;
>>> +
>>> + rc = kstrtoul(buf, 10, &val);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + driver->update_interval = val;
>>> + occ_set_update_interval(driver->occ, val);
>>> +
>>> + return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_update_interval,
>>> + store_update_interval);
>>> +
>>> +static ssize_t show_name(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + return snprintf(buf, PAGE_SIZE - 1, "occ\n");
>>> +}
>>> +
>>> +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
>>> +
>>> +static ssize_t show_user_powercap(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> +
>>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
>>> +}
>>> +
>>> +static ssize_t store_user_powercap(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> + u16 val;
>>> + int rc;
>>> +
>>> + rc = kstrtou16(buf, 10, &val);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + dev_dbg(dev, "set user powercap to: %d\n", val);
>>> + rc = occ_set_user_powercap(driver->occ, val);
>>> + if (rc) {
>>> + dev_err(dev, "set user powercap failed: 0x%x\n", rc);
>>> + if (rc == RESP_RETURN_CMD_INVAL) {
>>> + dev_err(dev, "set invalid powercap value: %d\n", val);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return rc;
>>> + }
>>> +
>>> + driver->user_powercap = val;
>>> +
>>> + return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO, show_user_powercap,
>>> + store_user_powercap);
>>> +
>>> +static void deinit_sensor_groups(struct device *dev,
>>> + struct sensor_group *sensor_groups)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
>>> + if (sensor_groups[i].group.attrs)
>>> + devm_kfree(dev, sensor_groups[i].group.attrs);
>>> + if (sensor_groups[i].sattr)
>>> + devm_kfree(dev, sensor_groups[i].sattr);
>>> + sensor_groups[i].group.attrs = NULL;
>>> + sensor_groups[i].sattr = NULL;
>>> + }
>>> +}
>>> +
>>> +static void sensor_attr_init(struct sensor_attr_data *sdata,
>>> + char *sensor_group_name,
>>> + char *attr_name,
>>> + ssize_t (*show)(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf))
>>> +{
>>> + sysfs_attr_init(&sdata->dev_attr.attr);
>>> +
>>> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
>>> + sensor_group_name, sdata->hwmon_index, attr_name);
>>> + sdata->dev_attr.attr.name = sdata->name;
>>> + sdata->dev_attr.attr.mode = S_IRUGO;
>>> + sdata->dev_attr.show = show;
>>> +}
>>> +
>>> +static int create_sensor_group(struct occ_sysfs *driver,
>>> + enum sensor_type type, int sensor_num)
>>> +{
>>> + struct device *dev = driver->dev;
>>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>>> + struct sensor_attr_data *sdata;
>>> + int rc, i;
>>> +
>>> + /* each sensor has 'label' and 'input' attributes */
>>> + sensor_groups[type].group.attrs =
>>> + devm_kzalloc(dev, sizeof(struct attribute *) *
>>> + sensor_num * 2 + 1, GFP_KERNEL);
>>> + if (!sensor_groups[type].group.attrs) {
>>> + rc = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
>>> + sensor_groups[type].sattr =
>>> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
>>> + sensor_num * 2, GFP_KERNEL);
>>> + if (!sensor_groups[type].sattr) {
>>> + rc = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
>>> + for (i = 0; i < sensor_num; i++) {
>>> + sdata = &sensor_groups[type].sattr[i];
>>> + /* hwmon attributes index starts from 1 */
>>> + sdata->hwmon_index = i + 1;
>>> + sdata->type = type;
>>> + sensor_attr_init(sdata, sensor_groups[type].name, "input",
>>> + show_input);
>>> + sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
>>> +
>>> + sdata = &sensor_groups[type].sattr[i + sensor_num];
>>> + sdata->hwmon_index = i + 1;
>>> + sdata->type = type;
>>> + sensor_attr_init(sdata, sensor_groups[type].name, "label",
>>> + show_label);
>>> + sensor_groups[type].group.attrs[i + sensor_num] =
>>> + &sdata->dev_attr.attr;
>>> + }
>>> +
>>> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
>>> + if (rc)
>>> + goto err;
>>> +
>>> + return 0;
>>> +err:
>>> + deinit_sensor_groups(dev, sensor_groups);
>>> + return rc;
>>> +}
>>> +
>>> +static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
>>> + char *attr_name, uint32_t hwmon_index,
>>> + uint32_t attr_id)
>>> +{
>>> + sdata->type = CAPS;
>>> + sdata->hwmon_index = hwmon_index;
>>> + sdata->attr_id = attr_id;
>>> +
>>> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
>>> + "caps", sdata->hwmon_index, attr_name);
>>> +
>>> + sysfs_attr_init(&sdata->dev_attr.attr);
>>> + sdata->dev_attr.attr.name = sdata->name;
>>> + sdata->dev_attr.attr.mode = S_IRUGO;
>>> + sdata->dev_attr.show = show_caps;
>>> +}
>>> +
>>> +static int create_caps_sensor_group(struct occ_sysfs *driver, int sensor_num)
>>> +{
>>> + struct device *dev = driver->dev;
>>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>>> + int field_num = driver->num_caps_fields;
>>> + struct sensor_attr_data *sdata;
>>> + int i, j, rc;
>>> +
>>> + sensor_groups[CAPS].group.attrs =
>>> + devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
>>> + field_num + 1, GFP_KERNEL);
>>> + if (!sensor_groups[CAPS].group.attrs) {
>>> + rc = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
>>> + sensor_groups[CAPS].sattr =
>>> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
>>> + sensor_num * field_num, GFP_KERNEL);
>>> + if (!sensor_groups[CAPS].sattr) {
>>> + rc = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
>>> + for (j = 0; j < sensor_num; ++j) {
>>> + for (i = 0; i < field_num; ++i) {
>>> + sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
>>> + caps_sensor_attr_init(sdata,
>>> + driver->caps_names[i], j + 1, i);
>>> + sensor_groups[CAPS].group.attrs[j * field_num + i] =
>>> + &sdata->dev_attr.attr;
>>> + }
>>> + }
>>> +
>>> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
>>> + if (rc)
>>> + goto err;
>>> +
>>> + return rc;
>>> +err:
>>> + deinit_sensor_groups(dev, sensor_groups);
>>> + return rc;
>>> +}
>>> +
>>> +static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
>>> +{
>>> + struct device *dev = driver->dev;
>>> +
>>> + device_remove_file(dev, &dev_attr_user_powercap);
>>> + device_remove_file(dev, &dev_attr_update_interval);
>>> + device_remove_file(dev, &dev_attr_name);
>>> +}
>>> +
>>> +static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
>>> +{
>>> + int i, rc, id, sensor_num;
>>> + struct device *dev = driver->dev;
>>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>>> + struct occ_blocks *resp = NULL;
>>> +
>>> + occ_get_response_blocks(driver->occ, &resp);
>>> +
>>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
>>> + resp->sensor_block_id[i] = -1;
>>> +
>>> + /* read sensor data from occ */
>>> + rc = occ_update_device(driver->occ);
>>> + if (rc) {
>>> + dev_err(dev, "cannot get occ sensor data: %d\n", rc);
>>> + return rc;
>>> + }
>>> + if (!resp->blocks)
>>> + return -ENOMEM;
>>> +
>>> + rc = device_create_file(dev, &dev_attr_name);
>>> + if (rc)
>>> + goto error;
>>> +
>>> + rc = device_create_file(dev, &dev_attr_update_interval);
>>> + if (rc)
>>> + goto error;
>>> +
>>> + if (resp->sensor_block_id[CAPS] >= 0) {
>>> + /* user powercap: only for master OCC */
>>> + rc = device_create_file(dev, &dev_attr_user_powercap);
>>> + if (rc)
>>> + goto error;
>>> + }
>>> +
>>> + sensor_groups[FREQ].name = "freq";
>>> + sensor_groups[TEMP].name = "temp";
>>> + sensor_groups[POWER].name = "power";
>>> + sensor_groups[CAPS].name = "caps";
>>> +
>>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
>>> + id = resp->sensor_block_id[i];
>>> + if (id < 0)
>>> + continue;
>>> +
>>> + sensor_num = resp->blocks[id].header.sensor_num;
>>> + if (i == CAPS)
>>> + rc = create_caps_sensor_group(driver, sensor_num);
>>> + else
>>> + rc = create_sensor_group(driver, i, sensor_num);
>>> + if (rc)
>>> + goto error;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +error:
>>> + dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
>>> + occ_remove_hwmon_attrs(driver);
>>> + return rc;
>>> +}
>>> +
>>> +static ssize_t show_occ_online(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> +
>>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
>>> +}
>>> +
>>> +static ssize_t store_occ_online(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>> + unsigned long val;
>>> + int rc;
>>> +
>>> + rc = kstrtoul(buf, 10, &val);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + if (val == 1) {
>>> + if (driver->occ_online)
>>> + return count;
>>> +
>>> + driver->dev = hwmon_device_register(dev);
>>
>> hwmon_device_register() is deprecated. Please consider using
>> devm_hwmon_device_register_with_info() or at least
>> hwmon_device_register_with_info().
>>
>> Uuh ... and registering a hwmon device based on writing into a sysfs attribute
>> is completely out of the question.
>>
>> Thanks,
>> Guenter
>>
>>> + if (IS_ERR(driver->dev))
>>> + return PTR_ERR(driver->dev);
>>> +
>>> + dev_set_drvdata(driver->dev, driver);
>>> +
>>> + rc = occ_create_hwmon_attrs(driver);
>>> + if (rc) {
>>> + hwmon_device_unregister(driver->dev);
>>> + driver->dev = NULL;
>>> + return rc;
>>> + }
>>> + } else if (val == 0) {
>>> + if (!driver->occ_online)
>>> + return count;
>>> +
>>> + occ_remove_hwmon_attrs(driver);
>>> + hwmon_device_unregister(driver->dev);
>>> + driver->dev = NULL;
>>> + } else
>>> + return -EINVAL;
>>> +
>>> + driver->occ_online = val;
>>> + return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
>>> + store_occ_online);
>>> +
>>> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>>> + struct occ_sysfs_config *config)
>>> +{
>>> + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
>>> + GFP_KERNEL);
>>> + int rc;
>>> +
>>> + if (!hwmon)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + hwmon->occ = occ;
>>> + hwmon->num_caps_fields = config->num_caps_fields;
>>> + hwmon->caps_names = config->caps_names;
>>> +
>>> + dev_set_drvdata(dev, hwmon);
>>> +
>>> + rc = device_create_file(dev, &dev_attr_online);
>>> + if (rc)
>>> + return ERR_PTR(rc);
>>> +
>>> + return hwmon;
>>> +}
>>> +EXPORT_SYMBOL(occ_sysfs_start);
>>> +
>>> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
>>> +{
>>> + if (driver->dev) {
>>> + occ_remove_hwmon_attrs(driver);
>>> + hwmon_device_unregister(driver->dev);
>>> + }
>>> +
>>> + device_remove_file(driver->dev, &dev_attr_online);
>>> +
>>> + devm_kfree(dev, driver);
>>
>> Thw point of using devm_ functions is not to require remove/free functions.
>> Something is completely wrong here if you need that call.
>>
>> Overall, this is architectually completely wrong. One does not register
>> or instantiate drivers based on writing into sysfs attributes. Please
>> reconsider your approach.
>>
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(occ_sysfs_stop);
>>> +
>>> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
>>> +MODULE_DESCRIPTION("OCC sysfs driver");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/hwmon/occ/occ_sysfs.h b/drivers/hwmon/occ/occ_sysfs.h
>>> new file mode 100644
>>> index 0000000..2a8044f
>>> --- /dev/null
>>> +++ b/drivers/hwmon/occ/occ_sysfs.h
>>> @@ -0,0 +1,52 @@
>>> +/*
>>> + * occ_sysfs.h - OCC sysfs interface
>>> + *
>>> + * This file contains the data structures and function prototypes for the OCC
>>> + * hwmon sysfs entries.
>>> + *
>>> + * Copyright 2016 IBM Corp.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#ifndef __OCC_SYSFS_H__
>>> +#define __OCC_SYSFS_H__
>>> +
>>> +#include "occ.h"
>>> +
>>> +struct sensor_group {
>>> + char *name;
>>> + struct sensor_attr_data *sattr;
>>> + struct attribute_group group;
>>> +};
>>> +
>>> +struct occ_sysfs_config {
>>> + unsigned int num_caps_fields;
>>> + char **caps_names;
>>> +};
>>> +
>>> +struct occ_sysfs {
>>> + struct device *dev;
>>> + struct occ *occ;
>>> +
>>> + u16 user_powercap;
>>> + bool occ_online;
>>> + struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
>>> + unsigned long update_interval;
>>> + unsigned int num_caps_fields;
>>> + char **caps_names;
>>> +};
>>> +
>>> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>>> + struct occ_sysfs_config *config);
>>> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
>>> +
>>> +#endif /* __OCC_SYSFS_H__ */
>>> --
>>> 1.9.1
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" 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] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-03 23:08 ` Eddie James
@ 2017-01-03 23:49 ` Brad Bishop
2017-01-04 0:06 ` Joel Stanley
0 siblings, 1 reply; 30+ messages in thread
From: Brad Bishop @ 2017-01-03 23:49 UTC (permalink / raw)
To: Eddie James; +Cc: Jeremy Kerr, Joel Stanley, Andrew Jeffery, OpenBMC Maillist
What if the driver is built into the kernel? Is there such a thing as module only drivers?
> On Jan 3, 2017, at 6:08 PM, Eddie James <eajames.ibm@gmail.com> wrote:
>
> Hi,
>
> Just talked to Andrew. Can we just issue a modprobe command from
> userspace once the system power is on? That should be pretty clean in
> the driver and will make the "online" attribute unnecessary.
>
> Eddie
>
> On Tue, Jan 3, 2017 at 8:30 AM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
>> The hwmon maintainer rejected Eddies OCC driver because of the online sysfs attribute. Any kernel hackers out there know what the architecturally correct way to make the occ hwmon device suspend/resume with the system power might be?
>>
>> -brad
>>
>>
>>> Begin forwarded message:
>>>
>>> From: Guenter Roeck <linux@roeck-us.net>
>>> Subject: Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
>>> Date: December 30, 2016 at 2:34:04 PM EST
>>> To: eajames.ibm@gmail.com
>>> Cc: jdelvare@suse.com, corbet@lwn.net, mark.rutland@arm.com, robh+dt@kernel.org, wsa@the-dreams.de, andrew@aj.id.au, joel@jms.id.au, devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, "Edward A. James" <eajames@us.ibm.com>
>>>
>>> On Fri, Dec 30, 2016 at 11:56:04AM -0600, eajames.ibm@gmail.com wrote:
>>>> From: "Edward A. James" <eajames@us.ibm.com>
>>>>
>>>> Add a generic mechanism to expose the sensors provided by the OCC in
>>>> sysfs.
>>>>
>>>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>>> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>>>> ---
>>>> Documentation/hwmon/occ | 48 +++++
>>>> drivers/hwmon/occ/Makefile | 2 +-
>>>> drivers/hwmon/occ/occ_sysfs.c | 492 ++++++++++++++++++++++++++++++++++++++++++
>>>> drivers/hwmon/occ/occ_sysfs.h | 52 +++++
>>>> 4 files changed, 593 insertions(+), 1 deletion(-)
>>>> create mode 100644 drivers/hwmon/occ/occ_sysfs.c
>>>> create mode 100644 drivers/hwmon/occ/occ_sysfs.h
>>>>
>>>> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
>>>> index 79d1642..1ee8689 100644
>>>> --- a/Documentation/hwmon/occ
>>>> +++ b/Documentation/hwmon/occ
>>>> @@ -25,6 +25,54 @@ Currently, all versions of the OCC support four types of sensor data: power,
>>>> temperature, frequency, and "caps," which indicate limits and thresholds used
>>>> internally on the OCC.
>>>>
>>>> +sysfs Entries
>>>> +-------------
>>>> +
>>>> +The OCC driver uses the hwmon sysfs framework to provide data to userspace.
>>>> +
>>>> +The driver exports two sysfs files for each frequency, temperature, and power
>>>> +sensor. These are "input" and "label". The input file contains the value of the
>>>> +sensor. The label file contains the sensor id. The sensor id is the unique
>>>> +internal OCC identifier. Sensor ids may be provided by the OCC specification.
>>>> +The names of these files will be in the following format:
>>>> + <sensor type><sensor index>_input
>>>> + <sensor type><sensor index>_label
>>>> +Sensor types will be one of "temp", "freq", or "power". The sensor index is
>>>> +an index to differentiate different sensor files. For example, a single
>>>> +temperature sensor will have two sysfs files: temp1_input and temp1_label.
>>>> +
>>>> +Caps sensors are exported differently. For each caps sensor, the driver will
>>>> +export 6 entries:
>>>> + curr_powercap - current power cap in watts
>>>> + curr_powerreading - current power output in watts
>>>> + norm_powercap - power cap without redundant power
>>>> + max_powercap - maximum power cap that can be set in watts
>>>> + min_powercap - minimum power cap that can be set in watts
>>>> + user_powerlimit - power limit specified by the user in watts
>>>> +In addition, the OCC driver for P9 will export a 7th entry:
>>>> + user_powerlimit_source - can be one of two values depending on who set
>>>> + the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
>>>> + in band from other source.
>>>> +The format for these files is caps<sensor index>_<entry type>. For example,
>>>> +caps1_curr_powercap.
>>>> +
>>>> +The driver also provides a number of sysfs entries through hwmon to better
>>>> +control the driver and monitor the OCC.
>>>> + powercap - read or write the OCC user power limit in watts.
>>>> + name - read the name of the driver
>>>> + update_interval - read or write the minimum interval for polling the
>>>> + OCC.
>>>> +
>>>> +The driver also exports a single sysfs file through the communication protocol
>>>> +device (see BMC - Host Communications). The filename is "online" and represents
>>>> +the status of the OCC with respect to the driver. The OCC can be in one of two
>>>> +states: OCC polling enabled or OCC polling disabled. The purpose of this file
>>>> +is to control the behavior of the driver and it's hwmon sysfs entries, not to
>>>> +infer any information about the state of the physical OCC. Reading the file
>>>> +returns either a 0 (polling disabled) or 1 (polling enabled). Writing 1 to the
>>>> +file enables OCC polling in the driver if communications can be established
>>>> +with the OCC. Writing a 0 to the driver disables OCC polling.
>>>> +
>>>> BMC - Host Communications
>>>> -------------------------
>>>>
>>>> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
>>>> index 93cb52f..a6881f9 100644
>>>> --- a/drivers/hwmon/occ/Makefile
>>>> +++ b/drivers/hwmon/occ/Makefile
>>>> @@ -1 +1 @@
>>>> -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
>>>> +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
>>>> diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
>>>> new file mode 100644
>>>> index 0000000..b0e063da
>>>> --- /dev/null
>>>> +++ b/drivers/hwmon/occ/occ_sysfs.c
>>>> @@ -0,0 +1,492 @@
>>>> +/*
>>>> + * occ_sysfs.c - OCC sysfs interface
>>>> + *
>>>> + * This file contains the methods and data structures for implementing the OCC
>>>> + * hwmon sysfs entries.
>>>> + *
>>>> + * Copyright 2016 IBM Corp.
>>>> + *
>>>> + * 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.
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/jiffies.h>
>>>> +#include <linux/hwmon.h>
>>>> +#include <linux/hwmon-sysfs.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/mutex.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/device.h>
>>>> +
>>>> +#include "occ_sysfs.h"
>>>> +
>>>> +#define MAX_SENSOR_ATTR_LEN 32
>>>> +
>>>> +#define RESP_RETURN_CMD_INVAL 0x13
>>>> +
>>>> +struct sensor_attr_data {
>>>> + enum sensor_type type;
>>>> + u32 hwmon_index;
>>>> + u32 attr_id;
>>>> + char name[MAX_SENSOR_ATTR_LEN];
>>>> + struct device_attribute dev_attr;
>>>> +};
>>>> +
>>>> +static ssize_t show_input(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + int val;
>>>> + struct sensor_attr_data *sdata = container_of(attr,
>>>> + struct sensor_attr_data,
>>>> + dev_attr);
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> +
>>>> + val = occ_get_sensor_value(driver->occ, sdata->type,
>>>> + sdata->hwmon_index - 1);
>>>> + if (sdata->type == TEMP)
>>>> + val *= 1000; /* in millidegree Celsius */
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>>> +}
>>>> +
>>>> +/* show_label provides the OCC sensor id. The sensor id will be either a
>>>> + * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
>>>> + * identify what each sensor represents, according to the OCC specification.
>>>> + */
>>>> +static ssize_t show_label(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + int val;
>>>> + struct sensor_attr_data *sdata = container_of(attr,
>>>> + struct sensor_attr_data,
>>>> + dev_attr);
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> +
>>>> + val = occ_get_sensor_id(driver->occ, sdata->type,
>>>> + sdata->hwmon_index - 1);
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>>> +}
>>>> +
>>>> +static ssize_t show_caps(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + int val;
>>>> + struct caps_sensor *sensor;
>>>> + struct sensor_attr_data *sdata = container_of(attr,
>>>> + struct sensor_attr_data,
>>>> + dev_attr);
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> +
>>>> + sensor = occ_get_sensor(driver->occ, CAPS);
>>>> + if (!sensor) {
>>>> + val = -1;
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>>> + }
>>>> +
>>>> + val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
>>>> + sdata->attr_id);
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>>>> +}
>>>> +
>>>> +static ssize_t show_update_interval(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->update_interval);
>>>> +}
>>>> +
>>>> +static ssize_t store_update_interval(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> + unsigned long val;
>>>> + int rc;
>>>> +
>>>> + rc = kstrtoul(buf, 10, &val);
>>>> + if (rc)
>>>> + return rc;
>>>> +
>>>> + driver->update_interval = val;
>>>> + occ_set_update_interval(driver->occ, val);
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_update_interval,
>>>> + store_update_interval);
>>>> +
>>>> +static ssize_t show_name(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + return snprintf(buf, PAGE_SIZE - 1, "occ\n");
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
>>>> +
>>>> +static ssize_t show_user_powercap(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
>>>> +}
>>>> +
>>>> +static ssize_t store_user_powercap(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> + u16 val;
>>>> + int rc;
>>>> +
>>>> + rc = kstrtou16(buf, 10, &val);
>>>> + if (rc)
>>>> + return rc;
>>>> +
>>>> + dev_dbg(dev, "set user powercap to: %d\n", val);
>>>> + rc = occ_set_user_powercap(driver->occ, val);
>>>> + if (rc) {
>>>> + dev_err(dev, "set user powercap failed: 0x%x\n", rc);
>>>> + if (rc == RESP_RETURN_CMD_INVAL) {
>>>> + dev_err(dev, "set invalid powercap value: %d\n", val);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + return rc;
>>>> + }
>>>> +
>>>> + driver->user_powercap = val;
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO, show_user_powercap,
>>>> + store_user_powercap);
>>>> +
>>>> +static void deinit_sensor_groups(struct device *dev,
>>>> + struct sensor_group *sensor_groups)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
>>>> + if (sensor_groups[i].group.attrs)
>>>> + devm_kfree(dev, sensor_groups[i].group.attrs);
>>>> + if (sensor_groups[i].sattr)
>>>> + devm_kfree(dev, sensor_groups[i].sattr);
>>>> + sensor_groups[i].group.attrs = NULL;
>>>> + sensor_groups[i].sattr = NULL;
>>>> + }
>>>> +}
>>>> +
>>>> +static void sensor_attr_init(struct sensor_attr_data *sdata,
>>>> + char *sensor_group_name,
>>>> + char *attr_name,
>>>> + ssize_t (*show)(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf))
>>>> +{
>>>> + sysfs_attr_init(&sdata->dev_attr.attr);
>>>> +
>>>> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
>>>> + sensor_group_name, sdata->hwmon_index, attr_name);
>>>> + sdata->dev_attr.attr.name = sdata->name;
>>>> + sdata->dev_attr.attr.mode = S_IRUGO;
>>>> + sdata->dev_attr.show = show;
>>>> +}
>>>> +
>>>> +static int create_sensor_group(struct occ_sysfs *driver,
>>>> + enum sensor_type type, int sensor_num)
>>>> +{
>>>> + struct device *dev = driver->dev;
>>>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>>>> + struct sensor_attr_data *sdata;
>>>> + int rc, i;
>>>> +
>>>> + /* each sensor has 'label' and 'input' attributes */
>>>> + sensor_groups[type].group.attrs =
>>>> + devm_kzalloc(dev, sizeof(struct attribute *) *
>>>> + sensor_num * 2 + 1, GFP_KERNEL);
>>>> + if (!sensor_groups[type].group.attrs) {
>>>> + rc = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>>>> + sensor_groups[type].sattr =
>>>> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
>>>> + sensor_num * 2, GFP_KERNEL);
>>>> + if (!sensor_groups[type].sattr) {
>>>> + rc = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>>>> + for (i = 0; i < sensor_num; i++) {
>>>> + sdata = &sensor_groups[type].sattr[i];
>>>> + /* hwmon attributes index starts from 1 */
>>>> + sdata->hwmon_index = i + 1;
>>>> + sdata->type = type;
>>>> + sensor_attr_init(sdata, sensor_groups[type].name, "input",
>>>> + show_input);
>>>> + sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
>>>> +
>>>> + sdata = &sensor_groups[type].sattr[i + sensor_num];
>>>> + sdata->hwmon_index = i + 1;
>>>> + sdata->type = type;
>>>> + sensor_attr_init(sdata, sensor_groups[type].name, "label",
>>>> + show_label);
>>>> + sensor_groups[type].group.attrs[i + sensor_num] =
>>>> + &sdata->dev_attr.attr;
>>>> + }
>>>> +
>>>> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
>>>> + if (rc)
>>>> + goto err;
>>>> +
>>>> + return 0;
>>>> +err:
>>>> + deinit_sensor_groups(dev, sensor_groups);
>>>> + return rc;
>>>> +}
>>>> +
>>>> +static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
>>>> + char *attr_name, uint32_t hwmon_index,
>>>> + uint32_t attr_id)
>>>> +{
>>>> + sdata->type = CAPS;
>>>> + sdata->hwmon_index = hwmon_index;
>>>> + sdata->attr_id = attr_id;
>>>> +
>>>> + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
>>>> + "caps", sdata->hwmon_index, attr_name);
>>>> +
>>>> + sysfs_attr_init(&sdata->dev_attr.attr);
>>>> + sdata->dev_attr.attr.name = sdata->name;
>>>> + sdata->dev_attr.attr.mode = S_IRUGO;
>>>> + sdata->dev_attr.show = show_caps;
>>>> +}
>>>> +
>>>> +static int create_caps_sensor_group(struct occ_sysfs *driver, int sensor_num)
>>>> +{
>>>> + struct device *dev = driver->dev;
>>>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>>>> + int field_num = driver->num_caps_fields;
>>>> + struct sensor_attr_data *sdata;
>>>> + int i, j, rc;
>>>> +
>>>> + sensor_groups[CAPS].group.attrs =
>>>> + devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
>>>> + field_num + 1, GFP_KERNEL);
>>>> + if (!sensor_groups[CAPS].group.attrs) {
>>>> + rc = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>>>> + sensor_groups[CAPS].sattr =
>>>> + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
>>>> + sensor_num * field_num, GFP_KERNEL);
>>>> + if (!sensor_groups[CAPS].sattr) {
>>>> + rc = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>>>> + for (j = 0; j < sensor_num; ++j) {
>>>> + for (i = 0; i < field_num; ++i) {
>>>> + sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
>>>> + caps_sensor_attr_init(sdata,
>>>> + driver->caps_names[i], j + 1, i);
>>>> + sensor_groups[CAPS].group.attrs[j * field_num + i] =
>>>> + &sdata->dev_attr.attr;
>>>> + }
>>>> + }
>>>> +
>>>> + rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
>>>> + if (rc)
>>>> + goto err;
>>>> +
>>>> + return rc;
>>>> +err:
>>>> + deinit_sensor_groups(dev, sensor_groups);
>>>> + return rc;
>>>> +}
>>>> +
>>>> +static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
>>>> +{
>>>> + struct device *dev = driver->dev;
>>>> +
>>>> + device_remove_file(dev, &dev_attr_user_powercap);
>>>> + device_remove_file(dev, &dev_attr_update_interval);
>>>> + device_remove_file(dev, &dev_attr_name);
>>>> +}
>>>> +
>>>> +static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
>>>> +{
>>>> + int i, rc, id, sensor_num;
>>>> + struct device *dev = driver->dev;
>>>> + struct sensor_group *sensor_groups = driver->sensor_groups;
>>>> + struct occ_blocks *resp = NULL;
>>>> +
>>>> + occ_get_response_blocks(driver->occ, &resp);
>>>> +
>>>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
>>>> + resp->sensor_block_id[i] = -1;
>>>> +
>>>> + /* read sensor data from occ */
>>>> + rc = occ_update_device(driver->occ);
>>>> + if (rc) {
>>>> + dev_err(dev, "cannot get occ sensor data: %d\n", rc);
>>>> + return rc;
>>>> + }
>>>> + if (!resp->blocks)
>>>> + return -ENOMEM;
>>>> +
>>>> + rc = device_create_file(dev, &dev_attr_name);
>>>> + if (rc)
>>>> + goto error;
>>>> +
>>>> + rc = device_create_file(dev, &dev_attr_update_interval);
>>>> + if (rc)
>>>> + goto error;
>>>> +
>>>> + if (resp->sensor_block_id[CAPS] >= 0) {
>>>> + /* user powercap: only for master OCC */
>>>> + rc = device_create_file(dev, &dev_attr_user_powercap);
>>>> + if (rc)
>>>> + goto error;
>>>> + }
>>>> +
>>>> + sensor_groups[FREQ].name = "freq";
>>>> + sensor_groups[TEMP].name = "temp";
>>>> + sensor_groups[POWER].name = "power";
>>>> + sensor_groups[CAPS].name = "caps";
>>>> +
>>>> + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
>>>> + id = resp->sensor_block_id[i];
>>>> + if (id < 0)
>>>> + continue;
>>>> +
>>>> + sensor_num = resp->blocks[id].header.sensor_num;
>>>> + if (i == CAPS)
>>>> + rc = create_caps_sensor_group(driver, sensor_num);
>>>> + else
>>>> + rc = create_sensor_group(driver, i, sensor_num);
>>>> + if (rc)
>>>> + goto error;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> +error:
>>>> + dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
>>>> + occ_remove_hwmon_attrs(driver);
>>>> + return rc;
>>>> +}
>>>> +
>>>> +static ssize_t show_occ_online(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> +
>>>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
>>>> +}
>>>> +
>>>> +static ssize_t store_occ_online(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct occ_sysfs *driver = dev_get_drvdata(dev);
>>>> + unsigned long val;
>>>> + int rc;
>>>> +
>>>> + rc = kstrtoul(buf, 10, &val);
>>>> + if (rc)
>>>> + return rc;
>>>> +
>>>> + if (val == 1) {
>>>> + if (driver->occ_online)
>>>> + return count;
>>>> +
>>>> + driver->dev = hwmon_device_register(dev);
>>>
>>> hwmon_device_register() is deprecated. Please consider using
>>> devm_hwmon_device_register_with_info() or at least
>>> hwmon_device_register_with_info().
>>>
>>> Uuh ... and registering a hwmon device based on writing into a sysfs attribute
>>> is completely out of the question.
>>>
>>> Thanks,
>>> Guenter
>>>
>>>> + if (IS_ERR(driver->dev))
>>>> + return PTR_ERR(driver->dev);
>>>> +
>>>> + dev_set_drvdata(driver->dev, driver);
>>>> +
>>>> + rc = occ_create_hwmon_attrs(driver);
>>>> + if (rc) {
>>>> + hwmon_device_unregister(driver->dev);
>>>> + driver->dev = NULL;
>>>> + return rc;
>>>> + }
>>>> + } else if (val == 0) {
>>>> + if (!driver->occ_online)
>>>> + return count;
>>>> +
>>>> + occ_remove_hwmon_attrs(driver);
>>>> + hwmon_device_unregister(driver->dev);
>>>> + driver->dev = NULL;
>>>> + } else
>>>> + return -EINVAL;
>>>> +
>>>> + driver->occ_online = val;
>>>> + return count;
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
>>>> + store_occ_online);
>>>> +
>>>> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>>>> + struct occ_sysfs_config *config)
>>>> +{
>>>> + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
>>>> + GFP_KERNEL);
>>>> + int rc;
>>>> +
>>>> + if (!hwmon)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + hwmon->occ = occ;
>>>> + hwmon->num_caps_fields = config->num_caps_fields;
>>>> + hwmon->caps_names = config->caps_names;
>>>> +
>>>> + dev_set_drvdata(dev, hwmon);
>>>> +
>>>> + rc = device_create_file(dev, &dev_attr_online);
>>>> + if (rc)
>>>> + return ERR_PTR(rc);
>>>> +
>>>> + return hwmon;
>>>> +}
>>>> +EXPORT_SYMBOL(occ_sysfs_start);
>>>> +
>>>> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
>>>> +{
>>>> + if (driver->dev) {
>>>> + occ_remove_hwmon_attrs(driver);
>>>> + hwmon_device_unregister(driver->dev);
>>>> + }
>>>> +
>>>> + device_remove_file(driver->dev, &dev_attr_online);
>>>> +
>>>> + devm_kfree(dev, driver);
>>>
>>> Thw point of using devm_ functions is not to require remove/free functions.
>>> Something is completely wrong here if you need that call.
>>>
>>> Overall, this is architectually completely wrong. One does not register
>>> or instantiate drivers based on writing into sysfs attributes. Please
>>> reconsider your approach.
>>>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(occ_sysfs_stop);
>>>> +
>>>> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
>>>> +MODULE_DESCRIPTION("OCC sysfs driver");
>>>> +MODULE_LICENSE("GPL");
>>>> diff --git a/drivers/hwmon/occ/occ_sysfs.h b/drivers/hwmon/occ/occ_sysfs.h
>>>> new file mode 100644
>>>> index 0000000..2a8044f
>>>> --- /dev/null
>>>> +++ b/drivers/hwmon/occ/occ_sysfs.h
>>>> @@ -0,0 +1,52 @@
>>>> +/*
>>>> + * occ_sysfs.h - OCC sysfs interface
>>>> + *
>>>> + * This file contains the data structures and function prototypes for the OCC
>>>> + * hwmon sysfs entries.
>>>> + *
>>>> + * Copyright 2016 IBM Corp.
>>>> + *
>>>> + * 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.
>>>> + */
>>>> +
>>>> +#ifndef __OCC_SYSFS_H__
>>>> +#define __OCC_SYSFS_H__
>>>> +
>>>> +#include "occ.h"
>>>> +
>>>> +struct sensor_group {
>>>> + char *name;
>>>> + struct sensor_attr_data *sattr;
>>>> + struct attribute_group group;
>>>> +};
>>>> +
>>>> +struct occ_sysfs_config {
>>>> + unsigned int num_caps_fields;
>>>> + char **caps_names;
>>>> +};
>>>> +
>>>> +struct occ_sysfs {
>>>> + struct device *dev;
>>>> + struct occ *occ;
>>>> +
>>>> + u16 user_powercap;
>>>> + bool occ_online;
>>>> + struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
>>>> + unsigned long update_interval;
>>>> + unsigned int num_caps_fields;
>>>> + char **caps_names;
>>>> +};
>>>> +
>>>> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>>>> + struct occ_sysfs_config *config);
>>>> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
>>>> +
>>>> +#endif /* __OCC_SYSFS_H__ */
>>>> --
>>>> 1.9.1
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" 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] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-03 23:49 ` Brad Bishop
@ 2017-01-04 0:06 ` Joel Stanley
2017-01-04 0:13 ` Brad Bishop
0 siblings, 1 reply; 30+ messages in thread
From: Joel Stanley @ 2017-01-04 0:06 UTC (permalink / raw)
To: Brad Bishop; +Cc: Eddie James, Jeremy Kerr, Andrew Jeffery, OpenBMC Maillist
On Wed, Jan 4, 2017 at 10:49 AM, Brad Bishop
<bradleyb@fuzziesquirrel.com> wrote:
> What if the driver is built into the kernel? Is there such a thing as module only drivers?
Our kernel is build with module loading disabled at present.
modprobing the driver when userspace is ready is a neat hack that we
could use for now. However, the driver will need a mechanism that
tells it to bind to the hardware when it becomes available. The same
mechanism would be useful for the fsi driver to initiate a scan and
bind it's client drivers.
Cheers,
Joel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-04 0:06 ` Joel Stanley
@ 2017-01-04 0:13 ` Brad Bishop
2017-01-04 0:25 ` Andrew Jeffery
0 siblings, 1 reply; 30+ messages in thread
From: Brad Bishop @ 2017-01-04 0:13 UTC (permalink / raw)
To: Joel Stanley; +Cc: Eddie James, Jeremy Kerr, Andrew Jeffery, OpenBMC Maillist
> On Jan 3, 2017, at 7:06 PM, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, Jan 4, 2017 at 10:49 AM, Brad Bishop
> <bradleyb@fuzziesquirrel.com> wrote:
>> What if the driver is built into the kernel? Is there such a thing as module only drivers?
>
> Our kernel is build with module loading disabled at present.
>
> modprobing the driver when userspace is ready is a neat hack that we
> could use for now. However, the driver will need a mechanism that
Do you happen to know if ‘a mechanism’ is something that needs to be invented or
if Linux already has something?
> tells it to bind to the hardware when it becomes available. The same
> mechanism would be useful for the fsi driver to initiate a scan and
> bind it's client drivers.
>
> Cheers,
>
> Joel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-04 0:13 ` Brad Bishop
@ 2017-01-04 0:25 ` Andrew Jeffery
2017-01-04 1:53 ` Andrew Jeffery
0 siblings, 1 reply; 30+ messages in thread
From: Andrew Jeffery @ 2017-01-04 0:25 UTC (permalink / raw)
To: Brad Bishop, Joel Stanley; +Cc: Eddie James, Jeremy Kerr, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]
On Tue, 2017-01-03 at 19:13 -0500, Brad Bishop wrote:
> > > > On Jan 3, 2017, at 7:06 PM, Joel Stanley <joel@jms.id.au> wrote:
> >
> > On Wed, Jan 4, 2017 at 10:49 AM, Brad Bishop
> > > > <bradleyb@fuzziesquirrel.com> wrote:
> > > What if the driver is built into the kernel? Is there such a thing as module only drivers?
> >
> > Our kernel is build with module loading disabled at present.
> >
> > modprobing the driver when userspace is ready is a neat hack that we
> > could use for now. However, the driver will need a mechanism that
>
> Do you happen to know if ‘a mechanism’ is something that needs to be invented or
> if Linux already has something?
I spoke with Rusty about this and by his response there's no existing
"event-bus" mechanism. His suggestions for the problem were:
1. As userspace is coordinating the host power-on, do the modprobe in
userspace
2. Push the power-on logic into the kernel so it can trigger hotplug
events to probe the drivers, and expose a power-{on,off}/{soft,hard}-
reboot interface to userspace.
Andrew
>
> > tells it to bind to the hardware when it becomes available. The same
> > mechanism would be useful for the fsi driver to initiate a scan and
> > bind it's client drivers.
> >
> > Cheers,
> >
> > Joel
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-04 0:25 ` Andrew Jeffery
@ 2017-01-04 1:53 ` Andrew Jeffery
2017-01-04 15:07 ` Eddie James
0 siblings, 1 reply; 30+ messages in thread
From: Andrew Jeffery @ 2017-01-04 1:53 UTC (permalink / raw)
To: Eddie James; +Cc: Jeremy Kerr, OpenBMC Maillist, Brad Bishop, Joel Stanley
[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]
On Wed, 2017-01-04 at 10:55 +1030, Andrew Jeffery wrote:
> On Tue, 2017-01-03 at 19:13 -0500, Brad Bishop wrote:
> > > > > On Jan 3, 2017, at 7:06 PM, Joel Stanley <joel@jms.id.au>
> > > > > wrote:
> > >
> > > On Wed, Jan 4, 2017 at 10:49 AM, Brad Bishop
> > > > > <bradleyb@fuzziesquirrel.com> wrote:
> > > >
> > > > What if the driver is built into the kernel? Is there such a
> > > > thing as module only drivers?
> > >
> > > Our kernel is build with module loading disabled at present.
> > >
> > > modprobing the driver when userspace is ready is a neat hack that
> > > we
> > > could use for now. However, the driver will need a mechanism that
> >
> > Do you happen to know if ‘a mechanism’ is something that needs to
> > be invented or
> > if Linux already has something?
>
> I spoke with Rusty about this and by his response there's no existing
> "event-bus" mechanism. His suggestions for the problem were:
>
> 1. As userspace is coordinating the host power-on, do the modprobe in
> userspace
After some discussion between Jeremy, Joel, Alistair and myself it was
decided that managing the problem from userspace was the way to go.
The rough approach is to describe the devices in the devicetree, but
mark them as 'status = "disabled";' so they are not bound to drivers on
boot. It is then userspace's job to trigger the binding as necessary,
but exactly how it should be done is unknown.
Eddie: Can you investigate what we might need to do to make this work?
Andrew
>
> 2. Push the power-on logic into the kernel so it can trigger hotplug
> events to probe the drivers, and expose a power-{on,off}/{soft,hard}-
> reboot interface to userspace.
>
> Andrew
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-04 1:53 ` Andrew Jeffery
@ 2017-01-04 15:07 ` Eddie James
0 siblings, 0 replies; 30+ messages in thread
From: Eddie James @ 2017-01-04 15:07 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Jeremy Kerr, OpenBMC Maillist, Brad Bishop, Joel Stanley
Will do Andrew.
Just had some discussion with Brad and Patrick. I think we can use the
"bind/unbind" sysfs api to cause the occ i2c device to probe when we
want it to. Will investigate that today.
Thanks,
Eddie
On Tue, Jan 3, 2017 at 7:53 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Wed, 2017-01-04 at 10:55 +1030, Andrew Jeffery wrote:
>> On Tue, 2017-01-03 at 19:13 -0500, Brad Bishop wrote:
>> > > > > On Jan 3, 2017, at 7:06 PM, Joel Stanley <joel@jms.id.au>
>> > > > > wrote:
>> > >
>> > > On Wed, Jan 4, 2017 at 10:49 AM, Brad Bishop
>> > > > > <bradleyb@fuzziesquirrel.com> wrote:
>> > > >
>> > > > What if the driver is built into the kernel? Is there such a
>> > > > thing as module only drivers?
>> > >
>> > > Our kernel is build with module loading disabled at present.
>> > >
>> > > modprobing the driver when userspace is ready is a neat hack that
>> > > we
>> > > could use for now. However, the driver will need a mechanism that
>> >
>> > Do you happen to know if ‘a mechanism’ is something that needs to
>> > be invented or
>> > if Linux already has something?
>>
>> I spoke with Rusty about this and by his response there's no existing
>> "event-bus" mechanism. His suggestions for the problem were:
>>
>> 1. As userspace is coordinating the host power-on, do the modprobe in
>> userspace
>
> After some discussion between Jeremy, Joel, Alistair and myself it was
> decided that managing the problem from userspace was the way to go.
>
> The rough approach is to describe the devices in the devicetree, but
> mark them as 'status = "disabled";' so they are not bound to drivers on
> boot. It is then userspace's job to trigger the binding as necessary,
> but exactly how it should be done is unknown.
>
> Eddie: Can you investigate what we might need to do to make this work?
>
> Andrew
>
>>
>> 2. Push the power-on logic into the kernel so it can trigger hotplug
>> events to probe the drivers, and expose a power-{on,off}/{soft,hard}-
>> reboot interface to userspace.
>>
>> Andrew
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2016-12-30 19:34 ` Guenter Roeck
2017-01-03 14:30 ` Fwd: " Brad Bishop
@ 2017-01-06 22:17 ` Edward James
2017-01-07 17:15 ` Guenter Roeck
1 sibling, 1 reply; 30+ messages in thread
From: Edward James @ 2017-01-06 22:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: andrew, corbet, devicetree, eajames.ibm, jdelvare, joel,
linux-doc, linux-hwmon, linux-i2c, linux-kernel, mark.rutland,
robh+dt, wsa
[-- Attachment #1: Type: text/plain, Size: 24685 bytes --]
Hi,
Thanks for your comments Guenter. Sorry to switch email addresses, but
gmail is rejecting my messages to this thread.
Guenter Roeck <linux@roeck-us.net> wrote on 12/30/2016 01:34:04 PM:
> On Fri, Dec 30, 2016 at 11:56:04AM -0600, eajames.ibm@gmail.com wrote:
> > From: "Edward A. James" <eajames@us.ibm.com>
> >
> > Add a generic mechanism to expose the sensors provided by the OCC in
> > sysfs.
> >
> > Signed-off-by: Edward A. James <eajames@us.ibm.com>
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > Documentation/hwmon/occ | 48 +++++
> > drivers/hwmon/occ/Makefile | 2 +-
> > drivers/hwmon/occ/occ_sysfs.c | 492 +++++++++++++++++++++++++++++
> +++++++++++++
> > drivers/hwmon/occ/occ_sysfs.h | 52 +++++
> > 4 files changed, 593 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/hwmon/occ/occ_sysfs.c
> > create mode 100644 drivers/hwmon/occ/occ_sysfs.h
> >
> > diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
> > index 79d1642..1ee8689 100644
> > --- a/Documentation/hwmon/occ
> > +++ b/Documentation/hwmon/occ
> > @@ -25,6 +25,54 @@ Currently, all versions of the OCC support four
> types of sensor data: power,
> > temperature, frequency, and "caps," which indicate limits and
> thresholds used
> > internally on the OCC.
> >
> > +sysfs Entries
> > +-------------
> > +
> > +The OCC driver uses the hwmon sysfs framework to provide data to
userspace.
> > +
> > +The driver exports two sysfs files for each frequency,
> temperature, and power
> > +sensor. These are "input" and "label". The input file contains
> the value of the
> > +sensor. The label file contains the sensor id. The sensor id is the
unique
> > +internal OCC identifier. Sensor ids may be provided by the OCC
> specification.
> > +The names of these files will be in the following format:
> > + <sensor type><sensor index>_input
> > + <sensor type><sensor index>_label
> > +Sensor types will be one of "temp", "freq", or "power". The sensor
index is
> > +an index to differentiate different sensor files. For example, a
single
> > +temperature sensor will have two sysfs files: temp1_input and
temp1_label.
> > +
> > +Caps sensors are exported differently. For each caps sensor, the
> driver will
> > +export 6 entries:
> > + curr_powercap - current power cap in watts
> > + curr_powerreading - current power output in watts
> > + norm_powercap - power cap without redundant power
> > + max_powercap - maximum power cap that can be set in watts
> > + min_powercap - minimum power cap that can be set in watts
> > + user_powerlimit - power limit specified by the user in watts
> > +In addition, the OCC driver for P9 will export a 7th entry:
> > + user_powerlimit_source - can be one of two values depending on who
set
> > + the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
> > + in band from other source.
> > +The format for these files is caps<sensor index>_<entry type>. For
example,
> > +caps1_curr_powercap.
> > +
> > +The driver also provides a number of sysfs entries through hwmon to
better
> > +control the driver and monitor the OCC.
> > + powercap - read or write the OCC user power limit in watts.
> > + name - read the name of the driver
> > + update_interval - read or write the minimum interval for polling
the
> > + OCC.
> > +
> > +The driver also exports a single sysfs file through the
> communication protocol
> > +device (see BMC - Host Communications). The filename is "online"
> and represents
> > +the status of the OCC with respect to the driver. The OCC can be
> in one of two
> > +states: OCC polling enabled or OCC polling disabled. The purpose
> of this file
> > +is to control the behavior of the driver and it's hwmon sysfs
> entries, not to
> > +infer any information about the state of the physical OCC. Reading the
file
> > +returns either a 0 (polling disabled) or 1 (polling enabled).
> Writing 1 to the
> > +file enables OCC polling in the driver if communications can be
established
> > +with the OCC. Writing a 0 to the driver disables OCC polling.
> > +
> > BMC - Host Communications
> > -------------------------
> >
> > diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> > index 93cb52f..a6881f9 100644
> > --- a/drivers/hwmon/occ/Makefile
> > +++ b/drivers/hwmon/occ/Makefile
> > @@ -1 +1 @@
> > -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
> > +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
> > diff --git a/drivers/hwmon/occ/occ_sysfs.c
b/drivers/hwmon/occ/occ_sysfs.c
> > new file mode 100644
> > index 0000000..b0e063da
> > --- /dev/null
> > +++ b/drivers/hwmon/occ/occ_sysfs.c
> > @@ -0,0 +1,492 @@
> > +/*
> > + * occ_sysfs.c - OCC sysfs interface
> > + *
> > + * This file contains the methods and data structures for
> implementing the OCC
> > + * hwmon sysfs entries.
> > + *
> > + * Copyright 2016 IBM Corp.
> > + *
> > + * 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.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/err.h>
> > +#include <linux/mutex.h>
> > +#include <linux/delay.h>
> > +#include <linux/kernel.h>
> > +#include <linux/device.h>
> > +
> > +#include "occ_sysfs.h"
> > +
> > +#define MAX_SENSOR_ATTR_LEN 32
> > +
> > +#define RESP_RETURN_CMD_INVAL 0x13
> > +
> > +struct sensor_attr_data {
> > + enum sensor_type type;
> > + u32 hwmon_index;
> > + u32 attr_id;
> > + char name[MAX_SENSOR_ATTR_LEN];
> > + struct device_attribute dev_attr;
> > +};
> > +
> > +static ssize_t show_input(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int val;
> > + struct sensor_attr_data *sdata = container_of(attr,
> > + struct sensor_attr_data,
> > + dev_attr);
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > +
> > + val = occ_get_sensor_value(driver->occ, sdata->type,
> > + sdata->hwmon_index - 1);
> > + if (sdata->type == TEMP)
> > + val *= 1000; /* in millidegree Celsius */
> > +
> > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> > +}
> > +
> > +/* show_label provides the OCC sensor id. The sensor id will be either
a
> > + * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
> > + * identify what each sensor represents, according to the OCC
> specification.
> > + */
> > +static ssize_t show_label(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int val;
> > + struct sensor_attr_data *sdata = container_of(attr,
> > + struct sensor_attr_data,
> > + dev_attr);
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > +
> > + val = occ_get_sensor_id(driver->occ, sdata->type,
> > + sdata->hwmon_index - 1);
> > +
> > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> > +}
> > +
> > +static ssize_t show_caps(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int val;
> > + struct caps_sensor *sensor;
> > + struct sensor_attr_data *sdata = container_of(attr,
> > + struct sensor_attr_data,
> > + dev_attr);
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > +
> > + sensor = occ_get_sensor(driver->occ, CAPS);
> > + if (!sensor) {
> > + val = -1;
> > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> > + }
> > +
> > + val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index -
1,
> > + sdata->attr_id);
> > +
> > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> > +}
> > +
> > +static ssize_t show_update_interval(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > +
> > + return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->
update_interval);
> > +}
> > +
> > +static ssize_t store_update_interval(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > + unsigned long val;
> > + int rc;
> > +
> > + rc = kstrtoul(buf, 10, &val);
> > + if (rc)
> > + return rc;
> > +
> > + driver->update_interval = val;
> > + occ_set_update_interval(driver->occ, val);
> > +
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO,
> show_update_interval,
> > + store_update_interval);
> > +
> > +static ssize_t show_name(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + return snprintf(buf, PAGE_SIZE - 1, "occ\n");
> > +}
> > +
> > +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
> > +
> > +static ssize_t show_user_powercap(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > +
> > + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
> > +}
> > +
> > +static ssize_t store_user_powercap(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > + u16 val;
> > + int rc;
> > +
> > + rc = kstrtou16(buf, 10, &val);
> > + if (rc)
> > + return rc;
> > +
> > + dev_dbg(dev, "set user powercap to: %d\n", val);
> > + rc = occ_set_user_powercap(driver->occ, val);
> > + if (rc) {
> > + dev_err(dev, "set user powercap failed: 0x%x\n", rc);
> > + if (rc == RESP_RETURN_CMD_INVAL) {
> > + dev_err(dev, "set invalid powercap value: %d\n", val);
> > + return -EINVAL;
> > + }
> > +
> > + return rc;
> > + }
> > +
> > + driver->user_powercap = val;
> > +
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO,
show_user_powercap,
> > + store_user_powercap);
> > +
> > +static void deinit_sensor_groups(struct device *dev,
> > + struct sensor_group *sensor_groups)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
> > + if (sensor_groups[i].group.attrs)
> > + devm_kfree(dev, sensor_groups[i].group.attrs);
> > + if (sensor_groups[i].sattr)
> > + devm_kfree(dev, sensor_groups[i].sattr);
> > + sensor_groups[i].group.attrs = NULL;
> > + sensor_groups[i].sattr = NULL;
> > + }
> > +}
> > +
> > +static void sensor_attr_init(struct sensor_attr_data *sdata,
> > + char *sensor_group_name,
> > + char *attr_name,
> > + ssize_t (*show)(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf))
> > +{
> > + sysfs_attr_init(&sdata->dev_attr.attr);
> > +
> > + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
> > + sensor_group_name, sdata->hwmon_index, attr_name);
> > + sdata->dev_attr.attr.name = sdata->name;
> > + sdata->dev_attr.attr.mode = S_IRUGO;
> > + sdata->dev_attr.show = show;
> > +}
> > +
> > +static int create_sensor_group(struct occ_sysfs *driver,
> > + enum sensor_type type, int sensor_num)
> > +{
> > + struct device *dev = driver->dev;
> > + struct sensor_group *sensor_groups = driver->sensor_groups;
> > + struct sensor_attr_data *sdata;
> > + int rc, i;
> > +
> > + /* each sensor has 'label' and 'input' attributes */
> > + sensor_groups[type].group.attrs =
> > + devm_kzalloc(dev, sizeof(struct attribute *) *
> > + sensor_num * 2 + 1, GFP_KERNEL);
> > + if (!sensor_groups[type].group.attrs) {
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + sensor_groups[type].sattr =
> > + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
> > + sensor_num * 2, GFP_KERNEL);
> > + if (!sensor_groups[type].sattr) {
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + for (i = 0; i < sensor_num; i++) {
> > + sdata = &sensor_groups[type].sattr[i];
> > + /* hwmon attributes index starts from 1 */
> > + sdata->hwmon_index = i + 1;
> > + sdata->type = type;
> > + sensor_attr_init(sdata, sensor_groups[type].name, "input",
> > + show_input);
> > + sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
> > +
> > + sdata = &sensor_groups[type].sattr[i + sensor_num];
> > + sdata->hwmon_index = i + 1;
> > + sdata->type = type;
> > + sensor_attr_init(sdata, sensor_groups[type].name, "label",
> > + show_label);
> > + sensor_groups[type].group.attrs[i + sensor_num] =
> > + &sdata->dev_attr.attr;
> > + }
> > +
> > + rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
> > + if (rc)
> > + goto err;
> > +
> > + return 0;
> > +err:
> > + deinit_sensor_groups(dev, sensor_groups);
> > + return rc;
> > +}
> > +
> > +static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
> > + char *attr_name, uint32_t hwmon_index,
> > + uint32_t attr_id)
> > +{
> > + sdata->type = CAPS;
> > + sdata->hwmon_index = hwmon_index;
> > + sdata->attr_id = attr_id;
> > +
> > + snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
> > + "caps", sdata->hwmon_index, attr_name);
> > +
> > + sysfs_attr_init(&sdata->dev_attr.attr);
> > + sdata->dev_attr.attr.name = sdata->name;
> > + sdata->dev_attr.attr.mode = S_IRUGO;
> > + sdata->dev_attr.show = show_caps;
> > +}
> > +
> > +static int create_caps_sensor_group(struct occ_sysfs *driver, int
> sensor_num)
> > +{
> > + struct device *dev = driver->dev;
> > + struct sensor_group *sensor_groups = driver->sensor_groups;
> > + int field_num = driver->num_caps_fields;
> > + struct sensor_attr_data *sdata;
> > + int i, j, rc;
> > +
> > + sensor_groups[CAPS].group.attrs =
> > + devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
> > + field_num + 1, GFP_KERNEL);
> > + if (!sensor_groups[CAPS].group.attrs) {
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + sensor_groups[CAPS].sattr =
> > + devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
> > + sensor_num * field_num, GFP_KERNEL);
> > + if (!sensor_groups[CAPS].sattr) {
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + for (j = 0; j < sensor_num; ++j) {
> > + for (i = 0; i < field_num; ++i) {
> > + sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
> > + caps_sensor_attr_init(sdata,
> > + driver->caps_names[i], j + 1, i);
> > + sensor_groups[CAPS].group.attrs[j * field_num + i] =
> > + &sdata->dev_attr.attr;
> > + }
> > + }
> > +
> > + rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
> > + if (rc)
> > + goto err;
> > +
> > + return rc;
> > +err:
> > + deinit_sensor_groups(dev, sensor_groups);
> > + return rc;
> > +}
> > +
> > +static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
> > +{
> > + struct device *dev = driver->dev;
> > +
> > + device_remove_file(dev, &dev_attr_user_powercap);
> > + device_remove_file(dev, &dev_attr_update_interval);
> > + device_remove_file(dev, &dev_attr_name);
> > +}
> > +
> > +static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
> > +{
> > + int i, rc, id, sensor_num;
> > + struct device *dev = driver->dev;
> > + struct sensor_group *sensor_groups = driver->sensor_groups;
> > + struct occ_blocks *resp = NULL;
> > +
> > + occ_get_response_blocks(driver->occ, &resp);
> > +
> > + for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
> > + resp->sensor_block_id[i] = -1;
> > +
> > + /* read sensor data from occ */
> > + rc = occ_update_device(driver->occ);
> > + if (rc) {
> > + dev_err(dev, "cannot get occ sensor data: %d\n", rc);
> > + return rc;
> > + }
> > + if (!resp->blocks)
> > + return -ENOMEM;
> > +
> > + rc = device_create_file(dev, &dev_attr_name);
> > + if (rc)
> > + goto error;
> > +
> > + rc = device_create_file(dev, &dev_attr_update_interval);
> > + if (rc)
> > + goto error;
> > +
> > + if (resp->sensor_block_id[CAPS] >= 0) {
> > + /* user powercap: only for master OCC */
> > + rc = device_create_file(dev, &dev_attr_user_powercap);
> > + if (rc)
> > + goto error;
> > + }
> > +
> > + sensor_groups[FREQ].name = "freq";
> > + sensor_groups[TEMP].name = "temp";
> > + sensor_groups[POWER].name = "power";
> > + sensor_groups[CAPS].name = "caps";
> > +
> > + for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
> > + id = resp->sensor_block_id[i];
> > + if (id < 0)
> > + continue;
> > +
> > + sensor_num = resp->blocks[id].header.sensor_num;
> > + if (i == CAPS)
> > + rc = create_caps_sensor_group(driver, sensor_num);
> > + else
> > + rc = create_sensor_group(driver, i, sensor_num);
> > + if (rc)
> > + goto error;
> > + }
> > +
> > + return 0;
> > +
> > +error:
> > + dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
> > + occ_remove_hwmon_attrs(driver);
> > + return rc;
> > +}
> > +
> > +static ssize_t show_occ_online(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > +
> > + return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
> > +}
> > +
> > +static ssize_t store_occ_online(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct occ_sysfs *driver = dev_get_drvdata(dev);
> > + unsigned long val;
> > + int rc;
> > +
> > + rc = kstrtoul(buf, 10, &val);
> > + if (rc)
> > + return rc;
> > +
> > + if (val == 1) {
> > + if (driver->occ_online)
> > + return count;
> > +
> > + driver->dev = hwmon_device_register(dev);
>
> hwmon_device_register() is deprecated. Please consider using
> devm_hwmon_device_register_with_info() or at least
> hwmon_device_register_with_info().
OK, I'll address this.
>
> Uuh ... and registering a hwmon device based on writing into a
sysfsattribute
> is completely out of the question.
>
> Thanks,
> Guenter
>
> > + if (IS_ERR(driver->dev))
> > + return PTR_ERR(driver->dev);
> > +
> > + dev_set_drvdata(driver->dev, driver);
> > +
> > + rc = occ_create_hwmon_attrs(driver);
> > + if (rc) {
> > + hwmon_device_unregister(driver->dev);
> > + driver->dev = NULL;
> > + return rc;
> > + }
> > + } else if (val == 0) {
> > + if (!driver->occ_online)
> > + return count;
> > +
> > + occ_remove_hwmon_attrs(driver);
> > + hwmon_device_unregister(driver->dev);
> > + driver->dev = NULL;
> > + } else
> > + return -EINVAL;
> > +
> > + driver->occ_online = val;
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
> > + store_occ_online);
> > +
> > +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> > + struct occ_sysfs_config *config)
> > +{
> > + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct
occ_sysfs),
> > + GFP_KERNEL);
> > + int rc;
> > +
> > + if (!hwmon)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + hwmon->occ = occ;
> > + hwmon->num_caps_fields = config->num_caps_fields;
> > + hwmon->caps_names = config->caps_names;
> > +
> > + dev_set_drvdata(dev, hwmon);
> > +
> > + rc = device_create_file(dev, &dev_attr_online);
> > + if (rc)
> > + return ERR_PTR(rc);
> > +
> > + return hwmon;
> > +}
> > +EXPORT_SYMBOL(occ_sysfs_start);
> > +
> > +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
> > +{
> > + if (driver->dev) {
> > + occ_remove_hwmon_attrs(driver);
> > + hwmon_device_unregister(driver->dev);
> > + }
> > +
> > + device_remove_file(driver->dev, &dev_attr_online);
> > +
> > + devm_kfree(dev, driver);
>
> Thw point of using devm_ functions is not to require remove/free
functions.
> Something is completely wrong here if you need that call.
>
> Overall, this is architectually completely wrong. One does not register
> or instantiate drivers based on writing into sysfs attributes. Please
> reconsider your approach.
We had some trouble designing this driver because the BMC only has
access to the OCC once the processor is powered on. This will happen
sometime after the BMC boots (this driver runs only on the BMC). With
no access to the OCC, we don't know what sensors are present on the
system without a large static enumeration. Also any sysfs files created
before we have OCC access won't be able to return any data.
Instead of the "online" attribute, what do you think about using the
"bind"/"unbind" API to probe the device from user space once the system
is powered on? All the hwmon registration would take place in the probe
function, it would just occur some time after boot.
Thanks again,
Eddie
>
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(occ_sysfs_stop);
> > +
> > +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> > +MODULE_DESCRIPTION("OCC sysfs driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/hwmon/occ/occ_sysfs.h
b/drivers/hwmon/occ/occ_sysfs.h
> > new file mode 100644
> > index 0000000..2a8044f
> > --- /dev/null
> > +++ b/drivers/hwmon/occ/occ_sysfs.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * occ_sysfs.h - OCC sysfs interface
> > + *
> > + * This file contains the data structures and function prototypes
> for the OCC
> > + * hwmon sysfs entries.
> > + *
> > + * Copyright 2016 IBM Corp.
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef __OCC_SYSFS_H__
> > +#define __OCC_SYSFS_H__
> > +
> > +#include "occ.h"
> > +
> > +struct sensor_group {
> > + char *name;
> > + struct sensor_attr_data *sattr;
> > + struct attribute_group group;
> > +};
> > +
> > +struct occ_sysfs_config {
> > + unsigned int num_caps_fields;
> > + char **caps_names;
> > +};
> > +
> > +struct occ_sysfs {
> > + struct device *dev;
> > + struct occ *occ;
> > +
> > + u16 user_powercap;
> > + bool occ_online;
> > + struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
> > + unsigned long update_interval;
> > + unsigned int num_caps_fields;
> > + char **caps_names;
> > +};
> > +
> > +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> > + struct occ_sysfs_config *config);
> > +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
> > +
> > +#endif /* __OCC_SYSFS_H__ */
> > --
> > 1.9.1
> >
>
[-- Attachment #2: Type: text/html, Size: 35433 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-06 22:17 ` Edward James
@ 2017-01-07 17:15 ` Guenter Roeck
2017-01-08 23:52 ` Andrew Jeffery
2017-01-10 17:41 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 30+ messages in thread
From: Guenter Roeck @ 2017-01-07 17:15 UTC (permalink / raw)
To: Edward James
Cc: andrew, corbet, devicetree, eajames.ibm, jdelvare, joel,
linux-doc, linux-hwmon, linux-i2c, linux-kernel, mark.rutland,
robh+dt, wsa
On 01/06/2017 02:17 PM, Edward James wrote:
[ ... ]
>> > +}
>> > +
>> > +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
>> > + store_occ_online);
>> > +
>> > +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
>> > + struct occ_sysfs_config *config)
>> > +{
>> > + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
>> > + GFP_KERNEL);
>> > + int rc;
>> > +
>> > + if (!hwmon)
>> > + return ERR_PTR(-ENOMEM);
>> > +
>> > + hwmon->occ = occ;
>> > + hwmon->num_caps_fields = config->num_caps_fields;
>> > + hwmon->caps_names = config->caps_names;
>> > +
>> > + dev_set_drvdata(dev, hwmon);
>> > +
>> > + rc = device_create_file(dev, &dev_attr_online);
>> > + if (rc)
>> > + return ERR_PTR(rc);
>> > +
>> > + return hwmon;
>> > +}
>> > +EXPORT_SYMBOL(occ_sysfs_start);
>> > +
>> > +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
>> > +{
>> > + if (driver->dev) {
>> > + occ_remove_hwmon_attrs(driver);
>> > + hwmon_device_unregister(driver->dev);
>> > + }
>> > +
>> > + device_remove_file(driver->dev, &dev_attr_online);
>> > +
>> > + devm_kfree(dev, driver);
>>
>> Thw point of using devm_ functions is not to require remove/free functions.
>> Something is completely wrong here if you need that call.
>>
>> Overall, this is architectually completely wrong. One does not register
>> or instantiate drivers based on writing into sysfs attributes. Please
>> reconsider your approach.
>
> We had some trouble designing this driver because the BMC only has
> access to the OCC once the processor is powered on. This will happen
> sometime after the BMC boots (this driver runs only on the BMC). With
> no access to the OCC, we don't know what sensors are present on the
> system without a large static enumeration. Also any sysfs files created
> before we have OCC access won't be able to return any data.
>
> Instead of the "online" attribute, what do you think about using the
> "bind"/"unbind" API to probe the device from user space once the system
> is powered on? All the hwmon registration would take place in the probe
> function, it would just occur some time after boot.
>
A more common approach would be to have a platform driver. That platform
driver would need a means to detect if the OCC is up and running, and
instantiate everything else once it is.
A trigger from user space is problematic because there is no guarantee
that the OCC is really up (or that it even exists).
An alternative might be to have the hwmon driver poll for the OCC,
but that would be a bit more difficult and might require a kernel thread
or maybe asynchronous probing.
Guenter
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-07 17:15 ` Guenter Roeck
@ 2017-01-08 23:52 ` Andrew Jeffery
2017-01-10 17:45 ` Benjamin Herrenschmidt
2017-01-10 17:41 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 30+ messages in thread
From: Andrew Jeffery @ 2017-01-08 23:52 UTC (permalink / raw)
To: Guenter Roeck, Edward James
Cc: corbet, devicetree, eajames.ibm, jdelvare, joel, linux-doc,
linux-hwmon, linux-i2c, linux-kernel, mark.rutland, robh+dt, wsa
[-- Attachment #1: Type: text/plain, Size: 4017 bytes --]
On Sat, 2017-01-07 at 09:15 -0800, Guenter Roeck wrote:
> On 01/06/2017 02:17 PM, Edward James wrote:
>
> [ ... ]
>
> > > > +}
> > > > +
> > > > +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
> > > > + store_occ_online);
> > > > +
> > > > +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> > > > + struct occ_sysfs_config *config)
> > > > +{
> > > > + struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
> > > > + GFP_KERNEL);
> > > > + int rc;
> > > > +
> > > > + if (!hwmon)
> > > > + return ERR_PTR(-ENOMEM);
> > > > +
> > > > + hwmon->occ = occ;
> > > > + hwmon->num_caps_fields = config->num_caps_fields;
> > > > + hwmon->caps_names = config->caps_names;
> > > > +
> > > > + dev_set_drvdata(dev, hwmon);
> > > > +
> > > > + rc = device_create_file(dev, &dev_attr_online);
> > > > + if (rc)
> > > > + return ERR_PTR(rc);
> > > > +
> > > > + return hwmon;
> > > > +}
> > > > +EXPORT_SYMBOL(occ_sysfs_start);
> > > > +
> > > > +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
> > > > +{
> > > > + if (driver->dev) {
> > > > + occ_remove_hwmon_attrs(driver);
> > > > + hwmon_device_unregister(driver->dev);
> > > > + }
> > > > +
> > > > + device_remove_file(driver->dev, &dev_attr_online);
> > > > +
> > > > + devm_kfree(dev, driver);
> > >
> > > Thw point of using devm_ functions is not to require remove/free functions.
> > > Something is completely wrong here if you need that call.
> > >
> > > Overall, this is architectually completely wrong. One does not register
> > > or instantiate drivers based on writing into sysfs attributes. Please
> > > reconsider your approach.
> >
> > We had some trouble designing this driver because the BMC only has
> > access to the OCC once the processor is powered on. This will happen
> > sometime after the BMC boots (this driver runs only on the BMC). With
> > no access to the OCC, we don't know what sensors are present on the
> > system without a large static enumeration. Also any sysfs files created
> > before we have OCC access won't be able to return any data.
> >
> > Instead of the "online" attribute, what do you think about using the
> > "bind"/"unbind" API to probe the device from user space once the system
> > is powered on? All the hwmon registration would take place in the probe
> > function, it would just occur some time after boot.
> >
>
> A more common approach would be to have a platform driver. That platform
> driver would need a means to detect if the OCC is up and running, and
> instantiate everything else once it is.
>
> A trigger from user space is problematic because there is no guarantee
> that the OCC is really up (or that it even exists).
This is true in general, but for the BMC case we have more information:
The host CPU power supply is controlled by several GPIOs from
userspace. Once we receive the "power-good" signal for the host CPU we
can bind the OCC driver and trigger the probe.
Alternatively, in the style of your first para, we could push the host
CPU state management into the kernel and expose a boot/reboot/power-off
API to userspace. That would give us a place to hook calls for
configuring and cleaning up any host-dependent drivers on the BMC.
The solution to the host-power-state problem is also applicable to the
OpenFSI patches that were recently sent out:
https://lkml.org/lkml/2016/12/6/732
The OpenFSI infra needs to re-scan for CFAMs when the host is powered
up.
>
> An alternative might be to have the hwmon driver poll for the OCC,
> but that would be a bit more difficult and might require a kernel thread
> or maybe asynchronous probing.
This was our thought as a fallback solution.
Andrew
>
> Guenter
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-08 23:52 ` Andrew Jeffery
@ 2017-01-10 17:45 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Herrenschmidt @ 2017-01-10 17:45 UTC (permalink / raw)
To: Andrew Jeffery, Guenter Roeck, Edward James
Cc: corbet, devicetree, eajames.ibm, jdelvare, joel, linux-doc,
linux-hwmon, linux-i2c, linux-kernel, mark.rutland, robh+dt, wsa
On Mon, 2017-01-09 at 10:22 +1030, Andrew Jeffery wrote:
> Alternatively, in the style of your first para, we could push the
> host
> CPU state management into the kernel and expose a boot/reboot/power-
> off
> API to userspace. That would give us a place to hook calls for
> configuring and cleaning up any host-dependent drivers on the BMC.
That's nasty. Each machine has a subtly different way of controller
host power, that would mean a different kernel driver for each of them,
which we are trying to avoid.
This is userspace policy.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-07 17:15 ` Guenter Roeck
2017-01-08 23:52 ` Andrew Jeffery
@ 2017-01-10 17:41 ` Benjamin Herrenschmidt
2017-01-10 17:51 ` Guenter Roeck
1 sibling, 1 reply; 30+ messages in thread
From: Benjamin Herrenschmidt @ 2017-01-10 17:41 UTC (permalink / raw)
To: Guenter Roeck, Edward James
Cc: andrew, corbet, devicetree, eajames.ibm, jdelvare, joel,
linux-doc, linux-hwmon, linux-i2c, linux-kernel, mark.rutland,
robh+dt, wsa
On Sat, 2017-01-07 at 09:15 -0800, Guenter Roeck wrote:
> > Instead of the "online" attribute, what do you think about using the
> > "bind"/"unbind" API to probe the device from user space once the system
> > is powered on? All the hwmon registration would take place in the probe
> > function, it would just occur some time after boot.
> >
>
> A more common approach would be to have a platform driver. That platform
> driver would need a means to detect if the OCC is up and running, and
> instantiate everything else once it is.
>
> A trigger from user space is problematic because there is no guarantee
> that the OCC is really up (or that it even exists).
>
> An alternative might be to have the hwmon driver poll for the OCC,
> but that would be a bit more difficult and might require a kernel thread
> or maybe asynchronous probing.
Hi Guenter !
I'm not sure I agree with you here ;-)
I'm don't know how much background you got (I missed the beginning of
the discussion) but basically this driver runs on the BMC (system
controller) of the POWER9 server, the OCC is inside the POWER9 chip
itself.
So the presence/power state of the OCC doesn't depend on the BMC
platform kernel code. The BMC userspace controls power up and down to the POWER9, and thus it's the one to know whether the remote is up.
If we were to create a "platform driver", all it would do is get input
from userspace exactly like that sysfs file :-)
So if you don't like the sysfs file that registers/de-registers, which
I sort-of understand, it's a bit of a hack (though a rather efficient
one), I think the bind/unbind approach makes sense. However, I wonder
whether the simplest and most efficient (remember this BMC has a really
slow CPU) is an "online" file sysfs file, though rather than
registering/de-registering the hwmon it would simply make it stop
accessing the HW (and return some known "offline" values).
Cheers,
Ben.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-10 17:41 ` Benjamin Herrenschmidt
@ 2017-01-10 17:51 ` Guenter Roeck
2017-01-10 23:01 ` Edward James
0 siblings, 1 reply; 30+ messages in thread
From: Guenter Roeck @ 2017-01-10 17:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Edward James, andrew, corbet, devicetree, eajames.ibm, jdelvare,
joel, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
mark.rutland, robh+dt, wsa
On Tue, Jan 10, 2017 at 11:41:44AM -0600, Benjamin Herrenschmidt wrote:
> On Sat, 2017-01-07 at 09:15 -0800, Guenter Roeck wrote:
> > > Instead of the "online" attribute, what do you think about using the
> > > "bind"/"unbind" API to probe the device from user space once the system
> > > is powered on? All the hwmon registration would take place in the probe
> > > function, it would just occur some time after boot.
> > >
> >
> > A more common approach would be to have a platform driver. That platform
> > driver would need a means to detect if the OCC is up and running, and
> > instantiate everything else once it is.
> >
> > A trigger from user space is problematic because there is no guarantee
> > that the OCC is really up (or that it even exists).
> >
> > An alternative might be to have the hwmon driver poll for the OCC,
> > but that would be a bit more difficult and might require a kernel thread
> > or maybe asynchronous probing.
>
> Hi Guenter !
>
> I'm not sure I agree with you here ;-)
>
> I'm don't know how much background you got (I missed the beginning of
> the discussion) but basically this driver runs on the BMC (system
> controller) of the POWER9 server, the OCC is inside the POWER9 chip
> itself.
>
> So the presence/power state of the OCC doesn't depend on the BMC
> platform kernel code. The BMC userspace controls power up and down to the POWER9, and thus it's the one to know whether the remote is up.
>
> If we were to create a "platform driver", all it would do is get input
> from userspace exactly like that sysfs file :-)
>
> So if you don't like the sysfs file that registers/de-registers, which
> I sort-of understand, it's a bit of a hack (though a rather efficient
> one), I think the bind/unbind approach makes sense. However, I wonder
> whether the simplest and most efficient (remember this BMC has a really
> slow CPU) is an "online" file sysfs file, though rather than
> registering/de-registering the hwmon it would simply make it stop
> accessing the HW (and return some known "offline" values).
>
I don't like that too much either; it still looks like a hack.
How about using bind/unbind then ?
Guenter
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
2017-01-10 17:51 ` Guenter Roeck
@ 2017-01-10 23:01 ` Edward James
0 siblings, 0 replies; 30+ messages in thread
From: Edward James @ 2017-01-10 23:01 UTC (permalink / raw)
To: Guenter Roeck
Cc: andrew, Benjamin Herrenschmidt, corbet, devicetree, eajames.ibm,
jdelvare, joel, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
mark.rutland, robh+dt, wsa
[-- Attachment #1: Type: text/plain, Size: 3175 bytes --]
Guenter Roeck <linux@roeck-us.net> wrote on 01/10/2017 11:51:12 AM:
> From: Guenter Roeck <linux@roeck-us.net>
> To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Edward James/Austin/IBM@IBMUS, andrew@aj.id.au, corbet@lwn.net,
> devicetree@vger.kernel.org, eajames.ibm@gmail.com,
> jdelvare@suse.com, joel@jms.id.au, linux-doc@vger.kernel.org, linux-
> hwmon@vger.kernel.org, linux-i2c@vger.kernel.org, linux-
> kernel@vger.kernel.org, mark.rutland@arm.com, robh+dt@kernel.org,
> wsa@the-dreams.de
> Date: 01/10/2017 11:51 AM
> Subject: Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
>
> On Tue, Jan 10, 2017 at 11:41:44AM -0600, Benjamin Herrenschmidt wrote:
> > On Sat, 2017-01-07 at 09:15 -0800, Guenter Roeck wrote:
> > > > Instead of the "online" attribute, what do you think about using
the
> > > > "bind"/"unbind" API to probe the device from user space once the
system
> > > > is powered on? All the hwmon registration would take place in the
probe
> > > > function, it would just occur some time after boot.
> > > >
> > >
> > > A more common approach would be to have a platform driver. That
platform
> > > driver would need a means to detect if the OCC is up and running, and
> > > instantiate everything else once it is.
> > >
> > > A trigger from user space is problematic because there is no
guarantee
> > > that the OCC is really up (or that it even exists).
> > >
> > > An alternative might be to have the hwmon driver poll for the OCC,
> > > but that would be a bit more difficult and might require a kernel
thread
> > > or maybe asynchronous probing.
> >
> > Hi Guenter !
> >
> > I'm not sure I agree with you here ;-)
> >
> > I'm don't know how much background you got (I missed the beginning of
> > the discussion) but basically this driver runs on the BMC (system
> > controller) of the POWER9 server, the OCC is inside the POWER9 chip
> > itself.
> >
> > So the presence/power state of the OCC doesn't depend on the BMC
> > platform kernel code. The BMC userspace controls power up and down
> to the POWER9, and thus it's the one to know whether the remote is up.
> >
> > If we were to create a "platform driver", all it would do is get input
> > from userspace exactly like that sysfs file :-)
> >
> > So if you don't like the sysfs file that registers/de-registers, which
> > I sort-of understand, it's a bit of a hack (though a rather efficient
> > one), I think the bind/unbind approach makes sense. However, I wonder
> > whether the simplest and most efficient (remember this BMC has a really
> > slow CPU) is an "online" file sysfs file, though rather than
> > registering/de-registering the hwmon it would simply make it stop
> > accessing the HW (and return some known "offline" values).
> >
>
> I don't like that too much either; it still looks like a hack.
> How about using bind/unbind then ?
Bind/unbind works very well in testing. Only thing we have to figure out is
how to stop the driver loading during BMC boot... but I don't think the
solution will be in this patch set. I'll put together a v2 for tomorrow.
Thanks,
Eddie
>
> Guenter
>
[-- Attachment #2: Type: text/html, Size: 3912 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH linux 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations
@ 2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
0 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
Add functions to send SCOM operations over I2C bus. The BMC can
communicate with the Power8 host processor over I2C, but needs to use SCOM
operations in order to access the OCC register space.
Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/occ_scom_i2c.c | 73 ++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_scom_i2c.h | 26 ++++++++++++++
2 files changed, 99 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h
diff --git a/drivers/hwmon/occ/occ_scom_i2c.c b/drivers/hwmon/occ/occ_scom_i2c.c
new file mode 100644
index 0000000..a922f83
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.c
@@ -0,0 +1,73 @@
+/*
+ * occ_scom_i2c.c - hwmon OCC driver
+ *
+ * This file contains the functions for performing SCOM operations over I2C bus
+ * to access the OCC.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+
+int occ_i2c_getscom(void *bus, u32 address, u64 *data)
+{
+ ssize_t rc;
+ u64 buf;
+ struct i2c_client *client = bus;
+
+ rc = i2c_master_send(client, (const char *)&address, sizeof(u32));
+ if (rc < 0)
+ return rc;
+ else if (rc != sizeof(u32))
+ return -EIO;
+
+ rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
+ if (rc < 0)
+ return rc;
+ else if (rc != sizeof(u64))
+ return -EIO;
+
+ *data = be64_to_cpu(buf);
+
+ return 0;
+}
+EXPORT_SYMBOL(occ_i2c_getscom);
+
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+ u32 buf[3];
+ ssize_t rc;
+ struct i2c_client *client = bus;
+
+ buf[0] = address;
+ buf[1] = data1;
+ buf[2] = data0;
+
+ rc = i2c_master_send(client, (const char *)buf, sizeof(u32) * 3);
+ if (rc < 0)
+ return rc;
+ else if (rc != sizeof(u32) * 3)
+ return -EIO;
+
+ return 0;
+}
+EXPORT_SYMBOL(occ_i2c_putscom);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("I2C OCC SCOM transport");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_scom_i2c.h b/drivers/hwmon/occ/occ_scom_i2c.h
new file mode 100644
index 0000000..945739c
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.h
@@ -0,0 +1,26 @@
+/*
+ * occ_scom_i2c.h - hwmon OCC driver
+ *
+ * This file contains function protoypes for peforming SCOM operations over I2C
+ * bus to access the OCC.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_SCOM_I2C_H__
+#define __OCC_SCOM_I2C_H__
+
+int occ_i2c_getscom(void *bus, u32 address, u64 *data);
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1);
+
+#endif /* __OCC_SCOM_I2C_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH linux 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations
@ 2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
0 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux-0h96xk9xTtrk1uMJSBkQmQ
Cc: jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
wsa-z923LK4zBo2bacvFa/9K2g, andrew-zrmu5oMJ5Fs,
joel-U3u1mxZcP9KHXe+LvDLADg, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Edward A. James
From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Add functions to send SCOM operations over I2C bus. The BMC can
communicate with the Power8 host processor over I2C, but needs to use SCOM
operations in order to access the OCC register space.
Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
drivers/hwmon/occ/occ_scom_i2c.c | 73 ++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_scom_i2c.h | 26 ++++++++++++++
2 files changed, 99 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h
diff --git a/drivers/hwmon/occ/occ_scom_i2c.c b/drivers/hwmon/occ/occ_scom_i2c.c
new file mode 100644
index 0000000..a922f83
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.c
@@ -0,0 +1,73 @@
+/*
+ * occ_scom_i2c.c - hwmon OCC driver
+ *
+ * This file contains the functions for performing SCOM operations over I2C bus
+ * to access the OCC.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+
+int occ_i2c_getscom(void *bus, u32 address, u64 *data)
+{
+ ssize_t rc;
+ u64 buf;
+ struct i2c_client *client = bus;
+
+ rc = i2c_master_send(client, (const char *)&address, sizeof(u32));
+ if (rc < 0)
+ return rc;
+ else if (rc != sizeof(u32))
+ return -EIO;
+
+ rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
+ if (rc < 0)
+ return rc;
+ else if (rc != sizeof(u64))
+ return -EIO;
+
+ *data = be64_to_cpu(buf);
+
+ return 0;
+}
+EXPORT_SYMBOL(occ_i2c_getscom);
+
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+ u32 buf[3];
+ ssize_t rc;
+ struct i2c_client *client = bus;
+
+ buf[0] = address;
+ buf[1] = data1;
+ buf[2] = data0;
+
+ rc = i2c_master_send(client, (const char *)buf, sizeof(u32) * 3);
+ if (rc < 0)
+ return rc;
+ else if (rc != sizeof(u32) * 3)
+ return -EIO;
+
+ return 0;
+}
+EXPORT_SYMBOL(occ_i2c_putscom);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("I2C OCC SCOM transport");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_scom_i2c.h b/drivers/hwmon/occ/occ_scom_i2c.h
new file mode 100644
index 0000000..945739c
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.h
@@ -0,0 +1,26 @@
+/*
+ * occ_scom_i2c.h - hwmon OCC driver
+ *
+ * This file contains function protoypes for peforming SCOM operations over I2C
+ * bus to access the OCC.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_SCOM_I2C_H__
+#define __OCC_SCOM_I2C_H__
+
+int occ_i2c_getscom(void *bus, u32 address, u64 *data);
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1);
+
+#endif /* __OCC_SCOM_I2C_H__ */
--
1.9.1
--
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] 30+ messages in thread
* [PATCH linux 4/6] hwmon: occ: Add callbacks for parsing P8 OCC datastructures
@ 2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
0 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
Add functions to parse the data structures that are specific to the OCC on
the POWER8 processor. These are the sensor data structures, including
temperature, frequency, power, and "caps."
Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/hwmon/occ | 9 ++
drivers/hwmon/occ/occ_p8.c | 217 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_p8.h | 30 +++++++
3 files changed, 256 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_p8.c
create mode 100644 drivers/hwmon/occ/occ_p8.h
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
index 1ee8689..a6b3dd6 100644
--- a/Documentation/hwmon/occ
+++ b/Documentation/hwmon/occ
@@ -25,6 +25,15 @@ Currently, all versions of the OCC support four types of sensor data: power,
temperature, frequency, and "caps," which indicate limits and thresholds used
internally on the OCC.
+The format for the POWER8 OCC sensor data can be found in the P8 OCC
+specification:
+github.com/open-power/docs/blob/master/occ/OCC_OpenPwr_FW_Interfaces.pdf
+This document provides the details of the OCC sensors: power, frequency,
+temperature, and caps. These sensor formats are specific to the POWER8 OCC. A
+number of data structures, such as command format, response headers, and the
+like, are also defined in this specification, and are common to both POWER8 and
+POWER9 OCCs.
+
sysfs Entries
-------------
diff --git a/drivers/hwmon/occ/occ_p8.c b/drivers/hwmon/occ/occ_p8.c
new file mode 100644
index 0000000..812df16
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.c
@@ -0,0 +1,217 @@
+/*
+ * occ_p8.c - OCC hwmon driver
+ *
+ * This file contains the Power8-specific methods and data structures for
+ * the OCC hwmon driver.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <asm/unaligned.h>
+
+#include "occ.h"
+#include "occ_p8.h"
+
+/* P8 OCC sensor data format */
+struct p8_occ_sensor {
+ u16 sensor_id;
+ u16 value;
+};
+
+struct p8_power_sensor {
+ u16 sensor_id;
+ u32 update_tag;
+ u32 accumulator;
+ u16 value;
+};
+
+struct p8_caps_sensor {
+ u16 curr_powercap;
+ u16 curr_powerreading;
+ u16 norm_powercap;
+ u16 max_powercap;
+ u16 min_powercap;
+ u16 user_powerlimit;
+};
+
+void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off,
+ int snum)
+{
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ {
+ struct p8_occ_sensor *os =
+ &(((struct p8_occ_sensor *)sensor)[snum]);
+
+ os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+ }
+ break;
+ case POWER:
+ {
+ struct p8_power_sensor *ps =
+ &(((struct p8_power_sensor *)sensor)[snum]);
+
+ ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ ps->update_tag =
+ be32_to_cpu(get_unaligned((u32 *)&data[off + 2]));
+ ps->accumulator =
+ be32_to_cpu(get_unaligned((u32 *)&data[off + 6]));
+ ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+ }
+ break;
+ case CAPS:
+ {
+ struct p8_caps_sensor *cs =
+ &(((struct p8_caps_sensor *)sensor)[snum]);
+
+ cs->curr_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ cs->curr_powerreading =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+ cs->norm_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+ cs->max_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 6]));
+ cs->min_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 8]));
+ cs->user_powerlimit =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+ }
+ break;
+ };
+}
+
+void *p8_alloc_sensor(int sensor_type, int num_sensors)
+{
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return kcalloc(num_sensors, sizeof(struct p8_occ_sensor),
+ GFP_KERNEL);
+ case POWER:
+ return kcalloc(num_sensors, sizeof(struct p8_power_sensor),
+ GFP_KERNEL);
+ case CAPS:
+ return kcalloc(num_sensors, sizeof(struct p8_caps_sensor),
+ GFP_KERNEL);
+ default:
+ return NULL;
+ }
+}
+
+int p8_get_sensor_value(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+
+ if (sensor_type == CAPS)
+ return -EINVAL;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -ENODEV;
+
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return ((struct p8_occ_sensor *)sensor)[snum].value;
+ case POWER:
+ return ((struct p8_power_sensor *)sensor)[snum].value;
+ default:
+ return -EINVAL;
+ }
+}
+
+int p8_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+ int i = snum;
+
+ if (sensor_type == CAPS)
+ return -EINVAL;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -ENODEV;
+
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return ((struct p8_occ_sensor *)sensor)[i].sensor_id;
+ case POWER:
+ return ((struct p8_power_sensor *)sensor)[i].sensor_id;
+ default:
+ return -EINVAL;
+ }
+}
+
+int p8_get_caps_value(void *sensor, int snum, int caps_field)
+{
+ struct p8_caps_sensor *caps_sensor = sensor;
+
+ switch (caps_field) {
+ case 0:
+ return caps_sensor[snum].curr_powercap;
+ case 1:
+ return caps_sensor[snum].curr_powerreading;
+ case 2:
+ return caps_sensor[snum].norm_powercap;
+ case 3:
+ return caps_sensor[snum].max_powercap;
+ case 4:
+ return caps_sensor[snum].min_powercap;
+ case 5:
+ return caps_sensor[snum].user_powerlimit;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct occ_ops p8_ops = {
+ .parse_sensor = p8_parse_sensor,
+ .alloc_sensor = p8_alloc_sensor,
+ .get_sensor_value = p8_get_sensor_value,
+ .get_sensor_id = p8_get_sensor_id,
+ .get_caps_value = p8_get_caps_value,
+};
+
+static const struct occ_config p8_config = {
+ .command_addr = 0xFFFF6000,
+ .response_addr = 0xFFFF7000,
+};
+
+struct occ *p8_occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops)
+{
+ return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config);
+}
+EXPORT_SYMBOL(p8_occ_start);
+
+int p8_occ_stop(struct occ *occ)
+{
+ return occ_stop(occ);
+}
+EXPORT_SYMBOL(p8_occ_stop);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("P8 OCC sensors");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_p8.h b/drivers/hwmon/occ/occ_p8.h
new file mode 100644
index 0000000..b44cdd4
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.h
@@ -0,0 +1,30 @@
+/*
+ * occ_p8.h - OCC hwmon driver
+ *
+ * This file contains Power8-specific function prototypes
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_P8_H__
+#define __OCC_P8_H__
+
+#include "scom.h"
+
+struct device;
+
+struct occ *p8_occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops);
+int p8_occ_stop(struct occ *occ);
+
+#endif /* __OCC_P8_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH linux 4/6] hwmon: occ: Add callbacks for parsing P8 OCC datastructures
@ 2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
0 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux-0h96xk9xTtrk1uMJSBkQmQ
Cc: jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
wsa-z923LK4zBo2bacvFa/9K2g, andrew-zrmu5oMJ5Fs,
joel-U3u1mxZcP9KHXe+LvDLADg, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Edward A. James
From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Add functions to parse the data structures that are specific to the OCC on
the POWER8 processor. These are the sensor data structures, including
temperature, frequency, power, and "caps."
Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Reviewed-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
Documentation/hwmon/occ | 9 ++
drivers/hwmon/occ/occ_p8.c | 217 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_p8.h | 30 +++++++
3 files changed, 256 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_p8.c
create mode 100644 drivers/hwmon/occ/occ_p8.h
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
index 1ee8689..a6b3dd6 100644
--- a/Documentation/hwmon/occ
+++ b/Documentation/hwmon/occ
@@ -25,6 +25,15 @@ Currently, all versions of the OCC support four types of sensor data: power,
temperature, frequency, and "caps," which indicate limits and thresholds used
internally on the OCC.
+The format for the POWER8 OCC sensor data can be found in the P8 OCC
+specification:
+github.com/open-power/docs/blob/master/occ/OCC_OpenPwr_FW_Interfaces.pdf
+This document provides the details of the OCC sensors: power, frequency,
+temperature, and caps. These sensor formats are specific to the POWER8 OCC. A
+number of data structures, such as command format, response headers, and the
+like, are also defined in this specification, and are common to both POWER8 and
+POWER9 OCCs.
+
sysfs Entries
-------------
diff --git a/drivers/hwmon/occ/occ_p8.c b/drivers/hwmon/occ/occ_p8.c
new file mode 100644
index 0000000..812df16
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.c
@@ -0,0 +1,217 @@
+/*
+ * occ_p8.c - OCC hwmon driver
+ *
+ * This file contains the Power8-specific methods and data structures for
+ * the OCC hwmon driver.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <asm/unaligned.h>
+
+#include "occ.h"
+#include "occ_p8.h"
+
+/* P8 OCC sensor data format */
+struct p8_occ_sensor {
+ u16 sensor_id;
+ u16 value;
+};
+
+struct p8_power_sensor {
+ u16 sensor_id;
+ u32 update_tag;
+ u32 accumulator;
+ u16 value;
+};
+
+struct p8_caps_sensor {
+ u16 curr_powercap;
+ u16 curr_powerreading;
+ u16 norm_powercap;
+ u16 max_powercap;
+ u16 min_powercap;
+ u16 user_powerlimit;
+};
+
+void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off,
+ int snum)
+{
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ {
+ struct p8_occ_sensor *os =
+ &(((struct p8_occ_sensor *)sensor)[snum]);
+
+ os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+ }
+ break;
+ case POWER:
+ {
+ struct p8_power_sensor *ps =
+ &(((struct p8_power_sensor *)sensor)[snum]);
+
+ ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ ps->update_tag =
+ be32_to_cpu(get_unaligned((u32 *)&data[off + 2]));
+ ps->accumulator =
+ be32_to_cpu(get_unaligned((u32 *)&data[off + 6]));
+ ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+ }
+ break;
+ case CAPS:
+ {
+ struct p8_caps_sensor *cs =
+ &(((struct p8_caps_sensor *)sensor)[snum]);
+
+ cs->curr_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ cs->curr_powerreading =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+ cs->norm_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+ cs->max_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 6]));
+ cs->min_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 8]));
+ cs->user_powerlimit =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+ }
+ break;
+ };
+}
+
+void *p8_alloc_sensor(int sensor_type, int num_sensors)
+{
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return kcalloc(num_sensors, sizeof(struct p8_occ_sensor),
+ GFP_KERNEL);
+ case POWER:
+ return kcalloc(num_sensors, sizeof(struct p8_power_sensor),
+ GFP_KERNEL);
+ case CAPS:
+ return kcalloc(num_sensors, sizeof(struct p8_caps_sensor),
+ GFP_KERNEL);
+ default:
+ return NULL;
+ }
+}
+
+int p8_get_sensor_value(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+
+ if (sensor_type == CAPS)
+ return -EINVAL;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -ENODEV;
+
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return ((struct p8_occ_sensor *)sensor)[snum].value;
+ case POWER:
+ return ((struct p8_power_sensor *)sensor)[snum].value;
+ default:
+ return -EINVAL;
+ }
+}
+
+int p8_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+ int i = snum;
+
+ if (sensor_type == CAPS)
+ return -EINVAL;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -ENODEV;
+
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return ((struct p8_occ_sensor *)sensor)[i].sensor_id;
+ case POWER:
+ return ((struct p8_power_sensor *)sensor)[i].sensor_id;
+ default:
+ return -EINVAL;
+ }
+}
+
+int p8_get_caps_value(void *sensor, int snum, int caps_field)
+{
+ struct p8_caps_sensor *caps_sensor = sensor;
+
+ switch (caps_field) {
+ case 0:
+ return caps_sensor[snum].curr_powercap;
+ case 1:
+ return caps_sensor[snum].curr_powerreading;
+ case 2:
+ return caps_sensor[snum].norm_powercap;
+ case 3:
+ return caps_sensor[snum].max_powercap;
+ case 4:
+ return caps_sensor[snum].min_powercap;
+ case 5:
+ return caps_sensor[snum].user_powerlimit;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct occ_ops p8_ops = {
+ .parse_sensor = p8_parse_sensor,
+ .alloc_sensor = p8_alloc_sensor,
+ .get_sensor_value = p8_get_sensor_value,
+ .get_sensor_id = p8_get_sensor_id,
+ .get_caps_value = p8_get_caps_value,
+};
+
+static const struct occ_config p8_config = {
+ .command_addr = 0xFFFF6000,
+ .response_addr = 0xFFFF7000,
+};
+
+struct occ *p8_occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops)
+{
+ return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config);
+}
+EXPORT_SYMBOL(p8_occ_start);
+
+int p8_occ_stop(struct occ *occ)
+{
+ return occ_stop(occ);
+}
+EXPORT_SYMBOL(p8_occ_stop);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("P8 OCC sensors");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_p8.h b/drivers/hwmon/occ/occ_p8.h
new file mode 100644
index 0000000..b44cdd4
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.h
@@ -0,0 +1,30 @@
+/*
+ * occ_p8.h - OCC hwmon driver
+ *
+ * This file contains Power8-specific function prototypes
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_P8_H__
+#define __OCC_P8_H__
+
+#include "scom.h"
+
+struct device;
+
+struct occ *p8_occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops);
+int p8_occ_stop(struct occ *occ);
+
+#endif /* __OCC_P8_H__ */
--
1.9.1
--
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] 30+ messages in thread
* [PATCH linux 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC
@ 2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
0 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
Add code to tie the hwmon sysfs code and the POWER8 OCC code together, as
well as probe the entire driver from the I2C bus. I2C is the communication
method between the BMC and the P8 OCC.
Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
.../devicetree/bindings/i2c/i2c-ibm-occ.txt | 13 ++
drivers/hwmon/occ/Kconfig | 14 ++
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/p8_occ_i2c.c | 141 +++++++++++++++++++++
4 files changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c
diff --git a/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
new file mode 100644
index 0000000..b0d2b36
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
@@ -0,0 +1,13 @@
+HWMON I2C driver for IBM POWER CPU OCC (On Chip Controller)
+
+Required properties:
+ - compatible: must be "ibm,p8-occ-i2c"
+ - reg: physical address
+
+Example:
+i2c3: i2c-bus@100 {
+ occ@50 {
+ compatible = "ibm,p8-occ-i2c";
+ reg = <0x50>;
+ };
+};
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
index cdb64a7..3a5188f 100644
--- a/drivers/hwmon/occ/Kconfig
+++ b/drivers/hwmon/occ/Kconfig
@@ -13,3 +13,17 @@ menuconfig SENSORS_PPC_OCC
This driver can also be built as a module. If so, the module
will be called occ.
+
+if SENSORS_PPC_OCC
+
+config SENSORS_PPC_OCC_P8_I2C
+ tristate "POWER8 OCC hwmon support"
+ depends on I2C
+ help
+ Provide a hwmon sysfs interface for the POWER8 On-Chip Controller,
+ exposing temperature, frequency and power measurements.
+
+ This driver can also be built as a module. If so, the module will be
+ called p8-occ-i2c.
+
+endif
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index a6881f9..9294b58 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
+obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o p8_occ_i2c.o
diff --git a/drivers/hwmon/occ/p8_occ_i2c.c b/drivers/hwmon/occ/p8_occ_i2c.c
new file mode 100644
index 0000000..0c65894
--- /dev/null
+++ b/drivers/hwmon/occ/p8_occ_i2c.c
@@ -0,0 +1,141 @@
+/*
+ * p8_occ_i2c.c - hwmon OCC driver
+ *
+ * This file contains the i2c layer for accessing the P8 OCC over i2c bus.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+#include "occ_p8.h"
+#include "occ_sysfs.h"
+
+#define P8_OCC_I2C_NAME "p8-occ-i2c"
+
+static char *caps_sensor_names[] = {
+ "curr_powercap",
+ "curr_powerreading",
+ "norm_powercap",
+ "max_powercap",
+ "min_powercap",
+ "user_powerlimit"
+};
+
+int p8_i2c_getscom(void *bus, u32 address, u64 *data)
+{
+ /* P8 i2c slave requires address to be shifted by 1 */
+ address = address << 1;
+
+ return occ_i2c_getscom(bus, address, data);
+}
+
+int p8_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+ /* P8 i2c slave requires address to be shifted by 1 */
+ address = address << 1;
+
+ return occ_i2c_putscom(bus, address, data0, data1);
+}
+
+static struct occ_bus_ops p8_bus_ops = {
+ .getscom = p8_i2c_getscom,
+ .putscom = p8_i2c_putscom,
+};
+
+static struct occ_sysfs_config p8_sysfs_config = {
+ .num_caps_fields = ARRAY_SIZE(caps_sensor_names),
+ .caps_names = caps_sensor_names,
+};
+
+static int p8_occ_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct occ *occ;
+ struct occ_sysfs *hwmon;
+
+ occ = p8_occ_start(&client->dev, client, &p8_bus_ops);
+ if (IS_ERR(occ))
+ return PTR_ERR(occ);
+
+ hwmon = occ_sysfs_start(&client->dev, occ, &p8_sysfs_config);
+ if (IS_ERR(hwmon))
+ return PTR_ERR(hwmon);
+
+ i2c_set_clientdata(client, hwmon);
+
+ return 0;
+}
+
+static int p8_occ_remove(struct i2c_client *client)
+{
+ int rc;
+ struct occ_sysfs *hwmon = i2c_get_clientdata(client);
+ struct occ *occ = hwmon->occ;
+
+ rc = occ_sysfs_stop(&client->dev, hwmon);
+
+ rc = rc || p8_occ_stop(occ);
+
+ return rc;
+}
+
+/* used by old-style board info. */
+static const struct i2c_device_id occ_ids[] = {
+ { P8_OCC_I2C_NAME, 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, occ_ids);
+
+/* used by device table */
+static const struct of_device_id occ_of_match[] = {
+ { .compatible = "ibm,p8-occ-i2c" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, occ_of_match);
+
+/*
+ * i2c-core uses i2c-detect() to detect device in below address list.
+ * If exists, address will be assigned to client.
+ * It is also possible to read address from device table.
+ */
+static const unsigned short normal_i2c[] = {0x50, 0x51, I2C_CLIENT_END };
+
+static struct i2c_driver p8_occ_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = P8_OCC_I2C_NAME,
+ .pm = NULL,
+ .of_match_table = occ_of_match,
+ },
+ .probe = p8_occ_probe,
+ .remove = p8_occ_remove,
+ .id_table = occ_ids,
+ .address_list = normal_i2c,
+};
+
+module_i2c_driver(p8_occ_driver);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("BMC P8 OCC hwmon driver");
+MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH linux 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC
@ 2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
0 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux-0h96xk9xTtrk1uMJSBkQmQ
Cc: jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
wsa-z923LK4zBo2bacvFa/9K2g, andrew-zrmu5oMJ5Fs,
joel-U3u1mxZcP9KHXe+LvDLADg, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Edward A. James
From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Add code to tie the hwmon sysfs code and the POWER8 OCC code together, as
well as probe the entire driver from the I2C bus. I2C is the communication
method between the BMC and the P8 OCC.
Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Reviewed-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
.../devicetree/bindings/i2c/i2c-ibm-occ.txt | 13 ++
drivers/hwmon/occ/Kconfig | 14 ++
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/p8_occ_i2c.c | 141 +++++++++++++++++++++
4 files changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c
diff --git a/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
new file mode 100644
index 0000000..b0d2b36
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
@@ -0,0 +1,13 @@
+HWMON I2C driver for IBM POWER CPU OCC (On Chip Controller)
+
+Required properties:
+ - compatible: must be "ibm,p8-occ-i2c"
+ - reg: physical address
+
+Example:
+i2c3: i2c-bus@100 {
+ occ@50 {
+ compatible = "ibm,p8-occ-i2c";
+ reg = <0x50>;
+ };
+};
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
index cdb64a7..3a5188f 100644
--- a/drivers/hwmon/occ/Kconfig
+++ b/drivers/hwmon/occ/Kconfig
@@ -13,3 +13,17 @@ menuconfig SENSORS_PPC_OCC
This driver can also be built as a module. If so, the module
will be called occ.
+
+if SENSORS_PPC_OCC
+
+config SENSORS_PPC_OCC_P8_I2C
+ tristate "POWER8 OCC hwmon support"
+ depends on I2C
+ help
+ Provide a hwmon sysfs interface for the POWER8 On-Chip Controller,
+ exposing temperature, frequency and power measurements.
+
+ This driver can also be built as a module. If so, the module will be
+ called p8-occ-i2c.
+
+endif
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index a6881f9..9294b58 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
+obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o p8_occ_i2c.o
diff --git a/drivers/hwmon/occ/p8_occ_i2c.c b/drivers/hwmon/occ/p8_occ_i2c.c
new file mode 100644
index 0000000..0c65894
--- /dev/null
+++ b/drivers/hwmon/occ/p8_occ_i2c.c
@@ -0,0 +1,141 @@
+/*
+ * p8_occ_i2c.c - hwmon OCC driver
+ *
+ * This file contains the i2c layer for accessing the P8 OCC over i2c bus.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+#include "occ_p8.h"
+#include "occ_sysfs.h"
+
+#define P8_OCC_I2C_NAME "p8-occ-i2c"
+
+static char *caps_sensor_names[] = {
+ "curr_powercap",
+ "curr_powerreading",
+ "norm_powercap",
+ "max_powercap",
+ "min_powercap",
+ "user_powerlimit"
+};
+
+int p8_i2c_getscom(void *bus, u32 address, u64 *data)
+{
+ /* P8 i2c slave requires address to be shifted by 1 */
+ address = address << 1;
+
+ return occ_i2c_getscom(bus, address, data);
+}
+
+int p8_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+ /* P8 i2c slave requires address to be shifted by 1 */
+ address = address << 1;
+
+ return occ_i2c_putscom(bus, address, data0, data1);
+}
+
+static struct occ_bus_ops p8_bus_ops = {
+ .getscom = p8_i2c_getscom,
+ .putscom = p8_i2c_putscom,
+};
+
+static struct occ_sysfs_config p8_sysfs_config = {
+ .num_caps_fields = ARRAY_SIZE(caps_sensor_names),
+ .caps_names = caps_sensor_names,
+};
+
+static int p8_occ_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct occ *occ;
+ struct occ_sysfs *hwmon;
+
+ occ = p8_occ_start(&client->dev, client, &p8_bus_ops);
+ if (IS_ERR(occ))
+ return PTR_ERR(occ);
+
+ hwmon = occ_sysfs_start(&client->dev, occ, &p8_sysfs_config);
+ if (IS_ERR(hwmon))
+ return PTR_ERR(hwmon);
+
+ i2c_set_clientdata(client, hwmon);
+
+ return 0;
+}
+
+static int p8_occ_remove(struct i2c_client *client)
+{
+ int rc;
+ struct occ_sysfs *hwmon = i2c_get_clientdata(client);
+ struct occ *occ = hwmon->occ;
+
+ rc = occ_sysfs_stop(&client->dev, hwmon);
+
+ rc = rc || p8_occ_stop(occ);
+
+ return rc;
+}
+
+/* used by old-style board info. */
+static const struct i2c_device_id occ_ids[] = {
+ { P8_OCC_I2C_NAME, 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, occ_ids);
+
+/* used by device table */
+static const struct of_device_id occ_of_match[] = {
+ { .compatible = "ibm,p8-occ-i2c" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, occ_of_match);
+
+/*
+ * i2c-core uses i2c-detect() to detect device in below address list.
+ * If exists, address will be assigned to client.
+ * It is also possible to read address from device table.
+ */
+static const unsigned short normal_i2c[] = {0x50, 0x51, I2C_CLIENT_END };
+
+static struct i2c_driver p8_occ_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = P8_OCC_I2C_NAME,
+ .pm = NULL,
+ .of_match_table = occ_of_match,
+ },
+ .probe = p8_occ_probe,
+ .remove = p8_occ_remove,
+ .id_table = occ_ids,
+ .address_list = normal_i2c,
+};
+
+module_i2c_driver(p8_occ_driver);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("BMC P8 OCC hwmon driver");
+MODULE_LICENSE("GPL");
--
1.9.1
--
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] 30+ messages in thread* Re: [PATCH linux 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC
2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
(?)
@ 2017-01-01 0:25 ` kbuild test robot
-1 siblings, 0 replies; 30+ messages in thread
From: kbuild test robot @ 2017-01-01 0:25 UTC (permalink / raw)
To: eajames.ibm
Cc: kbuild-all, linux, jdelvare, corbet, mark.rutland, robh+dt, wsa,
andrew, joel, devicetree, linux-doc, linux-hwmon, linux-i2c,
linux-kernel, Edward A. James
[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]
Hi Edward,
[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on v4.10-rc1 next-20161224]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/eajames-ibm-gmail-com/drivers-hwmon-Add-On-Chip-Controller-driver/20161231-021324
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: x86_64-randconfig-n0-01010656 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "occ_sysfs_start" [drivers/hwmon/occ/p8_occ_i2c.ko] undefined!
>> ERROR: "occ_sysfs_stop" [drivers/hwmon/occ/p8_occ_i2c.ko] undefined!
>> ERROR: "occ_stop" [drivers/hwmon/occ/occ_p8.ko] undefined!
>> ERROR: "occ_start" [drivers/hwmon/occ/occ_p8.ko] undefined!
>> ERROR: "occ_get_sensor" [drivers/hwmon/occ/occ_p8.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21665 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH linux 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC
2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
(?)
(?)
@ 2017-01-03 22:45 ` Rob Herring
-1 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2017-01-03 22:45 UTC (permalink / raw)
To: eajames.ibm
Cc: linux, jdelvare, corbet, mark.rutland, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
On Fri, Dec 30, 2016 at 11:56:07AM -0600, eajames.ibm@gmail.com wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> Add code to tie the hwmon sysfs code and the POWER8 OCC code together, as
> well as probe the entire driver from the I2C bus. I2C is the communication
> method between the BMC and the P8 OCC.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> .../devicetree/bindings/i2c/i2c-ibm-occ.txt | 13 ++
bindings/i2c/ is generally for host controllers. bindings/hwmon perhaps.
> drivers/hwmon/occ/Kconfig | 14 ++
> drivers/hwmon/occ/Makefile | 1 +
> drivers/hwmon/occ/p8_occ_i2c.c | 141 +++++++++++++++++++++
> 4 files changed, 169 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
> create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
> new file mode 100644
> index 0000000..b0d2b36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
> @@ -0,0 +1,13 @@
> +HWMON I2C driver for IBM POWER CPU OCC (On Chip Controller)
> +
> +Required properties:
> + - compatible: must be "ibm,p8-occ-i2c"
> + - reg: physical address
> +
> +Example:
> +i2c3: i2c-bus@100 {
> + occ@50 {
> + compatible = "ibm,p8-occ-i2c";
> + reg = <0x50>;
> + };
> +};
> diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
> index cdb64a7..3a5188f 100644
> --- a/drivers/hwmon/occ/Kconfig
> +++ b/drivers/hwmon/occ/Kconfig
> @@ -13,3 +13,17 @@ menuconfig SENSORS_PPC_OCC
>
> This driver can also be built as a module. If so, the module
> will be called occ.
> +
> +if SENSORS_PPC_OCC
> +
> +config SENSORS_PPC_OCC_P8_I2C
> + tristate "POWER8 OCC hwmon support"
> + depends on I2C
> + help
> + Provide a hwmon sysfs interface for the POWER8 On-Chip Controller,
> + exposing temperature, frequency and power measurements.
> +
> + This driver can also be built as a module. If so, the module will be
> + called p8-occ-i2c.
> +
> +endif
> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> index a6881f9..9294b58 100644
> --- a/drivers/hwmon/occ/Makefile
> +++ b/drivers/hwmon/occ/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
> +obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o p8_occ_i2c.o
> diff --git a/drivers/hwmon/occ/p8_occ_i2c.c b/drivers/hwmon/occ/p8_occ_i2c.c
> new file mode 100644
> index 0000000..0c65894
> --- /dev/null
> +++ b/drivers/hwmon/occ/p8_occ_i2c.c
> @@ -0,0 +1,141 @@
> +/*
> + * p8_occ_i2c.c - hwmon OCC driver
> + *
> + * This file contains the i2c layer for accessing the P8 OCC over i2c bus.
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * 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.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +
> +#include "scom.h"
> +#include "occ_scom_i2c.h"
> +#include "occ_p8.h"
> +#include "occ_sysfs.h"
> +
> +#define P8_OCC_I2C_NAME "p8-occ-i2c"
> +
> +static char *caps_sensor_names[] = {
> + "curr_powercap",
> + "curr_powerreading",
> + "norm_powercap",
> + "max_powercap",
> + "min_powercap",
> + "user_powerlimit"
> +};
> +
> +int p8_i2c_getscom(void *bus, u32 address, u64 *data)
> +{
> + /* P8 i2c slave requires address to be shifted by 1 */
> + address = address << 1;
> +
> + return occ_i2c_getscom(bus, address, data);
> +}
> +
> +int p8_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
> +{
> + /* P8 i2c slave requires address to be shifted by 1 */
> + address = address << 1;
> +
> + return occ_i2c_putscom(bus, address, data0, data1);
> +}
> +
> +static struct occ_bus_ops p8_bus_ops = {
> + .getscom = p8_i2c_getscom,
> + .putscom = p8_i2c_putscom,
> +};
> +
> +static struct occ_sysfs_config p8_sysfs_config = {
> + .num_caps_fields = ARRAY_SIZE(caps_sensor_names),
> + .caps_names = caps_sensor_names,
> +};
> +
> +static int p8_occ_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct occ *occ;
> + struct occ_sysfs *hwmon;
> +
> + occ = p8_occ_start(&client->dev, client, &p8_bus_ops);
> + if (IS_ERR(occ))
> + return PTR_ERR(occ);
> +
> + hwmon = occ_sysfs_start(&client->dev, occ, &p8_sysfs_config);
> + if (IS_ERR(hwmon))
> + return PTR_ERR(hwmon);
> +
> + i2c_set_clientdata(client, hwmon);
> +
> + return 0;
> +}
> +
> +static int p8_occ_remove(struct i2c_client *client)
> +{
> + int rc;
> + struct occ_sysfs *hwmon = i2c_get_clientdata(client);
> + struct occ *occ = hwmon->occ;
> +
> + rc = occ_sysfs_stop(&client->dev, hwmon);
> +
> + rc = rc || p8_occ_stop(occ);
> +
> + return rc;
> +}
> +
> +/* used by old-style board info. */
> +static const struct i2c_device_id occ_ids[] = {
> + { P8_OCC_I2C_NAME, 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, occ_ids);
> +
> +/* used by device table */
> +static const struct of_device_id occ_of_match[] = {
> + { .compatible = "ibm,p8-occ-i2c" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, occ_of_match);
> +
> +/*
> + * i2c-core uses i2c-detect() to detect device in below address list.
> + * If exists, address will be assigned to client.
> + * It is also possible to read address from device table.
> + */
> +static const unsigned short normal_i2c[] = {0x50, 0x51, I2C_CLIENT_END };
> +
> +static struct i2c_driver p8_occ_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = P8_OCC_I2C_NAME,
> + .pm = NULL,
> + .of_match_table = occ_of_match,
> + },
> + .probe = p8_occ_probe,
> + .remove = p8_occ_remove,
> + .id_table = occ_ids,
> + .address_list = normal_i2c,
> +};
> +
> +module_i2c_driver(p8_occ_driver);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("BMC P8 OCC hwmon driver");
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH linux 6/6] hwmon: occ: Add callbacks for parsing P9 OCC datastructures
2016-12-30 17:56 [PATCH linux 0/6] drivers: hwmon: Add On-Chip Controller driver eajames.ibm
` (4 preceding siblings ...)
2016-12-30 17:56 ` eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
@ 2016-12-30 17:56 ` eajames.ibm
5 siblings, 0 replies; 30+ messages in thread
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
To: linux
Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
Add functions to parse the data structures that are specific to the OCC on
the POWER9 processor. These are the sensor data structures, including
temperature, frequency, power, and "caps."
Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/hwmon/occ | 3 +
drivers/hwmon/occ/p9_occ.c | 243 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/p9_occ.h | 30 ++++++
3 files changed, 276 insertions(+)
create mode 100644 drivers/hwmon/occ/p9_occ.c
create mode 100644 drivers/hwmon/occ/p9_occ.h
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
index a6b3dd6..0f17c2f 100644
--- a/Documentation/hwmon/occ
+++ b/Documentation/hwmon/occ
@@ -34,6 +34,9 @@ number of data structures, such as command format, response headers, and the
like, are also defined in this specification, and are common to both POWER8 and
POWER9 OCCs.
+There is currently no public P9 OCC specification, and the data structures
+defined in the POWER9 OCC driver are subject to change.
+
sysfs Entries
-------------
diff --git a/drivers/hwmon/occ/p9_occ.c b/drivers/hwmon/occ/p9_occ.c
new file mode 100644
index 0000000..f69b469
--- /dev/null
+++ b/drivers/hwmon/occ/p9_occ.c
@@ -0,0 +1,243 @@
+/*
+ * p9.c - OCC hwmon driver
+ *
+ * This file contains the Power9-specific methods and data structures for
+ * the OCC hwmon driver.
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <asm/unaligned.h>
+
+#include "occ.h"
+#include "occ_p9.h"
+
+/* P9 OCC sensor data format */
+struct p9_temp_sensor {
+ u32 sensor_id;
+ u8 fru_type;
+ u8 value;
+};
+
+struct p9_freq_sensor {
+ u32 sensor_id;
+ u16 value;
+};
+
+struct p9_power_sensor {
+ u32 sensor_id;
+ u8 function_id;
+ u8 apss_channel;
+ u16 reserved;
+ u32 update_tag;
+ u64 accumulator;
+ u16 value;
+};
+
+struct p9_caps_sensor {
+ u16 curr_powercap;
+ u16 curr_powerreading;
+ u16 norm_powercap;
+ u16 max_powercap;
+ u16 min_powercap;
+ u16 user_powerlimit;
+ u8 user_powerlimit_source;
+};
+
+void p9_parse_sensor(u8 *data, void *sensor, int sensor_type, int off,
+ int snum)
+{
+ switch (sensor_type) {
+ case FREQ:
+ {
+ struct p9_freq_sensor *fs =
+ &(((struct p9_freq_sensor *)sensor)[snum]);
+
+ fs->sensor_id = be32_to_cpu(get_unaligned((u32 *)&data[off]));
+ fs->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+ }
+ break;
+ case TEMP:
+ {
+ struct p9_temp_sensor *ts =
+ &(((struct p9_temp_sensor *)sensor)[snum]);
+
+ ts->sensor_id = be32_to_cpu(get_unaligned((u32 *)&data[off]));
+ fs->fru_type = data[off + 4];
+ fs->value = data[off + 5];
+ }
+ break;
+ case POWER:
+ {
+ struct p9_power_sensor *ps =
+ &(((struct p9_power_sensor *)sensor)[snum]);
+
+ ps->sensor_id = be32_to_cpu(get_unaligned((u32 *)&data[off]));
+ ps->function_id = data[off + 4];
+ ps->apss_channel = data[off + 5];
+ ps->update_tag =
+ be32_to_cpu(get_unaligned((u32 *)&data[off + 8]));
+ ps->accumulator =
+ be64_to_cpu(get_unaligned((u64 *)&data[off + 12]));
+ ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 20]));
+ }
+ break;
+ case CAPS:
+ {
+ struct p9_caps_sensor *cs =
+ &(((struct p9_caps_sensor *)sensor)[snum]);
+
+ cs->curr_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off]));
+ cs->curr_powerreading =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+ cs->norm_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+ cs->max_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 6]));
+ cs->min_powercap =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 8]));
+ cs->user_powerlimit =
+ be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+ cs->user_powerlimit_source = data[off + 12];
+ }
+ break;
+ };
+}
+
+void *p9_alloc_sensor(int sensor_type, int num_sensors)
+{
+ switch (sensor_type) {
+ case FREQ:
+ return kcalloc(num_sensors, sizeof(struct p9_freq_sensor),
+ GFP_KERNEL);
+ case TEMP:
+ return kcalloc(num_sensors, sizeof(struct p9_temp_sensor),
+ GFP_KERNEL);
+ case POWER:
+ return kcalloc(num_sensors, sizeof(struct p9_power_sensor),
+ GFP_KERNEL);
+ case CAPS:
+ return kcalloc(num_sensors, sizeof(struct p9_caps_sensor),
+ GFP_KERNEL);
+ default:
+ return NULL;
+ }
+}
+
+int p9_get_sensor_value(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+
+ if (sensor_type == CAPS)
+ return -EINVAL;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -ENODEV;
+
+ switch (sensor_type) {
+ case FREQ:
+ return ((struct p9_freq_sensor *)sensor)[snum].value;
+ case TEMP:
+ return ((struct p9_temp_sensor *)sensor)[snum].value;
+ case POWER:
+ return ((struct p9_power_sensor *)sensor)[snum].value;
+ default:
+ return -EINVAL;
+ }
+}
+
+int p9_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+
+ if (sensor_type == CAPS)
+ return -EINVAL;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -ENODEV;
+
+ switch (sensor_type) {
+ case FREQ:
+ return ((struct p9_freq_sensor *)sensor)[snum].sensor_id;
+ case TEMP:
+ return ((struct p9_temp_sensor *)sensor)[snum].sensor_id;
+ case POWER:
+ return ((struct p9_power_sensor *)sensor)[snum].sensor_id;
+ default:
+ return -EINVAL;
+ }
+}
+
+int p9_get_caps_value(void *sensor, int snum, int caps_field)
+{
+ struct p9_caps_sensor *caps_sensor = sensor;
+
+ switch (caps_field) {
+ case 0:
+ return caps_sensor[snum].curr_powercap;
+ case 1:
+ return caps_sensor[snum].curr_powerreading;
+ case 2:
+ return caps_sensor[snum].norm_powercap;
+ case 3:
+ return caps_sensor[snum].max_powercap;
+ case 4:
+ return caps_sensor[snum].min_powercap;
+ case 5:
+ return caps_sensor[snum].user_powerlimit;
+ case 6:
+ return caps_sensor[snum].user_powerlimit_source;
+ default:
+ return -EINVAL;
+ }
+}
+static const struct occ_ops p9_ops = {
+ .parse_sensor = p9_parse_sensor,
+ .alloc_sensor = p9_alloc_sensor,
+ .get_sensor_value = p9_get_sensor_value,
+ .get_sensor_id = p9_get_sensor_id,
+ .get_caps_value = p9_get_caps_value,
+};
+
+static const struct occ_config p9_config = {
+ .command_addr = 0xFFFBE000,
+ .response_addr = 0xFFFBF000,
+};
+
+struct occ *p9_occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops)
+{
+ return occ_start(dev, bus, bus_ops, &p9_ops, &p9_config);
+}
+EXPORT_SYMBOL(occ_p9_start);
+
+int p9_occ_stop(struct occ *occ)
+{
+ return occ_stop(occ);
+}
+EXPORT_SYMBOL(occ_p9_stop);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("P9 OCC sensors");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/p9_occ.h b/drivers/hwmon/occ/p9_occ.h
new file mode 100644
index 0000000..8130873
--- /dev/null
+++ b/drivers/hwmon/occ/p9_occ.h
@@ -0,0 +1,30 @@
+/*
+ * occ_p9.h - OCC hwmon driver
+ *
+ * This file contains Power9-specific function prototypes
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef __OCC_P9_H__
+#define __OCC_P9_H__
+
+#include "scom.h"
+
+struct device;
+
+struct occ *p9_occ_start(struct device *dev, void *bus,
+ struct occ_bus_ops *bus_ops);
+int p9_occ_stop(struct occ *occ);
+
+#endif /* __OCC_P9_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread