On 04/19/2016 09:09 AM, Guenter Roeck wrote: > On 04/11/2016 01:50 PM, Andrew F. Davis wrote: >> Add support for the the INA3221 26v capable, Triple channel, >> Bi-Directional, Zero-Drift, Low-/High-Side, Current/Voltage Monitor >> with I2C interface. >> >> Signed-off-by: Andrew F. Davis <afd@xxxxxx> >> --- >> Documentation/hwmon/ina3221 | 32 ++++ >> drivers/hwmon/Kconfig | 11 ++ >> drivers/hwmon/Makefile | 1 + >> drivers/hwmon/ina3221.c | 398 >> ++++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 442 insertions(+) >> create mode 100644 Documentation/hwmon/ina3221 >> create mode 100644 drivers/hwmon/ina3221.c >> >> diff --git a/Documentation/hwmon/ina3221 b/Documentation/hwmon/ina3221 >> new file mode 100644 >> index 0000000..3de3814 >> --- /dev/null >> +++ b/Documentation/hwmon/ina3221 >> @@ -0,0 +1,32 @@ >> +Kernel driver ina3221 >> +===================== >> + >> +Supported chips: >> + * Texas Instruments INA3221 >> + Prefix: 'ina3221' >> + Addresses: I2C 0x40 - 0x43 >> + Datasheet: Publicly available at the Texas Instruments website >> + http://www.ti.com/ >> + >> +Author: Andrew F. Davis <afd@xxxxxx> >> + >> +Description >> +----------- >> + >> +The Texas Instruments INA3221 monitors voltage, current, and power on >> the high >> +side of up to three D.C. power supplies. The INA3221 monitors both >> shunt drop >> +and supply voltage, with programmable conversion times and averaging, >> current >> +and power are calculated host-side from these. >> + >> +Sysfs entries >> +------------- >> + >> +in[012]_input Bus voltages(mV) for channels 1, 2, and 3 >> respectively >> +in[345]_input Shunt voltages(mV) for channels 1, 2, and 3 >> respectively >> +curr[012]_input Current(mA) measurement for channels 1, 2, >> and 3 respectively > > Numbering for currents is [123], not [012]. > Okay, I'm guessing I should then change in to match? >> +power[012]_input Power(uW) measurement for channels 1, 2, and >> 3 respectively > > hwmon only reports what is there, and doesn't do calculations. The chip > doesn't report > the power, so please drop the power attributes. > Sure >> +shunt[012]_resistor Shunt resistance(uOhm) for channels 1, 2, and >> 3 respectively > > This should be a backup, if neither devicetree nor platform data are > provided, > not a primary means to provide the shunt resistor value. Also, there > should be a default, > to make sure the driver provides useful data even if nothing is configured. > See ina2xx.c for an example. > I'll add DT support for this. >> +critical[012]_alarm Critical alert voltage(mV) setting, activated >> when the >> + respective shunt voltage is above this value. >> +warning[012]_alarm Warning alert voltage(mV) setting, activated >> when the >> + respective shunt voltage average is above >> this value. > > Alarm attributes report the alarm status, not voltages. Use in[012]_max and > in[012]_crit attributes to set the limits. Report the alarm status with > in[012]_max_alarm and in[012]_crit_alarm (from register 0x0f, bit 9-7 and > 5-3). > Will fix. >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >> index 5c2d13a..de08242 100644 >> --- a/drivers/hwmon/Kconfig >> +++ b/drivers/hwmon/Kconfig >> @@ -1503,6 +1503,17 @@ config SENSORS_INA2XX >> This driver can also be built as a module. If so, the module >> will be called ina2xx. >> >> +config SENSORS_INA3221 >> + tristate "Texas Instruments INA3221 Triple Power Monitor" >> + depends on I2C >> + select REGMAP_I2C >> + help >> + If you say yes here you get support for the TI INA3221 Triple >> Power >> + Monitor. >> + >> + This driver can also be built as a module. If so, the module >> + will be called ina3221. >> + >> config SENSORS_TC74 >> tristate "Microchip TC74" >> depends on I2C >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >> index 58cc3ac..83e8ab0 100644 >> --- a/drivers/hwmon/Makefile >> +++ b/drivers/hwmon/Makefile >> @@ -77,6 +77,7 @@ obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o >> obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o >> obj-$(CONFIG_SENSORS_INA209) += ina209.o >> obj-$(CONFIG_SENSORS_INA2XX) += ina2xx.o >> +obj-$(CONFIG_SENSORS_INA3221) += ina3221.o >> obj-$(CONFIG_SENSORS_IT87) += it87.o >> obj-$(CONFIG_SENSORS_JC42) += jc42.o >> obj-$(CONFIG_SENSORS_JZ4740) += jz4740-hwmon.o >> diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c >> new file mode 100644 >> index 0000000..763e23b >> --- /dev/null >> +++ b/drivers/hwmon/ina3221.c >> @@ -0,0 +1,398 @@ >> +/* >> + * INA3221 Triple Current/Voltage Monitor >> + * >> + * Copyright (C) 2016 Texas Instruments Incorporated - >> http://www.ti.com/ >> + * Andrew F. Davis <afd@xxxxxx> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * General Public License for more details. >> + */ >> + >> +#include <linux/i2c.h> >> +#include <linux/module.h> >> +#include <linux/regmap.h> >> + >> +#include <linux/hwmon.h> >> +#include <linux/hwmon-sysfs.h> >> + >> +#define INA3221_DRIVER_NAME "ina3221" >> + >> +#define INA3221_CONFIG 0x00 >> +#define INA3221_SHUNT1 0x01 >> +#define INA3221_BUS1 0x02 >> +#define INA3221_SHUNT2 0x03 >> +#define INA3221_BUS2 0x04 >> +#define INA3221_SHUNT3 0x05 >> +#define INA3221_BUS3 0x06 >> +#define INA3221_CRIT1 0x07 >> +#define INA3221_WARN1 0x08 >> +#define INA3221_CRIT2 0x09 >> +#define INA3221_WARN2 0x0a >> +#define INA3221_CRIT3 0x0b >> +#define INA3221_WARN3 0x0c >> +#define INA3221_SHUNT_SUM 0x0d >> +#define INA3221_SHUNT_SUM_LIMIT 0x0e >> +#define INA3221_MASK_ENABLE 0x0f >> +#define INA3221_POWERV_HLIMIT 0x10 >> +#define INA3221_POWERV_LLIMIT 0x11 >> + >> +#define INA3221_CONFIG_MODE_SHUNT BIT(1) >> +#define INA3221_CONFIG_MODE_BUS BIT(2) >> +#define INA3221_CONFIG_MODE_CONTINUOUS BIT(3) >> + >> +enum ina3221_fields { >> + /* Configuration */ >> + F_MODE, F_SHUNT_CT, F_BUS_CT, F_AVG, >> + F_CHAN3_EN, F_CHAN2_EN, F_CHAN1_EN, F_RST, >> + >> + /* sentinel */ >> + F_MAX_FIELDS >> +}; >> + >> +static const struct reg_field ina3221_reg_fields[] = { >> + [F_MODE] = REG_FIELD(INA3221_CONFIG, 0, 2), >> + [F_SHUNT_CT] = REG_FIELD(INA3221_CONFIG, 3, 5), >> + [F_BUS_CT] = REG_FIELD(INA3221_CONFIG, 6, 8), >> + [F_AVG] = REG_FIELD(INA3221_CONFIG, 9, 11), >> + [F_CHAN3_EN] = REG_FIELD(INA3221_CONFIG, 12, 12), >> + [F_CHAN2_EN] = REG_FIELD(INA3221_CONFIG, 13, 13), >> + [F_CHAN1_EN] = REG_FIELD(INA3221_CONFIG, 14, 14), >> + [F_RST] = REG_FIELD(INA3221_CONFIG, 15, 15), >> +}; >> + >> +enum ina3221_channels { >> + INA3221_CHANNEL1, >> + INA3221_CHANNEL2, >> + INA3221_CHANNEL3, >> + INA3221_NUM_CHANNELS >> +}; >> + >> +static const int shunt_registers[] = { >> + [INA3221_CHANNEL1] = INA3221_SHUNT1, >> + [INA3221_CHANNEL2] = INA3221_SHUNT2, >> + [INA3221_CHANNEL3] = INA3221_SHUNT3, >> +}; >> + >> +static const int bus_registers[] = { >> + [INA3221_CHANNEL1] = INA3221_BUS1, >> + [INA3221_CHANNEL2] = INA3221_BUS2, >> + [INA3221_CHANNEL3] = INA3221_BUS3, >> +}; >> + >> +/** >> + * struct ina3221_data - device specific information >> + * @dev: Device structure >> + * @regmap: Register map of the device >> + * @fields: Register fields of the device >> + */ >> +struct ina3221_data { >> + struct device *dev; >> + struct regmap *regmap; >> + struct regmap_field *fields[F_MAX_FIELDS]; >> + unsigned int shunt_resistors[INA3221_NUM_CHANNELS]; >> +}; >> + >> +static int ina3221_read_value(struct ina3221_data *ina, unsigned int >> reg, >> + unsigned int *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(ina->regmap, reg, ®val); >> + if (ret) >> + return ret; >> + >> + *val = sign_extend32(regval >> 3, 12); >> + >> + return 0; >> +} >> + >> +static ssize_t ina3221_show_voltage(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); >> + struct ina3221_data *ina = dev_get_drvdata(dev); >> + unsigned int reg = sd_attr->index; >> + int val, voltage_mv, ret; >> + >> + ret = ina3221_read_value(ina, reg, &val); >> + if (ret) >> + return ret; >> + >> + if (reg == INA3221_BUS1 || >> + reg == INA3221_BUS2 || >> + reg == INA3221_BUS3) >> + voltage_mv = val * 8; >> + else >> + voltage_mv = DIV_ROUND_CLOSEST(val * 40, 1000); >> + >> + return snprintf(buf, PAGE_SIZE, "%d\n", voltage_mv); >> +} >> + >> +static ssize_t ina3221_set_voltage(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); >> + struct ina3221_data *ina = dev_get_drvdata(dev); >> + unsigned int reg = sd_attr->index; >> + int val, ret; >> + >> + ret = kstrtoint(buf, 10, &val); >> + if (ret) >> + return ret; >> + > Please clamp the value to valid voltage range. > ACK >> + val = DIV_ROUND_CLOSEST(val, 40000) << 3; >> + >> + ret = regmap_write(ina->regmap, reg, val); >> + if (ret) >> + return ret; >> + >> + return count; >> +} >> + >> +static ssize_t ina3221_show_current(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); >> + struct ina3221_data *ina = dev_get_drvdata(dev); >> + unsigned int channel = sd_attr->index; >> + unsigned int shunt_reg, resistance_uo; >> + int val, current_ma, shunt_voltage_mv, ret; >> + >> + shunt_reg = shunt_registers[channel]; >> + resistance_uo = ina->shunt_resistors[channel]; >> + if (resistance_uo == 0) { >> + current_ma = 0; >> + goto resistance_is_futile; > > Please no funny label names. http://i.imgur.com/2cZhiM4.jpg :) > Also, just make sure that shunt_resistors[] > is never 0 to avoid having to check the value here. > That works, will fix. >> + } >> + >> + ret = ina3221_read_value(ina, shunt_reg, &val); >> + if (ret) >> + return ret; >> + shunt_voltage_mv = val * 40000; >> + >> + current_ma = DIV_ROUND_CLOSEST(shunt_voltage_mv, resistance_uo); >> +resistance_is_futile: >> + return snprintf(buf, PAGE_SIZE, "%d\n", current_ma); >> +} >> + >> +static ssize_t ina3221_show_power(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); >> + struct ina3221_data *ina = dev_get_drvdata(dev); >> + unsigned int channel = sd_attr->index; >> + unsigned int shunt_reg, bus_reg, resistance_uo; >> + int val, power_uw, current_ma, shunt_voltage_mv, bus_voltage_mv, >> ret; >> + >> + shunt_reg = shunt_registers[channel]; >> + bus_reg = bus_registers[channel]; >> + resistance_uo = ina->shunt_resistors[channel]; >> + if (resistance_uo == 0) { >> + power_uw = 0; >> + goto sorry_i_couldnt_resist; >> + } >> + >> + ret = ina3221_read_value(ina, shunt_reg, &val); >> + if (ret) >> + return ret; >> + shunt_voltage_mv = val * 40000; >> + >> + current_ma = DIV_ROUND_CLOSEST(shunt_voltage_mv, resistance_uo); >> + >> + ret = ina3221_read_value(ina, bus_reg, &val); >> + if (ret) >> + return ret; >> + bus_voltage_mv = val * 8; >> + >> + power_uw = current_ma * bus_voltage_mv; >> +sorry_i_couldnt_resist: >> + return snprintf(buf, PAGE_SIZE, "%d\n", power_uw); >> +} >> + >> +static ssize_t ina3221_show_shunt(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); >> + struct ina3221_data *ina = dev_get_drvdata(dev); >> + unsigned int channel = sd_attr->index; >> + unsigned int resistance_uo; >> + >> + resistance_uo = ina->shunt_resistors[channel]; >> + >> + return snprintf(buf, PAGE_SIZE, "%d\n", resistance_uo); >> +} >> + >> +static ssize_t ina3221_set_shunt(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); >> + struct ina3221_data *ina = dev_get_drvdata(dev); >> + unsigned int channel = sd_attr->index; >> + int ret; >> + >> + ret = kstrtouint(buf, 10, &ina->shunt_resistors[channel]); >> + if (ret) >> + return ret; >> + > Please don't blindly accept values here. Wrong values should be rejected. > See ina2xx.c for an example. > I'll fix that and post v2 here shortly. Thanks, Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
