* [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver
@ 2016-11-07 23:15 eajames.ibm
2016-11-11 4:12 ` [RFC PATCH linux 0/9] " Andrew Jeffery
` (9 more replies)
0 siblings, 10 replies; 23+ messages in thread
From: eajames.ibm @ 2016-11-07 23:15 UTC (permalink / raw)
To: openbmc; +Cc: Edward A. James
From: "Edward A. James" <eajames@us.ibm.com>
This patchset provides a new OCC hwmon driver. There are a number
of issues with the existing driver. Firstly, i2c access was embedded
throughout the driver. Secondly, there is no way to easily differentiate
versions of the OCC.
This patchset addresses the first issue by abstracting the bus transfer
protocol into a modular structure. In this way, any low level transfer
method may be easily implemented.
The second issue is addressed by separating the "version specific" code
for the OCC and the common hwmon code. This task is not yet complete, but
the general structure is in place. Ultimately, different OCC versions
could be probed up using the device tree.
Edward A. James (5):
Revert "hwmon: Add Power8 OCC hwmon driver"
drivers: Add hwmon occ driver framework
drivers: hwmon OCC scom bus operations
drivers: Add hwmon OCC version specific functions
drivers: OCC hwmon driver and sysfs
.../devicetree/bindings/i2c/i2c-ibm-occ.txt | 2 +-
drivers/hwmon/Kconfig | 13 +-
drivers/hwmon/Makefile | 2 +-
drivers/hwmon/occ/Kconfig | 15 +
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/occ.c | 892 ++++++++++++++
drivers/hwmon/occ/occ.h | 58 +
drivers/hwmon/occ/occ_i2c.c | 147 +++
drivers/hwmon/occ/p8.c | 222 ++++
drivers/hwmon/occ/p8.h | 29 +
drivers/hwmon/occ/p9.c | 253 ++++
drivers/hwmon/occ/p9.h | 29 +
drivers/hwmon/occ/scom.h | 31 +
drivers/hwmon/power8_occ_i2c.c | 1254 --------------------
14 files changed, 1680 insertions(+), 1268 deletions(-)
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/occ_i2c.c
create mode 100644 drivers/hwmon/occ/p8.c
create mode 100644 drivers/hwmon/occ/p8.h
create mode 100644 drivers/hwmon/occ/p9.c
create mode 100644 drivers/hwmon/occ/p9.h
create mode 100644 drivers/hwmon/occ/scom.h
delete mode 100644 drivers/hwmon/power8_occ_i2c.c
--
1.9.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 5:16 ` Joel Stanley
2016-11-29 22:58 ` Eddie James
2016-11-11 4:12 ` [RFC PATCH linux 1/9] Revert "hwmon: Add Power8 OCC hwmon driver" Andrew Jeffery
` (8 subsequent siblings)
9 siblings, 2 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Hi Eddie,
> This patchset provides a new OCC hwmon driver. There are a number
> of issues with the existing driver. Firstly, i2c access was embedded
> throughout the driver. Secondly, there is no way to easily differentiate
> versions of the OCC.
>
> This patchset addresses the first issue by abstracting the bus transfer
> protocol into a modular structure. In this way, any low level transfer
> method may be easily implemented.
>
> The second issue is addressed by separating the "version specific" code
> for the OCC and the common hwmon code. This task is not yet complete, but
> the general structure is in place. Ultimately, different OCC versions
> could be probed up using the device tree.
I agree with all of this. However, I thought some of the layering could still
be improved, so rather than provide a review through prose I figured it might
be better for both of us if I mangled the patches directly.
I've attacked two things. The first is I found the series hard to review due to
the way the patches were split, as the patches went from trivial to huge (and
complex) in a fairly short step:
>
> Edward A. James (5):
> Revert "hwmon: Add Power8 OCC hwmon driver"
> drivers: Add hwmon occ driver framework
> drivers: hwmon OCC scom bus operations
> drivers: Add hwmon OCC version specific functions
> drivers: OCC hwmon driver and sysfs
So I have reworked them into what is now 8 patches (see below) and tried to
tell a bit more of a story with them (hopefully I succeeded!) Second, I tried
my hand at reworking the layering as mentioned above, such that hardware
interaction is split from sysfs, and that the SCOM transport and sensor
configuration is decoupled from the data parsing. You largely had all of these
in-place already, I just cut and pasted code around a bit.
Please review the patches and provide feedback! I'd like to hear arguments for
or against both of our approaches. Note though that I've only tested that these
patches compile, I absolutely have not done any further testing.
Joel: Any chance you could chime in with an architectural review?
When the dust has settled on the bigger issues we can start to look at the
finer details of the APIs (there are some bugs, oddities and complexities that
I would like to address before they are applied to openbmc/linux).
Cheers,
Andrew
Andrew Jeffery (8):
hwmon: Add core On-Chip Controller support for POWER CPUs
hwmon: occ: Add sysfs interface
hwmon: occ: Add I2C SCOM transport implementation
hwmon: occ: Add callbacks for parsing P8 OCC datastructures
hwmon: occ: Add hwmon implementation for the P8 OCC
arm: aspeed: dt: Fix OCC compatible strings
hwmon: occ: Add callbacks for parsing P9 OCC datastructures
wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC
Edward A. James (1):
Revert "hwmon: Add Power8 OCC hwmon driver"
.../devicetree/bindings/i2c/i2c-ibm-occ.txt | 13 -
arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts | 4 +-
arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts | 4 +-
arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts | 4 +-
arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 2 +-
drivers/hwmon/Kconfig | 13 +-
drivers/hwmon/Makefile | 2 +-
drivers/hwmon/occ/Kconfig | 38 +
drivers/hwmon/occ/Makefile | 3 +
drivers/hwmon/occ/occ.c | 494 ++++++++
drivers/hwmon/occ/occ.h | 79 ++
drivers/hwmon/occ/occ_p8.c | 219 ++++
drivers/hwmon/occ/occ_p8.h | 30 +
drivers/hwmon/occ/occ_p8_i2c.c | 133 +++
drivers/hwmon/occ/occ_p9.c | 261 ++++
drivers/hwmon/occ/occ_p9.h | 30 +
drivers/hwmon/occ/occ_p9_fsi.c | 93 ++
drivers/hwmon/occ/occ_scom_i2c.c | 68 ++
drivers/hwmon/occ/occ_scom_i2c.h | 20 +
drivers/hwmon/occ/occ_sysfs.c | 498 ++++++++
drivers/hwmon/occ/occ_sysfs.h | 51 +
drivers/hwmon/occ/scom.h | 50 +
drivers/hwmon/power8_occ_i2c.c | 1254 --------------------
23 files changed, 2076 insertions(+), 1287 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
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/occ_p8.c
create mode 100644 drivers/hwmon/occ/occ_p8.h
create mode 100644 drivers/hwmon/occ/occ_p8_i2c.c
create mode 100644 drivers/hwmon/occ/occ_p9.c
create mode 100644 drivers/hwmon/occ/occ_p9.h
create mode 100644 drivers/hwmon/occ/occ_p9_fsi.c
create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h
create mode 100644 drivers/hwmon/occ/occ_sysfs.c
create mode 100644 drivers/hwmon/occ/occ_sysfs.h
create mode 100644 drivers/hwmon/occ/scom.h
delete mode 100644 drivers/hwmon/power8_occ_i2c.c
--
2.9.3
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH linux 1/9] Revert "hwmon: Add Power8 OCC hwmon driver"
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
2016-11-11 4:12 ` [RFC PATCH linux 0/9] " Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 2/9] hwmon: Add core On-Chip Controller support for POWER CPUs Andrew Jeffery
` (7 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley; +Cc: Edward A . James, OpenBMC Maillist
From: "Edward A. James" <eajames@us.ibm.com>
Remove old P8 OCC driver.
---
.../devicetree/bindings/i2c/i2c-ibm-occ.txt | 13 -
drivers/hwmon/Kconfig | 13 -
drivers/hwmon/Makefile | 1 -
drivers/hwmon/power8_occ_i2c.c | 1254 --------------------
4 files changed, 1281 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
delete mode 100644 drivers/hwmon/power8_occ_i2c.c
diff --git a/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
deleted file mode 100644
index 9aab2df8c5ed..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
+++ /dev/null
@@ -1,13 +0,0 @@
-HWMON i2c driver for IBM POWER CPU OCC (On Chip Controller)
-
-Required properties:
- - compatible: must be "ibm,power8-occ-i2c"
- - reg: physical address
-
-Example:
-i2c3: i2c-bus@100 {
- occ@50 {
- compatible = "ibm,occ-i2c";
- reg = <0x50>;
- };
-};
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d4de8d5aeed4..367496c9a721 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1226,19 +1226,6 @@ config SENSORS_NSA320
This driver can also be built as a module. If so, the module
will be called nsa320-hwmon.
-config SENSORS_POWER8_OCC_I2C
- tristate "Power8 On Chip Controller i2c driver"
- depends on I2C
- help
- If you say yes here you get support to monitor Power8 CPU
- sensors via the On Chip Controller (OCC) over the i2c bus.
-
- 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 power8_occ_i2c.
-
config SENSORS_PCF8591
tristate "Philips PCF8591 ADC/DAC"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4455478e9e7a..d7ccb02ef220 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -128,7 +128,6 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o
obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o
obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o
obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
-obj-$(CONFIG_SENSORS_POWER8_OCC_I2C) += power8_occ_i2c.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/power8_occ_i2c.c b/drivers/hwmon/power8_occ_i2c.c
deleted file mode 100644
index 6de0e76ae21b..000000000000
--- a/drivers/hwmon/power8_occ_i2c.c
+++ /dev/null
@@ -1,1254 +0,0 @@
-/*
- * OCC HWMON driver - read IBM Power8 On Chip Controller sensor data via
- * i2c.
- *
- * Copyright 2015 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/i2c.h>
-#include <linux/hwmon.h>
-#include <linux/hwmon-sysfs.h>
-#include <linux/err.h>
-#include <linux/mutex.h>
-#include <linux/of.h>
-#include <linux/delay.h>
-#include <linux/kernel.h>
-#include <linux/device.h>
-
-#define OCC_I2C_ADDR 0x50
-#define OCC_I2C_NAME "occ-i2c"
-
-#define OCC_DATA_MAX 4096 /* 4KB at most */
-/* i2c read and write occ sensors */
-#define I2C_READ_ERROR 1
-#define I2C_WRITE_ERROR 2
-
-/* Defined in POWER8 Processor Registers Specification */
-/* 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
-/* See definition in:
- * https://github.com/open-power/docs/blob/master/occ/OCC_OpenPwr_FW_Interfaces.pdf
- */
-#define OCC_COMMAND_ADDR 0xFFFF6000
-#define OCC_RESPONSE_ADDR 0xFFFF7000
-
-#define MAX_SENSOR_ATTR_LEN 32
-
-enum sensor_t {
- freq,
- temp,
- power,
- caps,
- MAX_OCC_SENSOR_TYPE
-};
-
-/* OCC sensor data format */
-struct occ_sensor {
- uint16_t sensor_id;
- uint16_t value;
-};
-
-struct power_sensor {
- uint16_t sensor_id;
- uint32_t update_tag;
- uint32_t accumulator;
- uint16_t value;
-};
-
-struct caps_sensor {
- uint16_t curr_powercap;
- uint16_t curr_powerreading;
- uint16_t norm_powercap;
- uint16_t max_powercap;
- uint16_t min_powercap;
- uint16_t user_powerlimit;
-};
-
-struct sensor_data_block {
- uint8_t sensor_type[4];
- uint8_t reserved0;
- uint8_t sensor_format;
- uint8_t sensor_length;
- uint8_t sensor_num;
- struct occ_sensor *sensor;
- struct power_sensor *power;
- struct caps_sensor *caps;
-};
-
-struct occ_poll_header {
- uint8_t status;
- uint8_t ext_status;
- uint8_t occs_present;
- uint8_t config;
- uint8_t occ_state;
- uint8_t reserved0;
- uint8_t reserved1;
- uint8_t error_log_id;
- uint32_t error_log_addr_start;
- uint16_t error_log_length;
- uint8_t reserved2;
- uint8_t reserved3;
- uint8_t occ_code_level[16];
- uint8_t sensor_eye_catcher[6];
- uint8_t sensor_block_num;
- uint8_t sensor_data_version;
-};
-
-struct occ_response {
- uint8_t sequence_num;
- uint8_t cmd_type;
- uint8_t rtn_status;
- uint16_t data_length;
- struct occ_poll_header header;
- struct sensor_data_block *blocks;
- uint16_t chk_sum;
- int sensor_block_id[MAX_OCC_SENSOR_TYPE];
-};
-
-struct sensor_attr_data {
- enum sensor_t type;
- uint32_t hwmon_index;
- uint32_t attr_id;
- char name[MAX_SENSOR_ATTR_LEN];
- struct device_attribute dev_attr;
-};
-
-struct sensor_group {
- char *name;
- struct sensor_attr_data *sattr;
- struct attribute_group group;
-};
-
-/* data private to each client */
-struct occ_drv_data {
- struct i2c_client *client;
- struct device *hwmon_dev;
- struct mutex update_lock;
- bool valid;
- unsigned long last_updated;
- /* Minimum timer interval for sampling In jiffies */
- unsigned long update_interval;
- unsigned long occ_online;
- uint16_t user_powercap;
- struct occ_response occ_resp;
- struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
-};
-
-static void deinit_occ_resp_buf(struct occ_response *p)
-{
- int i;
-
- if (!p)
- return;
-
- if (!p->blocks)
- return;
-
- for (i = 0; i < p->header.sensor_block_num; i++) {
- kfree(p->blocks[i].sensor);
- kfree(p->blocks[i].power);
- kfree(p->blocks[i].caps);
- }
-
- kfree(p->blocks);
-
- memset(p, 0, sizeof(*p));
-
- for (i = 0; i < ARRAY_SIZE(p->sensor_block_id); i++)
- p->sensor_block_id[i] = -1;
-}
-
-static ssize_t occ_i2c_read(struct i2c_client *client, void *buf, size_t count)
-{
- WARN_ON(count > OCC_DATA_MAX);
-
- dev_dbg(&client->dev, "i2c_read: reading %zu bytes @0x%x.\n",
- count, client->addr);
- return i2c_master_recv(client, buf, count);
-}
-
-static ssize_t occ_i2c_write(struct i2c_client *client, const void *buf,
- size_t count)
-{
- WARN_ON(count > OCC_DATA_MAX);
-
- dev_dbg(&client->dev, "i2c_write: writing %zu bytes @0x%x.\n",
- count, client->addr);
- return i2c_master_send(client, buf, count);
-}
-
-/* read 8-byte value and put into data[offset] */
-static int occ_getscomb(struct i2c_client *client, uint32_t address,
- uint8_t *data, int offset)
-{
- uint32_t ret;
- char buf[8];
- int i;
-
- /* P8 i2c slave requires address to be shifted by 1 */
- address = address << 1;
-
- ret = occ_i2c_write(client, &address,
- sizeof(address));
-
- if (ret != sizeof(address))
- return -I2C_WRITE_ERROR;
-
- ret = occ_i2c_read(client, buf, sizeof(buf));
- if (ret != sizeof(buf))
- return -I2C_READ_ERROR;
-
- for (i = 0; i < 8; i++)
- data[offset + i] = buf[7 - i];
-
- return 0;
-}
-
-static int occ_putscom(struct i2c_client *client, uint32_t address,
- uint32_t data0, uint32_t data1)
-{
- uint32_t buf[3];
- uint32_t ret;
-
- /* P8 i2c slave requires address to be shifted by 1 */
- address = address << 1;
-
- buf[0] = address;
- buf[1] = data1;
- buf[2] = data0;
-
- ret = occ_i2c_write(client, buf, sizeof(buf));
- if (ret != sizeof(buf))
- return I2C_WRITE_ERROR;
-
- return 0;
-}
-
-static void *occ_get_sensor_by_type(struct occ_response *resp, enum sensor_t t)
-{
- void *sensor;
-
- if (!resp->blocks)
- return NULL;
-
- if (resp->sensor_block_id[t] == -1)
- return NULL;
-
- switch (t) {
- case temp:
- case freq:
- sensor = resp->blocks[resp->sensor_block_id[t]].sensor;
- break;
- case power:
- sensor = resp->blocks[resp->sensor_block_id[t]].power;
- break;
- case caps:
- sensor = resp->blocks[resp->sensor_block_id[t]].caps;
- break;
- default:
- sensor = NULL;
- }
-
- return sensor;
-}
-
-static int occ_renew_sensor(struct occ_response *resp, uint8_t sensor_length,
- uint8_t sensor_num, enum sensor_t t, int block)
-{
- void *sensor;
- int ret;
-
- sensor = occ_get_sensor_by_type(resp, t);
-
- /* empty sensor block, release older sensor data */
- if (sensor_num == 0 || sensor_length == 0) {
- kfree(sensor);
- return -1;
- }
-
- if (!sensor || sensor_num !=
- resp->blocks[resp->sensor_block_id[t]].sensor_num) {
- kfree(sensor);
- switch (t) {
- case temp:
- case freq:
- resp->blocks[block].sensor =
- kcalloc(sensor_num,
- sizeof(struct occ_sensor), GFP_KERNEL);
- if (!resp->blocks[block].sensor) {
- ret = -ENOMEM;
- goto err;
- }
- break;
- case power:
- resp->blocks[block].power =
- kcalloc(sensor_num,
- sizeof(struct power_sensor),
- GFP_KERNEL);
- if (!resp->blocks[block].power) {
- ret = -ENOMEM;
- goto err;
- }
- break;
- case caps:
- resp->blocks[block].caps =
- kcalloc(sensor_num,
- sizeof(struct caps_sensor), GFP_KERNEL);
- if (!resp->blocks[block].caps) {
- ret = -ENOMEM;
- goto err;
- }
- break;
- default:
- ret = -ENOMEM;
- goto err;
- }
- }
-
- return 0;
-err:
- deinit_occ_resp_buf(resp);
- return ret;
-}
-
-#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
-
-static inline uint16_t get_occdata_length(uint8_t *data)
-{
- return be16_to_cpup((const __be16 *)&data[RESP_DATA_LENGTH]);
-}
-
-static int parse_occ_response(struct i2c_client *client,
- uint8_t *data, struct occ_response *resp)
-{
- int b;
- int s;
- int ret;
- int dnum = SENSOR_BLOCK_OFFSET;
- struct occ_sensor *f_sensor;
- struct occ_sensor *t_sensor;
- struct power_sensor *p_sensor;
- struct caps_sensor *c_sensor;
- uint8_t sensor_block_num;
- uint8_t sensor_type[4];
- uint8_t sensor_format;
- uint8_t sensor_length;
- uint8_t sensor_num;
-
- /* check if the data is valid */
- if (strncmp(&data[SENSOR_STR_OFFSET], "SENSOR", 6) != 0) {
- dev_dbg(&client->dev,
- "ERROR: no SENSOR String in response\n");
- ret = -1;
- goto err;
- }
-
- sensor_block_num = data[SENSOR_BLOCK_NUM_OFFSET];
- if (sensor_block_num == 0) {
- dev_dbg(&client->dev, "ERROR: SENSOR block num is 0\n");
- ret = -1;
- goto err;
- }
-
- /* if sensor block has changed, re-malloc */
- if (sensor_block_num != resp->header.sensor_block_num) {
- deinit_occ_resp_buf(resp);
- resp->blocks = kcalloc(sensor_block_num,
- sizeof(struct sensor_data_block), GFP_KERNEL);
- if (!resp->blocks)
- return -ENOMEM;
- }
-
- memcpy(&resp->header, &data[RESP_HEADER_OFFSET], sizeof(resp->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(&client->dev, "Reading %d sensor blocks\n",
- resp->header.sensor_block_num);
- for (b = 0; b < sensor_block_num; b++) {
- /* 8-byte sensor block head */
- strncpy(sensor_type, &data[dnum], 4);
- sensor_format = data[dnum+5];
- sensor_length = data[dnum+6];
- sensor_num = data[dnum+7];
- dnum = dnum + 8;
-
- dev_dbg(&client->dev,
- "sensor block[%d]: type: %s, sensor_num: %d\n",
- b, sensor_type, sensor_num);
-
- if (strncmp(sensor_type, "FREQ", 4) == 0) {
- ret = occ_renew_sensor(resp, sensor_length,
- sensor_num, freq, b);
- if (ret)
- continue;
-
- resp->sensor_block_id[freq] = b;
- for (s = 0; s < sensor_num; s++) {
- f_sensor = &resp->blocks[b].sensor[s];
- f_sensor->sensor_id =
- be16_to_cpup((const __be16 *)
- &data[dnum]);
- f_sensor->value = be16_to_cpup((const __be16 *)
- &data[dnum+2]);
- dev_dbg(&client->dev,
- "sensor[%d]-[%d]: id: %u, value: %u\n",
- b, s, f_sensor->sensor_id,
- f_sensor->value);
- dnum = dnum + sensor_length;
- }
- } else if (strncmp(sensor_type, "TEMP", 4) == 0) {
- ret = occ_renew_sensor(resp, sensor_length,
- sensor_num, temp, b);
- if (ret)
- continue;
-
- resp->sensor_block_id[temp] = b;
- for (s = 0; s < sensor_num; s++) {
- t_sensor = &resp->blocks[b].sensor[s];
- t_sensor->sensor_id =
- be16_to_cpup((const __be16 *)
- &data[dnum]);
- t_sensor->value = be16_to_cpup((const __be16 *)
- &data[dnum+2]);
- dev_dbg(&client->dev,
- "sensor[%d]-[%d]: id: %u, value: %u\n",
- b, s, t_sensor->sensor_id,
- t_sensor->value);
- dnum = dnum + sensor_length;
- }
- } else if (strncmp(sensor_type, "POWR", 4) == 0) {
- ret = occ_renew_sensor(resp, sensor_length,
- sensor_num, power, b);
- if (ret)
- continue;
-
- resp->sensor_block_id[power] = b;
- for (s = 0; s < sensor_num; s++) {
- p_sensor = &resp->blocks[b].power[s];
- p_sensor->sensor_id =
- be16_to_cpup((const __be16 *)
- &data[dnum]);
- p_sensor->update_tag =
- be32_to_cpup((const __be32 *)
- &data[dnum+2]);
- p_sensor->accumulator =
- be32_to_cpup((const __be32 *)
- &data[dnum+6]);
- p_sensor->value = be16_to_cpup((const __be16 *)
- &data[dnum+10]);
-
- dev_dbg(&client->dev,
- "sensor[%d]-[%d]: id: %u, value: %u\n",
- b, s, p_sensor->sensor_id,
- p_sensor->value);
-
- dnum = dnum + sensor_length;
- }
- } else if (strncmp(sensor_type, "CAPS", 4) == 0) {
- ret = occ_renew_sensor(resp, sensor_length,
- sensor_num, caps, b);
- if (ret)
- continue;
-
- resp->sensor_block_id[caps] = b;
- for (s = 0; s < sensor_num; s++) {
- c_sensor = &resp->blocks[b].caps[s];
- c_sensor->curr_powercap =
- be16_to_cpup((const __be16 *)
- &data[dnum]);
- c_sensor->curr_powerreading =
- be16_to_cpup((const __be16 *)
- &data[dnum+2]);
- c_sensor->norm_powercap =
- be16_to_cpup((const __be16 *)
- &data[dnum+4]);
- c_sensor->max_powercap =
- be16_to_cpup((const __be16 *)
- &data[dnum+6]);
- c_sensor->min_powercap =
- be16_to_cpup((const __be16 *)
- &data[dnum+8]);
- c_sensor->user_powerlimit =
- be16_to_cpup((const __be16 *)
- &data[dnum+10]);
-
- dnum = dnum + sensor_length;
- dev_dbg(&client->dev, "CAPS sensor #%d:\n", s);
- dev_dbg(&client->dev, "curr_powercap is %x\n",
- c_sensor->curr_powercap);
- dev_dbg(&client->dev,
- "curr_powerreading is %x\n",
- c_sensor->curr_powerreading);
- dev_dbg(&client->dev, "norm_powercap is %x\n",
- c_sensor->norm_powercap);
- dev_dbg(&client->dev, "max_powercap is %x\n",
- c_sensor->max_powercap);
- dev_dbg(&client->dev, "min_powercap is %x\n",
- c_sensor->min_powercap);
- dev_dbg(&client->dev, "user_powerlimit is %x\n",
- c_sensor->user_powerlimit);
- }
-
- } else {
- dev_dbg(&client->dev,
- "ERROR: sensor type %s not supported\n",
- resp->blocks[b].sensor_type);
- ret = -1;
- goto err;
- }
-
- strncpy(resp->blocks[b].sensor_type, sensor_type, 4);
- resp->blocks[b].sensor_format = sensor_format;
- resp->blocks[b].sensor_length = sensor_length;
- resp->blocks[b].sensor_num = sensor_num;
- }
-
- return 0;
-err:
- deinit_occ_resp_buf(resp);
- return ret;
-}
-
-
-/* Refer to OCC interface document for OCC command format
- * https://github.com/open-power/docs/blob/master/occ/OCC_OpenPwr_FW_Interfaces.pdf
- */
-static uint8_t occ_send_cmd(struct i2c_client *client, uint8_t seq,
- uint8_t type, uint16_t length, uint8_t *data, uint8_t *resp)
-{
- uint32_t cmd1, cmd2;
- uint16_t checksum;
- int i;
-
- length = cpu_to_le16(length);
- cmd1 = (seq << 24) | (type << 16) | length;
- memcpy(&cmd2, data, length);
- cmd2 <<= ((4 - length) * 8);
-
- /* checksum: sum of every bytes of cmd1, cmd2 */
- checksum = 0;
- for (i = 0; i < 4; i++)
- checksum += (cmd1 >> (i * 8)) & 0xFF;
- for (i = 0; i < 4; i++)
- checksum += (cmd2 >> (i * 8)) & 0xFF;
- cmd2 |= checksum << ((2 - length) * 8);
-
- /* Init OCB */
- occ_putscom(client, OCB_STATUS_CONTROL_OR, 0x08000000, 0x00000000);
- occ_putscom(client, OCB_STATUS_CONTROL_AND, 0xFBFFFFFF, 0xFFFFFFFF);
-
- /* Send command */
- occ_putscom(client, OCB_ADDRESS, OCC_COMMAND_ADDR, 0x00000000);
- occ_putscom(client, OCB_ADDRESS, OCC_COMMAND_ADDR, 0x00000000);
- occ_putscom(client, OCB_DATA, cmd1, cmd2);
-
- /* Trigger attention */
- occ_putscom(client, ATTN_DATA, 0x01010000, 0x00000000);
-
- /* Get response data */
- occ_putscom(client, OCB_ADDRESS, OCC_RESPONSE_ADDR, 0x00000000);
- occ_getscomb(client, OCB_DATA, resp, 0);
-
- /* return status */
- return resp[2];
-}
-
-static int occ_get_all(struct i2c_client *client, struct occ_response *occ_resp)
-{
- uint8_t *occ_data;
- uint16_t num_bytes;
- int i;
- int ret;
- uint8_t poll_cmd_data;
-
- poll_cmd_data = 0x10;
-
- /*
- * TODO: fetch header, and then allocate the rest of the buffer based
- * on the header size. Assuming the OCC has a fixed sized header
- */
- occ_data = devm_kzalloc(&client->dev, OCC_DATA_MAX, GFP_KERNEL);
-
- ret = occ_send_cmd(client, 0, 0, 1, &poll_cmd_data, occ_data);
- if (ret) {
- dev_err(&client->dev, "ERROR: OCC Poll: 0x%x\n", ret);
- ret = -EINVAL;
- goto out;
- }
-
- num_bytes = get_occdata_length(occ_data);
-
- dev_dbg(&client->dev, "OCC data length: %d\n", num_bytes);
-
- if (num_bytes > OCC_DATA_MAX) {
- dev_dbg(&client->dev, "ERROR: OCC data length must be < 4KB\n");
- ret = -EINVAL;
- goto out;
- }
-
- if (num_bytes <= 0) {
- dev_dbg(&client->dev, "ERROR: OCC data length is zero\n");
- ret = -EINVAL;
- goto out;
- }
-
- /* read remaining data */
- for (i = 8; i < num_bytes + 8; i = i + 8)
- occ_getscomb(client, OCB_DATA, occ_data, i);
-
- ret = parse_occ_response(client, occ_data, occ_resp);
-
-out:
- devm_kfree(&client->dev, occ_data);
- return ret;
-}
-
-
-static int occ_update_device(struct device *dev)
-{
- struct occ_drv_data *data = dev_get_drvdata(dev);
- struct i2c_client *client = data->client;
- int ret = 0;
-
- mutex_lock(&data->update_lock);
-
- if (time_after(jiffies, data->last_updated + data->update_interval)
- || !data->valid) {
- data->valid = 1;
- ret = occ_get_all(client, &data->occ_resp);
- if (ret)
- data->valid = 0;
- data->last_updated = jiffies;
- }
- mutex_unlock(&data->update_lock);
-
- return ret;
-}
-
-
-static void *occ_get_sensor(struct device *hwmon_dev, enum sensor_t t)
-{
- struct device *dev = hwmon_dev->parent;
- struct occ_drv_data *data = dev_get_drvdata(dev);
- int ret;
-
- ret = occ_update_device(dev);
- if (ret != 0) {
- dev_dbg(dev, "ERROR: cannot get occ sensor data: %d\n", ret);
- return NULL;
- }
-
- return occ_get_sensor_by_type(&data->occ_resp, t);
-}
-
-static int occ_get_sensor_value(struct device *hwmon_dev, enum sensor_t t,
- int index)
-{
- void *sensor;
-
- if (t == caps)
- return -1;
-
- sensor = occ_get_sensor(hwmon_dev, t);
-
- if (!sensor)
- return -1;
-
- if (t == power)
- return ((struct power_sensor *)sensor)[index].value;
-
- return ((struct occ_sensor *)sensor)[index].value;
-}
-
-static int occ_get_sensor_id(struct device *hwmon_dev, enum sensor_t t,
- int index)
-{
- void *sensor;
-
- if (t == caps)
- return -1;
-
- sensor = occ_get_sensor(hwmon_dev, t);
-
- if (!sensor)
- return -1;
-
- if (t == power)
- return ((struct power_sensor *)sensor)[index].sensor_id;
-
- return ((struct occ_sensor *)sensor)[index].sensor_id;
-}
-
-/* sysfs attributes for occ hwmon device */
-
-static ssize_t show_input(struct device *hwmon_dev,
- struct device_attribute *da, char *buf)
-{
- struct sensor_attr_data *sdata = container_of(da,
- struct sensor_attr_data, dev_attr);
- int val;
-
- val = occ_get_sensor_value(hwmon_dev, sdata->type,
- sdata->hwmon_index - 1);
- if (sdata->type == temp)
- /* in millidegree Celsius */
- val *= 1000;
-
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
-}
-
-static ssize_t show_label(struct device *hwmon_dev,
- struct device_attribute *da, char *buf)
-{
- struct sensor_attr_data *sdata = container_of(da,
- struct sensor_attr_data, dev_attr);
- int val;
-
- val = occ_get_sensor_id(hwmon_dev, sdata->type,
- sdata->hwmon_index - 1);
-
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
-}
-
-static ssize_t show_caps(struct device *hwmon_dev,
- struct device_attribute *da, char *buf)
-{
- struct sensor_attr_data *sdata = container_of(da,
- struct sensor_attr_data, dev_attr);
- int nr = sdata->attr_id;
- int n = sdata->hwmon_index - 1;
- struct caps_sensor *sensor;
- int val;
-
- sensor = occ_get_sensor(hwmon_dev, caps);
- if (!sensor) {
- val = -1;
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
- }
-
- switch (nr) {
- case 0:
- val = sensor[n].curr_powercap;
- break;
- case 1:
- val = sensor[n].curr_powerreading;
- break;
- case 2:
- val = sensor[n].norm_powercap;
- break;
- case 3:
- val = sensor[n].max_powercap;
- break;
- case 4:
- val = sensor[n].min_powercap;
- break;
- case 5:
- val = sensor[n].user_powerlimit;
- break;
- default:
- val = -1;
- }
-
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
-}
-
-static ssize_t show_update_interval(struct device *hwmon_dev,
- struct device_attribute *attr, char *buf)
-{
- struct device *dev = hwmon_dev->parent;
- struct occ_drv_data *data = dev_get_drvdata(dev);
-
- return snprintf(buf, PAGE_SIZE - 1, "%u\n",
- jiffies_to_msecs(data->update_interval));
-}
-
-static ssize_t set_update_interval(struct device *hwmon_dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct device *dev = hwmon_dev->parent;
- struct occ_drv_data *data = dev_get_drvdata(dev);
- unsigned long val;
- int err;
-
- err = kstrtoul(buf, 10, &val);
- if (err)
- return err;
-
- data->update_interval = msecs_to_jiffies(val);
- return count;
-}
-static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO,
- show_update_interval, set_update_interval);
-
-static ssize_t show_name(struct device *hwmon_dev,
- struct device_attribute *attr, char *buf)
-{
- return snprintf(buf, PAGE_SIZE - 1, "%s\n", OCC_I2C_NAME);
-}
-static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
-
-static ssize_t show_user_powercap(struct device *hwmon_dev,
- struct device_attribute *attr, char *buf)
-{
- struct device *dev = hwmon_dev->parent;
- struct occ_drv_data *data = dev_get_drvdata(dev);
-
- return snprintf(buf, PAGE_SIZE - 1, "%u\n", data->user_powercap);
-}
-
-
-static ssize_t set_user_powercap(struct device *hwmon_dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct device *dev = hwmon_dev->parent;
- struct occ_drv_data *data = dev_get_drvdata(dev);
- struct i2c_client *client = data->client;
- uint16_t val;
- uint8_t resp[8];
- int err;
-
- err = kstrtou16(buf, 10, &val);
- if (err)
- return err;
-
- dev_dbg(dev, "set user powercap to: %u\n", val);
- val = cpu_to_le16(val);
- err = occ_send_cmd(client, 0, 0x22, 2, (uint8_t *)&val, resp);
- if (err != 0) {
- dev_dbg(dev,
- "ERROR: Set User Powercap: wrong return status: %x\n",
- err);
- if (err == 0x13)
- dev_info(dev,
- "ERROR: set invalid powercap value: %x\n", val);
- return -EINVAL;
- }
- data->user_powercap = val;
- return count;
-}
-static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO,
- show_user_powercap, set_user_powercap);
-
-static void deinit_sensor_groups(struct device *hwmon_dev,
- struct sensor_group *sensor_groups)
-{
- int cnt;
-
- for (cnt = 0; cnt < MAX_OCC_SENSOR_TYPE; cnt++) {
- if (sensor_groups[cnt].group.attrs)
- devm_kfree(hwmon_dev, sensor_groups[cnt].group.attrs);
- if (sensor_groups[cnt].sattr)
- devm_kfree(hwmon_dev, sensor_groups[cnt].sattr);
- sensor_groups[cnt].group.attrs = NULL;
- sensor_groups[cnt].sattr = NULL;
- }
-}
-
-static void occ_remove_hwmon_attrs(struct device *hwmon_dev)
-{
- struct occ_drv_data *data = dev_get_drvdata(hwmon_dev->parent);
- struct sensor_group *sensor_groups = data->sensor_groups;
- int i;
-
- if (!hwmon_dev)
- return;
-
- device_remove_file(hwmon_dev, &dev_attr_update_interval);
- device_remove_file(hwmon_dev, &dev_attr_name);
- device_remove_file(hwmon_dev, &dev_attr_user_powercap);
-
- for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++)
- sysfs_remove_group(&hwmon_dev->kobj, &sensor_groups[i].group);
-
- deinit_sensor_groups(hwmon_dev, sensor_groups);
-}
-
-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;
-}
-
-/* create hwmon sensor sysfs attributes */
-static int create_sensor_group(struct device *hwmon_dev, enum sensor_t type,
- int sensor_num)
-{
- struct occ_drv_data *data = dev_get_drvdata(hwmon_dev->parent);
- struct sensor_group *sensor_groups = data->sensor_groups;
- struct sensor_attr_data *sdata;
- int ret;
- int cnt;
-
- /* each sensor has 'label' and 'input' attributes */
- sensor_groups[type].group.attrs = devm_kzalloc(hwmon_dev,
- sizeof(struct attribute *) *
- sensor_num * 2 + 1, GFP_KERNEL);
- if (!sensor_groups[type].group.attrs) {
- ret = -ENOMEM;
- goto err;
- }
-
- sensor_groups[type].sattr = devm_kzalloc(hwmon_dev,
- sizeof(struct sensor_attr_data) *
- sensor_num * 2, GFP_KERNEL);
- if (!sensor_groups[type].sattr) {
- ret = -ENOMEM;
- goto err;
- }
-
- for (cnt = 0; cnt < sensor_num; cnt++) {
- sdata = &sensor_groups[type].sattr[cnt];
- /* hwomon attributes index starts from 1 */
- sdata->hwmon_index = cnt + 1;
- sdata->type = type;
- sensor_attr_init(sdata, sensor_groups[type].name, "input",
- show_input);
- sensor_groups[type].group.attrs[cnt] = &sdata->dev_attr.attr;
-
- sdata = &sensor_groups[type].sattr[cnt + sensor_num];
- sdata->hwmon_index = cnt + 1;
- sdata->type = type;
- sensor_attr_init(sdata, sensor_groups[type].name, "label",
- show_label);
- sensor_groups[type].group.attrs[cnt + sensor_num] =
- &sdata->dev_attr.attr;
- }
-
- ret = sysfs_create_group(&hwmon_dev->kobj, &sensor_groups[type].group);
- if (ret)
- goto err;
-
- return ret;
-err:
- deinit_sensor_groups(hwmon_dev, sensor_groups);
- return ret;
-}
-
-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;
-
- /* FIXME, to be compatible with user space app, we do not
- * generate caps1_* attributes.
- */
- if (sdata->hwmon_index == 1)
- snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s_%s",
- "caps", attr_name);
- else
- 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 char *caps_sensor_name[] = {
- "curr_powercap",
- "curr_powerreading",
- "norm_powercap",
- "max_powercap",
- "min_powercap",
- "user_powerlimit",
-};
-
-static int create_caps_sensor_group(struct device *hwmon_dev, int sensor_num)
-{
- struct occ_drv_data *data = dev_get_drvdata(hwmon_dev->parent);
- struct sensor_group *sensor_groups = data->sensor_groups;
- int field_num = ARRAY_SIZE(caps_sensor_name);
- struct sensor_attr_data *sdata;
- int ret;
- int cnt;
- int i;
-
- sensor_groups[caps].group.attrs = devm_kzalloc(hwmon_dev,
- sizeof(struct attribute *) *
- sensor_num * field_num + 1,
- GFP_KERNEL);
- if (!sensor_groups[caps].group.attrs) {
- ret = -ENOMEM;
- goto err;
- }
-
- sensor_groups[caps].sattr = devm_kzalloc(hwmon_dev,
- sizeof(struct sensor_attr_data) *
- sensor_num * field_num,
- GFP_KERNEL);
- if (!sensor_groups[caps].sattr) {
- ret = -ENOMEM;
- goto err;
- }
-
- for (cnt = 0; cnt < sensor_num; cnt++) {
- for (i = 0; i < field_num; i++) {
- sdata = &sensor_groups[caps].sattr[cnt * field_num + i];
- caps_sensor_attr_init(sdata, caps_sensor_name[i],
- cnt + 1, i);
- sensor_groups[caps].group.attrs[cnt * field_num + i] =
- &sdata->dev_attr.attr;
- }
- }
-
- ret = sysfs_create_group(&hwmon_dev->kobj, &sensor_groups[caps].group);
- if (ret)
- goto err;
-
- return ret;
-err:
- deinit_sensor_groups(hwmon_dev, sensor_groups);
- return ret;
-}
-
-static int occ_create_hwmon_attrs(struct device *dev)
-{
- struct occ_drv_data *drv_data = dev_get_drvdata(dev);
- struct device *hwmon_dev = drv_data->hwmon_dev;
- struct sensor_group *sensor_groups = drv_data->sensor_groups;
- int i;
- int sensor_num;
- int ret;
- struct occ_response *rsp;
- enum sensor_t t;
-
- rsp = &drv_data->occ_resp;
-
- for (i = 0; i < ARRAY_SIZE(rsp->sensor_block_id); i++)
- rsp->sensor_block_id[i] = -1;
-
- /* read sensor data from occ. */
- ret = occ_update_device(dev);
- if (ret != 0) {
- dev_dbg(dev, "ERROR: cannot get occ sensor data: %d\n", ret);
- return ret;
- }
- if (!rsp->blocks)
- return -1;
-
- ret = device_create_file(hwmon_dev, &dev_attr_name);
- if (ret)
- goto error;
-
- ret = device_create_file(hwmon_dev, &dev_attr_update_interval);
- if (ret)
- goto error;
-
- if (rsp->sensor_block_id[caps] >= 0) {
- /* user powercap: only for master OCC */
- ret = device_create_file(hwmon_dev, &dev_attr_user_powercap);
- if (ret)
- goto error;
- }
-
- sensor_groups[freq].name = "freq";
- sensor_groups[temp].name = "temp";
- sensor_groups[power].name = "power";
- sensor_groups[caps].name = "caps";
-
- for (t = 0; t < MAX_OCC_SENSOR_TYPE; t++) {
- if (rsp->sensor_block_id[t] < 0)
- continue;
-
- sensor_num =
- rsp->blocks[rsp->sensor_block_id[t]].sensor_num;
- if (t == caps)
- ret = create_caps_sensor_group(hwmon_dev, sensor_num);
- else
- ret = create_sensor_group(hwmon_dev, t, sensor_num);
- if (ret)
- goto error;
- }
-
- return 0;
-error:
- dev_err(dev, "ERROR: cannot create hwmon attributes\n");
- occ_remove_hwmon_attrs(drv_data->hwmon_dev);
- return ret;
-}
-
-static ssize_t show_occ_online(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct occ_drv_data *data = dev_get_drvdata(dev);
-
- return snprintf(buf, PAGE_SIZE - 1, "%lu\n", data->occ_online);
-}
-
-static ssize_t set_occ_online(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct occ_drv_data *data = dev_get_drvdata(dev);
- unsigned long val;
- int err;
-
- err = kstrtoul(buf, 10, &val);
- if (err)
- return err;
-
- if (val == 1) {
- if (data->occ_online == 1)
- return count;
-
- /* populate hwmon sysfs attr using sensor data */
- dev_dbg(dev, "occ register hwmon @0x%x\n", data->client->addr);
-
- data->hwmon_dev = hwmon_device_register(dev);
- if (IS_ERR(data->hwmon_dev))
- return PTR_ERR(data->hwmon_dev);
-
- err = occ_create_hwmon_attrs(dev);
- if (err) {
- hwmon_device_unregister(data->hwmon_dev);
- return err;
- }
- data->hwmon_dev->parent = dev;
- } else if (val == 0) {
- if (data->occ_online == 0)
- return count;
-
- occ_remove_hwmon_attrs(data->hwmon_dev);
- hwmon_device_unregister(data->hwmon_dev);
- data->hwmon_dev = NULL;
- } else
- return -EINVAL;
-
- data->occ_online = val;
- return count;
-}
-
-static DEVICE_ATTR(online, S_IWUSR | S_IRUGO,
- show_occ_online, set_occ_online);
-
-static int occ_create_i2c_sysfs_attr(struct device *dev)
-{
- /* create an i2c sysfs attribute, to indicate whether OCC is active */
- return device_create_file(dev, &dev_attr_online);
-}
-
-
-/* device probe and removal */
-
-enum occ_type {
- occ_id,
-};
-
-static int occ_probe(struct i2c_client *client, const struct i2c_device_id *id)
-{
- struct device *dev = &client->dev;
- struct occ_drv_data *data;
-
- data = devm_kzalloc(dev, sizeof(struct occ_drv_data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->client = client;
- i2c_set_clientdata(client, data);
- mutex_init(&data->update_lock);
- data->update_interval = HZ;
-
- occ_create_i2c_sysfs_attr(dev);
-
- dev_info(dev, "occ i2c driver ready: i2c addr@0x%x\n", client->addr);
-
- return 0;
-}
-
-static int occ_remove(struct i2c_client *client)
-{
- struct occ_drv_data *data = i2c_get_clientdata(client);
-
- /* free allocated sensor memory */
- deinit_occ_resp_buf(&data->occ_resp);
-
- device_remove_file(&client->dev, &dev_attr_online);
-
- if (!data->hwmon_dev)
- return 0;
-
- occ_remove_hwmon_attrs(data->hwmon_dev);
- hwmon_device_unregister(data->hwmon_dev);
- return 0;
-}
-
-/* used by old-style board info. */
-static const struct i2c_device_id occ_ids[] = {
- { OCC_I2C_NAME, occ_id, },
- { /* LIST END */ }
-};
-MODULE_DEVICE_TABLE(i2c, occ_ids);
-
-/* use by device table */
-static const struct of_device_id i2c_occ_of_match[] = {
- {.compatible = "ibm,occ-i2c"},
- {},
-};
-MODULE_DEVICE_TABLE(of, i2c_occ_of_match);
-
-/* i2c-core uses i2c-detect() to detect device in bellow 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 occ_driver = {
- .class = I2C_CLASS_HWMON,
- .driver = {
- .name = OCC_I2C_NAME,
- .pm = NULL,
- .of_match_table = i2c_occ_of_match,
- },
- .probe = occ_probe,
- .remove = occ_remove,
- .id_table = occ_ids,
- .address_list = normal_i2c,
-};
-
-module_i2c_driver(occ_driver);
-
-MODULE_AUTHOR("Li Yi <shliyi@cn.ibm.com>");
-MODULE_DESCRIPTION("BMC OCC hwmon driver");
-MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 2/9] hwmon: Add core On-Chip Controller support for POWER CPUs
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
2016-11-11 4:12 ` [RFC PATCH linux 0/9] " Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 1/9] Revert "hwmon: Add Power8 OCC hwmon driver" Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 3/9] hwmon: occ: Add sysfs interface Andrew Jeffery
` (6 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
The On-Chip Controller (OCC) exposes frequency and temperature sensor
information along with power draw measurement and capping for POWER
chips. The information is gathered by SCOM reads and writes over a
transport of choice, and structured in a flexible fashion to accomodate
differences in sensors across designs.
Add core support for querying and parsing the data structures provided
by the OCC via SCOMs.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/Kconfig | 2 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/occ/Kconfig | 15 ++
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/occ.c | 494 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ.h | 79 ++++++++
drivers/hwmon/occ/scom.h | 50 +++++
7 files changed, 642 insertions(+)
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/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 367496c9a721..f4b0180925a0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1226,6 +1226,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 d7ccb02ef220..f6af1a7713f1 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -165,6 +165,7 @@ obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
obj-$(CONFIG_PMBUS) += pmbus/
+obj-$(CONFIG_SENSORS_PPC_OCC) += occ/
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
new file mode 100644
index 000000000000..cdb64a71af6e
--- /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 000000000000..93cb52f8dd8c
--- /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 000000000000..18936c8ff923
--- /dev/null
+++ b/drivers/hwmon/occ/occ.c
@@ -0,0 +1,494 @@
+/*
+ * 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 "occ.h"
+
+#define OCC_DATA_MAX 4096
+
+/* 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
+
+#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
+
+#define MAX_SENSOR_ATTR_LEN 32
+
+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;
+};
+
+struct occ_response {
+ u8 sequence_num;
+ u8 cmd_type;
+ u8 rtn_status;
+ u16 data_length;
+ struct occ_poll_header header;
+ u16 chk_sum;
+ int sensor_block_id[MAX_OCC_SENSOR_TYPE];
+ struct sensor_data_block *blocks;
+};
+
+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->blocks)
+ return;
+
+ for (i = 0; i < resp->header.sensor_block_num; ++i) {
+ kfree(resp->blocks[i].sensors);
+ }
+
+ kfree(resp->blocks);
+
+ memset(resp, 0, sizeof(struct occ_response));
+
+ for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
+ resp->sensor_block_id[i] = -1;
+}
+
+static void *occ_get_sensor_by_type(struct occ_response *resp,
+ enum sensor_type t)
+{
+ if (!resp->blocks)
+ return NULL;
+
+ if (resp->sensor_block_id[t] == -1)
+ return NULL;
+
+ return resp->blocks[resp->sensor_block_id[t]].sensors;
+}
+
+static int occ_renew_sensor(struct occ *driver, u8 sensor_length,
+ u8 sensor_num, enum sensor_type t, int block)
+{
+ void *sensor;
+ int rc;
+ 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);
+ return -1;
+ }
+
+ if (!sensor || sensor_num !=
+ resp->blocks[resp->sensor_block_id[t]].sensor_num) {
+ kfree(sensor);
+ resp->blocks[block].sensors =
+ driver->ops.alloc_sensor(t, sensor_num);
+ if (!resp->blocks[block].sensors) {
+ rc = -ENOMEM;
+ goto err;
+ }
+ }
+
+ return 0;
+err:
+ deinit_occ_resp_buf(resp);
+ return rc;
+}
+
+static int parse_occ_response(struct occ *driver, u8 *data,
+ struct occ_response *resp)
+{
+ int b;
+ int s;
+ int rc;
+ int dnum = SENSOR_BLOCK_OFFSET;
+ u8 sensor_block_num;
+ u8 sensor_type[4];
+ u8 sensor_format;
+ u8 sensor_length;
+ u8 sensor_num;
+ struct device *dev = driver->dev;
+ void (*parse_sensor)(u8 *data, void *sensor, int sensor_type, int off,
+ int snum) = driver->ops.parse_sensor;
+
+ /* check if the data is valid */
+ if (strncmp(&data[SENSOR_STR_OFFSET], "SENSOR", 6) != 0) {
+ dev_dbg(dev, "ERROR: no SENSOR String in response\n");
+ rc = -1;
+ goto err;
+ }
+
+ sensor_block_num = data[SENSOR_BLOCK_NUM_OFFSET];
+ if (sensor_block_num == 0) {
+ dev_dbg(dev, "ERROR: SENSOR block num is 0\n");
+ rc = -1;
+ goto err;
+ }
+
+ /* if sensor block has changed, re-malloc */
+ if (sensor_block_num != resp->header.sensor_block_num) {
+ deinit_occ_resp_buf(resp);
+ resp->blocks = kcalloc(sensor_block_num,
+ sizeof(struct sensor_data_block),
+ GFP_KERNEL);
+ if (!resp->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++) {
+ /* 8-byte sensor block head */
+ strncpy(sensor_type, &data[dnum], 4);
+ sensor_format = data[dnum + 5];
+ sensor_length = data[dnum + 6];
+ sensor_num = data[dnum + 7];
+ dnum = dnum + 8;
+
+ dev_dbg(dev, "sensor block[%d]: type: %s, sensor_num: %d\n", b,
+ sensor_type, sensor_num);
+
+ if (strncmp(sensor_type, "FREQ", 4) == 0) {
+ rc = occ_renew_sensor(driver, sensor_length, sensor_num,
+ FREQ, b);
+ if (rc)
+ continue;
+
+ resp->sensor_block_id[FREQ] = b;
+ for (s = 0; s < sensor_num; s++) {
+ parse_sensor(data, resp->blocks[b].sensors,
+ FREQ, dnum, s);
+ dnum = dnum + sensor_length;
+ }
+ } else if (strncmp(sensor_type, "TEMP", 4) == 0) {
+ rc = occ_renew_sensor(driver, sensor_length,
+ sensor_num, TEMP, b);
+ if (rc)
+ continue;
+
+ resp->sensor_block_id[TEMP] = b;
+ for (s = 0; s < sensor_num; s++) {
+ parse_sensor(data, resp->blocks[b].sensors,
+ TEMP, dnum, s);
+ dnum = dnum + sensor_length;
+ }
+ } else if (strncmp(sensor_type, "POWR", 4) == 0) {
+ rc = occ_renew_sensor(driver, sensor_length,
+ sensor_num, POWER, b);
+ if (rc)
+ continue;
+
+ resp->sensor_block_id[POWER] = b;
+ for (s = 0; s < sensor_num; s++) {
+ parse_sensor(data, resp->blocks[b].sensors,
+ POWER, dnum, s);
+ dnum = dnum + sensor_length;
+ }
+ } else if (strncmp(sensor_type, "CAPS", 4) == 0) {
+ rc = occ_renew_sensor(driver, sensor_length,
+ sensor_num, CAPS, b);
+ if (rc)
+ continue;
+
+ resp->sensor_block_id[CAPS] = b;
+ for (s = 0; s < sensor_num; s++) {
+ parse_sensor(data, resp->blocks[b].sensors,
+ CAPS, dnum, s);
+ dnum = dnum + sensor_length;
+ }
+
+ } else {
+ dev_dbg(dev, "ERROR: sensor type %s not supported\n",
+ resp->blocks[b].sensor_type);
+ rc = -1;
+ goto err;
+ }
+
+ strncpy(resp->blocks[b].sensor_type, sensor_type, 4);
+ resp->blocks[b].sensor_format = sensor_format;
+ resp->blocks[b].sensor_length = sensor_length;
+ resp->blocks[b].sensor_num = sensor_num;
+ }
+
+ 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;
+ int i;
+
+ length = cpu_to_le16(length);
+ cmd1 = (seq << 24) | (type << 16) | length;
+ memcpy(&cmd2, data, length);
+ cmd2 <<= ((4 - length) * 8);
+
+ /* checksum: sum of every bytes of cmd1, cmd2 */
+ checksum = 0;
+ for (i = 0; i < 4; i++)
+ checksum += (cmd1 >> (i * 8)) & 0xFF;
+ for (i = 0; i < 4; i++)
+ checksum += (cmd2 >> (i * 8)) & 0xFF;
+ cmd2 |= checksum << ((2 - length) * 8);
+
+ /* Init OCB */
+ driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_OR, 0x08000000,
+ 0x00000000);
+ driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_AND,
+ 0xFBFFFFFF, 0xFFFFFFFF);
+
+ /* Send command */
+ driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+ driver->config.command_addr, 0x00000000);
+ driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+ driver->config.command_addr, 0x00000000);
+ driver->bus_ops.putscom(driver->bus, OCB_DATA, cmd1, cmd2);
+
+ /* Trigger attention */
+ driver->bus_ops.putscom(driver->bus, ATTN_DATA, 0x01010000,
+ 0x00000000);
+
+ /* Get response data */
+ driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+ driver->config.response_addr, 0x00000000);
+ driver->bus_ops.getscom(driver->bus, OCB_DATA, resp, 0);
+
+ /* return status */
+ return resp[2];
+}
+
+static inline u16 get_occdata_length(u8 *data)
+{
+ return be16_to_cpup((const __be16 *)&data[RESP_DATA_LENGTH]);
+}
+
+static int occ_get_all(struct occ *driver)
+{
+ int i = 0, rc;
+ u8 *occ_data;
+ u16 num_bytes;
+ u8 poll_cmd_data = 0x10;
+ 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, 0, 1, &poll_cmd_data, occ_data);
+ if (rc) {
+ dev_err(dev, "ERROR: OCC Poll: 0x%x\n", rc);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ num_bytes = get_occdata_length(occ_data);
+ dev_dbg(dev, "OCC data length: %d\n", num_bytes);
+
+ if (num_bytes > OCC_DATA_MAX) {
+ dev_dbg(dev, "ERROR: OCC data length must be < 4KB\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (num_bytes <= 0) {
+ dev_dbg(dev, "ERROR: OCC data length is zero\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* read remaining data */
+ for (i = 8; i < num_bytes + 8; i += 8)
+ driver->bus_ops.getscom(driver->bus, OCB_DATA, occ_data, i);
+
+ 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;
+
+ rc = occ_update_device(driver);
+ if (rc != 0) {
+ dev_dbg(driver->dev, "ERROR: 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->sensor_block_id = occ->response.sensor_block_id;
+ blocks->blocks = occ->response.blocks;
+}
+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_le16(cap);
+
+ return occ_send_cmd(occ, 0, 0x22, 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,
+ struct occ_ops ops, struct occ_config config)
+{
+ struct occ *driver = devm_kzalloc(dev, sizeof(struct occ), GFP_KERNEL);
+ if (!driver)
+ return ERR_PTR(-ENOMEM);
+
+ 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 000000000000..758d011e1617
--- /dev/null
+++ b/drivers/hwmon/occ/occ.h
@@ -0,0 +1,79 @@
+/*
+ * 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_driver;
+
+struct sensor_data_block {
+ u8 sensor_type[4];
+ u8 reserved0;
+ u8 sensor_format;
+ u8 sensor_length;
+ u8 sensor_num;
+ void *sensors;
+};
+
+enum sensor_type {
+ FREQ = 0,
+ TEMP,
+ POWER,
+ CAPS,
+ MAX_OCC_SENSOR_TYPE
+};
+
+struct occ;
+
+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;
+ 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 000000000000..ae5ed0a23ef8
--- /dev/null
+++ b/drivers/hwmon/occ/scom.h
@@ -0,0 +1,50 @@
+/*
+ * 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__
+
+#define SCOM_READ_ERROR 1
+#define SCOM_WRITE_ERROR 2
+/*
+ * 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 greater than
+ * or equal to offset + 8 bytes.
+ * @offset: offset into data pointer
+ *
+ * Returns 0 on success or one of -SCOM_READ_ERROR, -SCOM_WRITE_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 -SCOM_WRITE_ERROR on error
+ */
+struct occ_bus_ops {
+ int (*getscom)(void *bus, u32 address, u8 *data, size_t offset);
+ int (*putscom)(void *bus, u32 address, u32 data0, u32 data1);
+};
+
+#endif /* __SCOM_H__ */
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 3/9] hwmon: occ: Add sysfs interface
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (2 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 2/9] hwmon: Add core On-Chip Controller support for POWER CPUs Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation Andrew Jeffery
` (5 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Add a generic mechanism to expose the sensors provided by the OCC in
sysfs.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/Makefile | 2 +-
drivers/hwmon/occ/occ_sysfs.c | 498 ++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_sysfs.h | 51 +++++
3 files changed, 550 insertions(+), 1 deletion(-)
create mode 100644 drivers/hwmon/occ/occ_sysfs.c
create mode 100644 drivers/hwmon/occ/occ_sysfs.h
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index 93cb52f8dd8c..a6881f93067c 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 000000000000..456fd182a3e6
--- /dev/null
+++ b/drivers/hwmon/occ/occ_sysfs.c
@@ -0,0 +1,498 @@
+/*
+ * occ_sysfs.c - OCC sysfs interface
+ *
+ * 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 "occ_sysfs.h"
+
+#define OCC_DATA_MAX 4096
+
+/* 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
+
+#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
+
+#define MAX_SENSOR_ATTR_LEN 32
+
+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 *driver = dev_get_drvdata(dev);
+
+ val = occ_get_sensor_value(driver, sdata->type, sdata->hwmon_index - 1);
+ if (sdata->type == TEMP)
+ val *= 1000; /* in millidegree Celsius */
+
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
+}
+
+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 *driver = dev_get_drvdata(dev);
+
+ val = occ_get_sensor_id(driver, 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 *driver = dev_get_drvdata(dev);
+
+ sensor = occ_get_sensor(driver, CAPS);
+ if (!sensor) {
+ val = -1;
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
+ }
+
+ val = occ_get_caps_value(driver, 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: %u\n", val);
+ rc = occ_set_user_powercap(driver->occ, val);
+ if (rc != 0) {
+ dev_err(dev,
+ "ERROR: Set User Powercap: wrong return status: %x\n",
+ rc);
+ if (rc == 0x13)
+ dev_info(dev,
+ "ERROR: set invalid powercap value: %x\n",
+ val);
+ return -EINVAL;
+ }
+
+ 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 cnt;
+
+ for (cnt = 0; cnt < MAX_OCC_SENSOR_TYPE; cnt++) {
+ if (sensor_groups[cnt].group.attrs)
+ devm_kfree(dev, sensor_groups[cnt].group.attrs);
+ if (sensor_groups[cnt].sattr)
+ devm_kfree(dev, sensor_groups[cnt].sattr);
+ sensor_groups[cnt].group.attrs = NULL;
+ sensor_groups[cnt].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, cnt;
+
+ /* 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 (cnt = 0; cnt < sensor_num; cnt++) {
+ sdata = &sensor_groups[type].sattr[cnt];
+ /* hwmon attributes index starts from 1 */
+ sdata->hwmon_index = cnt + 1;
+ sdata->type = type;
+ sensor_attr_init(sdata, sensor_groups[type].name, "input",
+ show_input);
+ sensor_groups[type].group.attrs[cnt] = &sdata->dev_attr.attr;
+
+ sdata = &sensor_groups[type].sattr[cnt + sensor_num];
+ sdata->hwmon_index = cnt + 1;
+ sdata->type = type;
+ sensor_attr_init(sdata, sensor_groups[type].name, "label",
+ show_label);
+ sensor_groups[type].group.attrs[cnt + 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, *resp = &_resp;
+ 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 != 0) {
+ dev_err(dev, "ERROR: cannot get occ sensor data: %d\n", rc);
+ return rc;
+ }
+ if (!resp->blocks)
+ return -1;
+
+ 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].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, "ERROR: 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(*hwmon), GFP_KERNEL);
+ int rc;
+
+ if (!hwmon)
+ return ERR_PTR(-ENOMEM);
+
+ hwmon->dev = dev;
+ 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 occ_sysfs *driver)
+{
+ occ_remove_hwmon_attrs(driver);
+ hwmon_device_unregister(driver->dev);
+
+ device_remove_file(driver->dev, &dev_attr_online);
+
+ 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 000000000000..c12cd97bc3fc
--- /dev/null
+++ b/drivers/hwmon/occ/occ_sysfs.h
@@ -0,0 +1,51 @@
+#ifndef OCC_SYSFS_H
+#define OCC_SYSFS_H
+/*
+ * occ_sysfs.c - OCC sysfs interface
+ *
+ * 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 "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 occ_sysfs *driver);
+
+#endif
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (3 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 3/9] hwmon: occ: Add sysfs interface Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-29 22:59 ` Eddie James
2016-11-11 4:12 ` [RFC PATCH linux 5/9] hwmon: occ: Add callbacks for parsing P8 OCC datastructures Andrew Jeffery
` (4 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
The BMC queries the OCC on POWER8 chips with SCOMs over I2C.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/occ_scom_i2c.c | 68 ++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_scom_i2c.h | 20 ++++++++++++
2 files changed, 88 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 000000000000..b1a060331c4a
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.c
@@ -0,0 +1,68 @@
+/*
+ * occ_i2c.c - hwmon OCC driver
+ *
+ * This file contains the i2c layer for accessing the 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/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+
+#define OCC_I2C_NAME "occ-p8-i2c"
+
+int occ_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset)
+{
+ ssize_t rc;
+ u64 buf;
+ struct i2c_client *client = bus;
+
+ rc = i2c_master_send(client, (const char *)&address, sizeof(u32));
+ if (rc != sizeof(u32))
+ return -SCOM_WRITE_ERROR;
+
+ rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
+ if (rc != sizeof(u64))
+ return -SCOM_READ_ERROR;
+
+ *((u64 *)&data[offset]) = 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 != sizeof(u32) * 3)
+ return -SCOM_WRITE_ERROR;
+
+ 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 000000000000..92225f4cd584
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.h
@@ -0,0 +1,20 @@
+/*
+ * occ_i2c.c - hwmon OCC driver
+ *
+ * This file contains the i2c layer for accessing the 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.
+ */
+
+int occ_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset);
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1);
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 5/9] hwmon: occ: Add callbacks for parsing P8 OCC datastructures
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (4 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC Andrew Jeffery
` (3 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/occ_p8.c | 219 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_p8.h | 30 +++++++
2 files changed, 249 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_p8.c
create mode 100644 drivers/hwmon/occ/occ_p8.h
diff --git a/drivers/hwmon/occ/occ_p8.c b/drivers/hwmon/occ/occ_p8.c
new file mode 100644
index 000000000000..b2fa0cb6eb28
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.c
@@ -0,0 +1,219 @@
+/*
+ * 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 "occ.h"
+#include "occ_p8.h"
+
+/* P8 OCC sensor data format */
+struct occ_sensor_p8 {
+ u16 sensor_id;
+ u16 value;
+};
+
+struct power_sensor_p8 {
+ u16 sensor_id;
+ u32 update_tag;
+ u32 accumulator;
+ u16 value;
+};
+
+struct caps_sensor_p8 {
+ 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 occ_sensor_p8 *os =
+ &(((struct occ_sensor_p8 *)sensor)[snum]);
+ os->sensor_id =
+ be16_to_cpup((const __be16 *)&data[off]);
+ os->value =
+ be16_to_cpup((const __be16 *)&data[off + 2]);
+ }
+ break;
+ case POWER:
+ {
+ struct power_sensor_p8 *ps =
+ &(((struct power_sensor_p8 *)sensor)[snum]);
+ ps->sensor_id =
+ be16_to_cpup((const __be16 *)&data[off]);
+ ps->update_tag =
+ be32_to_cpup((const __be32 *)&data[off + 2]);
+ ps->accumulator =
+ be32_to_cpup((const __be32 *)&data[off + 6]);
+ ps->value =
+ be16_to_cpup((const __be16 *)&data[off + 10]);
+ }
+ break;
+ case CAPS:
+ {
+ struct caps_sensor_p8 *cs =
+ &(((struct caps_sensor_p8 *)sensor)[snum]);
+ cs->curr_powercap =
+ be16_to_cpup((const __be16 *)&data[off]);
+ cs->curr_powerreading =
+ be16_to_cpup((const __be16 *)&data[off + 2]);
+ cs->norm_powercap =
+ be16_to_cpup((const __be16 *)&data[off + 4]);
+ cs->max_powercap =
+ be16_to_cpup((const __be16 *)&data[off + 6]);
+ cs->min_powercap =
+ be16_to_cpup((const __be16 *)&data[off + 8]);
+ cs->user_powerlimit =
+ be16_to_cpup((const __be16 *)&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 occ_sensor_p8),
+ GFP_KERNEL);
+ case POWER:
+ return kcalloc(num_sensors,
+ sizeof(struct power_sensor_p8),
+ GFP_KERNEL);
+ case CAPS:
+ return kcalloc(num_sensors,
+ sizeof(struct caps_sensor_p8),
+ 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 -1;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -1;
+
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return ((struct occ_sensor_p8 *)sensor)[snum].value;
+ case POWER:
+ return ((struct power_sensor_p8 *)sensor)[snum].value;
+ default:
+ return -1;
+ }
+}
+
+int p8_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+ int i = snum;
+
+ if (sensor_type == CAPS)
+ return -1;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -1;
+
+ switch (sensor_type) {
+ case FREQ:
+ case TEMP:
+ return ((struct occ_sensor_p8 *)sensor)[i].sensor_id;
+ case POWER:
+ return ((struct power_sensor_p8 *)sensor)[i].sensor_id;
+ default:
+ return -1;
+ }
+}
+
+int p8_get_caps_value(void *sensor, int snum, int caps_field)
+{
+ struct caps_sensor_p8 *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 -1;
+ }
+}
+
+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 *occ_p8_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(occ_p8_start);
+
+int occ_p8_stop(struct occ *occ)
+{
+ return occ_stop(occ);
+}
+EXPORT_SYMBOL(occ_p8_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 000000000000..acae34323a6b
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.h
@@ -0,0 +1,30 @@
+/*
+ * 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 __P8_H__
+#define __P8_H__
+
+#include "scom.h"
+
+struct device;
+
+struct occ *occ_p8_start(struct device *dev, void *bus,
+ struct occ_bus_ops bus_ops);
+int occ_p8_stop(struct occ *occ);
+
+#endif /* __P8_H__ */
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (5 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 5/9] hwmon: occ: Add callbacks for parsing P8 OCC datastructures Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-29 23:00 ` Eddie James
2016-11-11 4:12 ` [RFC PATCH linux 7/9] arm: aspeed: dt: Fix OCC compatible strings Andrew Jeffery
` (2 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/Kconfig | 14 +++++
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/occ_p8_i2c.c | 133 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 148 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_p8_i2c.c
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
index cdb64a71af6e..085e1f370c42 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 occ-p8-i2c.
+
+endif
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index a6881f93067c..05a84e987127 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 occ_p8_i2c.o
diff --git a/drivers/hwmon/occ/occ_p8_i2c.c b/drivers/hwmon/occ/occ_p8_i2c.c
new file mode 100644
index 000000000000..1b66f302fe16
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8_i2c.c
@@ -0,0 +1,133 @@
+/*
+ * occ_p8_i2c.c - hwmon OCC driver
+ *
+ * This file contains the i2c layer for accessing the 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 OCC_I2C_NAME "occ-p8-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, u8 *data, size_t offset)
+{
+ /* P8 i2c slave requires address to be shifted by 1 */
+ address = address << 1;
+
+ return occ_i2c_getscom(bus, address, data, offset);
+}
+
+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 occ_p8_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct occ *occ;
+ struct occ_sysfs *hwmon;
+
+ occ = occ_p8_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 occ_p8_remove(struct i2c_client *client)
+{
+ return occ_p8_stop(i2c_get_clientdata(client));
+}
+
+/* used by old-style board info. */
+static const struct i2c_device_id occ_ids[] = {
+ { 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,occ-p8-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 occ_p8_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = OCC_I2C_NAME,
+ .pm = NULL,
+ .of_match_table = occ_of_match,
+ },
+ .probe = occ_p8_probe,
+ .remove = occ_p8_remove,
+ .id_table = occ_ids,
+ .address_list = normal_i2c,
+};
+
+module_i2c_driver(occ_p8_driver);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("BMC P8 OCC hwmon driver");
+MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 7/9] arm: aspeed: dt: Fix OCC compatible strings
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (6 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 8/9] hwmon: occ: Add callbacks for parsing P9 OCC datastructures Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC Andrew Jeffery
9 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts | 4 ++--
arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts | 4 ++--
arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts | 4 ++--
arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts b/arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts
index 6ca618bd331d..6bbc7817e823 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts
@@ -110,12 +110,12 @@
status = "okay";
occ@50 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x50>;
};
occ@51 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x51>;
};
};
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts b/arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts
index e677e9ff00b8..b88baf5d7add 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts
@@ -122,7 +122,7 @@
// turismo
occ@50 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x50>;
};
@@ -137,7 +137,7 @@
// turismo
occ@50 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x50>;
};
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts b/arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts
index a4471fb3b36b..461de1eaf108 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts
@@ -63,7 +63,7 @@
status = "okay";
occ@50 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x50>;
};
};
@@ -72,7 +72,7 @@
status = "okay";
occ@50 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x50>;
};
};
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
index 0fd60c4eafad..7426110f053f 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
@@ -127,7 +127,7 @@
status = "okay";
occ@50 {
- compatible = "ibm,occ-i2c";
+ compatible = "ibm,occ-p8-i2c";
reg = <0x50>;
};
};
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 8/9] hwmon: occ: Add callbacks for parsing P9 OCC datastructures
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (7 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 7/9] arm: aspeed: dt: Fix OCC compatible strings Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC Andrew Jeffery
9 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/occ_p9.c | 261 +++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/occ/occ_p9.h | 30 ++++++
2 files changed, 291 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_p9.c
create mode 100644 drivers/hwmon/occ/occ_p9.h
diff --git a/drivers/hwmon/occ/occ_p9.c b/drivers/hwmon/occ/occ_p9.c
new file mode 100644
index 000000000000..b8986290948b
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p9.c
@@ -0,0 +1,261 @@
+/*
+ * 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 "occ.h"
+#include "occ_p9.h"
+
+/* P9 OCC sensor data format */
+struct temp_sensor_p9 {
+ u32 sensor_id;
+ u8 fru_type;
+ u8 value;
+};
+
+struct freq_sensor_p9 {
+ u32 sensor_id;
+ u16 value;
+};
+
+struct power_sensor_p9 {
+ u32 sensor_id;
+ u8 function_id;
+ u8 apss_channel;
+ u16 reserved;
+ u32 update_tag;
+ u64 accumulator;
+ u16 value;
+};
+
+struct caps_sensor_p9 {
+ u16 curr_powercap;
+ u16 curr_powerreading;
+ u16 norm_powercap;
+ u16 max_powercap;
+ u16 min_powercap;
+ u16 user_powerlimit;
+ u8 user_powerlimit_source;
+};
+
+/*
+static char *caps_sensor_names[] = {
+ "curr_powercap",
+ "curr_powerreading",
+ "norm_powercap",
+ "max_powercap",
+ "min_powercap",
+ "user_powerlimit",
+ "user_powerlimit_source"
+};
+*/
+
+void p9_parse_sensor(u8 *data, void *sensor, int sensor_type, int off,
+ int snum)
+{
+ switch (sensor_type) {
+ case FREQ:
+ {
+ struct freq_sensor_p9 *fs =
+ &(((struct freq_sensor_p9 *)sensor)[snum]);
+ fs->sensor_id =
+ be32_to_cpup((const __be32 *)&data[off]);
+ fs->value =
+ be16_to_cpup((const __be16 *)&data[off + 4]);
+ }
+ break;
+ case TEMP:
+ {
+ struct temp_sensor_p9 *ts =
+ &(((struct temp_sensor_p9 *)sensor)[snum]);
+ ts->sensor_id =
+ be32_to_cpup((const __be32 *)&data[off]);
+ ts->fru_type = data[off + 4];
+ ts->value = data[off + 5];
+ }
+ break;
+ case POWER:
+ {
+ struct power_sensor_p9 *ps =
+ &(((struct power_sensor_p9 *)sensor)[snum]);
+ ps->sensor_id =
+ be32_to_cpup((const __be32 *)&data[off]);
+ ps->function_id = data[off + 4];
+ ps->apss_channel = data[off + 5];
+ /* two bytes reserved */
+ ps->update_tag =
+ be32_to_cpup((const __be32 *)&data[off + 8]);
+ ps->accumulator =
+ be64_to_cpup((const __be64 *)&data[off + 12]);
+ ps->value =
+ be16_to_cpup((const __be16 *)&data[off + 20]);
+ }
+ break;
+ case CAPS:
+ {
+ struct caps_sensor_p9 *cs =
+ &(((struct caps_sensor_p9 *)sensor)[snum]);
+ cs->curr_powercap =
+ be16_to_cpup((const __be16 *)&data[off]);
+ cs->curr_powerreading =
+ be16_to_cpup((const __be16 *)&data[off + 2]);
+ cs->norm_powercap =
+ be16_to_cpup((const __be16 *)&data[off + 4]);
+ cs->max_powercap =
+ be16_to_cpup((const __be16 *)&data[off + 6]);
+ cs->min_powercap =
+ be16_to_cpup((const __be16 *)&data[off + 8]);
+ cs->user_powerlimit =
+ be16_to_cpup((const __be16 *)&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 freq_sensor_p9),
+ GFP_KERNEL);
+ case TEMP:
+ return kcalloc(num_sensors,
+ sizeof(struct temp_sensor_p9),
+ GFP_KERNEL);
+ case POWER:
+ return kcalloc(num_sensors,
+ sizeof(struct power_sensor_p9),
+ GFP_KERNEL);
+ case CAPS:
+ return kcalloc(num_sensors,
+ sizeof(struct caps_sensor_p9),
+ 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 -1;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -1;
+
+ switch (sensor_type) {
+ case FREQ:
+ return ((struct freq_sensor_p9 *)sensor)[snum].value;
+ case TEMP:
+ return ((struct temp_sensor_p9 *)sensor)[snum].value;
+ case POWER:
+ return ((struct power_sensor_p9 *)sensor)[snum].value;
+ default:
+ return -1;
+ }
+}
+
+int p9_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+ void *sensor;
+ int i = snum;
+
+ if (sensor_type == CAPS)
+ return -1;
+
+ sensor = occ_get_sensor(driver, sensor_type);
+ if (!sensor)
+ return -1;
+
+ switch (sensor_type) {
+ case FREQ:
+ return ((struct freq_sensor_p9 *)sensor)[i].sensor_id;
+ case TEMP:
+ return ((struct temp_sensor_p9 *)sensor)[i].sensor_id;
+ case POWER:
+ return ((struct power_sensor_p9 *)sensor)[i].sensor_id;
+ default:
+ return -1;
+ }
+}
+
+int p9_get_caps_value(void *sensor, int snum, int caps_field)
+{
+ struct caps_sensor_p9 *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 -1;
+ }
+}
+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 *occ_p9_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 occ_p9_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/occ_p9.h b/drivers/hwmon/occ/occ_p9.h
new file mode 100644
index 000000000000..52b38278238b
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p9.h
@@ -0,0 +1,30 @@
+/*
+ * 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 __P9_H__
+#define __P9_H__
+
+#include "scom.h"
+#include "occ.h"
+
+struct device;
+
+struct occ *occ_p9_start(struct device *dev, void *bus, struct occ_bus_ops bus_ops);
+int occ_p9_stop(struct occ *occ);
+
+#endif /* __P9_H__ */
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
` (8 preceding siblings ...)
2016-11-11 4:12 ` [RFC PATCH linux 8/9] hwmon: occ: Add callbacks for parsing P9 OCC datastructures Andrew Jeffery
@ 2016-11-11 4:12 ` Andrew Jeffery
2016-11-29 23:00 ` Eddie James
9 siblings, 1 reply; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-11 4:12 UTC (permalink / raw)
To: eajames.ibm, Joel Stanley
Cc: Edward A . James, OpenBMC Maillist, Andrew Jeffery
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/occ/Kconfig | 9 ++++
drivers/hwmon/occ/Makefile | 1 +
drivers/hwmon/occ/occ_p9_fsi.c | 93 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 103 insertions(+)
create mode 100644 drivers/hwmon/occ/occ_p9_fsi.c
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
index 085e1f370c42..97b97b391bf9 100644
--- a/drivers/hwmon/occ/Kconfig
+++ b/drivers/hwmon/occ/Kconfig
@@ -26,4 +26,13 @@ config SENSORS_PPC_OCC_P8_I2C
This driver can also be built as a module. If so, the module will be
called occ-p8-i2c.
+config SENSORS_PPC_OCC_P9_FSI
+ tristate "POWER9 OCC hwmon support"
+ help
+ Provide a hwmon sysfs interface for the POWER9 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 occ-p9-fsi.
+
endif
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index 05a84e987127..77e47e10195e 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o occ_p8_i2c.o
+obj-$(CONFIG_SENSORS_PPC_OCC_P9_FSI) += occ_p9.o occ_p9_fsi.o
diff --git a/drivers/hwmon/occ/occ_p9_fsi.c b/drivers/hwmon/occ/occ_p9_fsi.c
new file mode 100644
index 000000000000..2f651d1b16bf
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p9_fsi.c
@@ -0,0 +1,93 @@
+/*
+ * occ_p9_fsi.c - hwmon OCC 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/err.h>
+#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 "scom.h"
+#include "occ_p9.h"
+#include "occ_sysfs.h"
+
+#define OCC_I2C_NAME "occ-p9-fsi"
+
+static char *caps_sensor_names[] = {
+ "curr_powercap",
+ "curr_powerreading",
+ "norm_powercap",
+ "max_powercap",
+ "min_powercap",
+ "user_powerlimit"
+};
+
+int p9_fsi_getscom(void *bus, u32 address, u8 *data, size_t offset)
+{
+ return -SCOM_READ_ERROR;
+}
+
+int p9_fsi_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+ return -SCOM_WRITE_ERROR;
+}
+
+static struct occ_bus_ops p9_bus_ops = {
+ .getscom = p9_fsi_getscom,
+ .putscom = p9_fsi_putscom,
+};
+
+static struct occ_sysfs_config p9_sysfs_config = {
+ .num_caps_fields = ARRAY_SIZE(caps_sensor_names),
+ .caps_names = caps_sensor_names,
+};
+
+int occ_p9_probe(struct device *dev, void *client)
+{
+ struct occ *occ;
+ struct occ_sysfs *hwmon;
+
+ occ = occ_p9_start(dev, client, p9_bus_ops);
+ if (IS_ERR(occ))
+ return PTR_ERR(occ);
+
+ hwmon = occ_sysfs_start(dev, occ, &p9_sysfs_config);
+ if (IS_ERR(hwmon))
+ return PTR_ERR(hwmon);
+
+ return 0;
+}
+
+int occ_p9_remove(void)
+{
+ struct occ *occ = NULL;
+
+ return occ_p9_stop(occ);
+}
+
+/* used by device table */
+static const struct of_device_id occ_of_match[] = {
+ { .compatible = "ibm,occ-p9-fsi" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, occ_of_match);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("BMC P9 OCC hwmon driver");
+MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver
2016-11-11 4:12 ` [RFC PATCH linux 0/9] " Andrew Jeffery
@ 2016-11-11 5:16 ` Joel Stanley
2016-11-29 22:58 ` Eddie James
1 sibling, 0 replies; 23+ messages in thread
From: Joel Stanley @ 2016-11-11 5:16 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Edward James, OpenBMC Maillist, eajames.ibm
[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]
On 11 Nov 2016 14:43, "Andrew Jeffery" <andrew@aj.id.au> wrote:
> So I have reworked them into what is now 8 patches (see below) and tried
to
> tell a bit more of a story with them (hopefully I succeeded!) Second, I
tried
> my hand at reworking the layering as mentioned above, such that hardware
> interaction is split from sysfs, and that the SCOM transport and sensor
> configuration is decoupled from the data parsing. You largely had all of
these
> in-place already, I just cut and pasted code around a bit.
Did you look at Jeremy and Chris B's FSI patches? I think you would be the
first client of their new interfaces.
> Please review the patches and provide feedback! I'd like to hear
arguments for
> or against both of our approaches. Note though that I've only tested that
these
> patches compile, I absolutely have not done any further testing.
>
> Joel: Any chance you could chime in with an architectural review?
No worries.
> When the dust has settled on the bigger issues we can start to look at the
> finer details of the APIs (there are some bugs, oddities and complexities
that
> I would like to address before they are applied to openbmc/linux).
I suggest the next step is upstream review. We can put them in the tree if
it's urgent, but I trust that you and Eddie will beat them into shape in no
time. If you succeed we can then apply the upstream versions directly.
Cheers,
Joel
[-- Attachment #2: Type: text/html, Size: 1714 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver
2016-11-11 4:12 ` [RFC PATCH linux 0/9] " Andrew Jeffery
2016-11-11 5:16 ` Joel Stanley
@ 2016-11-29 22:58 ` Eddie James
2016-11-30 1:09 ` Andrew Jeffery
1 sibling, 1 reply; 23+ messages in thread
From: Eddie James @ 2016-11-29 22:58 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
Hi Andrew,
Thanks a lot for putting together this new patchset! On the whole, it
looks very good! The split between sysfs, occ, and hardware transport
layers is solid. I'm pretty satisfied with your modified structuring,
although I had a couple of comments and questions. I'll send those out
in the applicable patches.
I also tested this on palmetto and barreleye systems and fixed a
couple of bugs. I went ahead and put them in a new patchset, which I'm
about to send out. For reference, here's my only diffs though:
Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
drivers/hwmon/occ/occ.c | 2 ++
drivers/hwmon/occ/occ_sysfs.c | 15 +++++++--------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/occ/occ.c b/drivers/hwmon/occ/occ.c
index 18936c8..d2fd81e 100644
--- a/drivers/hwmon/occ/occ.c
+++ b/drivers/hwmon/occ/occ.c
@@ -469,6 +470,7 @@ struct occ *occ_start(struct device *dev, void
*bus, struct occ_bus_ops bus_ops,
if (!driver)
return ERR_PTR(-ENOMEM);
+ driver->dev = dev;
driver->bus = bus;
driver->bus_ops = bus_ops;
driver->ops = ops;
diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
index ef45bd0..aab4fbe 100644
--- a/drivers/hwmon/occ/occ_sysfs.c
+++ b/drivers/hwmon/occ/occ_sysfs.c
@@ -63,9 +63,9 @@ static ssize_t show_input(struct device *dev,
struct sensor_attr_data *sdata = container_of(attr,
struct sensor_attr_data,
dev_attr);
- struct occ *driver = dev_get_drvdata(dev);
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
- val = occ_get_sensor_value(driver, sdata->type, sdata->hwmon_index - 1);
+ val = occ_get_sensor_value(driver->occ, sdata->type,
sdata->hwmon_index - 1);
if (sdata->type == TEMP)
val *= 1000; /* in millidegree Celsius */
@@ -79,9 +79,9 @@ static ssize_t show_label(struct device *dev,
struct sensor_attr_data *sdata = container_of(attr,
struct sensor_attr_data,
dev_attr);
- struct occ *driver = dev_get_drvdata(dev);
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
- val = occ_get_sensor_id(driver, sdata->type,
+ val = occ_get_sensor_id(driver->occ, sdata->type,
sdata->hwmon_index - 1);
return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
@@ -95,15 +95,15 @@ static ssize_t show_caps(struct device *dev,
struct sensor_attr_data *sdata = container_of(attr,
struct sensor_attr_data,
dev_attr);
- struct occ *driver = dev_get_drvdata(dev);
+ struct occ_sysfs *driver = dev_get_drvdata(dev);
- sensor = occ_get_sensor(driver, CAPS);
+ 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, sensor, sdata->hwmon_index - 1,
+ val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
sdata->attr_id);
return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
@@ -467,7 +467,6 @@ struct occ_sysfs *occ_sysfs_start(struct device
*dev, struct occ *occ,
if (!hwmon)
return ERR_PTR(-ENOMEM);
- hwmon->dev = dev;
hwmon->occ = occ;
hwmon->num_caps_fields = config->num_caps_fields;
hwmon->caps_names = config->caps_names;
--
1.9.1
On Thu, Nov 10, 2016 at 10:12 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Hi Eddie,
>
>> This patchset provides a new OCC hwmon driver. There are a number
>> of issues with the existing driver. Firstly, i2c access was embedded
>> throughout the driver. Secondly, there is no way to easily differentiate
>> versions of the OCC.
>>
>> This patchset addresses the first issue by abstracting the bus transfer
>> protocol into a modular structure. In this way, any low level transfer
>> method may be easily implemented.
>>
>> The second issue is addressed by separating the "version specific" code
>> for the OCC and the common hwmon code. This task is not yet complete, but
>> the general structure is in place. Ultimately, different OCC versions
>> could be probed up using the device tree.
>
> I agree with all of this. However, I thought some of the layering could still
> be improved, so rather than provide a review through prose I figured it might
> be better for both of us if I mangled the patches directly.
>
> I've attacked two things. The first is I found the series hard to review due to
> the way the patches were split, as the patches went from trivial to huge (and
> complex) in a fairly short step:
>
>>
>> Edward A. James (5):
>> Revert "hwmon: Add Power8 OCC hwmon driver"
>> drivers: Add hwmon occ driver framework
>> drivers: hwmon OCC scom bus operations
>> drivers: Add hwmon OCC version specific functions
>> drivers: OCC hwmon driver and sysfs
>
> So I have reworked them into what is now 8 patches (see below) and tried to
> tell a bit more of a story with them (hopefully I succeeded!) Second, I tried
> my hand at reworking the layering as mentioned above, such that hardware
> interaction is split from sysfs, and that the SCOM transport and sensor
> configuration is decoupled from the data parsing. You largely had all of these
> in-place already, I just cut and pasted code around a bit.
>
> Please review the patches and provide feedback! I'd like to hear arguments for
> or against both of our approaches. Note though that I've only tested that these
> patches compile, I absolutely have not done any further testing.
>
> Joel: Any chance you could chime in with an architectural review?
>
> When the dust has settled on the bigger issues we can start to look at the
> finer details of the APIs (there are some bugs, oddities and complexities that
> I would like to address before they are applied to openbmc/linux).
>
> Cheers,
>
> Andrew
>
> Andrew Jeffery (8):
> hwmon: Add core On-Chip Controller support for POWER CPUs
> hwmon: occ: Add sysfs interface
> hwmon: occ: Add I2C SCOM transport implementation
> hwmon: occ: Add callbacks for parsing P8 OCC datastructures
> hwmon: occ: Add hwmon implementation for the P8 OCC
> arm: aspeed: dt: Fix OCC compatible strings
> hwmon: occ: Add callbacks for parsing P9 OCC datastructures
> wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC
>
> Edward A. James (1):
> Revert "hwmon: Add Power8 OCC hwmon driver"
>
> .../devicetree/bindings/i2c/i2c-ibm-occ.txt | 13 -
> arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts | 4 +-
> arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts | 4 +-
> arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts | 4 +-
> arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 2 +-
> drivers/hwmon/Kconfig | 13 +-
> drivers/hwmon/Makefile | 2 +-
> drivers/hwmon/occ/Kconfig | 38 +
> drivers/hwmon/occ/Makefile | 3 +
> drivers/hwmon/occ/occ.c | 494 ++++++++
> drivers/hwmon/occ/occ.h | 79 ++
> drivers/hwmon/occ/occ_p8.c | 219 ++++
> drivers/hwmon/occ/occ_p8.h | 30 +
> drivers/hwmon/occ/occ_p8_i2c.c | 133 +++
> drivers/hwmon/occ/occ_p9.c | 261 ++++
> drivers/hwmon/occ/occ_p9.h | 30 +
> drivers/hwmon/occ/occ_p9_fsi.c | 93 ++
> drivers/hwmon/occ/occ_scom_i2c.c | 68 ++
> drivers/hwmon/occ/occ_scom_i2c.h | 20 +
> drivers/hwmon/occ/occ_sysfs.c | 498 ++++++++
> drivers/hwmon/occ/occ_sysfs.h | 51 +
> drivers/hwmon/occ/scom.h | 50 +
> drivers/hwmon/power8_occ_i2c.c | 1254 --------------------
> 23 files changed, 2076 insertions(+), 1287 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
> 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/occ_p8.c
> create mode 100644 drivers/hwmon/occ/occ_p8.h
> create mode 100644 drivers/hwmon/occ/occ_p8_i2c.c
> create mode 100644 drivers/hwmon/occ/occ_p9.c
> create mode 100644 drivers/hwmon/occ/occ_p9.h
> create mode 100644 drivers/hwmon/occ/occ_p9_fsi.c
> create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
> create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h
> create mode 100644 drivers/hwmon/occ/occ_sysfs.c
> create mode 100644 drivers/hwmon/occ/occ_sysfs.h
> create mode 100644 drivers/hwmon/occ/scom.h
> delete mode 100644 drivers/hwmon/power8_occ_i2c.c
>
> --
> 2.9.3
>
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation
2016-11-11 4:12 ` [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation Andrew Jeffery
@ 2016-11-29 22:59 ` Eddie James
2016-11-30 0:30 ` Andrew Jeffery
0 siblings, 1 reply; 23+ messages in thread
From: Eddie James @ 2016-11-29 22:59 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
Hi Andrew,
Just a general comment on this one. I'm not sure it's worth having an
extra layer just to isolate these functions from the
p8_i2c_getscom/putscom. Those just do the address shift for P8. But
there's no situation where i2c is used that's not on the P8. So this
seems like unnecessary complexity to me. I'm not sure, just a thought.
Thanks,
Eddie
On Thu, Nov 10, 2016 at 10:12 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> The BMC queries the OCC on POWER8 chips with SCOMs over I2C.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/hwmon/occ/occ_scom_i2c.c | 68 ++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/occ/occ_scom_i2c.h | 20 ++++++++++++
> 2 files changed, 88 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 000000000000..b1a060331c4a
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_scom_i2c.c
> @@ -0,0 +1,68 @@
> +/*
> + * occ_i2c.c - hwmon OCC driver
> + *
> + * This file contains the i2c layer for accessing the 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/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include "scom.h"
> +#include "occ_scom_i2c.h"
> +
> +#define OCC_I2C_NAME "occ-p8-i2c"
> +
> +int occ_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset)
> +{
> + ssize_t rc;
> + u64 buf;
> + struct i2c_client *client = bus;
> +
> + rc = i2c_master_send(client, (const char *)&address, sizeof(u32));
> + if (rc != sizeof(u32))
> + return -SCOM_WRITE_ERROR;
> +
> + rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
> + if (rc != sizeof(u64))
> + return -SCOM_READ_ERROR;
> +
> + *((u64 *)&data[offset]) = 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 != sizeof(u32) * 3)
> + return -SCOM_WRITE_ERROR;
> +
> + 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 000000000000..92225f4cd584
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_scom_i2c.h
> @@ -0,0 +1,20 @@
> +/*
> + * occ_i2c.c - hwmon OCC driver
> + *
> + * This file contains the i2c layer for accessing the 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.
> + */
> +
> +int occ_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset);
> +int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1);
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC
2016-11-11 4:12 ` [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC Andrew Jeffery
@ 2016-11-29 23:00 ` Eddie James
2016-11-30 1:00 ` Andrew Jeffery
0 siblings, 1 reply; 23+ messages in thread
From: Eddie James @ 2016-11-29 23:00 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
Hi Andrew,
One comment on this one.
Thanks,
Eddie
On Thu, Nov 10, 2016 at 10:12 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/hwmon/occ/Kconfig | 14 +++++
> drivers/hwmon/occ/Makefile | 1 +
> drivers/hwmon/occ/occ_p8_i2c.c | 133 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 148 insertions(+)
> create mode 100644 drivers/hwmon/occ/occ_p8_i2c.c
>
> diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
> index cdb64a71af6e..085e1f370c42 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 occ-p8-i2c.
> +
> +endif
> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> index a6881f93067c..05a84e987127 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 occ_p8_i2c.o
> diff --git a/drivers/hwmon/occ/occ_p8_i2c.c b/drivers/hwmon/occ/occ_p8_i2c.c
> new file mode 100644
> index 000000000000..1b66f302fe16
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_p8_i2c.c
> @@ -0,0 +1,133 @@
> +/*
> + * occ_p8_i2c.c - hwmon OCC driver
> + *
> + * This file contains the i2c layer for accessing the 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 OCC_I2C_NAME "occ-p8-i2c"
> +
> +static char *caps_sensor_names[] = {
> + "curr_powercap",
> + "curr_powerreading",
> + "norm_powercap",
> + "max_powercap",
> + "min_powercap",
> + "user_powerlimit"
> +};
Does it make more sense to define this in the P8 callbacks code?
> +
> +int p8_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset)
> +{
> + /* P8 i2c slave requires address to be shifted by 1 */
> + address = address << 1;
> +
> + return occ_i2c_getscom(bus, address, data, offset);
> +}
> +
> +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,
> +};
Same as above for this structure.
> +
> +static int occ_p8_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct occ *occ;
> + struct occ_sysfs *hwmon;
> +
> + occ = occ_p8_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);
I see why you defined those here. But I guess I don't see why
occ_sysfs_config has to be separate from occ_config structure. It
could be populated in occ_p8_start and then passed to occ_sysfs_start
here.
> + if (IS_ERR(hwmon))
> + return PTR_ERR(hwmon);
> +
> + i2c_set_clientdata(client, hwmon);
> +
> + return 0;
> +}
> +
> +static int occ_p8_remove(struct i2c_client *client)
> +{
> + return occ_p8_stop(i2c_get_clientdata(client));
> +}
> +
> +/* used by old-style board info. */
> +static const struct i2c_device_id occ_ids[] = {
> + { 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,occ-p8-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 occ_p8_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = OCC_I2C_NAME,
> + .pm = NULL,
> + .of_match_table = occ_of_match,
> + },
> + .probe = occ_p8_probe,
> + .remove = occ_p8_remove,
> + .id_table = occ_ids,
> + .address_list = normal_i2c,
> +};
> +
> +module_i2c_driver(occ_p8_driver);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("BMC P8 OCC hwmon driver");
> +MODULE_LICENSE("GPL");
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC
2016-11-11 4:12 ` [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC Andrew Jeffery
@ 2016-11-29 23:00 ` Eddie James
2016-11-30 1:04 ` Andrew Jeffery
0 siblings, 1 reply; 23+ messages in thread
From: Eddie James @ 2016-11-29 23:00 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
Hi Andrew,
Looks pretty good. Do we need to get this finalized before getting
into our kernel or submitting it upstream? I don't think the SBE or
FSI interfaces are nailed down, are they?
Eddie
On Thu, Nov 10, 2016 at 10:12 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/hwmon/occ/Kconfig | 9 ++++
> drivers/hwmon/occ/Makefile | 1 +
> drivers/hwmon/occ/occ_p9_fsi.c | 93 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 103 insertions(+)
> create mode 100644 drivers/hwmon/occ/occ_p9_fsi.c
>
> diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
> index 085e1f370c42..97b97b391bf9 100644
> --- a/drivers/hwmon/occ/Kconfig
> +++ b/drivers/hwmon/occ/Kconfig
> @@ -26,4 +26,13 @@ config SENSORS_PPC_OCC_P8_I2C
> This driver can also be built as a module. If so, the module will be
> called occ-p8-i2c.
>
> +config SENSORS_PPC_OCC_P9_FSI
> + tristate "POWER9 OCC hwmon support"
> + help
> + Provide a hwmon sysfs interface for the POWER9 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 occ-p9-fsi.
> +
> endif
> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> index 05a84e987127..77e47e10195e 100644
> --- a/drivers/hwmon/occ/Makefile
> +++ b/drivers/hwmon/occ/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
> obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o occ_p8_i2c.o
> +obj-$(CONFIG_SENSORS_PPC_OCC_P9_FSI) += occ_p9.o occ_p9_fsi.o
> diff --git a/drivers/hwmon/occ/occ_p9_fsi.c b/drivers/hwmon/occ/occ_p9_fsi.c
> new file mode 100644
> index 000000000000..2f651d1b16bf
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_p9_fsi.c
> @@ -0,0 +1,93 @@
> +/*
> + * occ_p9_fsi.c - hwmon OCC 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/err.h>
> +#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 "scom.h"
> +#include "occ_p9.h"
> +#include "occ_sysfs.h"
> +
> +#define OCC_I2C_NAME "occ-p9-fsi"
> +
> +static char *caps_sensor_names[] = {
> + "curr_powercap",
> + "curr_powerreading",
> + "norm_powercap",
> + "max_powercap",
> + "min_powercap",
> + "user_powerlimit"
> +};
> +
> +int p9_fsi_getscom(void *bus, u32 address, u8 *data, size_t offset)
> +{
> + return -SCOM_READ_ERROR;
> +}
> +
> +int p9_fsi_putscom(void *bus, u32 address, u32 data0, u32 data1)
> +{
> + return -SCOM_WRITE_ERROR;
> +}
> +
> +static struct occ_bus_ops p9_bus_ops = {
> + .getscom = p9_fsi_getscom,
> + .putscom = p9_fsi_putscom,
> +};
> +
> +static struct occ_sysfs_config p9_sysfs_config = {
> + .num_caps_fields = ARRAY_SIZE(caps_sensor_names),
> + .caps_names = caps_sensor_names,
> +};
> +
> +int occ_p9_probe(struct device *dev, void *client)
> +{
> + struct occ *occ;
> + struct occ_sysfs *hwmon;
> +
> + occ = occ_p9_start(dev, client, p9_bus_ops);
> + if (IS_ERR(occ))
> + return PTR_ERR(occ);
> +
> + hwmon = occ_sysfs_start(dev, occ, &p9_sysfs_config);
> + if (IS_ERR(hwmon))
> + return PTR_ERR(hwmon);
> +
> + return 0;
> +}
> +
> +int occ_p9_remove(void)
> +{
> + struct occ *occ = NULL;
> +
> + return occ_p9_stop(occ);
> +}
> +
> +/* used by device table */
> +static const struct of_device_id occ_of_match[] = {
> + { .compatible = "ibm,occ-p9-fsi" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, occ_of_match);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("BMC P9 OCC hwmon driver");
> +MODULE_LICENSE("GPL");
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation
2016-11-29 22:59 ` Eddie James
@ 2016-11-30 0:30 ` Andrew Jeffery
0 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-30 0:30 UTC (permalink / raw)
To: Eddie James; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 5250 bytes --]
On Tue, 2016-11-29 at 16:59 -0600, Eddie James wrote:
> Hi Andrew,
>
> Just a general comment on this one. I'm not sure it's worth having an
> extra layer just to isolate these functions from the
> p8_i2c_getscom/putscom. Those just do the address shift for P8. But
> there's no situation where i2c is used that's not on the P8. So this
> seems like unnecessary complexity to me. I'm not sure, just a thought.
My perspective is that baking P8 knowledge into the I2C SCOM operations
makes the code surprising. Maybe renaming the functions is enough,
something like p8_occ_getscom(). To me it doesn't seem like much cost
in complexity to separate the transport from implementation-specific
quirks with the addressing, and gaining a general approach in the
process.
Maybe Joel can weigh in here?
Andrew
>
> Thanks,
> Eddie
>
> > On Thu, Nov 10, 2016 at 10:12 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> > The BMC queries the OCC on POWER8 chips with SCOMs over I2C.
> >
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > drivers/hwmon/occ/occ_scom_i2c.c | 68 ++++++++++++++++++++++++++++++++++++++++
> > drivers/hwmon/occ/occ_scom_i2c.h | 20 ++++++++++++
> > 2 files changed, 88 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 000000000000..b1a060331c4a
> > --- /dev/null
> > +++ b/drivers/hwmon/occ/occ_scom_i2c.c
> > @@ -0,0 +1,68 @@
> > +/*
> > + * occ_i2c.c - hwmon OCC driver
> > + *
> > + * This file contains the i2c layer for accessing the 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/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +
> > +#include "scom.h"
> > +#include "occ_scom_i2c.h"
> > +
> > +#define OCC_I2C_NAME "occ-p8-i2c"
> > +
> > +int occ_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset)
> > +{
> > + ssize_t rc;
> > + u64 buf;
> > + struct i2c_client *client = bus;
> > +
> > + rc = i2c_master_send(client, (const char *)&address, sizeof(u32));
> > + if (rc != sizeof(u32))
> > + return -SCOM_WRITE_ERROR;
> > +
> > + rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
> > + if (rc != sizeof(u64))
> > + return -SCOM_READ_ERROR;
> > +
> > + *((u64 *)&data[offset]) = 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 != sizeof(u32) * 3)
> > + return -SCOM_WRITE_ERROR;
> > +
> > + 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 000000000000..92225f4cd584
> > --- /dev/null
> > +++ b/drivers/hwmon/occ/occ_scom_i2c.h
> > @@ -0,0 +1,20 @@
> > +/*
> > + * occ_i2c.c - hwmon OCC driver
> > + *
> > + * This file contains the i2c layer for accessing the 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.
> > + */
> > +
> > +int occ_i2c_getscom(void *bus, u32 address, u8 *data, size_t offset);
> > +int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1);
> > --
> > 2.9.3
> >
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC
2016-11-29 23:00 ` Eddie James
@ 2016-11-30 1:00 ` Andrew Jeffery
2016-11-30 16:17 ` Eddie James
0 siblings, 1 reply; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-30 1:00 UTC (permalink / raw)
To: Eddie James; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 2555 bytes --]
On Tue, 2016-11-29 at 17:00 -0600, Eddie James wrote:
> > +
> > +static int occ_p8_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct occ *occ;
> > + struct occ_sysfs *hwmon;
> > +
> > + occ = occ_p8_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);
>
> I see why you defined those here. But I guess I don't see why
> occ_sysfs_config has to be separate from occ_config structure. It
> could be populated in occ_p8_start and then passed to occ_sysfs_start
> here.
I admit getting to this point was a little tricky, and I'm not
completely satisfied by what I've come up with. Here's my reasoning:
I felt that only half of your struct occ_protocol dealt with SCOM
protocol problems, specifically the command_addr and response_addr
members. num_caps_fields and caps_names were only used in the sysfs
code where they are relevant for interpretation of the payload, but are
not in transacting SCOMs. Therefore I removed them from struct
occ_protocol.
I also renamed struct occ_protocol to occ_config. I could live with
either name but my feeling is it's more obvious that the consumer of
the API needs to populate a struct with 'config' in its name.
Given I'd removed num_caps_fields and caps_names I had to put the
information somewhere. One place might be in struct occ as I've
interpreted your suggestion above. However, I was trying to keep struct
occ opaque so in occ.c could fully encapsulate its behaviour. As struct
occ is opaque, occ_sysfs.c has no visibility into its members and thus
can't access anything we stash in it, e.g. num_caps_fields and
caps_names.
That leaves us with needing some other solution, one of which I've
implemented above. It achieves the goals I was outlining above and
keeps the sysfs code free of any implementation specifics, at the cost
of describing the information in a slightly weird spot.
An alternative might be to define struct occ_sysfs_config in occ_p8.c
and add an extern to occ_p8.h. That bakes sysfs knowledge into the p8
header which to me feels more unfortunate than my current approach. Or,
define say p8_caps_names and p8_num_caps in occ_p8.c and extern those
in occ_p8.h and use them to define a struct occ_sysfs_config here. What
do you think?
Andrew
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC
2016-11-29 23:00 ` Eddie James
@ 2016-11-30 1:04 ` Andrew Jeffery
0 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-30 1:04 UTC (permalink / raw)
To: Eddie James; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 5608 bytes --]
On Tue, 2016-11-29 at 17:00 -0600, Eddie James wrote:
> Hi Andrew,
>
> Looks pretty good. Do we need to get this finalized before getting
> into our kernel or submitting it upstream? I don't think the SBE or
> FSI interfaces are nailed down, are they?
This patch is a complete hack only to illustrate how the abstractions
would work for P9. It doesn't actually work in any way. So yes, we
definitely need to finalise it before it goes anywhere.
FSI is coming together but we don't yet have it in the tree.
Andrew
>
> Eddie
>
> On Thu, Nov 10, 2016 at 10:12 PM, Andrew Jeffery <andrew@aj.id.au>
> wrote:
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > drivers/hwmon/occ/Kconfig | 9 ++++
> > drivers/hwmon/occ/Makefile | 1 +
> > drivers/hwmon/occ/occ_p9_fsi.c | 93
> > ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 103 insertions(+)
> > create mode 100644 drivers/hwmon/occ/occ_p9_fsi.c
> >
> > diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
> > index 085e1f370c42..97b97b391bf9 100644
> > --- a/drivers/hwmon/occ/Kconfig
> > +++ b/drivers/hwmon/occ/Kconfig
> > @@ -26,4 +26,13 @@ config SENSORS_PPC_OCC_P8_I2C
> > This driver can also be built as a module. If so, the
> > module will be
> > called occ-p8-i2c.
> >
> > +config SENSORS_PPC_OCC_P9_FSI
> > + tristate "POWER9 OCC hwmon support"
> > + help
> > + Provide a hwmon sysfs interface for the POWER9 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 occ-p9-fsi.
> > +
> > endif
> > diff --git a/drivers/hwmon/occ/Makefile
> > b/drivers/hwmon/occ/Makefile
> > index 05a84e987127..77e47e10195e 100644
> > --- a/drivers/hwmon/occ/Makefile
> > +++ b/drivers/hwmon/occ/Makefile
> > @@ -1,2 +1,3 @@
> > obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
> > obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o
> > occ_p8_i2c.o
> > +obj-$(CONFIG_SENSORS_PPC_OCC_P9_FSI) += occ_p9.o occ_p9_fsi.o
> > diff --git a/drivers/hwmon/occ/occ_p9_fsi.c
> > b/drivers/hwmon/occ/occ_p9_fsi.c
> > new file mode 100644
> > index 000000000000..2f651d1b16bf
> > --- /dev/null
> > +++ b/drivers/hwmon/occ/occ_p9_fsi.c
> > @@ -0,0 +1,93 @@
> > +/*
> > + * occ_p9_fsi.c - hwmon OCC 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/err.h>
> > +#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 "scom.h"
> > +#include "occ_p9.h"
> > +#include "occ_sysfs.h"
> > +
> > +#define OCC_I2C_NAME "occ-p9-fsi"
> > +
> > +static char *caps_sensor_names[] = {
> > + "curr_powercap",
> > + "curr_powerreading",
> > + "norm_powercap",
> > + "max_powercap",
> > + "min_powercap",
> > + "user_powerlimit"
> > +};
> > +
> > +int p9_fsi_getscom(void *bus, u32 address, u8 *data, size_t
> > offset)
> > +{
> > + return -SCOM_READ_ERROR;
> > +}
> > +
> > +int p9_fsi_putscom(void *bus, u32 address, u32 data0, u32 data1)
> > +{
> > + return -SCOM_WRITE_ERROR;
> > +}
> > +
> > +static struct occ_bus_ops p9_bus_ops = {
> > + .getscom = p9_fsi_getscom,
> > + .putscom = p9_fsi_putscom,
> > +};
> > +
> > +static struct occ_sysfs_config p9_sysfs_config = {
> > + .num_caps_fields = ARRAY_SIZE(caps_sensor_names),
> > + .caps_names = caps_sensor_names,
> > +};
> > +
> > +int occ_p9_probe(struct device *dev, void *client)
> > +{
> > + struct occ *occ;
> > + struct occ_sysfs *hwmon;
> > +
> > + occ = occ_p9_start(dev, client, p9_bus_ops);
> > + if (IS_ERR(occ))
> > + return PTR_ERR(occ);
> > +
> > + hwmon = occ_sysfs_start(dev, occ, &p9_sysfs_config);
> > + if (IS_ERR(hwmon))
> > + return PTR_ERR(hwmon);
> > +
> > + return 0;
> > +}
> > +
> > +int occ_p9_remove(void)
> > +{
> > + struct occ *occ = NULL;
> > +
> > + return occ_p9_stop(occ);
> > +}
> > +
> > +/* used by device table */
> > +static const struct of_device_id occ_of_match[] = {
> > + { .compatible = "ibm,occ-p9-fsi" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, occ_of_match);
> > +
> > +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> > +MODULE_DESCRIPTION("BMC P9 OCC hwmon driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.9.3
> >
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver
2016-11-29 22:58 ` Eddie James
@ 2016-11-30 1:09 ` Andrew Jeffery
2016-12-06 14:53 ` Eddie James
0 siblings, 1 reply; 23+ messages in thread
From: Andrew Jeffery @ 2016-11-30 1:09 UTC (permalink / raw)
To: Eddie James; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 846 bytes --]
On Tue, 2016-11-29 at 16:58 -0600, Eddie James wrote:
> Hi Andrew,
>
> Thanks a lot for putting together this new patchset! On the whole, it
> looks very good! The split between sysfs, occ, and hardware transport
> layers is solid. I'm pretty satisfied with your modified structuring,
> although I had a couple of comments and questions. I'll send those out
> in the applicable patches.
I've just finished replying to your comments. The most controversial
one was the cap_names/num_cap_names problem, but we have some
alternatives there.
>
> I also tested this on palmetto and barreleye systems and fixed a
> couple of bugs.
Great!
> I went ahead and put them in a new patchset, which I'm
> about to send out. For reference, here's my only diffs though:
Good ol' void conversions eh.
Thanks for testing!
Andrew
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC
2016-11-30 1:00 ` Andrew Jeffery
@ 2016-11-30 16:17 ` Eddie James
0 siblings, 0 replies; 23+ messages in thread
From: Eddie James @ 2016-11-30 16:17 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
On Tue, Nov 29, 2016 at 7:00 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Tue, 2016-11-29 at 17:00 -0600, Eddie James wrote:
>> > +
>> > +static int occ_p8_probe(struct i2c_client *client,
>> > + const struct i2c_device_id *id)
>> > +{
>> > + struct occ *occ;
>> > + struct occ_sysfs *hwmon;
>> > +
>> > + occ = occ_p8_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);
>>
>> I see why you defined those here. But I guess I don't see why
>> occ_sysfs_config has to be separate from occ_config structure. It
>> could be populated in occ_p8_start and then passed to occ_sysfs_start
>> here.
>
> I admit getting to this point was a little tricky, and I'm not
> completely satisfied by what I've come up with. Here's my reasoning:
>
> I felt that only half of your struct occ_protocol dealt with SCOM
> protocol problems, specifically the command_addr and response_addr
> members. num_caps_fields and caps_names were only used in the sysfs
> code where they are relevant for interpretation of the payload, but are
> not in transacting SCOMs. Therefore I removed them from struct
> occ_protocol.
Makes sense.
>
> I also renamed struct occ_protocol to occ_config. I could live with
> either name but my feeling is it's more obvious that the consumer of
> the API needs to populate a struct with 'config' in its name.
>
> Given I'd removed num_caps_fields and caps_names I had to put the
> information somewhere. One place might be in struct occ as I've
> interpreted your suggestion above. However, I was trying to keep struct
> occ opaque so in occ.c could fully encapsulate its behaviour. As struct
> occ is opaque, occ_sysfs.c has no visibility into its members and thus
> can't access anything we stash in it, e.g. num_caps_fields and
> caps_names.
>
> That leaves us with needing some other solution, one of which I've
> implemented above. It achieves the goals I was outlining above and
> keeps the sysfs code free of any implementation specifics, at the cost
> of describing the information in a slightly weird spot.
>
> An alternative might be to define struct occ_sysfs_config in occ_p8.c
> and add an extern to occ_p8.h. That bakes sysfs knowledge into the p8
> header which to me feels more unfortunate than my current approach. Or,
> define say p8_caps_names and p8_num_caps in occ_p8.c and extern those
> in occ_p8.h and use them to define a struct occ_sysfs_config here. What
> do you think?
I think your solution is fine, I was just a bit confused as to why you
decided to do that. Let's stick with your solution, it's better than
putting it in the headers.
Thanks,
Eddie
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver
2016-11-30 1:09 ` Andrew Jeffery
@ 2016-12-06 14:53 ` Eddie James
2016-12-06 14:55 ` Andrew Jeffery
0 siblings, 1 reply; 23+ messages in thread
From: Eddie James @ 2016-12-06 14:53 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
Hi Andrew,
What's the status on this? I last sent out a tested patch set that I'm
pretty happy with. I'm not sure what the next step for me is, any
ideas?
Thanks,
Eddie
On Tue, Nov 29, 2016 at 7:09 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Tue, 2016-11-29 at 16:58 -0600, Eddie James wrote:
>> Hi Andrew,
>>
>> Thanks a lot for putting together this new patchset! On the whole, it
>> looks very good! The split between sysfs, occ, and hardware transport
>> layers is solid. I'm pretty satisfied with your modified structuring,
>> although I had a couple of comments and questions. I'll send those out
>> in the applicable patches.
>
> I've just finished replying to your comments. The most controversial
> one was the cap_names/num_cap_names problem, but we have some
> alternatives there.
>
>>
>> I also tested this on palmetto and barreleye systems and fixed a
>> couple of bugs.
>
> Great!
>
>> I went ahead and put them in a new patchset, which I'm
>> about to send out. For reference, here's my only diffs though:
>
> Good ol' void conversions eh.
>
> Thanks for testing!
>
> Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver
2016-12-06 14:53 ` Eddie James
@ 2016-12-06 14:55 ` Andrew Jeffery
0 siblings, 0 replies; 23+ messages in thread
From: Andrew Jeffery @ 2016-12-06 14:55 UTC (permalink / raw)
To: Eddie James; +Cc: Joel Stanley, Edward A . James, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 364 bytes --]
On Tue, 2016-12-06 at 08:53 -0600, Eddie James wrote:
> Hi Andrew,
>
> What's the status on this? I last sent out a tested patch set that I'm
> pretty happy with. I'm not sure what the next step for me is, any
> ideas?
>
Sorry, I've been busy on other things but will now have some time to
look at the series. I'll take a look in the morning.
Andrew
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-12-06 14:55 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-07 23:15 [PATCH linux v5 0/5] On-Chip Controller (OCC) hwmon driver eajames.ibm
2016-11-11 4:12 ` [RFC PATCH linux 0/9] " Andrew Jeffery
2016-11-11 5:16 ` Joel Stanley
2016-11-29 22:58 ` Eddie James
2016-11-30 1:09 ` Andrew Jeffery
2016-12-06 14:53 ` Eddie James
2016-12-06 14:55 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 1/9] Revert "hwmon: Add Power8 OCC hwmon driver" Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 2/9] hwmon: Add core On-Chip Controller support for POWER CPUs Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 3/9] hwmon: occ: Add sysfs interface Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 4/9] hwmon: occ: Add I2C SCOM transport implementation Andrew Jeffery
2016-11-29 22:59 ` Eddie James
2016-11-30 0:30 ` Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 5/9] hwmon: occ: Add callbacks for parsing P8 OCC datastructures Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 6/9] hwmon: occ: Add hwmon implementation for the P8 OCC Andrew Jeffery
2016-11-29 23:00 ` Eddie James
2016-11-30 1:00 ` Andrew Jeffery
2016-11-30 16:17 ` Eddie James
2016-11-11 4:12 ` [RFC PATCH linux 7/9] arm: aspeed: dt: Fix OCC compatible strings Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 8/9] hwmon: occ: Add callbacks for parsing P9 OCC datastructures Andrew Jeffery
2016-11-11 4:12 ` [RFC PATCH linux 9/9] wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC Andrew Jeffery
2016-11-29 23:00 ` Eddie James
2016-11-30 1:04 ` Andrew Jeffery
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.