On Fri, Aug 26, 2011 at 08:29:53AM -0400, Jean Delvare wrote:
> On Thu, 25 Aug 2011 19:12:35 -0700, Guenter Roeck wrote:
> > ADM1276 is mostly compatible to ADM1275, with added support for input power
> > measurement. Add support for it to the ADM1275 driver.
> >
> > Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> > ---
> > Documentation/hwmon/adm1275 | 32 ++++++++++++----------
> > drivers/hwmon/pmbus/Kconfig | 5 ++-
> > drivers/hwmon/pmbus/adm1275.c | 58 ++++++++++++++++++++++++++++++++++++----
> > 3 files changed, 73 insertions(+), 22 deletions(-)
>
> Looks overall sane, except for one thing:
>
> >
> > diff --git a/Documentation/hwmon/adm1275 b/Documentation/hwmon/adm1275
> > index c438c98..ab70d96 100644
> > --- a/Documentation/hwmon/adm1275
> > +++ b/Documentation/hwmon/adm1275
> > @@ -6,6 +6,10 @@ Supported chips:
> > Prefix: 'adm1275'
> > Addresses scanned: -
> > Datasheet: www.analog.com/static/imported-files/data_sheets/ADM1275.pdf
> > + * Analog Devices ADM1276
> > + Prefix: 'adm1276'
> > + Addresses scanned: -
> > + Datasheet: www.analog.com/static/imported-files/data_sheets/ADM1276.pdf
> >
> > Author: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> >
> > @@ -13,13 +17,13 @@ Author: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> > Description
> > -----------
> >
> > -This driver supports hardware montoring for Analog Devices ADM1275 Hot-Swap
> > -Controller and Digital Power Monitor.
> > +This driver supports hardware montoring for Analog Devices ADM1275 and ADM1276
> > +Hot-Swap Controller and Digital Power Monitor.
> >
> > -The ADM1275 is a hot-swap controller that allows a circuit board to be removed
> > -from or inserted into a live backplane. It also features current and voltage
> > -readback via an integrated 12-bit analog-to-digital converter (ADC), accessed
> > -using a PMBus. interface.
> > +ADM1275 and ADM1276 are hot-swap controllers that allow a circuit board to be
> > +removed from or inserted into a live backplane. They also feature current and
> > +voltage readback via an integrated 12-bit analog-to-digital converter (ADC),
> > +accessed using a PMBus interface.
> >
> > The driver is a client driver to the core PMBus driver. Please see
> > Documentation/hwmon/pmbus for details on PMBus client drivers.
> > @@ -48,18 +52,18 @@ attributes are write-only, all other attributes are read-only.
> >
> > in1_label "vin1" or "vout1" depending on chip variant and
> > configuration.
> > -in1_input Measured voltage. From READ_VOUT register.
> > -in1_min Minumum Voltage. From VOUT_UV_WARN_LIMIT register.
> > -in1_max Maximum voltage. From VOUT_OV_WARN_LIMIT register.
> > -in1_min_alarm Voltage low alarm. From VOLTAGE_UV_WARNING status.
> > -in1_max_alarm Voltage high alarm. From VOLTAGE_OV_WARNING status.
> > +in1_input Measured voltage.
> > +in1_min Minumum Voltage.
> > +in1_max Maximum voltage.
> > +in1_min_alarm Voltage low alarm.
> > +in1_max_alarm Voltage high alarm.
> > in1_highest Historical maximum voltage.
> > in1_reset_history Write any value to reset history.
> >
> > curr1_label "iout1"
> > -curr1_input Measured current. From READ_IOUT register.
> > -curr1_max Maximum current. From IOUT_OC_WARN_LIMIT register.
> > -curr1_max_alarm Current high alarm. From IOUT_OC_WARN_LIMIT register.
> > +curr1_input Measured current.
> > +curr1_max Maximum current.
> > +curr1_max_alarm Current high alarm.
> > curr1_lcrit Critical minimum current. Depending on the chip
> > configuration, either curr1_lcrit or curr1_crit is
> > supported, but not both.
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index c9237b9..3757558 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -26,11 +26,12 @@ config SENSORS_PMBUS
> > be called pmbus.
> >
> > config SENSORS_ADM1275
> > - tristate "Analog Devices ADM1275"
> > + tristate "Analog Devices ADM1275 and compatibles"
> > default n
> > help
> > If you say yes here you get hardware monitoring support for Analog
> > - Devices ADM1275 Hot-Swap Controller and Digital Power Monitor.
> > + Devices ADM1275 and ADM1276 Hot-Swap Controller and Digital Power
> > + Monitor.
> >
> > This driver can also be built as a module. If so, the module will
> > be called adm1275.
> > diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
> > index 061e7e7..cb04291 100644
> > --- a/drivers/hwmon/pmbus/adm1275.c
> > +++ b/drivers/hwmon/pmbus/adm1275.c
> > @@ -23,6 +23,8 @@
> > #include <linux/i2c.h>
> > #include "pmbus.h"
> >
> > +enum chips { adm1275, adm1276 };
> > +
> > #define ADM1275_PEAK_IOUT 0xd0
> > #define ADM1275_PEAK_VIN 0xd1
> > #define ADM1275_PEAK_VOUT 0xd2
> > @@ -36,9 +38,12 @@
> >
> > #define ADM1275_IOUT_WARN2_SELECT (1 << 4)
> >
> > +#define ADM1276_PEAK_PIN 0xda
> > +
> > #define ADM1275_MFR_STATUS_IOUT_WARN2 (1 << 0)
> >
> > struct adm1275_data {
> > + int id;
> > bool have_oc_fault;
> > struct pmbus_driver_info info;
> > };
> > @@ -78,11 +83,24 @@ static int adm1275_read_word_data(struct i2c_client *client, int page, int reg)
> > case PMBUS_VIRT_READ_VIN_MAX:
> > ret = pmbus_read_word_data(client, 0, ADM1275_PEAK_VIN);
> > break;
> > + case PMBUS_VIRT_READ_PIN_MAX:
> > + if (data->id != adm1276) {
> > + ret = -EINVAL;
> > + break;
> > + }
> > + ret = pmbus_read_word_data(client, 0, ADM1276_PEAK_PIN);
> > + break;
> > case PMBUS_VIRT_RESET_IOUT_HISTORY:
> > case PMBUS_VIRT_RESET_VOUT_HISTORY:
> > case PMBUS_VIRT_RESET_VIN_HISTORY:
> > ret = 0;
> > break;
> > + case PMBUS_VIRT_RESET_PIN_HISTORY:
> > + if (data->id != adm1276)
> > + ret = -EINVAL;
> > + else
> > + ret = 0;
> > + break;
>
> As with the previous patch, I am confused by the mix of -EINVAL and
> -ENODATA for unsupported features.
>
Same as before - the calling code reacts differently.
> Also please don't forget to update wiki/Devices once this patch goes
> upstream.
>
Sure.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
