hwmon: occ: Add callbacks for parsing P9 OCC datastructures
RFC PATCH v6 8/9 hwmon: occ: Add callbacks for parsing P9 OCC datastructures
From: "Edward A. James" <[email protected]>
--- 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
drivers/hwmon/occ/occ_p9.c new file +261 -0
@@ -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 <[email protected]>" ); +MODULE_DESCRIPTION ("P9 OCC sensors" ); +MODULE_LICENSE ("GPL" );
drivers/hwmon/occ/occ_p9.h new file +30 -0
@@ -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__ */
Signature · 1 line
1.9.1
Quoted 2 lines · On Tue, 2016-11-29 at 17:04 -0600, [email protected] wrote:
Quoted 1 line
From: "Edward A. James" <[email protected]>
The usual here :) More words pointing to relevant documentation and sections defining the returned data.
FWIW I don't actually know where this documentation is so can't verify the behaviour.
The rest of the comments are mostly minor issues.
Quoted 41 lines
Signed-off-by Edward A. James <[email protected]>---
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 100644drivers/hwmon/occ/occ_p9.hdrivers/hwmon/occ/occ_p9.c new file +27
-0@@ -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>
drivers/hwmon/occ/occ_p9.c line 37
Quoted 10 lines
drivers/hwmon/occ/occ_p9.c new file +10
-0+ +#include "occ.h" +#include "occ_p9.h" + +/* P9 OCC sensor data format */ +struct temp_sensor_p9 { + u32 sensor_id; + u8 fru_type; + u8 value; +};
Whatever approach we apply for naming things in the P8 code we should also apply here.
Quoted 65 lines
drivers/hwmon/occ/occ_p9.c new file +65
-0+ +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 =
drivers/hwmon/occ/occ_p9.c line 112
Quoted 10 lines
drivers/hwmon/occ/occ_p9.c new file +10
-0+ &(((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]);
This looks to be unaligned. Please fix it.
Quoted 125 lines
drivers/hwmon/occ/occ_p9.c new file +125
-0+ 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,
drivers/hwmon/occ/occ_p9.c line 247
Quoted 10 lines
drivers/hwmon/occ/occ_p9.c new file +10
-0+ .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)
Again we're copying bus_ops here. Can/should we take a pointer instead?
Quoted 30 lines
drivers/hwmon/occ/occ_p9.c new file +11
-0+{ + 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); +Quoted 1 line
+MODULE_AUTHOR("Eddie James <[email protected]>");
drivers/hwmon/occ/occ_p9.c new file +2
-0+MODULE_DESCRIPTION ("P9 OCC sensors" ); +MODULE_LICENSE ("GPL" );drivers/hwmon/occ/occ_p9.h new file +10
-0@@ -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
drivers/hwmon/occ/occ_p9.h line 20
Quoted 10 lines
drivers/hwmon/occ/occ_p9.h new file +10
-0+* (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__
Picking on my own guard/file naming problem again :)
drivers/hwmon/occ/occ_p9.h line 30
Quoted 10 lines
drivers/hwmon/occ/occ_p9.h new file +10
-0+ +#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__ */
Andrew