| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20110226024552.GA14032@ericsson.com>
Date: Fri, 25 Feb 2011 18:45:52 -0800
From: Guenter Roeck <guenter.roeck@...csson.com>
To: Jonathan Cameron <jic23@....ac.uk>
CC: Jean Delvare <khali@...ux-fr.org>,
Jonathan Cameron <kernel@...23.retrosnub.co.uk>,
Randy Dunlap <rdunlap@...otime.net>,
Greg Schnorr <gschnorr@...co.com>,
"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/5] hwmon: PMBus device driver
On Fri, Feb 25, 2011 at 03:23:10PM -0500, Jonathan Cameron wrote:
> On 02/17/11 19:00, Guenter Roeck wrote:
> > This driver adds support for hardware monitoring features of various PMBus
> > devices.
>
> Hi Guenter, I'm afraid this isn't the most thorough review ever, but there
> are a few questions I'd like answered inline. These devices are pretty ugly.
>
Hi Jonathan,
given the complexity of the driver I am very happy you found the time!
> I'm also a little unhappy with how often you clear faults without it being
> obvious why. Please have another look at this and add clarifying comments...
>
Mostly that was to make me feel safer. I removed some; hope it is better now.
> I've suggested use of some macros inline. Not sure whether it's actualy a good
> idea though. What do you think?
>
I actually had more function macros in the code, but I am getting a bit away
from it after I noticed that it often just obfuscates the code. Only reason
for not removing the remaining the function macros is that I didn't find
a clean way to replace them with actual code or functions.
> Other than request for a few more comments to explain bits that weren't immediately
> obvious, the only bit I'd really question is the necessity of the list of pmbus
> devices. I might have missed something there though...
>
That is pretty much just to be able to clean up an entry. More see below.
> Thanks,
>
> Jonathan
>
> >
> > Signed-off-by: Guenter Roeck <guenter.roeck@...csson.com>
> > ---
> > drivers/hwmon/Kconfig | 25 +
> > drivers/hwmon/Makefile | 4 +
> > drivers/hwmon/pmbus.c | 234 +++++++
> > drivers/hwmon/pmbus.h | 307 +++++++++
> > drivers/hwmon/pmbus_core.c | 1611 ++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/i2c/pmbus.h | 45 ++
> > 6 files changed, 2226 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/hwmon/pmbus.c
> > create mode 100644 drivers/hwmon/pmbus.h
> > create mode 100644 drivers/hwmon/pmbus_core.c
> > create mode 100644 include/linux/i2c/pmbus.h
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 773e484..9f20e56 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -734,6 +734,31 @@ config SENSORS_PCF8591
> > These devices are hard to detect and rarely found on mainstream
> > hardware. If unsure, say N.
> >
> > +config PMBUS
> > + tristate "PMBus support"
Side note: that should really be a boolean. Will be fixed in next version.
> > + depends on I2C && EXPERIMENTAL
> > + default n
> > + help
> > + Say yes here if you want to enable PMBus support.
> > +
> > + This driver can also be built as a module. If so, the module will
> > + be called pmbus_core.
> > +
> > +if PMBUS
> > +
> > +config SENSORS_PMBUS
> > + tristate "Generic PMBus devices"
> > + default n
> > + help
> > + If you say yes here you get hardware monitoring support for generic
> > + PMBus devices, including but not limited to BMR450, BMR451, BMR453,
> > + BMR454, and LTC2978.
> > +
> > + This driver can also be built as a module. If so, the module will
> > + be called pmbus.
> > +
> > +endif # 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 dde02d9..f7f158b 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -112,6 +112,10 @@ obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
> > obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
> > obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
> >
> > +# PMBus drivers
> > +obj-$(CONFIG_PMBUS) += pmbus_core.o
> > +obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> > +
> > ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
> > EXTRA_CFLAGS += -DDEBUG
> > endif
> > diff --git a/drivers/hwmon/pmbus.c b/drivers/hwmon/pmbus.c
> > new file mode 100644
> > index 0000000..e5c3527
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus.c
> > @@ -0,0 +1,234 @@
> > +/*
> > + * Hardware monitoring driver for PMBus devices
> > + *
> > + * Copyright (c) 2010, 2011 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/mutex.h>
> > +#include <linux/list.h>
> > +#include <linux/i2c.h>
> > +#include "pmbus.h"
> > +
> > +struct pmbus_entry {
> > + struct list_head list;
> > + struct i2c_client *client;
> > + struct pmbus_driver_info info;
> > +};
> > +
> I'd like to see a comment to explain that (I think) this
> list only contains pmbus drivers directly managed by this driver.
> (and not say the max8688?)
Correct, and its only purpose is to be able to clean up the locally allocated
struct pmbus_driver_info. Other drivers may have static platform data, and thus
not have to clean it up.
> > +static LIST_HEAD(pmbus_list);
> > +static DEFINE_MUTEX(pmbus_list_mutex);
> > +
> > +/*
> > + * Find sensor groups and status registers on each page.
> > + */
> > +static void pmbus_find_sensor_groups(struct i2c_client *client,
> > + struct pmbus_driver_info *info)
> > +{
> > + int page;
> > +
> > + /* Sensors detected on page 0 only */
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_VIN))
> > + info->func[0] |= PMBUS_HAVE_VIN;
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_VCAP))
> > + info->func[0] |= PMBUS_HAVE_VCAP;
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_IIN))
> > + info->func[0] |= PMBUS_HAVE_IIN;
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_PIN))
> > + info->func[0] |= PMBUS_HAVE_PIN;
> > + if (info->func[0]
> > + && pmbus_check_byte_register(client, 0, PMBUS_STATUS_INPUT))
> > + info->func[0] |= PMBUS_HAVE_STATUS_INPUT;
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_FAN_SPEED_1)) {
> > + info->func[0] |= PMBUS_HAVE_FAN12;
> > + if (pmbus_check_byte_register(client, 0, PMBUS_STATUS_FAN_12))
> > + info->func[0] |= PMBUS_HAVE_STATUS_FAN12;
> > + }
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_FAN_SPEED_3)) {
> > + info->func[0] |= PMBUS_HAVE_FAN34;
> > + if (pmbus_check_byte_register(client, 0, PMBUS_STATUS_FAN_34))
> > + info->func[0] |= PMBUS_HAVE_STATUS_FAN34;
> > + }
> > + if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_1)) {
> > + info->func[0] |= PMBUS_HAVE_TEMP;
> > + if (pmbus_check_byte_register(client, 0,
> > + PMBUS_STATUS_TEMPERATURE))
> > + info->func[0] |= PMBUS_HAVE_STATUS_TEMP;
> > + }
> > +
> > + /* Sensors detected on all pages */
> > + for (page = 0; page < info->pages; page++) {
> > + if (pmbus_check_word_register(client, page, PMBUS_READ_VOUT)) {
> > + info->func[page] |= PMBUS_HAVE_VOUT;
> > + if (pmbus_check_byte_register(client, page,
> > + PMBUS_STATUS_VOUT))
> > + info->func[page] |= PMBUS_HAVE_STATUS_VOUT;
> > + }
> > + if (pmbus_check_word_register(client, page, PMBUS_READ_IOUT)) {
> > + info->func[page] |= PMBUS_HAVE_IOUT;
> > + if (pmbus_check_byte_register(client, 0,
> > + PMBUS_STATUS_IOUT))
> > + info->func[page] |= PMBUS_HAVE_STATUS_IOUT;
> > + }
> > + if (pmbus_check_word_register(client, page, PMBUS_READ_POUT))
> > + info->func[page] |= PMBUS_HAVE_POUT;
> > + }
> > +}
> > +
> > +/*
> > + * Identify chip parameters.
> > + */
> > +static int pmbus_identify(struct i2c_client *client,
> > + struct pmbus_driver_info *info)
> > +{
> > + if (!info->pages) {
> > + /*
> > + * Check if the PAGE command is supported. If it is,
> > + * keep setting the page number until it fails or until the
> > + * maximum number of pages has been reached. Assume that
> > + * this is the number of pages supported by the chip.
> > + */
> > + if (pmbus_check_byte_register(client, 0, PMBUS_PAGE)) {
> > + int page;
> > +
> > + for (page = 1; page < PMBUS_PAGES; page++) {
> > + if (pmbus_set_page(client, page) < 0)
> > + break;
> > + }
> > + pmbus_set_page(client, 0);
> > + info->pages = page;
> > + } else {
> > + info->pages = 1;
> > + }
> > + }
> > +
> > + /*
> > + * We should check if the COEFFICIENTS register is supported.
> > + * If it is, and the chip is configured for direct mode, we can read
> > + * the coefficients from the chip, one set per group of sensor
> > + * registers.
> > + *
> > + * To do this, we will need access to a chip which actually supports the
> > + * COEFFICIENTS command, since the command is too complex to implement
> > + * without testing it.
> > + */
> > +
> > + /* Try to find sensor groups */
> > + pmbus_find_sensor_groups(client, info);
> > +
> > + pmbus_clear_faults(client);
> > +
> > + return 0;
> > +}
> > +
> > +static void pmbus_entry_remove(struct i2c_client *client)
> > +{
> > + struct pmbus_entry *p;
> > +
> > + mutex_lock(&pmbus_list_mutex);
> > + list_for_each_entry(p, &pmbus_list, list) {
> > + if (p->client == client) {
> > + list_del(&p->list);
> > + kfree(p);
> > + break;
> > + }
> > + }
> > + mutex_unlock(&pmbus_list_mutex);
> > +}
> > +
> > +static int pmbus_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct pmbus_entry *pe;
> > + int ret;
> > +
> > + pe = kzalloc(sizeof(struct pmbus_entry), GFP_KERNEL);
> > + if (!pe)
> > + return -ENOMEM;
> > +
> > + pe->client = client;
> > + mutex_lock(&pmbus_list_mutex);
> > + list_add_tail(&pe->list, &pmbus_list);
> > + mutex_unlock(&pmbus_list_mutex);
> I'm a little unclear why the list is needed. Looks like it is used
> for cleanup purposes. i2c_set_clientdata gets called with a struct pmbus_data *data
Correct.
> in pmbus_do_probe. That structure has a pointer to struct pmbus_driver_info info
> currently effectively stored in this list. Hence, if we don't have the list
> I think we can create a pmbus_driver_info struct directly here and get to it via
> ((struct pmbus_data *)(i2c_get_clientdata(client))->info to free the structure
> on remove.
>
You are right, though that would require context information which is currently
not exported from pmbus_core.c. Specifically, pmbus.c would need to know about
struct pmbus_data which is private to pmbus_core.c. I would prefer to keep it that way.
Maybe I could add an API call into pmbus_core.c to retrieve a pointer to pmbus_driver_info.
Do you think that would make sense ? I am a bit at odds with myself if the list or an
API function (or something else ?) would be better.
> > +
> > + pe->info.pages = id->driver_data;
> > + pe->info.identify = pmbus_identify;
> > +
> > + ret = pmbus_do_probe(client, id, &pe->info);
> > + if (ret < 0)
> > + goto out;
> > + return 0;
> > +
> > +out:
> > + pmbus_entry_remove(client);
> > + return ret;
> > +}
> > +
> > +static int pmbus_remove(struct i2c_client *client)
> > +{
> > + int ret;
> > +
> > + ret = pmbus_do_remove(client);
> > + pmbus_entry_remove(client);
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * Use driver_data to set the number of pages supported by the chip.
> > + */
> > +static const struct i2c_device_id pmbus_id[] = {
> > + {"bmr450", 1},
> > + {"bmr451", 1},
> > + {"bmr453", 1},
> > + {"bmr454", 1},
> > + {"ltc2978", 8},
> > + {"pmbus", 0},
> > + {}
> > +};
> > +
> > +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("Generic 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..2dc4e01
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus.h
> > @@ -0,0 +1,307 @@
> > +/*
> > + * pmbus.h - Common defines and structures for PMBus devices
> > + *
> > + * Copyright (c) 2010, 2011 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_FAN_CONFIG_12 0x3A
> > +#define PMBUS_FAN_COMMAND_1 0x3B
> > +#define PMBUS_FAN_COMMAND_2 0x3C
> > +#define PMBUS_FAN_CONFIG_34 0x3D
> > +#define PMBUS_FAN_COMMAND_3 0x3E
> > +#define PMBUS_FAN_COMMAND_4 0x3F
> > +
> > +#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_FAN_12 0x81
> > +#define PMBUS_STATUS_FAN_34 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
> > +
> > +/*
> > + * 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
> > +
> > +/*
> > + * Fan configuration
> > + */
> > +#define PB_FAN_2_PULSE_MASK ((1 << 0) | (1 << 1))
> > +#define PB_FAN_2_RPM (1 << 2)
> > +#define PB_FAN_2_INSTALLED (1 << 3)
> > +#define PB_FAN_1_PULSE_MASK ((1 << 4) | (1 << 5))
> > +#define PB_FAN_1_RPM (1 << 6)
> > +#define PB_FAN_1_INSTALLED (1 << 7)
> > +
> > +/*
> > + * 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)
> > +
> > +/*
> > + * STATUS_FAN
> > + */
> > +#define PB_FAN_AIRFLOW_WARNING (1<<0)
> > +#define PB_FAN_AIRFLOW_FAULT (1<<1)
> > +#define PB_FAN_FAN2_SPEED_OVERRIDE (1<<2)
> > +#define PB_FAN_FAN1_SPEED_OVERRIDE (1<<3)
> > +#define PB_FAN_FAN2_WARNING (1<<4)
> > +#define PB_FAN_FAN1_WARNING (1<<5)
> > +#define PB_FAN_FAN2_FAULT (1<<6)
> > +#define PB_FAN_FAN1_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)
> > +
> > +enum pmbus_sensor_classes {
> > + PSC_VOLTAGE_IN = 0,
> > + PSC_VOLTAGE_OUT,
> > + PSC_CURRENT_IN,
> > + PSC_CURRENT_OUT,
> > + PSC_POWER,
> > + PSC_TEMPERATURE,
> > + PSC_FAN,
> > + PSC_NUM_CLASSES /* Number of power sensor classes */
> > +};
> > +
> > +#define PMBUS_PAGES 32 /* Per PMBus specification */
> > +
> > +/* Functionality bit mask */
> > +#define PMBUS_HAVE_VIN (1 << 0)
> > +#define PMBUS_HAVE_VCAP (1 << 1)
> > +#define PMBUS_HAVE_VOUT (1 << 2)
> > +#define PMBUS_HAVE_IIN (1 << 3)
> > +#define PMBUS_HAVE_IOUT (1 << 4)
> > +#define PMBUS_HAVE_PIN (1 << 5)
> > +#define PMBUS_HAVE_POUT (1 << 6)
> > +#define PMBUS_HAVE_FAN12 (1 << 7)
> > +#define PMBUS_HAVE_FAN34 (1 << 8)
> > +#define PMBUS_HAVE_TEMP (1 << 9)
> > +#define PMBUS_HAVE_TEMP2 (1 << 10)
> > +#define PMBUS_HAVE_TEMP3 (1 << 11)
> > +#define PMBUS_HAVE_STATUS_VOUT (1 << 12)
> > +#define PMBUS_HAVE_STATUS_IOUT (1 << 13)
> > +#define PMBUS_HAVE_STATUS_INPUT (1 << 14)
> > +#define PMBUS_HAVE_STATUS_TEMP (1 << 15)
> > +#define PMBUS_HAVE_STATUS_FAN12 (1 << 16)
> > +#define PMBUS_HAVE_STATUS_FAN34 (1 << 17)
> > +
> > +struct pmbus_driver_info {
> > + int pages; /* Total number of pages */
> > + bool direct[PSC_NUM_CLASSES];
> > + /* true if device uses direct data format
> > + for the given sensor class */
> > + /*
> > + * Support one set of coefficients for each sensor type
> > + * Used for chips providing data in direct mode.
> > + */
> > + int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
> > + int b[PSC_NUM_CLASSES]; /* offset */
> > + int R[PSC_NUM_CLASSES]; /* exponent */
> > +
> > + u32 func[PMBUS_PAGES]; /* Functionality, per page */
> > + /*
> > + * The get_status function maps manufacturing specific status values
> > + * into PMBus standard status values.
> > + */
> > + int (*get_status)(struct i2c_client *client, int page, int reg);
> > + /*
> > + * The identify function determines supported PMBus functionality.
> > + */
> > + int (*identify)(struct i2c_client *client,
> > + struct pmbus_driver_info *info);
> > +};
> > +
> > +/* Function declarations */
> > +
> > +int pmbus_set_page(struct i2c_client *client, u8 page);
> > +int pmbus_read_word_data(struct i2c_client *client, u8 page, u8 reg);
> > +int pmbus_read_byte_data(struct i2c_client *client, u8 page, u8 reg);
> I suppose this makes sense for completeness. But I don't think you currently
> use read_byte_data in any of the drivers so it needn't be here..
>
Yes, it is just for completeness.
> > +void pmbus_clear_faults(struct i2c_client *client);
> > +bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
> > +bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
> > +int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
> > + struct pmbus_driver_info *info);
> > +int pmbus_do_remove(struct i2c_client *client);
> > +
> > +#endif /* PB_H */
> > diff --git a/drivers/hwmon/pmbus_core.c b/drivers/hwmon/pmbus_core.c
> > new file mode 100644
> > index 0000000..fe18722
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus_core.c
> > @@ -0,0 +1,1611 @@
> > +/*
> > + * Hardware monitoring driver for PMBus devices
> > + *
> > + * Copyright (c) 2010, 2011 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 <linux/i2c/pmbus.h>
> > +#include "pmbus.h"
> > +
> > +/*
> > + * Constants needed to determine number of sensors, booleans, and labels.
> > + */
> > +#define PMBUS_MAX_INPUT_SENSORS 11 /* 6*volt, 3*curr, 2*power */
> > +#define PMBUS_VOUT_SENSORS_PER_PAGE 5 /* input, min, max, lcrit,
> > + crit */
> > +#define PMBUS_IOUT_SENSORS_PER_PAGE 4 /* input, min, max, crit */
> > +#define PMBUS_POUT_SENSORS_PER_PAGE 4 /* input, cap, max, crit */
> > +#define PMBUS_MAX_SENSORS_PER_FAN 1 /* input */
> > +#define PMBUS_MAX_SENSORS_PER_TEMP 5 /* input, min, max, lcrit,
> > + crit */
> > +
> > +#define PMBUS_MAX_INPUT_BOOLEANS 7 /* v: min_alarm, max_alarm,
> > + lcrit_alarm, crit_alarm;
> > + c: alarm, crit_alarm;
> > + p: crit_alarm */
> > +#define PMBUS_VOUT_BOOLEANS_PER_PAGE 4 /* min_alarm, max_alarm,
> > + lcrit_alarm, crit_alarm */
> > +#define PMBUS_IOUT_BOOLEANS_PER_PAGE 3 /* alarm, lcrit_alarm,
> > + crit_alarm */
> > +#define PMBUS_POUT_BOOLEANS_PER_PAGE 2 /* alarm, crit_alarm */
> > +#define PMBUS_MAX_BOOLEANS_PER_FAN 2 /* alarm, fault */
> > +#define PMBUS_MAX_BOOLEANS_PER_TEMP 4 /* min_alarm, max_alarm,
> > + lcrit_alarm, crit_alarm */
> > +
> > +#define PMBUS_MAX_INPUT_LABELS 4 /* vin, vcap, iin, pin */
> > +
> > +/*
> > + * status, status_vout, status_iout, status_fans, and status_temp
> > + * are paged. status_input and status_fan34 are unpaged.
> > + * status_fan34 is a special case to handle a second set of fans
> > + * on page 0.
> > + */
> > +#define PB_NUM_STATUS_REG (PMBUS_PAGES * 5 + 2)
> > +
> > +/*
> > + * Index into status register array, per status register group
> > + */
> > +#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_FAN_BASE (PB_STATUS_IOUT_BASE + PMBUS_PAGES)
> > +#define PB_STATUS_FAN34_BASE (PB_STATUS_FAN_BASE + PMBUS_PAGES)
> > +#define PB_STATUS_INPUT_BASE (PB_STATUS_FAN34_BASE + 1)
> > +#define PB_STATUS_TEMP_BASE (PB_STATUS_INPUT_BASE + 1)
> > +
> > +struct pmbus_sensor {
> > + char name[I2C_NAME_SIZE]; /* sysfs sensor name */
> > + struct sensor_device_attribute attribute;
> > + u8 page; /* page number */
> > + u8 reg; /* register */
> > + enum pmbus_sensor_classes class; /* sensor class */
> > + bool update; /* runtime sensor update needed */
> > + int data; /* Sensor data.
> > + Negative if there was a read error */
> > +};
> > +
> > +struct pmbus_boolean {
> > + char name[I2C_NAME_SIZE]; /* sysfs boolean name */
> > + struct sensor_device_attribute attribute;
> > +};
> > +
> > +struct pmbus_label {
> > + char name[I2C_NAME_SIZE]; /* sysfs label name */
> > + struct sensor_device_attribute attribute;
> > + char label[I2C_NAME_SIZE]; /* label */
> > +};
> > +
> > +struct pmbus_data {
> > + struct device *hwmon_dev;
> > +
> > + u32 flags; /* from platform data */
> > +
> > + int exponent; /* linear mode: exponent for output voltages */
> > +
> > + const struct pmbus_driver_info *info;
> > +
> > + int max_attributes;
> > + int num_attributes;
> > + struct attribute **attributes;
> > + struct attribute_group group;
> > +
> > + /*
> > + * Sensors cover both sensor and limit registers.
> > + */
> > + int max_sensors;
> > + int num_sensors;
> > + struct pmbus_sensor *sensors;
> > + /*
> > + * Booleans are used for alarms.
> > + * Values are determined from status registers.
> > + */
> > + int max_booleans;
> > + int num_booleans;
> > + struct pmbus_boolean *booleans;
> > + /*
> > + * Labels are used to map generic names (e.g., "in1")
> > + * to PMBus specific names (e.g., "vin" or "vout1").
> > + */
> > + int max_labels;
> > + int num_labels;
> > + struct pmbus_label *labels;
> > +
> > + struct mutex update_lock;
> > + bool valid;
> > + unsigned long last_updated; /* in jiffies */
> > +
> > + /*
> > + * A single status register covers multiple attributes,
> > + * so we keep them all together.
> > + */
> > + u8 status_bits;
> > + u8 status[PB_NUM_STATUS_REG];
> > +
> > + u8 currpage;
> > +};
> > +
> > +int pmbus_set_page(struct i2c_client *client, u8 page)
> > +{
> > + struct pmbus_data *data = i2c_get_clientdata(client);
> > + int rv = 0;
> > + int newpage;
> > +
> > + if (page != data->currpage) {
> > + rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> > + newpage = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> > + if (newpage != page)
> > + rv = -EINVAL;
> > + else
> > + data->currpage = page;
> > + }
> > + return rv;
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_set_page);
> > +
> > +static int pmbus_write_byte(struct i2c_client *client, u8 page, u8 value)
> > +{
> > + int rv;
> > +
> > + rv = pmbus_set_page(client, page);
> > + if (rv < 0)
> > + return rv;
> > +
> > + return i2c_smbus_write_byte(client, value);
> > +}
> > +
> > +static int pmbus_write_word_data(struct i2c_client *client, u8 page, u8 reg,
> > + u16 word)
> > +{
> > + int rv;
> > +
> > + rv = pmbus_set_page(client, page);
> > + if (rv < 0)
> > + return rv;
> > +
> > + return i2c_smbus_write_word_data(client, reg, word);
> > +}
> > +
> > +int pmbus_read_word_data(struct i2c_client *client, u8 page, u8 reg)
> > +{
> > + int rv;
> > +
> > + rv = pmbus_set_page(client, page);
> > + if (rv < 0)
> > + return rv;
> > +
> > + return i2c_smbus_read_word_data(client, reg);
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_read_word_data);
> > +
> > +int pmbus_read_byte_data(struct i2c_client *client, u8 page, u8 reg)
> > +{
> > + int rv;
> > +
> > + rv = pmbus_set_page(client, page);
> > + if (rv < 0)
> > + return rv;
> > +
> > + return i2c_smbus_read_byte_data(client, reg);
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_read_byte_data);
> > +
> > +void pmbus_clear_faults(struct i2c_client *client)
> > +{
> > + struct pmbus_data *data = i2c_get_clientdata(client);
> > + int i;
> > +
> > + for (i = 0; i < data->info->pages; i++)
> > + pmbus_write_byte(client, i, PMBUS_CLEAR_FAULTS);
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_clear_faults);
> > +
> > +bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg)
> > +{
> > + int rv, status;
> > + struct pmbus_data *data = i2c_get_clientdata(client);
> > +
> > + pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
> > + rv = pmbus_read_byte_data(client, page, reg);
> > + if (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK)) {
> > + status = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE);
> > + if (status < 0 || (status & PB_STATUS_CML)) {
> > + int status2 = pmbus_read_byte_data(client, page,
> > + PMBUS_STATUS_CML);
> > + if (status2 < 0
> > + || (status2 & PB_CML_FAULT_INVALID_COMMAND)) {
> > + dev_dbg(&client->dev,
> > + "page %d reg 0x%x status 0x%x-0x%x\n",
> > + page, reg, status, status2);
> > + rv = -EINVAL;
> > + }
> > + }
> > + }
> > + pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
> > + return rv >= 0;
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_check_byte_register);
> > +
> > +bool pmbus_check_word_register(struct i2c_client *client, int page, int reg)
> > +{
> > + int rv, status;
> > + struct pmbus_data *data = i2c_get_clientdata(client);
> > +
> > + pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
> If there have been faults, do we not want to have handled them?
> That is, should there be any circumstance under which we would want
> to clear one here?x
That is one place where it is really needed. Status codes are sticky, so if
"PB_CML_FAULT_INVALID_COMMAND" was set for whatever reason, it would be reported
below even if the following access is ok.
> > + rv = pmbus_read_word_data(client, page, reg);
>
> I think this is idenical to the check code in the previous function.
> Worth pulling out to a utility function used by both?
Yes, excellent idea.
> > + if (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK)) {
> > + status = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE);
> > + if (status < 0 || (status & PB_STATUS_CML)) {
> > + int status2 = pmbus_read_byte_data(client, page,
> > + PMBUS_STATUS_CML);
> > + if (status2 < 0
> > + || (status2 & PB_CML_FAULT_INVALID_COMMAND)) {
> > + dev_dbg(&client->dev,
> > + "page %d reg 0x%x status 0x%x-0x%x\n",
> > + page, reg, status, status2);
> > + rv = -EINVAL;
> > + }
> > + }
> > + }
> > + pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
This one isn't strictly needed, though, so I removed it.
> > + return rv >= 0;
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_check_word_register);
> > +
> > +static int pmbus_get_status(struct i2c_client *client, int page, int reg)
> > +{
> > + struct pmbus_data *data = i2c_get_clientdata(client);
> > + const struct pmbus_driver_info *info = data->info;
> > + int status;
> > +
> > + if (info->get_status) {
> > + status = info->get_status(client, page, reg);
> > + if (status != -ENODATA)
> > + return status;
> > + }
> > + return pmbus_read_byte_data(client, page, reg);
> > +}
> > +
> > +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);
> > + const struct pmbus_driver_info *info = data->info;
> > +
> > + mutex_lock(&data->update_lock);
> > + if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
> > + int i;
> > +
> > + for (i = 0; i < info->pages; i++)
> > + data->status[PB_STATUS_BASE + i]
> > + = pmbus_read_byte_data(client, i,
> > + PMBUS_STATUS_BYTE);
> > + for (i = 0; i < info->pages; i++) {
> > + if (!(info->func[i] & PMBUS_HAVE_STATUS_VOUT))
> > + continue;
> > + data->status[PB_STATUS_VOUT_BASE + i]
> > + = pmbus_get_status(client, i, PMBUS_STATUS_VOUT);
> > + }
> > + for (i = 0; i < info->pages; i++) {
> > + if (!(info->func[i] & PMBUS_HAVE_STATUS_IOUT))
> > + continue;
> > + data->status[PB_STATUS_IOUT_BASE + i]
> > + = pmbus_get_status(client, i, PMBUS_STATUS_IOUT);
> > + }
> > + for (i = 0; i < info->pages; i++) {
> > + if (!(info->func[i] & PMBUS_HAVE_STATUS_TEMP))
> > + continue;
> > + data->status[PB_STATUS_TEMP_BASE + i]
> > + = pmbus_get_status(client, i,
> > + PMBUS_STATUS_TEMPERATURE);
> > + }
> > + for (i = 0; i < info->pages; i++) {
> > + if (!(info->func[i] & PMBUS_HAVE_STATUS_FAN12))
> > + continue;
> > + data->status[PB_STATUS_FAN_BASE + i]
> > + = pmbus_get_status(client, i, PMBUS_STATUS_FAN_12);
> > + }
> > +
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT)
> > + data->status[PB_STATUS_INPUT_BASE]
> > + = pmbus_get_status(client, 0, PMBUS_STATUS_INPUT);
> > +
> > + if (info->func[0] & PMBUS_HAVE_STATUS_FAN34)
> > + data->status[PB_STATUS_FAN34_BASE]
> > + = pmbus_get_status(client, 0, PMBUS_STATUS_FAN_34);
> > +
> > + for (i = 0; i < data->num_sensors; i++) {
> > + struct pmbus_sensor *sensor = &data->sensors[i];
> > +
> > + if (!data->valid || sensor->update)
> > + sensor->data
> > + = pmbus_read_word_data(client, sensor->page,
> > + sensor->reg);
> > + }
> > + pmbus_clear_faults(client);
> > + data->last_updated = jiffies;
> > + data->valid = 1;
> > + }
> > + mutex_unlock(&data->update_lock);
> > + return data;
> > +}
> > +
> > +/*
> > + * Convert linear sensor values to milli- or micro-units
> > + * depending on sensor type.
> > + */
> > +static int pmbus_reg2data_linear(struct pmbus_data *data,
> > + struct pmbus_sensor *sensor)
> > +{
> > + s16 exponent, mantissa;
> > + long val;
> > +
> > + if (sensor->class == PSC_VOLTAGE_OUT) {
> > + exponent = data->exponent;
> > + mantissa = (s16) sensor->data;
> > + } else {
> > + exponent = (sensor->data >> 11) & 0x001f;
> > + mantissa = sensor->data & 0x07ff;
> > +
> > + if (exponent > 0x0f)
> > + exponent |= 0xffe0; /* sign extend exponent */
> > + if (mantissa > 0x03ff)
> > + mantissa |= 0xf800; /* sign extend mantissa */
> > + }
> > +
> > + val = mantissa;
> > +
> > + /* scale result to milli-units for all sensors except fans */
> > + if (sensor->class != PSC_FAN)
> > + val = val * 1000L;
> > +
> > + /* scale result to micro-units for power sensors */
> > + if (sensor->class == PSC_POWER)
> > + val = val * 1000L;
> > +
> > + if (exponent >= 0)
> > + val <<= exponent;
> > + else
> > + val >>= -exponent;
> > +
> > + return (int)val;
> > +}
> > +
> > +/*
> > + * Convert direct sensor values to milli- or micro-units
> > + * depending on sensor type.
> > + */
> > +static int pmbus_reg2data_direct(struct pmbus_data *data,
> > + struct pmbus_sensor *sensor)
> > +{
> > + long val = (s16) sensor->data;
> > + long m, b, R;
> > +
> > + m = data->info->m[sensor->class];
> > + b = data->info->b[sensor->class];
> > + R = data->info->R[sensor->class];
> > +
> > + if (m == 0)
> > + return 0;
> > +
> > + /* X = 1/m * (Y * 10^-R - b) */
> > + R = -R;
> > + /* scale result to milli-units for everything but fans */
> > + if (sensor->class != PSC_FAN) {
> > + R += 3;
> > + b *= 1000;
> > + }
> > +
> > + /* scale result to micro-units for power sensors */
> > + if (sensor->class == PSC_POWER) {
> > + R += 3;
> > + b *= 1000;
> > + }
> > +
> > + while (R > 0) {
> > + val *= 10;
> > + R--;
> > + }
> > + while (R < 0) {
> > + val = DIV_ROUND_CLOSEST(val, 10);
> > + R++;
> > + }
> > +
> > + return (int)((val - b) / m);
> > +}
> > +
> > +static int pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
> > +{
> > + int val;
> > +
> > + if (data->info->direct[sensor->class])
> > + val = pmbus_reg2data_direct(data, sensor);
> > + else
> > + val = pmbus_reg2data_linear(data, sensor);
> > +
> > + 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;
> > + }
> > +
> > + if (class == PSC_VOLTAGE_OUT) {
> > + /*
> > + * For a static exponents, we don't have a choice
> > + * but to adjust the value to it.
> > + */
> > + if (data->exponent < 0)
> > + val <<= -data->exponent;
> > + else
> > + val >>= data->exponent;
> > + val = DIV_ROUND_CLOSEST(val, 1000);
> > + if (val > 0x7fff)
> > + val = 0x7fff;
> > + return negative ? -val : val;
> > + }
> > +
> > + /* Power is in uW. Convert to mW before converting. */
> > + if (class == PSC_POWER)
> > + val = DIV_ROUND_CLOSEST(val, 1000L);
> > +
> > + /*
> > + * For simplicity, convert fan data to milli-units
> > + * before calculating the exponent.
> > + */
> > + if (class == PSC_FAN)
> > + val = val * 1000;
> > +
> > + /* 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->info->m[class];
> > + b = data->info->b[class];
> > + R = data->info->R[class];
> > +
> > + /* Power is in uW. Adjust R and b. */
> > + if (class == PSC_POWER) {
> > + R -= 3;
> > + b *= 1000;
> > + }
> > +
> > + /* Calculate Y = (m * X + b) * 10^R */
> > + if (class != PSC_FAN) {
> > + R -= 3; /* Adjust R and b for data in milli-units */
> > + b *= 1000;
> > + }
> > + val = val * m + b;
> > +
> > + 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 regval;
> > +
> > + if (data->info->direct[class])
> > + regval = pmbus_data2reg_direct(data, class, val);
> > + else
> > + regval = pmbus_data2reg_linear(data, class, val);
> > +
> > + return regval;
> > +}
> > +
> > +/*
> > + * 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 int pmbus_get_boolean(struct pmbus_data *data, int index, int *val)
> > +{
> > + u8 s1 = (index >> 24) & 0xff;
> > + u8 s2 = (index >> 16) & 0xff;
> > + u8 reg = (index >> 8) & 0xff;
> > + u8 mask = index & 0xff;
> > + int status;
> > + u8 regval;
> > +
> > + status = data->status[reg];
> > + if (status < 0)
> > + return status;
> > +
> > + regval = status & mask;
>
> This test could do with a simple explanatory comment (which is another
> way of saying I'm not quite clear why this makes sense!)
You mean the following test ? If both s1 and s2 are 0, the function
returns the result of (status & mask). This is for all booleans
which are created with pmbus_add_boolean_reg().
Otherwise, the result is determined by comparison as described above.
I'll add additional comments to describe this in more detail.
> > + if (!s1 && !s2)
> > + *val = !!regval;
> > + else {
> > + int v1, v2;
> > + struct pmbus_sensor *sensor1, *sensor2;
> > +
> > + sensor1 = &data->sensors[s1];
> > + if (sensor1->data < 0)
> > + return sensor1->data;
> > + sensor2 = &data->sensors[s2];
> > + if (sensor2->data < 0)
> > + return sensor2->data;
> > +
> > + v1 = pmbus_reg2data(data, sensor1);
> > + v2 = pmbus_reg2data(data, sensor2);
> > + *val = !!(regval && v1 >= v2);
> > + }
> > + return 0;
> > +}
> > +
> > +static ssize_t pmbus_show_boolean(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);
> > + int val;
> > + int err;
> > +
> > + err = pmbus_get_boolean(data, attr->index, &val);
> > + if (err)
> > + return err;
> > + return snprintf(buf, PAGE_SIZE, "%d\n", val);
> > +}
> > +
> > +static ssize_t pmbus_show_sensor(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);
> > + struct pmbus_sensor *sensor;
> > +
> > + sensor = &data->sensors[attr->index];
> > + if (sensor->data < 0)
> > + return sensor->data;
> > +
> > + return snprintf(buf, PAGE_SIZE, "%d\n", pmbus_reg2data(data, sensor));
> > +}
> > +
> > +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 regval;
> > +
> > + if (strict_strtol(buf, 10, &val) < 0)
> > + return -EINVAL;
> > +
> > + mutex_lock(&data->update_lock);
> > + regval = pmbus_data2reg(data, sensor->class, val);
> > + ret = pmbus_write_word_data(client, sensor->page, sensor->reg, regval);
> > + if (ret < 0)
> > + rv = ret;
> > + else
> > + data->sensors[attr->index].data = regval;
> > + 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, _type, _show, _set) \
> > +do { \
> > + struct sensor_device_attribute *a \
> > + = &data->_type##s[data->num_##_type##s].attribute; \
> > + BUG_ON(data->num_attributes >= data->max_attributes); \
> > + 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, _type, \
> > + pmbus_show_##_type, NULL)
> > +
> > +#define PMBUS_ADD_SET_ATTR(data, _name, _type, _idx) \
> > + PMBUS_ADD_ATTR(data, _name, _idx, S_IWUSR | S_IRUGO, _type, \
> > + pmbus_show_##_type, pmbus_set_##_type)
> > +
> > +static void pmbus_add_boolean(struct pmbus_data *data,
> > + const char *name, const char *type, int seq,
> Is val the best name for the next variable? It isn't obvious to me what
> value this is... Gets pushed into an index by the PMBUS_ADD_GET_ATTR macro...
> Perhaps keep the idx naming?
Not sure myself. val is composed of up to four individual parameters.
I'll name it idx.
> > + int val)
> > +{
> > + struct pmbus_boolean *boolean;
> > +
> > + BUG_ON(data->num_booleans >= data->max_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 >= data->max_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 >= data->max_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++;
> > +}
> > +
> const?
yes
> > +static int pmbus_temp_registers[] = {
> > + PMBUS_READ_TEMPERATURE_1,
> > + PMBUS_READ_TEMPERATURE_2,
> > + PMBUS_READ_TEMPERATURE_3
> > +};
> > +
> > +static int pmbus_fan_registers[] = {
> > + PMBUS_READ_FAN_SPEED_1,
> > + PMBUS_READ_FAN_SPEED_2,
> > + PMBUS_READ_FAN_SPEED_3,
> > + PMBUS_READ_FAN_SPEED_4
> > +};
> > +
> > +static int pmbus_fan_config_registers[] = {
> > + PMBUS_FAN_CONFIG_12,
> > + PMBUS_FAN_CONFIG_12,
> > + PMBUS_FAN_CONFIG_34,
> > + PMBUS_FAN_CONFIG_34
> > +};
> > +
> > +static int pmbus_fan_status_registers[] = {
> > + PMBUS_STATUS_FAN_12,
> > + PMBUS_STATUS_FAN_12,
> > + PMBUS_STATUS_FAN_34,
> > + PMBUS_STATUS_FAN_34
> > +};
> > +
> > +/*
> > + * Determine maximum number of sensors, booleans, and labels.
> > + * To keep things simple, only make a rough high estimate.
> > + */
> > +static void pmbus_find_max_attr(struct i2c_client *client,
> > + struct pmbus_data *data)
> > +{
> > + const struct pmbus_driver_info *info = data->info;
> > + int page, max_sensors, max_booleans, max_labels;
> > +
> > + max_sensors = PMBUS_MAX_INPUT_SENSORS;
> > + max_booleans = PMBUS_MAX_INPUT_BOOLEANS;
> > + max_labels = PMBUS_MAX_INPUT_LABELS;
> > +
> > + for (page = 0; page < info->pages; page++) {
> > + if (info->func[page] & PMBUS_HAVE_VOUT) {
> > + max_sensors += PMBUS_VOUT_SENSORS_PER_PAGE;
> > + max_booleans += PMBUS_VOUT_BOOLEANS_PER_PAGE;
> > + max_labels++;
> > + }
> > + if (info->func[page] & PMBUS_HAVE_IOUT) {
> > + max_sensors += PMBUS_IOUT_SENSORS_PER_PAGE;
> > + max_booleans += PMBUS_IOUT_BOOLEANS_PER_PAGE;
> > + max_labels++;
> > + }
> > + if (info->func[page] & PMBUS_HAVE_POUT) {
> > + max_sensors += PMBUS_POUT_SENSORS_PER_PAGE;
> > + max_booleans += PMBUS_POUT_BOOLEANS_PER_PAGE;
> > + max_labels++;
> > + }
> > + if (info->func[page] & PMBUS_HAVE_FAN12) {
> > + if (page == 0) {
> > + max_sensors +=
> > + ARRAY_SIZE(pmbus_fan_registers) *
> > + PMBUS_MAX_SENSORS_PER_FAN;
> > + max_booleans +=
> > + ARRAY_SIZE(pmbus_fan_registers) *
> > + PMBUS_MAX_BOOLEANS_PER_FAN;
> > + } else {
> > + max_sensors += PMBUS_MAX_SENSORS_PER_FAN;
> > + max_booleans += PMBUS_MAX_BOOLEANS_PER_FAN;
> > + }
> > + }
> > + if (info->func[page] & PMBUS_HAVE_TEMP) {
> > + if (page == 0) {
> > + max_sensors +=
> > + ARRAY_SIZE(pmbus_temp_registers) *
> > + PMBUS_MAX_SENSORS_PER_TEMP;
> > + max_booleans +=
> > + ARRAY_SIZE(pmbus_temp_registers) *
> > + PMBUS_MAX_BOOLEANS_PER_TEMP;
> > + } else {
> > + max_sensors += PMBUS_MAX_SENSORS_PER_TEMP;
> > + max_booleans += PMBUS_MAX_BOOLEANS_PER_TEMP;
> > + }
> > + }
> > + }
> > + data->max_sensors = max_sensors;
> > + data->max_booleans = max_booleans;
> > + data->max_labels = max_labels;
> > + data->max_attributes = max_sensors + max_booleans + max_labels;
> > +}
> > +
> > +/*
> > + * Search for attributes. Allocate sensors, booleans, and labels as needed.
> > + */
> > +static void pmbus_find_attributes(struct i2c_client *client,
> > + struct pmbus_data *data)
> > +{
> > + const struct pmbus_driver_info *info = data->info;
> > + int page, i0, i1, in_index;
> > +
> > + /*
> > + * Input voltage sensors
> > + */
> > + in_index = 1;
> > + if (info->func[0] & PMBUS_HAVE_VIN) {
> > + bool have_alarm = 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_IN, true);
>
> This next lot do rather look like they could be done using an inline function
> or a macro...
>
> Something like...
> #define bob(LIM, NAME, POST, WHAT, TYPE)
> if (pmbus_check_word_register(client, 0, LIM)) {
> i1 = data->num_sensors;
> pmbus_add_sensor(data, NAME, POST, in_index, 0, LIM, WHAT, false);
> if (info->func[0] & PMBUS_HAV_STATUS_INPUT) {
> pmbus_add_boolean_reg(data, NAME, POST##_alarm, in_index, PBU_STATUS_INPUT_BASE,
> TYPE);
> have_alarm = true;
> }
> }
>
> bob(PMBUS_VIN_UV_WARN_LIMIT, "in", "min", PSC_VOLTAGE_IN, PB_VOLTAGE_UV_WARNING);
> bob(PMBUS_VIN_UV_FAULT_LIMIT, "in", "lcrit", PSC_VOLTAGE_IN, PB_VOLTAGE_UV_FAULT);
> bob(PMBUS_VIN_OV_WARN_LIMIT, "in", "max", PSC_VOLTAGE_IN, PB_VOLTAGE_OV_WARNING);
> etc.
>
> basically I'm observing a lot of shared code in these... Still maybe this
> just acts to hide what is going on.
I think I would prefer to keep it as-is. I am not really a friend of function macros
anymore.
It would be different if I can extract some of the code into functions, but right now
I don't see how I could do that.
> > + 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_IN, false);
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT) {
> > + pmbus_add_boolean_reg(data, "in", "min_alarm",
> > + in_index,
> > + PB_STATUS_INPUT_BASE,
> > + PB_VOLTAGE_UV_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + 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_IN, false);
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT) {
> > + pmbus_add_boolean_reg(data, "in", "lcrit_alarm",
> > + in_index,
> > + PB_STATUS_INPUT_BASE,
> > + PB_VOLTAGE_UV_FAULT);
> > + have_alarm = 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_IN, false);
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT) {
> > + pmbus_add_boolean_reg(data, "in", "max_alarm",
> > + in_index,
> > + PB_STATUS_INPUT_BASE,
> > + PB_VOLTAGE_OV_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + 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_IN, false);
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT) {
> > + pmbus_add_boolean_reg(data, "in", "crit_alarm",
> > + in_index,
> > + PB_STATUS_INPUT_BASE,
> > + PB_VOLTAGE_OV_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + /*
> > + * Add generic alarm attribute only if there are no individual
> > + * attributes.
> > + */
> > + if (!have_alarm)
> > + pmbus_add_boolean_reg(data, "in", "alarm",
> > + in_index,
> > + PB_STATUS_BASE,
> > + PB_STATUS_VIN_UV);
> > + in_index++;
> > + }
> > + if (info->func[0] & PMBUS_HAVE_VCAP) {
> > + pmbus_add_label(data, "in", in_index, "vcap", 0);
> > + pmbus_add_sensor(data, "in", "input", in_index, 0,
> > + PMBUS_READ_VCAP, PSC_VOLTAGE_IN, true);
> > + in_index++;
> > + }
> > +
> > + /*
> > + * Output voltage sensors
> > + */
> > + for (page = 0; page < info->pages; page++) {
> > + bool have_alarm = false;
> > +
> > + if (!(info->func[page] & PMBUS_HAVE_VOUT))
> > + continue;
> > +
> > + i0 = data->num_sensors;
> > + pmbus_add_label(data, "in", in_index, "vout", page + 1);
> > + pmbus_add_sensor(data, "in", "input", in_index, page,
> > + PMBUS_READ_VOUT, PSC_VOLTAGE_OUT, true);
> Looks like that macro needs a few minor additions and you can use it here as well.
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_VOUT_UV_WARN_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "in", "min", in_index, page,
> > + PMBUS_VOUT_UV_WARN_LIMIT,
> > + PSC_VOLTAGE_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_VOUT) {
> > + pmbus_add_boolean_reg(data, "in", "min_alarm",
> > + in_index,
> > + PB_STATUS_VOUT_BASE +
> > + page,
> > + PB_VOLTAGE_UV_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_VOUT_UV_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "in", "lcrit", in_index, page,
> > + PMBUS_VOUT_UV_FAULT_LIMIT,
> > + PSC_VOLTAGE_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_VOUT) {
> > + pmbus_add_boolean_reg(data, "in", "lcrit_alarm",
> > + in_index,
> > + PB_STATUS_VOUT_BASE +
> > + page,
> > + PB_VOLTAGE_UV_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_VOUT_OV_WARN_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "in", "max", in_index, page,
> > + PMBUS_VOUT_OV_WARN_LIMIT,
> > + PSC_VOLTAGE_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_VOUT) {
> > + pmbus_add_boolean_reg(data, "in", "max_alarm",
> > + in_index,
> > + PB_STATUS_VOUT_BASE +
> > + page,
> > + PB_VOLTAGE_OV_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_VOUT_OV_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "in", "crit", in_index, page,
> > + PMBUS_VOUT_OV_FAULT_LIMIT,
> > + PSC_VOLTAGE_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_VOUT) {
> > + pmbus_add_boolean_reg(data, "in", "crit_alarm",
> > + in_index,
> > + PB_STATUS_VOUT_BASE +
> > + page,
> > + PB_VOLTAGE_OV_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + /*
> > + * Add generic alarm attribute only if there are no individual
> > + * attributes.
> > + */
> > + if (!have_alarm)
> > + pmbus_add_boolean_reg(data, "in", "alarm",
> > + in_index,
> > + PB_STATUS_BASE + page,
> > + PB_STATUS_VOUT_OV);
> > + in_index++;
> > + }
> > +
> > + /*
> > + * Current sensors
> > + */
> > +
> > + /*
> > + * Input current sensors
> > + */
> > + in_index = 1;
> > + if (info->func[0] & PMBUS_HAVE_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_IN, 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_IN, false);
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT) {
> > + pmbus_add_boolean_reg(data, "curr", "max_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_IN, false);
> > + if (info->func[0] & PMBUS_HAVE_STATUS_INPUT)
> > + pmbus_add_boolean_reg(data, "curr",
> > + "crit_alarm",
> > + in_index,
> > + PB_STATUS_INPUT_BASE,
> > + PB_IIN_OC_FAULT);
> > + }
> > + in_index++;
> > + }
> > +
> > + /*
> > + * Output current sensors
> > + */
> > + for (page = 0; page < info->pages; page++) {
> > + bool have_alarm = false;
> > +
> > + if (!(info->func[page] & PMBUS_HAVE_IOUT))
> > + continue;
> > +
> > + i0 = data->num_sensors;
> > + pmbus_add_label(data, "curr", in_index, "iout", page + 1);
> > + pmbus_add_sensor(data, "curr", "input", in_index, page,
> > + PMBUS_READ_IOUT, PSC_CURRENT_OUT, true);
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_IOUT_OC_WARN_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "curr", "max", in_index, page,
> > + PMBUS_IOUT_OC_WARN_LIMIT,
> > + PSC_CURRENT_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_IOUT) {
> > + pmbus_add_boolean_reg(data, "curr", "max_alarm",
> > + in_index,
> > + PB_STATUS_IOUT_BASE +
> > + page, PB_IOUT_OC_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_IOUT_UC_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "curr", "lcrit", in_index, page,
> > + PMBUS_IOUT_UC_FAULT_LIMIT,
> > + PSC_CURRENT_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_IOUT) {
> > + pmbus_add_boolean_reg(data, "curr",
> > + "lcrit_alarm",
> > + in_index,
> > + PB_STATUS_IOUT_BASE +
> > + page, PB_IOUT_UC_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_IOUT_OC_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "curr", "crit", in_index, page,
> > + PMBUS_IOUT_OC_FAULT_LIMIT,
> > + PSC_CURRENT_OUT, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_IOUT) {
> > + pmbus_add_boolean_reg(data, "curr",
> > + "crit_alarm",
> > + in_index,
> > + PB_STATUS_IOUT_BASE +
> > + page, PB_IOUT_OC_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + /*
> > + * Add generic alarm attribute only if there are no individual
> > + * attributes.
> > + */
> > + if (!have_alarm)
> > + pmbus_add_boolean_reg(data, "curr", "alarm",
> > + in_index,
> > + PB_STATUS_BASE + page,
> > + PB_STATUS_IOUT_OC);
> > + in_index++;
> > + }
> > +
> > + /*
> > + * Power sensors
> > + */
> > + /*
> > + * Input Power sensors
> > + */
> > + in_index = 1;
> > + if (info->func[0] & PMBUS_HAVE_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 (info->func[0] & PMBUS_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 (page = 0; page < info->pages; page++) {
> > + bool need_alarm = false;
> > +
> > + if (!(info->func[page] & PMBUS_HAVE_POUT))
> > + continue;
> > +
> > + i0 = data->num_sensors;
> > + pmbus_add_label(data, "power", in_index, "pout", page + 1);
> > + pmbus_add_sensor(data, "power", "input", in_index, page,
> > + 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, page, PMBUS_POUT_MAX)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "power", "cap", in_index, page,
> > + PMBUS_POUT_MAX, PSC_POWER, false);
> > + need_alarm = true;
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_POUT_OP_WARN_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "power", "max", in_index, page,
> > + PMBUS_POUT_OP_WARN_LIMIT, PSC_POWER,
> > + false);
> > + need_alarm = true;
> > + }
> > + if (need_alarm && (info->func[page] & PMBUS_HAVE_STATUS_IOUT))
> > + pmbus_add_boolean_reg(data, "power", "alarm",
> > + in_index,
> > + PB_STATUS_IOUT_BASE + page,
> > + PB_POUT_OP_WARNING
> > + | PB_POWER_LIMITING);
> > +
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_POUT_OP_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "power", "crit", in_index, page,
> > + PMBUS_POUT_OP_FAULT_LIMIT, PSC_POWER,
> > + false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_IOUT)
> > + pmbus_add_boolean_reg(data, "power",
> > + "crit_alarm",
> > + in_index,
> > + PB_STATUS_IOUT_BASE
> > + + page,
> > + PB_POUT_OP_FAULT);
> > + }
> > + in_index++;
> > + }
> > +
> > + /*
> > + * Temperature sensors
> > + */
> > + in_index = 1;
> > + for (page = 0; page < info->pages; page++) {
> > + int t, temps;
> > +
> > + if (!(info->func[page] & PMBUS_HAVE_TEMP))
> > + continue;
> > +
> > + temps = page ? 1 : ARRAY_SIZE(pmbus_temp_registers);
> > + for (t = 0; t < temps; t++) {
> > + bool have_alarm = false;
> > +
> > + if (!pmbus_check_word_register
> > + (client, page, pmbus_temp_registers[t]))
> > + break;
> > +
> > + i0 = data->num_sensors;
> > + pmbus_add_sensor(data, "temp", "input", in_index, page,
> > + pmbus_temp_registers[t],
> > + PSC_TEMPERATURE, true);
> > +
> > + /*
> > + * PMBus provides only one status register for TEMP1-3.
> > + * Thus, we can not use the status register to determine
> > + * which of the three sensors actually caused an alarm.
> > + * Always compare current temperature against the limit
> > + * registers to determine alarm conditions for a
> > + * specific sensor.
> > + */
> > + if (pmbus_check_word_register
> > + (client, page, PMBUS_UT_WARN_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "temp", "min", in_index,
> > + page, PMBUS_UT_WARN_LIMIT,
> > + PSC_TEMPERATURE, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_TEMP) {
> > + pmbus_add_boolean_cmp(data, "temp",
> > + "min_alarm", in_index, i1, i0,
> > + PB_STATUS_TEMP_BASE + page,
> > + PB_TEMP_UT_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_UT_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "temp", "lcrit",
> > + in_index, page,
> > + PMBUS_UT_FAULT_LIMIT,
> > + PSC_TEMPERATURE, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_TEMP) {
> > + pmbus_add_boolean_cmp(data, "temp",
> > + "lcrit_alarm", in_index, i1, i0,
> > + PB_STATUS_TEMP_BASE + page,
> > + PB_TEMP_UT_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register
> > + (client, page, PMBUS_OT_WARN_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "temp", "max", in_index,
> > + page, PMBUS_OT_WARN_LIMIT,
> > + PSC_TEMPERATURE, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_TEMP) {
> > + pmbus_add_boolean_cmp(data, "temp",
> > + "max_alarm", in_index, i0, i1,
> > + PB_STATUS_TEMP_BASE + page,
> > + PB_TEMP_OT_WARNING);
> > + have_alarm = true;
> > + }
> > + }
> > + if (pmbus_check_word_register(client, page,
> > + PMBUS_OT_FAULT_LIMIT)) {
> > + i1 = data->num_sensors;
> > + pmbus_add_sensor(data, "temp", "crit", in_index,
> > + page, PMBUS_OT_FAULT_LIMIT,
> > + PSC_TEMPERATURE, false);
> > + if (info->func[page] & PMBUS_HAVE_STATUS_TEMP) {
> > + pmbus_add_boolean_cmp(data, "temp",
> > + "crit_alarm", in_index, i0, i1,
> > + PB_STATUS_TEMP_BASE + page,
> > + PB_TEMP_OT_FAULT);
> > + have_alarm = true;
> > + }
> > + }
> > + /*
> > + * Last resort - we were not able to create any alarm
> > + * registers. Report alarm for all sensors using the
> > + * status register temperature alarm bit.
> > + */
> > + if (!have_alarm)
> > + pmbus_add_boolean_reg(data, "temp", "alarm",
> > + in_index,
> > + PB_STATUS_BASE + page,
> > + PB_STATUS_TEMPERATURE);
> > + in_index++;
> > + }
> > + }
> > +
> > + /*
> > + * Fans
> > + */
> > + in_index = 1;
> > + for (page = 0; page < info->pages; page++) {
> > + int fans, f;
> > +
> > + if (!(info->func[page] & PMBUS_HAVE_FAN12))
> > + continue;
> > +
> > + fans = page ? 1 : ARRAY_SIZE(pmbus_fan_registers);
> > + for (f = 0; f < fans; f++) {
> > + int regval;
> > +
> > + if (!pmbus_check_word_register(client, page,
> > + pmbus_fan_registers[f])
> > + || !pmbus_check_byte_register(client, page,
> > + pmbus_fan_config_registers[f]))
> > + break;
> > +
> > + /*
> > + * Skip fan if not installed.
> > + * Each fan configuration register covers multiple fans,
> > + * so we have to do some magic.
> > + */
> > + regval = pmbus_read_byte_data(client, page,
> > + pmbus_fan_config_registers[f]);
> > + if (regval < 0 ||
> > + (!(regval & (PB_FAN_1_INSTALLED >> ((f & 1) * 4)))))
> > + continue;
> > +
> > + i0 = data->num_sensors;
> > + pmbus_add_sensor(data, "fan", "input", in_index, page,
> > + pmbus_fan_registers[f], PSC_FAN, true);
> > +
> > + /*
> > + * Each fan status register covers multiple fans,
> > + * so we have to do some magic.
> > + */
> > + if (pmbus_check_byte_register
> > + (client, page, pmbus_fan_status_registers[f])) {
> > + int base;
> > +
> > + if (f > 1) /* fan 3, 4 */
> > + base = PB_STATUS_FAN34_BASE;
> > + else
> > + base = PB_STATUS_FAN_BASE + page;
> > + pmbus_add_boolean_reg(data, "fan", "alarm",
> > + in_index, base,
> > + PB_FAN_FAN1_WARNING >> (f & 1));
> > + pmbus_add_boolean_reg(data, "fan", "fault",
> > + in_index, base,
> > + PB_FAN_FAN1_FAULT >> (f & 1));
> > + }
> > + in_index++;
> > + }
> > + }
> > +}
> > +
> > +/*
> > + * Identify chip parameters.
> > + * This function is called for all chips.
> > + */
> > +static int pmbus_identify_common(struct i2c_client *client,
> > + struct pmbus_data *data)
> > +{
> > + int vout_mode, exponent;
> > +
> > + vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
> > + if (vout_mode >= 0) {
> > + /*
> > + * Not all chips support the VOUT_MODE command,
> > + * so a failure to read it is not an error.
> > + */
> > + switch (vout_mode >> 5) {
> > + case 0: /* linear mode */
> > + if (data->info->direct[PSC_VOLTAGE_OUT])
> > + return -ENODEV;
> > +
> > + exponent = vout_mode & 0x1f;
> > + /* and sign-extend it */
> > + if (exponent & 0x10)
> > + exponent |= ~0x1f;
> > + data->exponent = exponent;
> > + break;
> > + case 2: /* direct mode */
> > + if (!data->info->direct[PSC_VOLTAGE_OUT])
> > + return -ENODEV;
> > + break;
> > + default:
> > + return -ENODEV;
> > + }
> > + }
> > +
> > + /* Determine maximum number of sensors, booleans, and labels */
> > + pmbus_find_max_attr(client, data);
> > +
> > + pmbus_clear_faults(client);
> > + return 0;
> > +}
> > +
> > +int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
> > + struct pmbus_driver_info *info)
> > +{
> > + const struct pmbus_platform_data *pdata = client->dev.platform_data;
> > + struct pmbus_data *data;
> > + int ret;
> > +
> > + if (!info) {
> Why insist on platform data? Can't we define a default as the pdata looks
> very simple?
There are no mandatory registers in PMBus, so I have no idea what a default
would or could be. Basic idea is that platform data (ie supported register groups)
is either determined via pmbus.c, or provided by a chip driver. If a chip driver
is not needed, pmbus.c should be sufficient and provides the necessary platform data.
An "empty" chip driver which does not provide platform data does not fit into this model.
Note that this is not platform_data (pdata), which is actually optional.
"info" is struct pmbus_driver_info; I would not call that simple.
> > + dev_err(&client->dev, "Missing chip information");
> > + return -ENODEV;
> > + }
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WRITE_BYTE
> > + | I2C_FUNC_SMBUS_BYTE_DATA
> > + | I2C_FUNC_SMBUS_WORD_DATA))
> > + return -ENODEV;
> > +
> > + data = kzalloc(sizeof(*data), GFP_KERNEL);
> > + if (!data) {
> > + dev_err(&client->dev, "No memory to allocate driver data\n");
> > + return -ENOMEM;
> > + }
> > +
> > + i2c_set_clientdata(client, data);
> > + mutex_init(&data->update_lock);
> > +
> > + /*
> > + * Bail out if status register or PMBus revision register
> > + * does not exist.
> > + */
> > + if (i2c_smbus_read_byte_data(client, PMBUS_STATUS_BYTE) < 0
> > + || i2c_smbus_read_byte_data(client, PMBUS_REVISION) < 0) {
> > + dev_err(&client->dev,
> > + "Status or revision register not found\n");
> > + ret = -ENODEV;
> > + goto out_data;
> > + }
> > +
> > + if (pdata)
> > + data->flags = pdata->flags;
> > + data->info = info;
> > +
> perhaps a clarifying comment explaining why some devices might not have
> an identify function?
I'll add some text to pmbus.h.
> > + if (info->identify) {
> > + ret = (*info->identify)(client, info);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "Chip identification failed\n");
> > + goto out_data;
> > + }
> > + }
> > +
> > + if (info->pages <= 0 || info->pages > PMBUS_PAGES) {
> > + dev_err(&client->dev, "Bad number of PMBus pages: %d\n",
> > + info->pages);
> > + ret = -EINVAL;
> > + goto out_data;
> > + }
> > + /*
> > + * Bail out if more than one page was configured, but we can not
> > + * select the highest page. This is an indication that the wrong
> > + * chip type was selected. Better bail out now than keep
> > + * returning errors later on.
> > + */
> > + if (info->pages > 1 && pmbus_set_page(client, info->pages - 1) < 0) {
> > + dev_err(&client->dev, "Failed to select page %d\n",
> > + info->pages - 1);
> > + ret = -EINVAL;
> > + goto out_data;
> > + }
> > +
> > + ret = pmbus_identify_common(client, data);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "Failed to identify chip capabilities\n");
> > + goto out_data;
> > + }
> > +
> > + ret = -ENOMEM;
> > + data->sensors = kzalloc(sizeof(struct pmbus_sensor) * data->max_sensors,
> > + GFP_KERNEL);
> > + if (!data->sensors) {
> > + dev_err(&client->dev, "No memory to allocate sensor data\n");
> > + goto out_data;
> > + }
> > +
> > + data->booleans = kzalloc(sizeof(struct pmbus_boolean)
> > + * data->max_booleans, GFP_KERNEL);
> > + if (!data->booleans) {
> > + dev_err(&client->dev, "No memory to allocate boolean data\n");
> > + goto out_sensors;
> > + }
> > +
> > + data->labels = kzalloc(sizeof(struct pmbus_label) * data->max_labels,
> > + GFP_KERNEL);
> > + if (!data->labels) {
> > + dev_err(&client->dev, "No memory to allocate label data\n");
> > + goto out_booleans;
> > + }
> > +
> > + data->attributes = kzalloc(sizeof(struct attribute *)
> > + * data->max_attributes, GFP_KERNEL);
> > + if (!data->attributes) {
> > + dev_err(&client->dev, "No memory to allocate attribute data\n");
> > + goto out_labels;
> > + }
> > +
> > + pmbus_find_attributes(client, data);
> > +
> > + /*
> > + * If there are no attributes, something is wrong.
> > + * Bail out instead of trying to register nothing.
> > + */
> > + if (!data->num_attributes) {
> > + dev_err(&client->dev, "No attributes found\n");
> > + ret = -ENODEV;
> > + goto out_attributes;
> > + }
> > +
> > + /* Register sysfs hooks */
> > + data->group.attrs = data->attributes;
> > + ret = sysfs_create_group(&client->dev.kobj, &data->group);
> > + if (ret) {
> > + dev_err(&client->dev, "Failed to create sysfs entries\n");
> > + goto out_attributes;
> > + }
> > + data->hwmon_dev = hwmon_device_register(&client->dev);
> > + if (IS_ERR(data->hwmon_dev)) {
> > + ret = PTR_ERR(data->hwmon_dev);
> > + dev_err(&client->dev, "Failed to register hwmon device\n");
> > + goto out_hwmon_device_register;
> > + }
> > + return 0;
> > +
> > +out_hwmon_device_register:
> > + sysfs_remove_group(&client->dev.kobj, &data->group);
> > +out_attributes:
> > + kfree(data->attributes);
> > +out_labels:
> > + kfree(data->labels);
> > +out_booleans:
> > + kfree(data->booleans);
> > +out_sensors:
> > + kfree(data->sensors);
> > +out_data:
> > + kfree(data);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_do_probe);
> > +
> > +int pmbus_do_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->attributes);
> > + kfree(data->labels);
> > + kfree(data->booleans);
> > + kfree(data->sensors);
> > + kfree(data);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(pmbus_do_remove);
> > +
> > +MODULE_AUTHOR("Guenter Roeck");
> > +MODULE_DESCRIPTION("PMBus core driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/i2c/pmbus.h b/include/linux/i2c/pmbus.h
> > new file mode 100644
> > index 0000000..69280db
> > --- /dev/null
> > +++ b/include/linux/i2c/pmbus.h
> > @@ -0,0 +1,45 @@
> > +/*
> > + * Hardware monitoring driver for PMBus devices
> > + *
> > + * Copyright (c) 2010, 2011 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_
> > +
> > +/* flags */
> > +
> > +/*
> > + * PMBUS_SKIP_STATUS_CHECK
> > + *
> > + * During register detection, skip checking the status register for
> > + * communication or command errors.
> > + *
> > + * Some PMBus chips respond with valid data when trying to read an unsupported
> > + * register. For such chips, checking the status register is mandatory when
> > + * trying to determine if a chip register exists or not.
> > + * Other PMBus chips don't support the STATUS_CML register, or report
> > + * communication errors for no explicable reason. For such chips, checking
> > + * the status register must be disabled.
> > + */
> > +#define PMBUS_SKIP_STATUS_CHECK (1 << 0)
> > +
> > +struct pmbus_platform_data {
> > + u32 flags; /* Device specific flags */
> > +};
> > +
> > +#endif /* _PMBUS_H_ */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists