lkml.org 
[lkml]   [2010]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH/RFC 2/4] hwmon: PMBus device driver
    On 06/28/10 22:56, Guenter Roeck wrote:
    Hi Guenter,

    A few questions and inital comments...
    Ouch the pmbus spec is tricky to follow!

    > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
    > ---
    > drivers/hwmon/Kconfig | 12 +
    > drivers/hwmon/Makefile | 1 +
    > drivers/hwmon/pmbus.c | 1227 ++++++++++++++++++++++++++++++++++++++++++++++++
    > drivers/hwmon/pmbus.h | 209 ++++++++
    > 4 files changed, 1449 insertions(+), 0 deletions(-)
    > create mode 100644 drivers/hwmon/pmbus.c
    > create mode 100644 drivers/hwmon/pmbus.h
    >
    > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
    > index e19cf8e..8d53cf7 100644
    > --- a/drivers/hwmon/Kconfig
    > +++ b/drivers/hwmon/Kconfig
    > @@ -702,6 +702,18 @@ config SENSORS_PCF8591
    > These devices are hard to detect and rarely found on mainstream
    > hardware. If unsure, say N.
    >
    > +config SENSORS_PMBUS
    > + tristate "PMBus devices"
    > + depends on I2C && EXPERIMENTAL
    > + default n
    > + help
    > + If you say yes here you get hardware monitoring support for various
    > + PMBus devices, including but not limited to BMR45x, LTC2978, MAX16064,
    > + MAX8688, and UCD92xx.
    > +
    > + This driver can also be built as a module. If so, the module will
    > + be called pmbus.
    > +
    > config SENSORS_SHT15
    > tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
    > depends on GENERIC_GPIO
    > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
    > index 2138ceb..88b043e 100644
    > --- a/drivers/hwmon/Makefile
    > +++ b/drivers/hwmon/Makefile
    > @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
    > obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
    > obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
    > obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
    > +obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
    > obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
    > obj-$(CONFIG_SENSORS_SHT15) += sht15.o
    > obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o
    > diff --git a/drivers/hwmon/pmbus.c b/drivers/hwmon/pmbus.c
    > new file mode 100644
    > index 0000000..418ee2c
    > --- /dev/null
    > +++ b/drivers/hwmon/pmbus.c
    > @@ -0,0 +1,1227 @@
    > +/*
    > + * Hardware monitoring driver for PMBus devices
    > + *
    > + * Copyright (C) 2010 Ericsson AB.
    > + *
    > + * This program is free software; you can redistribute it and/or modify
    > + * it under the terms of the GNU General Public License as published by
    > + * the Free Software Foundation; either version 2 of the License, or
    > + * (at your option) any later version.
    > + *
    > + * This program is distributed in the hope that it will be useful,
    > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
    > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
    > + * GNU General Public License for more details.
    > + *
    > + * You should have received a copy of the GNU General Public License
    > + * along with this program; if not, write to the Free Software
    > + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
    > + */
    > +
    > +#include <linux/kernel.h>
    > +#include <linux/module.h>
    > +#include <linux/init.h>
    > +#include <linux/err.h>
    > +#include <linux/slab.h>
    > +#include <linux/i2c.h>
    > +#include <linux/hwmon.h>
    > +#include <linux/hwmon-sysfs.h>
    > +#include <linux/delay.h>
    > +#include "pmbus.h"
    > +
    > +#define PMBUS_SENSORS 64
    > +#define PMBUS_BOOLEANS 64
    > +#define PMBUS_LABELS 32
    > +#define PMBUS_NUM_ATTR (PMBUS_BOOLEANS + PMBUS_SENSORS + PMBUS_LABELS)
    > +#define PMBUS_PAGES 8
    > +
    > +static int pages;
    > +module_param(pages, int, 0);
    > +MODULE_PARM_DESC(pages, "Number of sensor pages");
    Why is this a module parameter? If you do it like this
    you will be overriding page count for all devices in the system.
    Is that the intent?

    > +
    > +enum chips { ltc2978, max16064, max8688, pmbus, ucd9220, ucd9240 };
    > +
    > +enum pmbus_sensor_classes {
    > + PSC_VOLTAGE = 0,
    > + PSC_TEMPERATURE,
    > + PSC_CURRENT,
    > + PSC_POWER,

    Perhaps name this PSC_MAX_LABEL or similar to indicate it is just here to
    allow the number of possible classes to be established.
    > + SENSOR_CLASSES
    > +};
    > +
    > +struct pmbus_config {
    > + int pages; /* Total number of pages (= output sensors) */
    > + bool direct; /* true if device uses direct data format */
    > + /*
    > + * Support one set of coefficients for each sensor type
    > + * Used for chips providing data in direct mode.
    > + */
    > + int m[SENSOR_CLASSES]; /* mantissa for direct data format */
    > + int b[SENSOR_CLASSES]; /* offset */
    > + int R[SENSOR_CLASSES]; /* exponent */
    > +};
    > +
    Can we name these something to do with PB to cut down on chance of name
    clashes.
    > +#define HAVE_STATUS_VOUT (1<<0)
    > +#define HAVE_STATUS_IOUT (1<<1)
    > +#define HAVE_STATUS_INPUT (1<<2)
    > +#define HAVE_STATUS_TEMP (1<<3)
    > +
    > +#define PB_STATUS_BASE 0
    > +#define PB_STATUS_VOUT_BASE (PB_STATUS_BASE + PMBUS_PAGES)
    > +#define PB_STATUS_IOUT_BASE (PB_STATUS_VOUT_BASE + PMBUS_PAGES)
    > +#define PB_STATUS_INPUT_BASE (PB_STATUS_IOUT_BASE + PMBUS_PAGES)
    > +#define PB_STATUS_TEMP_BASE (PB_STATUS_INPUT_BASE + 1)
    > +
    > +struct pmbus_sensor {
    > + char name[I2C_NAME_SIZE]; /* sysfs sensor name */
    > + u8 page; /* page number */
    > + u8 reg; /* register */
    > + enum pmbus_sensor_classes class;/* sensor class */
    > + bool update; /* runtime sensor update needed */
    > +};
    > +
    What are the next two for?
    > +struct pmbus_boolean {
    > + char name[I2C_NAME_SIZE]; /* sysfs boolean name */
    > +};
    > +
    > +struct pmbus_label {
    > + char name[I2C_NAME_SIZE]; /* sysfs label name */
    > + char label[I2C_NAME_SIZE]; /* label */
    > +};
    > +
    > +struct pmbus_data {
    > + struct device *hwmon_dev;
    > + enum chips type;
    > +
    > + bool direct;
    > + int pages;
    > +
    > + /* Coefficients, for chips providing data in direct mode */
    > + int m[SENSOR_CLASSES]; /* mantissa for direct data format */
    > + int b[SENSOR_CLASSES]; /* offset */
    > + int R[SENSOR_CLASSES]; /* exponent */
    > +
    Any chance of getting rid of one of the following? The 'attributes'
    array is must an array of pointers to elements within sensor_devices_attributes.
    It would prevent easy usage of sysfs_create_group so maybe it is better
    to just have the redunancy here.

    > + struct sensor_device_attribute
    > + sensor_device_attributes[PMBUS_NUM_ATTR];
    > + struct attribute *attributes[PMBUS_NUM_ATTR];
    > + int num_attributes;
    > + struct attribute_group group;
    > +
    > + int num_sensors;
    > + struct pmbus_sensor sensors[PMBUS_SENSORS];
    > + int num_booleans;
    > + struct pmbus_boolean booleans[PMBUS_BOOLEANS];
    > + int num_labels;
    Can you comment on what these labels are for?
    > + struct pmbus_label labels[PMBUS_LABELS];
    > +
    > + struct mutex update_lock;
    > + bool valid;
    > + unsigned long last_updated; /* in jiffies */
    > + u8 status_bits;

    What is the theoretical maximum number of pages?
    Is it the 8 you have above? A quick look at the rev 1 spec
    suggests 0-1F so 32. That is going to make for some big
    arrays of fixed size whatever the chip in question needs.

    Could you do some further grouping into suitable structures?
    Perhaps put sensor_data in the pmbus_sensor struct?
    Can status also be moved into such a struct? I'm a little
    unclear how it is assocated with the various sensors inputs?

    After such grouping, perhaps the allocation can be made dynamic.
    > + /*
    > + * status, status_vout, status_iout are paged.
    > + * status_input and status_temp are unpaged.
    > + */
    > + u8 status[PMBUS_PAGES * 3 + 2];
    > + u16 sensor_data[PMBUS_SENSORS];
    > +
    > + u8 currpage;
    > +};
    > +

    Do these need to be complete or do only some devices support direct mode
    reads?
    > +static const struct pmbus_config pmbus_config[] = {
    > + [pmbus] = {
    > + .pages = 1,
    > + },
    > + [ltc2978] = {
    > + .pages = 8,
    > + },
    > + [max16064] = {
    > + .pages = 4,
    > + .direct = true,
    > + .m[PSC_VOLTAGE] = 19995,
    > + .b[PSC_VOLTAGE] = 0,
    > + .R[PSC_VOLTAGE] = -1,
    > + .m[PSC_TEMPERATURE] = -7612,
    > + .b[PSC_TEMPERATURE] = 335,
    > + .R[PSC_TEMPERATURE] = -3,
    > + },
    > + [max8688] = {
    > + .pages = 1,
    > + .direct = true,
    > + .m[PSC_VOLTAGE] = 19995,
    > + .b[PSC_VOLTAGE] = 0,
    > + .R[PSC_VOLTAGE] = -1,
    > + .m[PSC_TEMPERATURE] = -7612,
    > + .b[PSC_TEMPERATURE] = 335,
    > + .R[PSC_TEMPERATURE] = -3,
    > + .m[PSC_CURRENT] = 23109,
    > + .b[PSC_CURRENT] = 0,
    > + .R[PSC_CURRENT] = -2,
    > + },
    > + [ucd9220] = {
    > + .pages = 2,
    > + },
    > + [ucd9240] = {
    > + .pages = 4,
    > + },
    > +};
    > +
    > +static int pmbus_set_page(struct i2c_client *client, u8 page)
    > +{
    > + struct pmbus_data *data = i2c_get_clientdata(client);
    > + int rv = 0;
    > +
    > + if (page != data->currpage) {
    > + rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
    > + data->currpage = page;
    > + }
    > + return rv;
    > +}
    > +
    > +static int pmbus_write_byte(struct i2c_client *client, u8 page, u8 value)
    > +{
    > + if (pmbus_set_page(client, page) < 0)
    > + return -1;
    > +
    > + return i2c_smbus_write_byte(client, value);
    > +}
    > +
    > +static int pmbus_write_word_data(struct i2c_client *client, u8 page, u8 reg,
    > + u16 word)
    > +{
    > + if (pmbus_set_page(client, page) < 0)
    > + return -1;
    > +
    > + return i2c_smbus_write_word_data(client, reg, word);
    > +}
    > +
    > +static int pmbus_read_word_data(struct i2c_client *client, u8 page, u8 reg)
    > +{
    > + if (pmbus_set_page(client, page) < 0)
    > + return -1;
    > +
    > + return i2c_smbus_read_word_data(client, reg);
    > +}
    > +
    > +static int pmbus_read_byte_data(struct i2c_client *client, u8 page, u8 reg)
    > +{
    > + if (pmbus_set_page(client, page) < 0)
    > + return -1;
    > +
    > + return i2c_smbus_read_byte_data(client, reg);
    > +}
    > +
    > +static void pmbus_clear_faults(struct i2c_client *client)
    > +{
    > + struct pmbus_data *data = i2c_get_clientdata(client);
    > + int i;
    > +
    > + for (i = 0; i < data->pages; i++)
    > + pmbus_write_byte(client, i, PMBUS_CLEAR_FAULTS);
    > +}
    > +
    > +static bool pmbus_check_byte_register(struct i2c_client *client, int page,
    > + int reg)
    > +{
    Roll everything into one line.

    return (pmbus_read_byte_data(client, page, reg) >= 0);
    > + int rv;
    > +
    > + rv = pmbus_read_byte_data(client, page, reg);
    > + return (rv >= 0);
    > +}
    > +
    > +static bool pmbus_check_word_register(struct i2c_client *client, int page,
    > + int reg)
    > +{
    Another one liner.
    > + int rv;
    > +
    > + rv = pmbus_read_word_data(client, page, reg);
    > + return (rv >= 0);
    > +}
    > +
    > +static struct pmbus_data *pmbus_update_device(struct device *dev)
    > +{
    > + struct i2c_client *client = to_i2c_client(dev);
    > + struct pmbus_data *data = i2c_get_clientdata(client);
    > + struct pmbus_data *ret = data;
    > +
    > + mutex_lock(&data->update_lock);
    > + if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
    > + int i, val;
    > +
    > + for (i = 0; i < data->pages; i++) {
    > + val = pmbus_read_byte_data(client, i,
    > + PMBUS_STATUS_BYTE);
    > + if (val < 0) {
    > + ret = ERR_PTR(val);
    > + goto abort;
    > + }
    > + data->status[PB_STATUS_BASE + i] = val;
    > + }
    > + if (data->status_bits & HAVE_STATUS_VOUT)
    > + for (i = 0; i < data->pages; i++) {
    > + val = pmbus_read_byte_data(client, i,
    > + PMBUS_STATUS_VOUT);
    > + if (val < 0) {
    > + ret = ERR_PTR(val);
    > + goto abort;
    > + }
    > + data->status[PB_STATUS_VOUT_BASE + i] = val;
    > + }
    > + if (data->status_bits & HAVE_STATUS_IOUT)
    > + for (i = 0; i < data->pages; i++) {
    > + val = pmbus_read_byte_data(client, i,
    > + PMBUS_STATUS_IOUT);
    > + if (val < 0) {
    > + ret = ERR_PTR(val);
    > + goto abort;
    > + }
    > + data->status[PB_STATUS_IOUT_BASE + i] = val;
    > + }
    > + if (data->status_bits & HAVE_STATUS_INPUT) {
    > + val = pmbus_read_byte_data(client, 0,
    > + PMBUS_STATUS_INPUT);
    > + if (val < 0) {
    > + ret = ERR_PTR(val);
    > + goto abort;
    > + }
    > + data->status[PB_STATUS_INPUT_BASE] = val;
    > + }
    > + if (data->status_bits & HAVE_STATUS_TEMP) {
    > + val = pmbus_read_byte_data(client, 0,
    > + PMBUS_STATUS_TEMPERATURE);
    > + if (val < 0) {
    > + ret = ERR_PTR(val);
    > + goto abort;
    > + }
    > + data->status[PB_STATUS_TEMP_BASE] = val;
    > + }
    > + for (i = 0; i < data->num_sensors; i++) {
    > + struct pmbus_sensor *sensor = &data->sensors[i];
    > +
    > + if (!data->valid || sensor->update) {
    > + val = pmbus_read_word_data(client, sensor->page,
    > + sensor->reg);
    > + if (val < 0) {
    > + ret = ERR_PTR(val);
    > + goto abort;
    > + }
    > + data->sensor_data[i] = val;
    > + }
    > + }
    > + pmbus_clear_faults(client);
    > + data->last_updated = jiffies;
    > + data->valid = 1;
    > + }
    > +abort:
    > + mutex_unlock(&data->update_lock);
    > + return ret;
    > +}
    > +
    > +/*
    > + * Convert linear sensor values to milli- or micro-units
    > + * depending on sensor type.
    > + */
    > +static long pmbus_reg2data_linear(struct pmbus_data *data,
    > + enum pmbus_sensor_classes class, u16 adc)
    > +{
    > + s16 exponent, mantissa;
    > + long val;
    > +
    > + exponent = adc >> 11;
    > + mantissa = adc & 0x07ff;
    > +
    > + if (exponent > 0x0f)
    > + exponent |= 0xffe0; /* sign extend exponent */
    > + if (mantissa > 0x03ff)
    > + mantissa |= 0xf800; /* sign extend mantissa */
    > +
    > + /* scale result to milli-units */
    > + val = mantissa * 1000L;
    > +
    > + /* scale result to micro-units for power sensors */
    > + if (class == PSC_POWER)
    > + val = val * 1000L;
    > +
    > + if (exponent > 0)
    > + val <<= exponent;
    Don't need this check...
    val >>= 0 is same as val.
    > + else if (exponent < 0)
    > + val >>= -exponent;
    > +
    > + return val;
    > +}
    > +
    > +/*
    > + * Convert direct sensor values to milli- or micro-units
    > + * depending on sensor type.
    > + */
    > +static long pmbus_reg2data_direct(struct pmbus_data *data,
    > + enum pmbus_sensor_classes class, u16 adc)
    > +{
    > + long val = (s16) adc;
    > + long m, b, R;
    > +
    > + m = data->m[class];
    > + b = data->b[class];
    > + R = data->R[class];
    > +
    > + if (m == 0)
    > + return 0;
    > +
    > + /* X = 1/m * (Y * 10^-R - b) */
    > + R = -R + 3; /* scale result to milli-units */
    > + b *= 1000; /* subtract milli-units */
    > +
    > + /* scale result to micro-units for power sensors */
    > + if (class == PSC_POWER) {
    > + R += 3;
    > + b *= 1000;
    > + }
    > +
    > + while (R > 0) {
    > + val *= 10;
    > + R--;
    > + }
    > + while (R < 0) {
    > + val = DIV_ROUND_CLOSEST(val, 10);
    > + R++;
    > + }
    > +
    > + return (val - b) / m;
    > +}
    > +
    > +static long pmbus_reg2data(struct pmbus_data *data,
    > + enum pmbus_sensor_classes class, u16 adc)
    > +{
    > + long val;
    > +
    > + if (data->direct)
    > + val = pmbus_reg2data_direct(data, class, adc);
    > + else
    > + val = pmbus_reg2data_linear(data, class, adc);
    > +
    > + return val;
    > +}
    > +
    > +#define MAX_MANTISSA (1023 * 1000)
    > +#define MIN_MANTISSA (511 * 1000)
    > +
    > +static u16 pmbus_data2reg_linear(struct pmbus_data *data,
    > + enum pmbus_sensor_classes class, long val)
    > +{
    > + s16 exponent = 0, mantissa = 0;
    > + bool negative = false;
    > +
    > + /* simple case */
    > + if (val == 0)
    > + return 0;
    > +
    > + if (val < 0) {
    > + negative = true;
    > + val = -val;
    > + }
    > +
    > + /* Power is in uW. Convert to mW before converting. */
    > + if (class == PSC_POWER)
    > + val = DIV_ROUND_CLOSEST(val, 1000L);
    > +
    > + /* Reduce large mantissa until it fits into 10 bit */
    > + while (val >= MAX_MANTISSA && exponent < 15) {
    > + exponent++;
    > + val >>= 1;
    > + }
    > + /* Increase small mantissa to improve precision */
    > + while (val < MIN_MANTISSA && exponent > -15) {
    > + exponent--;
    > + val <<= 1;
    > + }
    > +
    > + /* Convert mantissa from milli-units to units */
    > + mantissa = DIV_ROUND_CLOSEST(val, 1000);
    > +
    > + /* Ensure that resulting number is within range */
    > + if (mantissa > 0x3ff)
    > + mantissa = 0x3ff;
    > +
    > + /* restore sign */
    > + if (negative)
    > + mantissa = -mantissa;
    > +
    > + /* Convert to 5 bit exponent, 11 bit mantissa */
    > + return (mantissa & 0x7ff) | ((exponent << 11) & 0xf800);
    > +}
    > +
    > +static u16 pmbus_data2reg_direct(struct pmbus_data *data,
    > + enum pmbus_sensor_classes class, long val)
    > +{
    > + long m, b, R;
    > +
    > + m = data->m[class];
    > + b = data->b[class];
    > + R = data->R[class];
    > +
    > + /* Power is in uW. Adjust R. */
    > + if (class == PSC_POWER)
    > + R -= 3;
    > +
    > + /* Calculate Y = (m * X + b) * 10^R */
    > + R -= 3; /* Adjust R for data in milli-units */
    > + val *= m;
    > + val += b * 1000;
    > +
    > + while (R > 0) {
    > + val *= 10;
    > + R--;
    > + }
    > + while (R < 0) {
    > + val = DIV_ROUND_CLOSEST(val, 10);
    > + R++;
    > + }
    > +
    > + return val;
    > +}
    > +
    > +static u16 pmbus_data2reg(struct pmbus_data *data,
    > + enum pmbus_sensor_classes class, long val)
    > +{
    > + u16 reg;
    > +
    > + if (data->direct)
    > + reg = pmbus_data2reg_direct(data, class, val);
    > + else
    > + reg = pmbus_data2reg_linear(data, class, val);
    > +
    > + return reg;
    > +}
    > +
    > +/* Return converted value from given object */
    > +static long pmbus_get_sensor(struct pmbus_data *data, int index)
    > +{
    > + return pmbus_reg2data(data, data->sensors[index].class,
    > + data->sensor_data[index]);
    > +}
    > +
    > +/*
    > + * Return boolean calculated from converted data.
    > + * <index> defines a status register index and mask, and optionally
    > + * two sensor indexes.
    > + * The upper half-word references the two sensors,
    > + * the lower half word references status register and mask.
    > + * The function returns true if (status[reg] & mask) is true and,
    > + * if specified, if v1 >= v2.
    > + * To determine if an object exceeds upper limits, specify <v, limit>.
    > + * To determine if an object exceeds lower limits, specify <limit, v>.
    > + */
    > +static long pmbus_get_boolean(struct pmbus_data *data, int index)
    > +{
    > + u8 s1 = (index >> 24) & 0xff;
    > + u8 s2 = (index >> 16) & 0xff;
    > + u8 reg = (index >> 8) & 0xff;
    > + u8 mask = index & 0xff;
    > + int rv = 0;
    > + u8 regval = data->status[reg] & mask;
    > +
    > + if (!s1 && !s2)
    > + rv = !!regval;
    > + else {
    > + int v1, v2;
    > +
    > + v1 = pmbus_reg2data(data, data->sensors[s1].class,
    > + data->sensor_data[s1]);
    > + v2 = pmbus_reg2data(data, data->sensors[s2].class,
    > + data->sensor_data[s2]);
    > + rv = !!(regval && v1 >= v2);
    > + }
    > + return rv;
    > +}
    > +
    > +#define PMBUS_SHOW_INT(what) \
    > + static ssize_t pmbus_show_##what(struct device *dev, \
    > + struct device_attribute *da, \
    > + char *buf) \
    > +{ \
    > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); \
    > + struct pmbus_data *data = pmbus_update_device(dev); \
    > + long val; \
    > + if (IS_ERR(data)) \
    > + return PTR_ERR(data); \
    > + val = pmbus_get_##what(data, attr->index); \
    > + return snprintf(buf, PAGE_SIZE, "%ld\n", val); \
    > +}
    > +
    > +PMBUS_SHOW_INT(sensor);
    > +PMBUS_SHOW_INT(boolean);
    > +
    > +static ssize_t pmbus_set_sensor(struct device *dev,
    > + struct device_attribute *devattr,
    > + const char *buf, size_t count)
    > +{
    > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
    > + struct i2c_client *client = to_i2c_client(dev);
    > + struct pmbus_data *data = i2c_get_clientdata(client);
    > + struct pmbus_sensor *sensor = &data->sensors[attr->index];
    > + ssize_t rv = count;
    > + long val = 0;
    > + int ret;
    > + u16 reg;
    > +
    > + if (strict_strtol(buf, 10, &val) < 0)
    > + return -EINVAL;
    > +
    > + mutex_lock(&data->update_lock);
    > + reg = pmbus_data2reg(data, sensor->class, val);
    > + ret = pmbus_write_word_data(client, sensor->page, sensor->reg, reg);
    > + if (ret < 0)
    > + rv = ret;
    > + else
    > + data->sensor_data[attr->index] = reg;
    > + mutex_unlock(&data->update_lock);
    > + return rv;
    > +}
    > +
    > +static ssize_t pmbus_show_label(struct device *dev,
    > + struct device_attribute *da,
    > + char *buf)
    > +{
    > + struct i2c_client *client = to_i2c_client(dev);
    > + struct pmbus_data *data = i2c_get_clientdata(client);
    > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
    > +
    > + return snprintf(buf, PAGE_SIZE, "%s\n",
    > + data->labels[attr->index].label);
    > +}
    > +
    > +#define PMBUS_ADD_ATTR(data, _name, _idx, _mode, _show, _set) \
    > +do { \
    > + struct sensor_device_attribute *a \
    > + = &data->sensor_device_attributes[data->num_attributes];\
    > + BUG_ON(data->num_attributes >=PMBUS_NUM_ATTR); \
    > + a->dev_attr.attr.name = _name; \
    > + a->dev_attr.attr.mode = _mode; \
    > + a->dev_attr.show = _show; \
    > + a->dev_attr.store = _set; \
    > + a->index = _idx; \
    > + data->attributes[data->num_attributes] = &a->dev_attr.attr; \
    > + data->num_attributes++; \
    > +} while (0)
    > +
    > +#define PMBUS_ADD_GET_ATTR(data, _name, _type, _idx) \
    > + PMBUS_ADD_ATTR(data, _name, _idx, S_IRUGO, \
    > + pmbus_show_##_type, NULL)
    > +
    > +#define PMBUS_ADD_SET_ATTR(data, _name, _type, _idx) \
    > + PMBUS_ADD_ATTR(data, _name, _idx, S_IWUSR | S_IRUGO, \
    > + pmbus_show_##_type, pmbus_set_##_type)
    > +
    > +static void pmbus_add_boolean(struct pmbus_data *data,
    > + const char *name, const char *type, int seq,
    > + int val)
    > +{
    > + struct pmbus_boolean *boolean;
    > +
    > + BUG_ON(data->num_booleans >= PMBUS_BOOLEANS);
    > +
    > + boolean = &data->booleans[data->num_booleans];
    > +
    > + snprintf(boolean->name, sizeof(boolean->name), "%s%d_%s",
    > + name, seq, type);
    > + PMBUS_ADD_GET_ATTR(data, boolean->name, boolean, val);
    > + data->num_booleans++;
    > +}
    > +
    > +static void pmbus_add_boolean_reg(struct pmbus_data *data,
    > + const char *name, const char *type,
    > + int seq, int reg, int bit)
    > +{
    > + pmbus_add_boolean(data, name, type, seq,
    > + (reg << 8) | bit);
    > +}
    > +
    > +static void pmbus_add_boolean_cmp(struct pmbus_data *data,
    > + const char *name, const char *type,
    > + int seq, int i1, int i2, int reg,
    > + int mask)
    > +{
    > + pmbus_add_boolean(data, name, type, seq,
    > + (i1 << 24) | (i2 << 16) | (reg << 8) | mask);
    > +}
    > +
    > +static void pmbus_add_sensor(struct pmbus_data *data,
    > + const char *name, const char *type, int seq,
    > + int page, int reg, enum pmbus_sensor_classes class,
    > + bool update)
    > +{
    > + struct pmbus_sensor *sensor;
    > +
    > + BUG_ON(data->num_sensors >= PMBUS_SENSORS);
    > +
    > + sensor = &data->sensors[data->num_sensors];
    > + snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
    > + name, seq, type);
    > + sensor->page = page;
    > + sensor->reg = reg;
    > + sensor->class = class;
    > + sensor->update = update;
    > + if (update)
    > + PMBUS_ADD_GET_ATTR(data, sensor->name, sensor,
    > + data->num_sensors);
    > + else
    > + PMBUS_ADD_SET_ATTR(data, sensor->name, sensor,
    > + data->num_sensors);
    > + data->num_sensors++;
    > +}
    > +
    > +static void pmbus_add_label(struct pmbus_data *data,
    > + const char *name, int seq,
    > + const char *lstring, int index)
    > +{
    > + struct pmbus_label *label;
    > +
    > + BUG_ON(data->num_labels >= PMBUS_LABELS);
    > +
    > + label = &data->labels[data->num_labels];
    > + snprintf(label->name, sizeof(label->name), "%s%d_label",
    > + name, seq);
    > + if (!index)
    > + strncpy(label->label, lstring, sizeof(label->label) - 1);
    > + else
    > + snprintf(label->label, sizeof(label->label), "%s%d", lstring,
    > + index);
    > +
    > + PMBUS_ADD_GET_ATTR(data, label->name, label, data->num_labels);
    > + data->num_labels++;
    > +}
    > +
    > +static int pmbus_temp_sensors[] = {
    > + PMBUS_READ_TEMPERATURE_1,
    > + PMBUS_READ_TEMPERATURE_2,
    > + PMBUS_READ_TEMPERATURE_3
    > +};
    > +
    > +static int pmbus_probe(struct i2c_client *client,
    > + const struct i2c_device_id *id)
    > +{
    > + struct pmbus_data *data;
    > + int i, in_index, ret;
    > + int i0, i1;
    > +
    > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA
    > + | I2C_FUNC_SMBUS_WORD_DATA))
    > + return -ENODEV;
    > +
    This is an odd way of doing this. Assign ret only if (!data) rather
    than here.
    > + ret = -ENOMEM;
    > + data = kzalloc(sizeof(*data), GFP_KERNEL);
    > + if (!data)
    > + goto out;
    > +
    > + i2c_set_clientdata(client, data);
    > + mutex_init(&data->update_lock);
    > +
    Why not use the name rather than the id?
    > + dev_dbg(&client->dev, "PMBus device type %d", (int)id->driver_data);
    > +
    > + data->type = id->driver_data;

    Perhaps neater to put a pointer directly to pmbus_config in pmbus_data
    structure and use that?
    > + data->direct = pmbus_config[data->type].direct;
    > + data->pages = pmbus_config[data->type].pages;
    Please justify this parameter (may be elsewhere!) as I'm unconvinced it is
    a good idea when you may well have multiple pmbus devices in a system?
    > + if (pages > 0)
    > + data->pages = pages;
    > +
    Again, if one just has a pointer to the pmbus_config hten no need to copy
    these across.
    > + if (data->direct)
    > + for (i = 0; i < SENSOR_CLASSES; i++) {
    > + data->m[i] = pmbus_config[data->type].m[i];
    > + data->b[i] = pmbus_config[data->type].b[i];
    > + data->R[i] = pmbus_config[data->type].R[i];
    > + }
    > +
    > + if (data->type == pmbus) {
    > + /*
    > + * We could try to determine supported options here.
    > + * However, it appears that hardly any chips support
    > + * the CAPABILITY, QUERY, or COEFFICIENTS commands.
    > + * Thus, we just use default basic settings.
    > + */
    > + }
    > +
    > + /*
    > + * Bail out if the number of pages is bad, or if more than
    > + * one page was configured and the chip has no PAGE register.
    > + */
    I think you only need to run this if you have the pages module parameter
    specified (or there is a bug in your driver / someone is lying about what
    is wired up, neither of which should be explicitly handled)
    > + if (data->pages > PMBUS_PAGES ||
    > + (data->pages > 1 && !pmbus_check_byte_register(client, 0,
    > + PMBUS_PAGE))) {
    > + dev_err(&client->dev, "%d pages but no PAGE register",
    > + data->pages);
    > + ret = -EINVAL;
    > + goto out_data;
    > + }
    > +
    > + /*
    > + * Bail out if status register or PMBus revision register
    > + * does not exist.
    > + */
    Again, doesn't this just mean you don't have a pmbus device? If this driver's
    probe is running you should be sure you have one.
    > + if (!pmbus_check_byte_register(client, 0, PMBUS_STATUS_BYTE)
    > + || !pmbus_check_byte_register(client, 0, PMBUS_REVISION)) {
    > + ret = -ENODEV;
    > + goto out_data;
    > + }
    > +
    > + /*
    > + * Identify supported status registers
    > + */
    > + if (pmbus_check_byte_register(client, 0, PMBUS_STATUS_VOUT))
    > + data->status_bits |= HAVE_STATUS_VOUT;
    > + if (pmbus_check_byte_register(client, 0, PMBUS_STATUS_IOUT))
    > + data->status_bits |= HAVE_STATUS_IOUT;
    > + if (pmbus_check_byte_register(client, 0, PMBUS_STATUS_INPUT))
    > + data->status_bits |= HAVE_STATUS_INPUT;
    > + if (pmbus_check_byte_register(client, 0, PMBUS_STATUS_TEMPERATURE))
    > + data->status_bits |= HAVE_STATUS_TEMP;
    > +
    > + /*
    > + * Input voltage sensors
    > + */
    > + in_index = 1;
    > + if (pmbus_check_word_register(client, 0, PMBUS_READ_VIN)) {
    > + bool have_fault = false;
    > +
    > + i0 = data->num_sensors;
    > + pmbus_add_label(data, "in", in_index, "vin", 0);
    > + pmbus_add_sensor(data, "in", "input", in_index,
    > + 0, PMBUS_READ_VIN, PSC_VOLTAGE, true);
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_VIN_UV_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "min", in_index,
    > + 0, PMBUS_VIN_UV_WARN_LIMIT,
    > + PSC_VOLTAGE, false);
    > + if (data->status_bits & HAVE_STATUS_INPUT)
    > + pmbus_add_boolean_reg(data, "in", "min_alarm",
    > + in_index,
    > + PB_STATUS_INPUT_BASE,
    > + PB_VOLTAGE_UV_WARNING);
    > + }
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_VIN_UV_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "lcrit", in_index,
    > + 0, PMBUS_VIN_UV_FAULT_LIMIT,
    > + PSC_VOLTAGE, false);
    > + if (data->status_bits & HAVE_STATUS_INPUT)
    > + have_fault = true;
    > + }
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_VIN_OV_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "max", in_index,
    > + 0, PMBUS_VIN_OV_WARN_LIMIT,
    > + PSC_VOLTAGE, false);
    > + if (data->status_bits & HAVE_STATUS_INPUT)
    > + pmbus_add_boolean_reg(data, "in", "max_alarm",
    > + in_index,
    > + PB_STATUS_INPUT_BASE,
    > + PB_VOLTAGE_OV_WARNING);
    > + }
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_VIN_OV_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "crit", in_index,
    > + 0, PMBUS_VIN_OV_FAULT_LIMIT,
    > + PSC_VOLTAGE, false);
    > + if (data->status_bits & HAVE_STATUS_INPUT)
    > + have_fault = true;
    > + }
    > + if (have_fault) {
    > + pmbus_add_boolean_reg(data, "in", "fault",
    > + in_index,
    > + PB_STATUS_INPUT_BASE,
    > + PB_VOLTAGE_UV_FAULT
    > + | PB_VOLTAGE_OV_FAULT);
    > + }
    > + in_index++;
    > + }
    > + if (pmbus_check_word_register(client, 0, PMBUS_READ_VCAP)) {
    > + pmbus_add_label(data, "in", in_index, "vcap", 0);
    > + pmbus_add_sensor(data, "in", "input", in_index, 0,
    > + PMBUS_READ_VCAP, PSC_VOLTAGE, true);
    > + in_index++;
    > + }
    > +
    > + /*
    > + * Output voltage sensors
    > + */
    > + for (i = 0; i < data->pages; i++) {
    > + bool have_fault = false;
    > +
    > + if (!pmbus_check_word_register(client, i, PMBUS_READ_VOUT))
    > + break;
    > +
    > + i0 = data->num_sensors;
    > + pmbus_add_label(data, "in", in_index, "vout", i+1);
    > + pmbus_add_sensor(data, "in", "input", in_index, i,
    > + PMBUS_READ_VOUT, PSC_VOLTAGE, true);
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_VOUT_UV_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "min", in_index, i,
    > + PMBUS_VOUT_UV_WARN_LIMIT, PSC_VOLTAGE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_VOUT)
    > + pmbus_add_boolean_reg(data, "in", "min_alarm",
    > + in_index,
    > + PB_STATUS_VOUT_BASE + i,
    > + PB_VOLTAGE_UV_WARNING);
    > + }
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_VOUT_UV_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "lcrit", in_index, i,
    > + PMBUS_VOUT_UV_FAULT_LIMIT, PSC_VOLTAGE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_VOUT)
    > + have_fault = true;
    > + }
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_VOUT_OV_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "max", in_index, i,
    > + PMBUS_VOUT_OV_WARN_LIMIT, PSC_VOLTAGE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_VOUT) {
    > + pmbus_add_boolean_reg(data, "in", "max_alarm",
    > + in_index,
    > + PB_STATUS_VOUT_BASE + i,
    > + PB_VOLTAGE_OV_WARNING);
    > + }
    > + }
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_VOUT_OV_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "in", "crit", in_index, i,
    > + PMBUS_VOUT_OV_FAULT_LIMIT, PSC_VOLTAGE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_VOUT)
    > + have_fault = true;
    > + }
    > + if (have_fault) {
    > + pmbus_add_boolean_reg(data, "in", "fault",
    > + in_index,
    > + PB_STATUS_VOUT_BASE + i,
    > + PB_VOLTAGE_UV_FAULT
    > + | PB_VOLTAGE_OV_FAULT);
    > + }
    > + in_index++;
    > + }
    > +
    > + /*
    > + * Current sensors
    > + */
    > +
    > + /*
    > + * Input current sensors
    > + */
    > + in_index = 1;
    > + if (pmbus_check_word_register(client, 0, PMBUS_READ_IIN)) {
    > + i0 = data->num_sensors;
    > + pmbus_add_label(data, "curr", in_index, "iin", 0);
    > + pmbus_add_sensor(data, "curr", "input", in_index,
    > + 0, PMBUS_READ_IIN, PSC_CURRENT, true);
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_IIN_OC_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "curr", "max", in_index,
    > + 0, PMBUS_IIN_OC_WARN_LIMIT,
    > + PSC_CURRENT, false);
    > + if (data->status_bits & HAVE_STATUS_INPUT) {
    > + pmbus_add_boolean_reg(data, "curr", "alarm",
    > + in_index,
    > + PB_STATUS_INPUT_BASE,
    > + PB_IIN_OC_WARNING);
    > + }
    > + }
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_IIN_OC_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "curr", "crit", in_index,
    > + 0, PMBUS_IIN_OC_FAULT_LIMIT,
    > + PSC_CURRENT, false);
    > + if (data->status_bits & HAVE_STATUS_INPUT) {
    > + pmbus_add_boolean_reg(data, "curr", "fault",
    > + in_index,
    > + PB_STATUS_INPUT_BASE,
    > + PB_IIN_OC_FAULT);
    > + }
    > + }
    > + in_index++;
    > + }
    > +
    > + /*
    > + * Output Current sensors
    > + */
    > + for (i = 0; i < data->pages; i++) {
    > + bool have_fault = false;
    > +
    > + if (!pmbus_check_word_register(client, i, PMBUS_READ_IOUT))
    > + break;
    > +
    > + i0 = data->num_sensors;
    > + pmbus_add_label(data, "curr", in_index, "iout", i+1);
    > + pmbus_add_sensor(data, "curr", "input", in_index, i,
    > + PMBUS_READ_IOUT, PSC_CURRENT, true);
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_IOUT_OC_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "curr", "max", in_index, i,
    > + PMBUS_IOUT_OC_WARN_LIMIT, PSC_CURRENT,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_IOUT)
    > + pmbus_add_boolean_reg(data, "curr", "alarm",
    > + in_index,
    > + PB_STATUS_IOUT_BASE + i,
    > + PB_IOUT_OC_WARNING);
    > + }
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_IOUT_OC_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "curr", "crit", in_index, i,
    > + PMBUS_IOUT_OC_FAULT_LIMIT, PSC_CURRENT,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_IOUT)
    > + have_fault = true;
    > + }
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_IOUT_UC_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "curr", "lcrit", in_index, i,
    > + PMBUS_IOUT_UC_FAULT_LIMIT, PSC_CURRENT,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_IOUT)
    > + have_fault = true;
    > + }
    > + if (have_fault) {
    > + pmbus_add_boolean_reg(data, "curr", "fault",
    > + in_index,
    > + PB_STATUS_IOUT_BASE + i,
    > + PB_IOUT_UC_FAULT
    > + | PB_IOUT_OC_FAULT);
    > + }
    > + in_index++;
    > + }
    > +
    > + /*
    > + * Power sensors
    > + */
    > + /*
    > + * Input Power sensors
    > + */
    > + in_index = 1;
    > + if (pmbus_check_word_register(client, 0, PMBUS_READ_PIN)) {
    > + i0 = data->num_sensors;
    > + pmbus_add_label(data, "power", in_index, "pin", 0);
    > + pmbus_add_sensor(data, "power", "input", in_index,
    > + 0, PMBUS_READ_PIN, PSC_POWER, true);
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_PIN_OP_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "power", "max", in_index,
    > + 0, PMBUS_PIN_OP_WARN_LIMIT, PSC_POWER,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_INPUT)
    > + pmbus_add_boolean_reg(data, "power", "alarm",
    > + in_index,
    > + PB_STATUS_INPUT_BASE,
    > + PB_PIN_OP_WARNING);
    > + }
    > + in_index++;
    > + }
    > +
    > + /*
    > + * Output Power sensors
    > + */
    > + for (i = 0; i < data->pages; i++) {
    > + bool need_alarm = false;
    > +
    > + if (!pmbus_check_word_register(client, i, PMBUS_READ_POUT))
    > + break;
    > +
    > + i0 = data->num_sensors;
    > + pmbus_add_label(data, "power", in_index, "pout", i+1);
    > + pmbus_add_sensor(data, "power", "input", in_index, i,
    > + PMBUS_READ_POUT, PSC_POWER, true);
    > + /*
    > + * Per hwmon sysfs API, power_cap is to be used to limit output
    > + * power.
    > + * We have two registers related to maximum output power,
    > + * PMBUS_POUT_MAX and PMBUS_POUT_OP_WARN_LIMIT.
    > + * PMBUS_POUT_MAX matches the powerX_cap attribute definition.
    > + * There is no attribute in the API to match
    > + * PMBUS_POUT_OP_WARN_LIMIT. We use powerX_max for now.
    > + */
    > + if (pmbus_check_word_register(client, i, PMBUS_POUT_MAX)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "power", "cap", in_index, i,
    > + PMBUS_POUT_MAX, PSC_POWER, false);
    > + need_alarm = true;
    > + }
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_POUT_OP_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "power", "max", in_index, i,
    > + PMBUS_POUT_OP_WARN_LIMIT, PSC_POWER,
    > + false);
    > + need_alarm = true;
    > + }
    > + if (need_alarm && (data->status_bits & HAVE_STATUS_IOUT))
    > + pmbus_add_boolean_reg(data, "power", "alarm",
    > + in_index,
    > + PB_STATUS_IOUT_BASE,
    > + PB_POUT_OP_WARNING
    > + | PB_POWER_LIMITING);
    > +
    > + if (pmbus_check_word_register(client, i,
    > + PMBUS_POUT_OP_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "power", "crit", in_index, i,
    > + PMBUS_POUT_OP_FAULT_LIMIT, PSC_POWER,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_IOUT)
    > + pmbus_add_boolean_reg(data, "power", "fault",
    > + in_index,
    > + PB_STATUS_IOUT_BASE,
    > + PB_POUT_OP_FAULT);
    > + }
    > + in_index++;
    > + }
    > +
    > + /*
    > + * Temperature sensors
    > + */
    > + in_index = 1;
    > + for (i = 0; i < ARRAY_SIZE(pmbus_temp_sensors); i++) {
    > + if (!pmbus_check_word_register(client, 0,
    > + pmbus_temp_sensors[i]))
    > + break;
    > +
    > + i0 = data->num_sensors;
    > + pmbus_add_sensor(data, "temp", "input", in_index, 0,
    > + pmbus_temp_sensors[i], PSC_TEMPERATURE, true);
    > +
    > + /*
    > + * PMBus provides only one status register for all temperature
    > + * sensors. Thus, we can not use the status register to
    > + * determine which of the sensors actually caused an alarm
    > + * or fault. Always compare the current temperature against
    > + * the limit registers to determine warning or fault conditions.
    > + */
    > + if (pmbus_check_word_register(client, 0, PMBUS_UT_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "temp", "min", in_index, 0,
    > + PMBUS_UT_WARN_LIMIT, PSC_TEMPERATURE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_TEMP)
    > + pmbus_add_boolean_cmp(data, "temp", "min_alarm",
    > + in_index, i1, i0,
    > + PB_STATUS_TEMP_BASE,
    > + PB_TEMP_UT_WARNING);
    > + }
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_UT_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "temp", "lcrit", in_index, 0,
    > + PMBUS_UT_FAULT_LIMIT, PSC_TEMPERATURE,
    > + false);
    > + }
    > + if (pmbus_check_word_register(client, 0, PMBUS_OT_WARN_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "temp", "max", in_index, 0,
    > + PMBUS_OT_WARN_LIMIT, PSC_TEMPERATURE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_TEMP)
    > + pmbus_add_boolean_cmp(data, "temp", "max_alarm",
    > + in_index, i0, i1,
    > + PB_STATUS_TEMP_BASE,
    > + PB_TEMP_OT_WARNING);
    > + }
    > + if (pmbus_check_word_register(client, 0,
    > + PMBUS_OT_FAULT_LIMIT)) {
    > + i1 = data->num_sensors;
    > + pmbus_add_sensor(data, "temp", "crit", in_index, 0,
    > + PMBUS_OT_FAULT_LIMIT, PSC_TEMPERATURE,
    > + false);
    > + if (data->status_bits & HAVE_STATUS_TEMP)
    > + pmbus_add_boolean_cmp(data, "temp",
    > + "crit_alarm",
    > + in_index, i0, i1,
    > + PB_STATUS_TEMP_BASE,
    > + PB_TEMP_OT_FAULT);
    > + }
    > + in_index++;
    > + }
    > +
    > + pmbus_clear_faults(client);
    > +
    > + ret = -ENODEV;
    > + /* Register sysfs hooks */
    > + data->group.attrs = data->attributes;
    > + ret = sysfs_create_group(&client->dev.kobj, &data->group);
    > + if (ret)
    > + goto out_data;
    > + data->hwmon_dev = hwmon_device_register(&client->dev);
    > + if (IS_ERR(data->hwmon_dev)) {
    > + ret = PTR_ERR(data->hwmon_dev);
    > + goto out_hwmon_device_register;
    > + }
    > + return 0;
    > +out_hwmon_device_register:
    > + sysfs_remove_group(&client->dev.kobj, &data->group);
    > +out_data:
    > + kfree(data);
    > +out:
    > + return ret;
    > +}
    > +
    > +static int pmbus_remove(struct i2c_client *client)
    > +{
    > + struct pmbus_data *data = i2c_get_clientdata(client);
    > + hwmon_device_unregister(data->hwmon_dev);
    > + sysfs_remove_group(&client->dev.kobj, &data->group);
    > + kfree(data);
    > + return 0;
    > +}
    > +
    > +static const struct i2c_device_id pmbus_id[] = {
    > + {"bmr45x", pmbus},
    > + {"ltc2978", ltc2978},
    > + {"max16064", max16064},
    > + {"max8688", max8688},
    > + {"pmbus", pmbus},
    > + {"ucd921x", pmbus},
    > + {"ucd9220", ucd9220},
    > + {"ucd9240", ucd9240},
    > + {}
    > +};
    > +
    > +MODULE_DEVICE_TABLE(i2c, pmbus_id);
    > +
    > +/* This is the driver that will be inserted */
    > +static struct i2c_driver pmbus_driver = {
    > + .driver = {
    > + .name = "pmbus",
    > + },
    > + .probe = pmbus_probe,
    > + .remove = pmbus_remove,
    > + .id_table = pmbus_id,
    > +};
    > +
    > +static int __init pmbus_init(void)
    > +{
    > + return i2c_add_driver(&pmbus_driver);
    > +}
    > +
    > +static void __exit pmbus_exit(void)
    > +{
    > + i2c_del_driver(&pmbus_driver);
    > +}
    > +
    > +MODULE_AUTHOR("Guenter Roeck");
    > +MODULE_DESCRIPTION("PMBus driver");
    > +MODULE_LICENSE("GPL");
    > +module_init(pmbus_init);
    > +module_exit(pmbus_exit);
    > diff --git a/drivers/hwmon/pmbus.h b/drivers/hwmon/pmbus.h
    > new file mode 100644
    > index 0000000..2a8a027
    > --- /dev/null
    > +++ b/drivers/hwmon/pmbus.h
    > @@ -0,0 +1,209 @@
    > +/*
    > + * pmbus.h - Common defines and structures for PMBus devices
    > + *
    > + * Copyright (c) 2010 Ericsson AB.
    > + *
    > + * This program is free software; you can redistribute it and/or modify
    > + * it under the terms of the GNU General Public License as published by
    > + * the Free Software Foundation; either version 2 of the License, or
    > + * (at your option) any later version.
    > + *
    > + * This program is distributed in the hope that it will be useful,
    > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
    > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
    > + * GNU General Public License for more details.
    > + *
    > + * You should have received a copy of the GNU General Public License
    > + * along with this program; if not, write to the Free Software
    > + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
    > + */
    > +
    > +#ifndef PMBUS_H
    > +#define PMBUS_H
    > +
    > +/*
    > + * Registers
    > + */
    > +#define PMBUS_PAGE 0x00
    > +#define PMBUS_OPERATION 0x01
    > +#define PMBUS_ON_OFF_CONFIG 0x02
    > +#define PMBUS_CLEAR_FAULTS 0x03
    > +#define PMBUS_PHASE 0x04
    > +
    > +#define PMBUS_CAPABILITY 0x19
    > +#define PMBUS_QUERY 0x1A
    > +
    > +#define PMBUS_VOUT_MODE 0x20
    > +#define PMBUS_VOUT_COMMAND 0x21
    > +#define PMBUS_VOUT_TRIM 0x22
    > +#define PMBUS_VOUT_CAL_OFFSET 0x23
    > +#define PMBUS_VOUT_MAX 0x24
    > +#define PMBUS_VOUT_MARGIN_HIGH 0x25
    > +#define PMBUS_VOUT_MARGIN_LOW 0x26
    > +#define PMBUS_VOUT_TRANSITION_RATE 0x27
    > +#define PMBUS_VOUT_DROOP 0x28
    > +#define PMBUS_VOUT_SCALE_LOOP 0x29
    > +#define PMBUS_VOUT_SCALE_MONITOR 0x2A
    > +
    > +#define PMBUS_COEFFICIENTS 0x30
    > +#define PMBUS_POUT_MAX 0x31
    > +
    > +#define PMBUS_VOUT_OV_FAULT_LIMIT 0x40
    > +#define PMBUS_VOUT_OV_FAULT_RESPONSE 0x41
    > +#define PMBUS_VOUT_OV_WARN_LIMIT 0x42
    > +#define PMBUS_VOUT_UV_WARN_LIMIT 0x43
    > +#define PMBUS_VOUT_UV_FAULT_LIMIT 0x44
    > +#define PMBUS_VOUT_UV_FAULT_RESPONSE 0x45
    > +#define PMBUS_IOUT_OC_FAULT_LIMIT 0x46
    > +#define PMBUS_IOUT_OC_FAULT_RESPONSE 0x47
    > +#define PMBUS_IOUT_OC_LV_FAULT_LIMIT 0x48
    > +#define PMBUS_IOUT_OC_LV_FAULT_RESPONSE 0x49
    > +#define PMBUS_IOUT_OC_WARN_LIMIT 0x4A
    > +#define PMBUS_IOUT_UC_FAULT_LIMIT 0x4B
    > +#define PMBUS_IOUT_UC_FAULT_RESPONSE 0x4C
    > +
    > +#define PMBUS_OT_FAULT_LIMIT 0x4F
    > +#define PMBUS_OT_FAULT_RESPONSE 0x50
    > +#define PMBUS_OT_WARN_LIMIT 0x51
    > +#define PMBUS_UT_WARN_LIMIT 0x52
    > +#define PMBUS_UT_FAULT_LIMIT 0x53
    > +#define PMBUS_UT_FAULT_RESPONSE 0x54
    > +#define PMBUS_VIN_OV_FAULT_LIMIT 0x55
    > +#define PMBUS_VIN_OV_FAULT_RESPONSE 0x56
    > +#define PMBUS_VIN_OV_WARN_LIMIT 0x57
    > +#define PMBUS_VIN_UV_WARN_LIMIT 0x58
    > +#define PMBUS_VIN_UV_FAULT_LIMIT 0x59
    > +
    > +#define PMBUS_IIN_OC_FAULT_LIMIT 0x5B
    > +#define PMBUS_IIN_OC_WARN_LIMIT 0x5D
    > +
    > +#define PMBUS_POUT_OP_FAULT_LIMIT 0x68
    > +#define PMBUS_POUT_OP_WARN_LIMIT 0x6A
    > +#define PMBUS_PIN_OP_WARN_LIMIT 0x6B
    > +
    > +#define PMBUS_STATUS_BYTE 0x78
    > +#define PMBUS_STATUS_WORD 0x79
    > +#define PMBUS_STATUS_VOUT 0x7A
    > +#define PMBUS_STATUS_IOUT 0x7B
    > +#define PMBUS_STATUS_INPUT 0x7C
    > +#define PMBUS_STATUS_TEMPERATURE 0x7D
    > +#define PMBUS_STATUS_CML 0x7E
    > +#define PMBUS_STATUS_OTHER 0x7F
    > +#define PMBUS_STATUS_MFR_SPECIFIC 0x80
    > +#define PMBUS_STATUS_FANS_1_2 0x81
    > +#define PMBUS_STATUS_FANS_3_4 0x82
    > +
    > +#define PMBUS_READ_VIN 0x88
    > +#define PMBUS_READ_IIN 0x89
    > +#define PMBUS_READ_VCAP 0x8A
    > +#define PMBUS_READ_VOUT 0x8B
    > +#define PMBUS_READ_IOUT 0x8C
    > +#define PMBUS_READ_TEMPERATURE_1 0x8D
    > +#define PMBUS_READ_TEMPERATURE_2 0x8E
    > +#define PMBUS_READ_TEMPERATURE_3 0x8F
    > +#define PMBUS_READ_FAN_SPEED_1 0x90
    > +#define PMBUS_READ_FAN_SPEED_2 0x91
    > +#define PMBUS_READ_FAN_SPEED_3 0x92
    > +#define PMBUS_READ_FAN_SPEED_4 0x93
    > +#define PMBUS_READ_DUTY_CYCLE 0x94
    > +#define PMBUS_READ_FREQUENCY 0x95
    > +#define PMBUS_READ_POUT 0x96
    > +#define PMBUS_READ_PIN 0x97
    > +
    > +#define PMBUS_REVISION 0x98
    > +#define PMBUS_MFR_ID 0x99
    > +#define PMBUS_MFR_MODEL 0x9A
    > +#define PMBUS_MFR_REVISION 0x9B
    > +#define PMBUS_MFR_LOCATION 0x9C
    > +#define PMBUS_MFR_DATE 0x9D
    > +#define PMBUS_MFR_SERIAL 0x9E
    > +
    > +#define LTC2978_MFR_SPECIAL_ID 0xE7
    > +
    > +/*
    > + * CAPABILITY
    > + */
    > +#define PB_CAPABILITY_SMBALERT (1<<4)
    > +#define PB_CAPABILITY_ERROR_CHECK (1<<7)
    > +
    > +/*
    > + * VOUT_MODE
    > + */
    > +#define PB_VOUT_MODE_MODE_MASK 0xe0
    > +#define PB_VOUT_MODE_PARAM_MASK 0x1f
    > +
    > +#define PB_VOUT_MODE_LINEAR 0x00
    > +#define PB_VOUT_MODE_VID 0x20
    > +#define PB_VOUT_MODE_DIRECT 0x40
    > +
    > +/*
    > + * STATUS_BYTE, STATUS_WORD (lower)
    > + */
    > +#define PB_STATUS_NONE_ABOVE (1<<0)
    > +#define PB_STATUS_CML (1<<1)
    > +#define PB_STATUS_TEMPERATURE (1<<2)
    > +#define PB_STATUS_VIN_UV (1<<3)
    > +#define PB_STATUS_IOUT_OC (1<<4)
    > +#define PB_STATUS_VOUT_OV (1<<5)
    > +#define PB_STATUS_OFF (1<<6)
    > +#define PB_STATUS_BUSY (1<<7)
    > +
    > +/*
    > + * STATUS_WORD (upper)
    > + */
    > +#define PB_STATUS_UNKNOWN (1<<8)
    > +#define PB_STATUS_OTHER (1<<9)
    > +#define PB_STATUS_FANS (1<<10)
    > +#define PB_STATUS_POWER_GOOD_N (1<<11)
    > +#define PB_STATUS_WORD_MFR (1<<12)
    > +#define PB_STATUS_INPUT (1<<13)
    > +#define PB_STATUS_IOUT_POUT (1<<14)
    > +#define PB_STATUS_VOUT (1<<15)
    > +
    > +/*
    > + * STATUS_IOUT
    > + */
    > +#define PB_POUT_OP_WARNING (1<<0)
    > +#define PB_POUT_OP_FAULT (1<<1)
    > +#define PB_POWER_LIMITING (1<<2)
    > +#define PB_CURRENT_SHARE_FAULT (1<<3)
    > +#define PB_IOUT_UC_FAULT (1<<4)
    > +#define PB_IOUT_OC_WARNING (1<<5)
    > +#define PB_IOUT_OC_LV_FAULT (1<<6)
    > +#define PB_IOUT_OC_FAULT (1<<7)
    > +
    > +/*
    > + * STATUS_VOUT, STATUS_INPUT
    > + */
    > +#define PB_VOLTAGE_UV_FAULT (1<<4)
    > +#define PB_VOLTAGE_UV_WARNING (1<<5)
    > +#define PB_VOLTAGE_OV_WARNING (1<<6)
    > +#define PB_VOLTAGE_OV_FAULT (1<<7)
    > +
    > +/*
    > + * STATUS_INPUT
    > + */
    > +#define PB_PIN_OP_WARNING (1<<0)
    > +#define PB_IIN_OC_WARNING (1<<1)
    > +#define PB_IIN_OC_FAULT (1<<2)
    > +
    > +/*
    > + * STATUS_TEMPERATURE
    > + */
    > +#define PB_TEMP_UT_FAULT (1<<4)
    > +#define PB_TEMP_UT_WARNING (1<<5)
    > +#define PB_TEMP_OT_WARNING (1<<6)
    > +#define PB_TEMP_OT_FAULT (1<<7)
    > +
    > +/*
    > + * CML_FAULT_STATUS
    > + */
    > +#define PB_CML_FAULT_OTHER_MEM_LOGIC (1<<0)
    > +#define PB_CML_FAULT_OTHER_COMM (1<<1)
    > +#define PB_CML_FAULT_PROCESSOR (1<<3)
    > +#define PB_CML_FAULT_MEMORY (1<<4)
    > +#define PB_CML_FAULT_PACKET_ERROR (1<<5)
    > +#define PB_CML_FAULT_INVALID_DATA (1<<6)
    > +#define PB_CML_FAULT_INVALID_COMMAND (1<<7)
    > +
    > +#endif /* PB_H */



    \
     
     \ /
      Last update: 2010-06-29 15:41    [W:0.136 / U:30.304 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site