| Message ID | 1486509056-25838-5-git-send-email-eajames@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Feb 8, 2017 at 9:40 AM, <eajames@linux.vnet.ibm.com> wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Add functions to parse the data structures that are specific to the OCC on > the POWER8 processor. These are the sensor data structures, including > temperature, frequency, power, and "caps." > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > Documentation/hwmon/occ | 9 ++ > drivers/hwmon/occ/occ_p8.c | 248 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/hwmon/occ/occ_p8.h | 30 ++++++ > 3 files changed, 287 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 0000000..5c61fc4 > --- /dev/null > +++ b/drivers/hwmon/occ/occ_p8.c > +void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off, > + int snum) > +{ > + switch (sensor_type) { > + case FREQ: > + case TEMP: > + { > + struct p8_occ_sensor *os = > + &(((struct p8_occ_sensor *)sensor)[snum]); > + > + os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > + os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2])); > + } > + break; > + case POWER: > + { > + struct p8_power_sensor *ps = > + &(((struct p8_power_sensor *)sensor)[snum]); > + > + ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > + ps->update_tag = > + be32_to_cpu(get_unaligned((u32 *)&data[off + 2])); This might be more readable if you wrote a cast_get_unaliged_be32_to_cpu() macro. > + ps->accumulator = > + be32_to_cpu(get_unaligned((u32 *)&data[off + 6])); > + ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10])); > + } > + break; > + case CAPS: > + { > +const u32 *p8_get_sensor_hwmon_configs() > +{ > + return p8_sensor_hwmon_configs; > +} > +EXPORT_SYMBOL(p8_get_sensor_hwmon_configs); > + > +struct occ *p8_occ_start(struct device *dev, void *bus, > + struct occ_bus_ops *bus_ops) > +{ > + return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config); > +} > +EXPORT_SYMBOL(p8_occ_start); We don't need to export these symbols; they're not used outside of the OCC module. The same goes for all of the exports you've made in this series. I suggest we re-architect the drivers so we build all of the objects and link them into one module for each platform, instead of having an occ module and occ-p8/occ-p9 modules and i2c modules that all depend on each other. The Makefile could look like this: obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += hwmon_occ_p8.o obj-$(CONFIG_SENSORS_PPC_OCC_P9) += hwmon_occ_p9.o hwmon_occ_p8-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o p8_occ_i2c.o occ_sysfs.o occ.o hwmon_occ_p9-$(CONFIG_SENSORS_PPC_OCC_P9) += occ_p9.o occ_sysfs.o occ.o And the Kbuild like this: menuconfig SENSORS_PPC_OCC bool "PPC On-Chip Controller" if SENSORS_PPC_OCC config SENSORS_PPC_OCC_P8_I2C bool "POWER8 OCC hwmon support" depends on I2C config SENSORS_PPC_OCC_P9 bool "POWER9 OCC hwmon support" endif -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2017-02-10 at 16:01 +1030, Joel Stanley wrote: > > On Wed, Feb 8, 2017 at 9:40 AM, <eajames@linux.vnet.ibm.com> wrote: > > > > From: "Edward A. James" <eajames@us.ibm.com> > > > > Add functions to parse the data structures that are specific to the OCC on > > the POWER8 processor. These are the sensor data structures, including > > temperature, frequency, power, and "caps." > > > > > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > --- > > Documentation/hwmon/occ | 9 ++ > > drivers/hwmon/occ/occ_p8.c | 248 +++++++++++++++++++++++++++++++++++++++++++++ > > drivers/hwmon/occ/occ_p8.h | 30 ++++++ > > 3 files changed, 287 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 0000000..5c61fc4 > > --- /dev/null > > +++ b/drivers/hwmon/occ/occ_p8.c > > +void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off, > > + int snum) > > +{ > > + switch (sensor_type) { > > + case FREQ: > > + case TEMP: > > + { > > + struct p8_occ_sensor *os = > > + &(((struct p8_occ_sensor *)sensor)[snum]); > > + > > + os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > > + os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2])); > > + } > > + break; > > + case POWER: > > + { > > + struct p8_power_sensor *ps = > > + &(((struct p8_power_sensor *)sensor)[snum]); > > + > > + ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > > + ps->update_tag = > > + be32_to_cpu(get_unaligned((u32 *)&data[off + 2])); > > This might be more readable if you wrote a > cast_get_unaliged_be32_to_cpu() macro. > > > + ps->accumulator = > > + be32_to_cpu(get_unaligned((u32 *)&data[off + 6])); > > + ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10])); > > + } > > + break; > > + case CAPS: > > + { > > +const u32 *p8_get_sensor_hwmon_configs() > > +{ > > + return p8_sensor_hwmon_configs; > > +} > > +EXPORT_SYMBOL(p8_get_sensor_hwmon_configs); > > + > > +struct occ *p8_occ_start(struct device *dev, void *bus, > > + struct occ_bus_ops *bus_ops) > > +{ > > + return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config); > > +} > > +EXPORT_SYMBOL(p8_occ_start); > > We don't need to export these symbols; they're not used outside of the > OCC module. The same goes for all of the exports you've made in this > series. Sorry, this was my doing in an attempt to get everything to build as modules rather than just built-in. I should have studied Documentation/kbuild/modules.txt a bit more. > > I suggest we re-architect the drivers so we build all of the objects > and link them into one module for each platform, instead of having an > occ module and occ-p8/occ-p9 modules and i2c modules that all depend > on each other. The Makefile could look like this: > > obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += hwmon_occ_p8.o > obj-$(CONFIG_SENSORS_PPC_OCC_P9) += hwmon_occ_p9.o > > hwmon_occ_p8-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o > occ_p8.o p8_occ_i2c.o occ_sysfs.o occ.o > hwmon_occ_p9-$(CONFIG_SENSORS_PPC_OCC_P9) += occ_p9.o occ_sysfs.o occ.o > > And the Kbuild like this: > > menuconfig SENSORS_PPC_OCC > bool "PPC On-Chip Controller" > > if SENSORS_PPC_OCC > > config SENSORS_PPC_OCC_P8_I2C > bool "POWER8 OCC hwmon support" > depends on I2C > > config SENSORS_PPC_OCC_P9 > bool "POWER9 OCC hwmon support" > > endif Given we can drop the exports that's a much more sensible idea. Andrew
On Mon, Feb 13, 2017 at 11:47:22AM +1030, Andrew Jeffery wrote: > On Fri, 2017-02-10 at 16:01 +1030, Joel Stanley wrote: > > > On Wed, Feb 8, 2017 at 9:40 AM, <eajames@linux.vnet.ibm.com> wrote: > > > > > From: "Edward A. James" <eajames@us.ibm.com> > > > > > > Add functions to parse the data structures that are specific to the OCC on > > > the POWER8 processor. These are the sensor data structures, including > > > temperature, frequency, power, and "caps." > > > > > > > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > --- > > > Documentation/hwmon/occ | 9 ++ > > > drivers/hwmon/occ/occ_p8.c | 248 +++++++++++++++++++++++++++++++++++++++++++++ > > > drivers/hwmon/occ/occ_p8.h | 30 ++++++ > > > 3 files changed, 287 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 0000000..5c61fc4 > > > --- /dev/null > > > +++ b/drivers/hwmon/occ/occ_p8.c > > > +void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off, > > > + int snum) > > > +{ > > > + switch (sensor_type) { > > > + case FREQ: > > > + case TEMP: > > > + { > > > + struct p8_occ_sensor *os = > > > + &(((struct p8_occ_sensor *)sensor)[snum]); > > > + > > > + os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > > > + os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2])); > > > + } > > > + break; > > > + case POWER: > > > + { > > > + struct p8_power_sensor *ps = > > > + &(((struct p8_power_sensor *)sensor)[snum]); > > > + > > > + ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > > > + ps->update_tag = > > > + be32_to_cpu(get_unaligned((u32 *)&data[off + 2])); > > > > This might be more readable if you wrote a > > cast_get_unaliged_be32_to_cpu() macro. > > > > > + ps->accumulator = > > > + be32_to_cpu(get_unaligned((u32 *)&data[off + 6])); > > > + ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10])); > > > + } > > > + break; > > > + case CAPS: > > > + { > > > +const u32 *p8_get_sensor_hwmon_configs() > > > +{ > > > + return p8_sensor_hwmon_configs; > > > +} > > > +EXPORT_SYMBOL(p8_get_sensor_hwmon_configs); > > > + > > > +struct occ *p8_occ_start(struct device *dev, void *bus, > > > + struct occ_bus_ops *bus_ops) > > > +{ > > > + return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config); > > > +} > > > +EXPORT_SYMBOL(p8_occ_start); > > > > We don't need to export these symbols; they're not used outside of the > > OCC module. The same goes for all of the exports you've made in this > > series. > > Sorry, this was my doing in an attempt to get everything to build as > modules rather than just built-in. I should have studied > Documentation/kbuild/modules.txt a bit more. > > > > > I suggest we re-architect the drivers so we build all of the objects > > and link them into one module for each platform, instead of having an > > occ module and occ-p8/occ-p9 modules and i2c modules that all depend > > on each other. The Makefile could look like this: > > > > obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += hwmon_occ_p8.o > > obj-$(CONFIG_SENSORS_PPC_OCC_P9) += hwmon_occ_p9.o > > > > hwmon_occ_p8-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o > > occ_p8.o p8_occ_i2c.o occ_sysfs.o occ.o > > hwmon_occ_p9-$(CONFIG_SENSORS_PPC_OCC_P9) += occ_p9.o occ_sysfs.o occ.o > > Please note that the above will still result in separate modules, one per object file. Is this really what you want ? I don't even know what happens if an object file is specified twice, so you may also have to make sure that CONFIG_SENSORS_PPC_OCC_P8_I2C and CONFIG_SENSORS_PPC_OCC_P9 are either-or configurations. Again, I am not really sure if that is what you want. > > And the Kbuild like this: > > > > menuconfig SENSORS_PPC_OCC > > bool "PPC On-Chip Controller" > > > > if SENSORS_PPC_OCC > > > > config SENSORS_PPC_OCC_P8_I2C > > bool "POWER8 OCC hwmon support" > > depends on I2C > > > > config SENSORS_PPC_OCC_P9 > > bool "POWER9 OCC hwmon support" > > If both are bool, ie there won't be any module support, I don't really see the point of specifying occ_sysfs.o and occ.o twice above. > > endif > > Given we can drop the exports that's a much more sensible idea. > Please make sure that both "allmodconfig" and "allyesconfig" still build after this change. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ index d0bdf06..143951e 100644 --- a/Documentation/hwmon/occ +++ b/Documentation/hwmon/occ @@ -25,6 +25,15 @@ Currently, all versions of the OCC support four types of sensor data: power, temperature, frequency, and "caps," which indicate limits and thresholds used internally on the OCC. +The format for the POWER8 OCC sensor data can be found in the P8 OCC +specification: +github.com/open-power/docs/blob/master/occ/OCC_OpenPwr_FW_Interfaces.pdf +This document provides the details of the OCC sensors: power, frequency, +temperature, and caps. These sensor formats are specific to the POWER8 OCC. A +number of data structures, such as command format, response headers, and the +like, are also defined in this specification, and are common to both POWER8 and +POWER9 OCCs. + sysfs Entries ------------- diff --git a/drivers/hwmon/occ/occ_p8.c b/drivers/hwmon/occ/occ_p8.c new file mode 100644 index 0000000..5c61fc4 --- /dev/null +++ b/drivers/hwmon/occ/occ_p8.c @@ -0,0 +1,248 @@ +/* + * occ_p8.c - OCC hwmon driver + * + * This file contains the Power8-specific methods and data structures for + * the OCC hwmon driver. + * + * Copyright 2016 IBM Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <asm/unaligned.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/hwmon.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include "occ.h" +#include "occ_p8.h" + +/* P8 OCC sensor data format */ +struct p8_occ_sensor { + u16 sensor_id; + u16 value; +}; + +struct p8_power_sensor { + u16 sensor_id; + u32 update_tag; + u32 accumulator; + u16 value; +}; + +struct p8_caps_sensor { + u16 curr_powercap; + u16 curr_powerreading; + u16 norm_powercap; + u16 max_powercap; + u16 min_powercap; + u16 user_powerlimit; +}; + +static const u32 p8_sensor_hwmon_configs[MAX_OCC_SENSOR_TYPE] = { + HWMON_I_INPUT | HWMON_I_LABEL, /* freq: value | label */ + HWMON_T_INPUT | HWMON_T_LABEL, /* temp: value | label */ + /* power: value | label | accumulator | update_tag */ + HWMON_P_INPUT | HWMON_P_LABEL | HWMON_P_AVERAGE | + HWMON_P_AVERAGE_INTERVAL, + /* caps: curr | max | min | norm | user */ + HWMON_P_CAP | HWMON_P_CAP_MAX | HWMON_P_CAP_MIN | HWMON_P_MAX | + HWMON_P_ALARM, +}; + +void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off, + int snum) +{ + switch (sensor_type) { + case FREQ: + case TEMP: + { + struct p8_occ_sensor *os = + &(((struct p8_occ_sensor *)sensor)[snum]); + + os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); + os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2])); + } + break; + case POWER: + { + struct p8_power_sensor *ps = + &(((struct p8_power_sensor *)sensor)[snum]); + + ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); + ps->update_tag = + be32_to_cpu(get_unaligned((u32 *)&data[off + 2])); + ps->accumulator = + be32_to_cpu(get_unaligned((u32 *)&data[off + 6])); + ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10])); + } + break; + case CAPS: + { + struct p8_caps_sensor *cs = + &(((struct p8_caps_sensor *)sensor)[snum]); + + cs->curr_powercap = + be16_to_cpu(get_unaligned((u16 *)&data[off])); + cs->curr_powerreading = + be16_to_cpu(get_unaligned((u16 *)&data[off + 2])); + cs->norm_powercap = + be16_to_cpu(get_unaligned((u16 *)&data[off + 4])); + cs->max_powercap = + be16_to_cpu(get_unaligned((u16 *)&data[off + 6])); + cs->min_powercap = + be16_to_cpu(get_unaligned((u16 *)&data[off + 8])); + cs->user_powerlimit = + be16_to_cpu(get_unaligned((u16 *)&data[off + 10])); + } + break; + }; +} + +void *p8_alloc_sensor(struct device *dev, int sensor_type, int num_sensors) +{ + switch (sensor_type) { + case FREQ: + case TEMP: + return devm_kzalloc(dev, num_sensors * + sizeof(struct p8_occ_sensor), GFP_KERNEL); + case POWER: + return devm_kzalloc(dev, num_sensors * + sizeof(struct p8_power_sensor), + GFP_KERNEL); + case CAPS: + return devm_kzalloc(dev, num_sensors * + sizeof(struct p8_caps_sensor), GFP_KERNEL); + default: + return NULL; + } +} + +int p8_get_sensor(struct occ *driver, int sensor_type, int sensor_num, + u32 hwmon, long *val) +{ + int rc = 0; + void *sensor; + + if (sensor_type == POWER) { + if (hwmon == hwmon_power_cap || hwmon == hwmon_power_cap_max || + hwmon == hwmon_power_cap_min || hwmon == hwmon_power_max || + hwmon == hwmon_power_alarm) + sensor_type = CAPS; + } + + sensor = occ_get_sensor(driver, sensor_type); + if (!sensor) + return -ENODEV; + + switch (sensor_type) { + case FREQ: + case TEMP: + { + struct p8_occ_sensor *os = + &(((struct p8_occ_sensor *)sensor)[sensor_num]); + + if (hwmon == hwmon_in_input || hwmon == hwmon_temp_input) + *val = os->value; + else if (hwmon == hwmon_in_label || hwmon == hwmon_temp_label) + *val = os->sensor_id; + else + rc = -EOPNOTSUPP; + } + break; + case POWER: + { + struct p8_power_sensor *ps = + &(((struct p8_power_sensor *)sensor)[sensor_num]); + + switch (hwmon) { + case hwmon_power_input: + *val = ps->value; + break; + case hwmon_power_label: + *val = ps->sensor_id; + break; + case hwmon_power_average: + *val = ps->accumulator; + break; + case hwmon_power_average_interval: + *val = ps->update_tag; + break; + default: + rc = -EOPNOTSUPP; + } + } + break; + case CAPS: + { + struct p8_caps_sensor *cs = + &(((struct p8_caps_sensor *)sensor)[sensor_num]); + + switch (hwmon) { + case hwmon_power_cap: + *val = cs->curr_powercap; + break; + case hwmon_power_cap_max: + *val = cs->max_powercap; + break; + case hwmon_power_cap_min: + *val = cs->min_powercap; + break; + case hwmon_power_max: + *val = cs->norm_powercap; + break; + case hwmon_power_alarm: + *val = cs->user_powerlimit; + break; + default: + rc = -EOPNOTSUPP; + } + } + break; + default: + rc = -EINVAL; + } + + return rc; +} + +static const struct occ_ops p8_ops = { + .parse_sensor = p8_parse_sensor, + .alloc_sensor = p8_alloc_sensor, + .get_sensor = p8_get_sensor, +}; + +static const struct occ_config p8_config = { + .command_addr = 0xFFFF6000, + .response_addr = 0xFFFF7000, +}; + +const u32 *p8_get_sensor_hwmon_configs() +{ + return p8_sensor_hwmon_configs; +} +EXPORT_SYMBOL(p8_get_sensor_hwmon_configs); + +struct occ *p8_occ_start(struct device *dev, void *bus, + struct occ_bus_ops *bus_ops) +{ + return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config); +} +EXPORT_SYMBOL(p8_occ_start); + +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>"); +MODULE_DESCRIPTION("P8 OCC sensors"); +MODULE_LICENSE("GPL"); diff --git a/drivers/hwmon/occ/occ_p8.h b/drivers/hwmon/occ/occ_p8.h new file mode 100644 index 0000000..3fe6417 --- /dev/null +++ b/drivers/hwmon/occ/occ_p8.h @@ -0,0 +1,30 @@ +/* + * occ_p8.h - OCC hwmon driver + * + * This file contains Power8-specific function prototypes + * + * Copyright 2016 IBM Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __OCC_P8_H__ +#define __OCC_P8_H__ + +#include "scom.h" + +struct device; + +const u32 *p8_get_sensor_hwmon_configs(void); +struct occ *p8_occ_start(struct device *dev, void *bus, + struct occ_bus_ops *bus_ops); + +#endif /* __OCC_P8_H__ */