On 04/10/2016 10:16 AM, Guenter Roeck wrote: > On 04/10/2016 04:57 AM, Jonathan Cameron wrote: >> On 08/04/16 19:19, Andrew F. Davis wrote: >>> The INA3221 is a three-channel, high-side current and bus voltage >>> monitor >>> with an I2C and SMBUS compatible interface. >>> >>> Signed-off-by: Andrew F. Davis <afd@xxxxxx> >> Hi Andrew, >> >> My immediate thought on this one is whether it would be better off in >> hwmon. >> I'm not convinced either way from a quick glance at the data sheet, >> but the >> question needs to be addressed. >> > > It looks like a typical hardware monitoring device to me. > >> It's not exactly a clean fit for the IIO ABI either at the moment though >> I think some elements of that could be easily sorted out. >> The key think in my mind is to look at what is actually being measured >> rather than assume all the external components will be as the datasheet >> assumes them to be... >> >> + where do the alert lines actually go? >> > In a typical application they would connect to interrupts or to gpio pins. > They could also trigger some direct action, such as turning on a fan, > though that is unlikely nowadays. The 'critical' pin might sometimes > trigger a system reset. > > Some more comments inline. > > Guenter > >> Jonathan >>> --- >>> .../ABI/testing/sysfs-bus-iio-adc-ina3221 | 23 ++ >>> drivers/iio/adc/Kconfig | 7 + >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/ina3221.c | 417 >>> +++++++++++++++++++++ >>> 4 files changed, 448 insertions(+) >>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ina3221 >>> create mode 100644 drivers/iio/adc/ina3221.c >>> >>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ina3221 >>> b/Documentation/ABI/testing/sysfs-bus-iio-adc-ina3221 >>> new file mode 100644 >>> index 0000000..bbe4f8c >>> --- /dev/null >>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ina3221 >>> @@ -0,0 +1,23 @@ >>> +What: >>> /sys/bus/iio/devices/iio:deviceX/in_voltageY_shunt_(raw|scale) >>> +What: >>> /sys/bus/iio/devices/iio:deviceX/in_voltageY_bus_(raw|scale) >>> +Date: April 2016 >>> +KernelVersion: 4.7 >>> +Contact: Andrew F. Davis <afd@xxxxxx> >>> +Description: >>> + Shunt and Bus voltage for each channel. >> Units etc need specifying or at least a reference to the main >> in_voltageY_raw >> docs etc. These are really completely separate measurements be it >> differential measurements where the + on one is the - on the other >> (second is really a >> unipolar measurement as it's relative to 0). I wonder if we are >> better off supporting them as such so define this device as having the >> channels. >> in_voltage1-voltage0_raw (shunt voltage 1) >> in_voltage0_raw (bus voltage 1) >> in_voltage3-voltage2_raw (shunt voltage 2) >> in_voltage2_raw (bus voltage 2) >> in_voltage5-voltage4_raw (shunt voltage 3) >> in_voltage4_raw >> >> That I think reflects the actual measuring options, without applying >> assumptions on what is connected to them. Yes the datasheet says you >> should >> put a shunt between the first two connections and the load between the >> second connection and the 0 volt line, but I can't see anything >> preventing >> this being used differently... > > Shunt voltage (or voltage difference) has a LSB of 40uV. Using it for > anything else but current measurement doesn't really make much sense. > I would report it not as voltage but as current, but that is from > my filtered hwmon point of view, so maybe it doesn't really apply here. > I would need to know the shunt resistance, I could get this from the user somehow (DT/sysfs) but I decided to report directly from the ADC and let the reader do the math so I don't have to make any use assumptions. >>> + >>> +What: >>> /sys/bus/iio/devices/iio:deviceX/shunt_integration_time[_available] >>> +What: >>> /sys/bus/iio/devices/iio:deviceX/bus_integration_time[_available] >>> +Date: April 2016 >>> +KernelVersion: 4.7 >>> +Contact: Andrew F. Davis <afd@xxxxxx> >>> +Description: >>> + Shunt and Bus integration time for each channel. >> I'd keep these tightly associated with the channels as then they become >> standard abi elements, just for channels with extended names. >>> + >>> +What: >>> /sys/bus/iio/devices/iio:deviceX/in_voltageY_shunt_critical_(raw|scale) >>> +What: >>> /sys/bus/iio/devices/iio:deviceX/in_voltageY_shunt_warning_(raw|scale) >>> +Date: April 2016 >>> +KernelVersion: 4.7 >>> +Contact: Andrew F. Davis <afd@xxxxxx> >>> +Description: >>> + Shunt voltage critical and warning trigger levels. >> This is why I think this may really belong in hwmon. >> The way you currently have this done is a bodge on the relevant IIO >> interfaces. >> If there is good reason to look at multiple equivalent event types on a >> given channel in IIO we can look at adding that support to the core. >> This is a lot more common in explicit hardware monitoring chips than in >> more general ADCs. >> >> Also I see nothing in the driver that is actually detecting if these >> conditions have been passed? If that is assumed to be a problem for >> external >> hardware then it should be clearly documented as such. >> >>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >>> index af4aea7..d713c9c 100644 >>> --- a/drivers/iio/adc/Kconfig >>> +++ b/drivers/iio/adc/Kconfig >>> @@ -232,6 +232,13 @@ config IMX7D_ADC >>> This driver can also be built as a module. If so, the module >>> will be >>> called imx7d_adc. >>> >>> +config INA3221 >>> + tristate "Texas Instruments INA3221 Triple Power Monitor" >>> + depends on I2C >>> + select REGMAP_I2C >>> + help >>> + Say yes here to build support for TI INA3221 Triple Power >>> Monitor. >> Need more detail here really. Something about what it provides and >> the name >> of the module would be conventional. >>> + >>> config LP8788_ADC >>> tristate "LP8788 ADC driver" >>> depends on MFD_LP8788 >>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile >>> index 0cb7921..eebe0c6 100644 >>> --- a/drivers/iio/adc/Makefile >>> +++ b/drivers/iio/adc/Makefile >>> @@ -24,6 +24,7 @@ obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o >>> obj-$(CONFIG_HI8435) += hi8435.o >>> obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o >>> obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o >>> +obj-$(CONFIG_INA3221) += ina3221.o >>> obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o >>> obj-$(CONFIG_MAX1027) += max1027.o >>> obj-$(CONFIG_MAX1363) += max1363.o >>> diff --git a/drivers/iio/adc/ina3221.c b/drivers/iio/adc/ina3221.c >>> new file mode 100644 >>> index 0000000..e5b9df97 >>> --- /dev/null >>> +++ b/drivers/iio/adc/ina3221.c >>> @@ -0,0 +1,417 @@ >>> +/* >>> + * 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/iio/iio.h> >>> +#include <linux/iio/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), >>> +}; >>> + >>> +#define is_bus_reg(_reg) \ >>> + (_reg == INA3221_BUS1 || \ >>> + _reg == INA3221_BUS2 || \ >>> + _reg == 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]; >>> +}; >>> + >>> +/** >>> + * struct ina3221_reg_lookup - value element in iio lookup table map >>> + * @integer: Integer component of value >>> + * @fract: Fractional component of value >>> + */ >>> +struct ina3221_reg_lookup { >>> + int integer; >>> + int fract; >>> +}; >>> + >>> +static const struct ina3221_reg_lookup ina3221_conv_time_table[] = { >>> + {.fract = 140}, {.fract = 204}, {.fract = 332}, {.fract = 588}, >>> + {.fract = 1100}, {.fract = 2116}, {.fract = 4156}, {.fract = 8244}, >>> +}; >>> + >>> +static const int ina3221_avg_table[] = { 1, 4, 16, 64, 128, 256, >>> 512, 1024 }; >>> +static IIO_CONST_ATTR(oversampling_ratio_available, "1 4 16 64 128 >>> 256 512 1024"); >>> + >>> +static int ina3221_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct ina3221_data *ina = iio_priv(indio_dev); >>> + unsigned int regval; >>> + int ret; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + ret = regmap_read(ina->regmap, chan->address, ®val); >>> + if (ret) >>> + return ret; >>> + >>> + *val = (s16)sign_extend32(regval >> 3, 12); >>> + >>> + return IIO_VAL_INT; >>> + >>> + case IIO_CHAN_INFO_SCALE: >>> + if (is_bus_reg(chan->address)) { >>> + *val = 8; >>> + *val2 = 0; >>> + } else { >>> + *val = 0; >>> + *val2 = 40000; >>> + } >>> + >>> + return IIO_VAL_INT_PLUS_MICRO; >>> + >>> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO: >>> + ret = regmap_field_read(ina->fields[F_AVG], ®val); >>> + if (ret) >>> + return ret; >>> + >>> + *val = ina3221_avg_table[regval]; >>> + >>> + return IIO_VAL_INT; >>> + } >>> + >>> + return -EINVAL; >>> +} >>> + >>> +static int ina3221_write_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int val, int val2, long mask) >>> +{ >>> + struct ina3221_data *ina = iio_priv(indio_dev); >>> + int i; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + return regmap_write(ina->regmap, chan->address, val << 3); >>> + >>> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO: >>> + if (val2) >>> + return -EINVAL; >>> + for (i = 0; i < ARRAY_SIZE(ina3221_avg_table); i++) >>> + if (ina3221_avg_table[i] == val) >>> + break; >>> + if (i == ARRAY_SIZE(ina3221_avg_table)) >>> + return -EINVAL; >>> + >>> + return regmap_field_write(ina->fields[F_AVG], i); >>> + } >>> + >>> + return -EINVAL; >>> +} >>> + >>> +#define INA3221_CHAN(_channel, _address, _name) { \ >>> + .type = IIO_VOLTAGE, \ >>> + .channel = (_channel), \ >>> + .address = (_address), \ >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ >>> + BIT(IIO_CHAN_INFO_SCALE), \ >>> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ >>> + .extend_name = _name, \ >>> + .indexed = true, \ >>> +} >>> + >>> +static const struct iio_chan_spec ina3221_channels[] = { >>> + INA3221_CHAN(1, INA3221_SHUNT1, "shunt"), >>> + INA3221_CHAN(1, INA3221_BUS1, "bus"), >>> + INA3221_CHAN(1, INA3221_CRIT1, "shunt_critical"), >>> + INA3221_CHAN(1, INA3221_WARN1, "shunt_warning"), >>> + >>> + INA3221_CHAN(2, INA3221_SHUNT2, "shunt"), >>> + INA3221_CHAN(2, INA3221_BUS2, "bus"), >>> + INA3221_CHAN(2, INA3221_CRIT2, "shunt_critical"), >>> + INA3221_CHAN(2, INA3221_WARN2, "shunt_warning"), >>> + >>> + INA3221_CHAN(3, INA3221_SHUNT3, "shunt"), >>> + INA3221_CHAN(3, INA3221_BUS3, "bus"), >>> + INA3221_CHAN(3, INA3221_CRIT3, "shunt_critical"), >>> + INA3221_CHAN(3, INA3221_WARN3, "shunt_warning"), >> I'm not really sure how these work at all... You can set the thresholds >> but how does the driver know if they have been tripped? >> Or are we dealing with the assumption that it is a problem for external >> hardware? >> > 'shunt' is really the current reported as voltage. 'bus' is the actual > voltage. Unless I am missing it, the driver won't know if the thresholds > have tripped. It would need an interrupt handler and read the status > register to know that. But that isn't really relevant for an iio driver, > or is it ? What would it do if the limits are tripped ? > I agree, this is really the issue that makes me want to go hwmod side with this part, although the interrupts could be made to trigger an IIO_EVENT. >>> +}; >>> + >>> +struct ina3221_attr { >>> + struct device_attribute dev_attr; >>> + struct device_attribute dev_attr_available; >>> + unsigned int field; >>> + const struct ina3221_reg_lookup *table; >>> + unsigned int table_size; >>> +}; >>> + >>> +#define to_ina3221_attr(_dev_attr) \ >>> + container_of(_dev_attr, struct ina3221_attr, dev_attr) >>> + >>> +#define to_ina3221_attr_available(_dev_attr) \ >>> + container_of(_dev_attr, struct ina3221_attr, dev_attr_available) >>> + >>> +static ssize_t ina3221_show_register(struct device *dev, >>> + struct device_attribute *attr, >>> + char *buf) >>> +{ >>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>> + struct ina3221_data *ina = iio_priv(indio_dev); >>> + struct ina3221_attr *ina3221_attr = to_ina3221_attr(attr); >>> + unsigned int reg_val; >>> + int vals[2]; >>> + int ret; >>> + >>> + ret = regmap_field_read(ina->fields[ina3221_attr->field], >>> ®_val); >>> + if (ret) >>> + return ret; >>> + >>> + vals[0] = ina3221_attr->table[reg_val].integer; >>> + vals[1] = ina3221_attr->table[reg_val].fract; >>> + >>> + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); >>> +} >>> + >>> +static ssize_t ina3221_store_register(struct device *dev, >>> + struct device_attribute *attr, >>> + const char *buf, size_t count) >>> +{ >>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>> + struct ina3221_data *ina = iio_priv(indio_dev); >>> + struct ina3221_attr *ina3221_attr = to_ina3221_attr(attr); >>> + long val; >>> + int integer, fract, ret; >>> + >>> + ret = iio_str_to_fixpoint(buf, 100000, &integer, &fract); >>> + if (ret) >>> + return ret; >>> + >>> + if (integer < 0) >>> + return -EINVAL; >>> + >>> + for (val = 0; val < ina3221_attr->table_size; val++) >>> + if (ina3221_attr->table[val].integer == integer && >>> + ina3221_attr->table[val].fract == fract) { >>> + ret = >>> regmap_field_write(ina->fields[ina3221_attr->field], val); >>> + if (ret) >>> + return ret; >>> + >>> + return count; >>> + } >>> + >>> + return -EINVAL; >>> +} >>> + >>> +static ssize_t ina3221_show_available(struct device *dev, >>> + struct device_attribute *attr, >>> + char *buf) >>> +{ >>> + struct ina3221_attr *ina3221_attr = >>> to_ina3221_attr_available(attr); >>> + ssize_t len = 0; >>> + int i; >>> + >>> + for (i = 0; i < ina3221_attr->table_size; i++) >>> + len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", >>> + ina3221_attr->table[i].integer, >>> + ina3221_attr->table[i].fract); >>> + >>> + if (len > 0) >>> + buf[len - 1] = '\n'; >>> + >>> + return len; >>> +} >>> + >>> +#define INA3221_ATTR(_name, _field, _table) \ >>> + struct ina3221_attr ina3221_attr_##_name = { \ >>> + .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ >>> + ina3221_show_register, \ >>> + ina3221_store_register), \ >>> + .dev_attr_available = __ATTR(_name##_available, S_IRUGO, \ >>> + ina3221_show_available, NULL), \ >>> + .field = _field, \ >>> + .table = _table, \ >>> + .table_size = ARRAY_SIZE(_table), \ >>> + } >>> + >>> +static INA3221_ATTR(shunt_integration_time, F_SHUNT_CT, >>> ina3221_conv_time_table); >>> +static INA3221_ATTR(bus_integration_time, F_BUS_CT, >>> ina3221_conv_time_table); >>> + >>> +static struct attribute *ina3221_attributes[] = { >>> + &ina3221_attr_shunt_integration_time.dev_attr.attr, >>> + &ina3221_attr_shunt_integration_time.dev_attr_available.attr, >>> + &ina3221_attr_bus_integration_time.dev_attr.attr, >>> + &ina3221_attr_bus_integration_time.dev_attr_available.attr, >>> + &iio_const_attr_oversampling_ratio_available.dev_attr.attr, >>> + NULL, >>> +}; >>> + >>> +static const struct attribute_group ina3221_attribute_group = { >>> + .attrs = ina3221_attributes, >>> +}; >>> + >>> +static const struct iio_info ina3221_iio_info = { >>> + .driver_module = THIS_MODULE, >>> + .attrs = &ina3221_attribute_group, >>> + .read_raw = ina3221_read_raw, >>> + .write_raw = ina3221_write_raw, >>> +}; >>> + >>> +static const struct regmap_range ina3221_yes_ranges[] = { >>> + regmap_reg_range(INA3221_SHUNT1, INA3221_BUS3), >>> + regmap_reg_range(INA3221_MASK_ENABLE, INA3221_MASK_ENABLE), >>> +}; >>> + >>> +static const struct regmap_access_table ina3221_volatile_table = { >>> + .yes_ranges = ina3221_yes_ranges, >>> + .n_yes_ranges = ARRAY_SIZE(ina3221_yes_ranges), >>> +}; >>> + >>> +static const struct regmap_config ina3221_regmap_config = { >>> + .reg_bits = 8, >>> + .val_bits = 16, >>> + >>> + .cache_type = REGCACHE_RBTREE, >>> + .volatile_table = &ina3221_volatile_table, >>> +}; >>> + >>> +#ifdef CONFIG_OF >>> +static const struct of_device_id ina3221_of_match[] = { >>> + { .compatible = "ti,ina3221", }, >>> + { /* sentinel */ } >>> +}; >>> +MODULE_DEVICE_TABLE(of, ina3221_of_match); >>> +#endif >>> + >>> +static int ina3221_probe(struct i2c_client *client, >>> + const struct i2c_device_id *id) >>> +{ >>> + struct iio_dev *indio_dev; >>> + struct ina3221_data *ina; >>> + int i, ret; >>> + >>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ina)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + i2c_set_clientdata(client, indio_dev); >>> + >>> + ina = iio_priv(indio_dev); >>> + >>> + ina->dev = &client->dev; >>> + >>> + ina->regmap = devm_regmap_init_i2c(client, &ina3221_regmap_config); >>> + if (IS_ERR(ina->regmap)) { >>> + dev_err(ina->dev, "Unable to allocate register map\n"); >>> + return PTR_ERR(ina->regmap); >>> + } >>> + >>> + for (i = 0; i < F_MAX_FIELDS; i++) { >>> + ina->fields[i] = devm_regmap_field_alloc(ina->dev, >>> + ina->regmap, >>> + ina3221_reg_fields[i]); >>> + if (IS_ERR(ina->fields[i])) { >>> + dev_err(ina->dev, "Unable to allocate regmap fields\n"); >>> + return PTR_ERR(ina->fields[i]); >>> + } >>> + } >>> + >>> + ret = regmap_field_write(ina->fields[F_RST], true); >>> + if (ret) { >>> + dev_err(ina->dev, "Unable to reset device\n"); >>> + return ret; >>> + } >>> + >>> + indio_dev->modes = INDIO_DIRECT_MODE; >>> + indio_dev->dev.parent = ina->dev; >>> + indio_dev->channels = ina3221_channels; >>> + indio_dev->num_channels = ARRAY_SIZE(ina3221_channels); >>> + indio_dev->name = INA3221_DRIVER_NAME; >>> + indio_dev->info = &ina3221_iio_info; >>> + >>> + ret = devm_iio_device_register(ina->dev, indio_dev); >>> + if (ret) { >>> + dev_err(ina->dev, "Unable to register IIO device\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static const struct i2c_device_id ina3221_ids[] = { >>> + { "ina3221", 0 }, >>> + { /* sentinel */ } >>> +}; >>> +MODULE_DEVICE_TABLE(i2c, ina3221_ids); >>> + >>> +static struct i2c_driver ina3221_i2c_driver = { >>> + .driver = { >>> + .name = INA3221_DRIVER_NAME, >>> + .of_match_table = of_match_ptr(ina3221_of_match), > > Is that really necessary ? I don't see any chip specific properties > Having said that, specifying shunt resistor values might be useful, > but since the driver reports it as voltage it would not really > add any value. > Not necessary, I just left it in incase I did want to add a shunt resistor value to DT like the ina2xx driver. I'll remove this until then. Andrew >>> + }, >>> + .probe = ina3221_probe, >>> + .id_table = ina3221_ids, >>> +}; >>> +module_i2c_driver(ina3221_i2c_driver); >>> + >>> +MODULE_AUTHOR("Andrew F. Davis <afd@xxxxxx>"); >>> +MODULE_DESCRIPTION("Texas Instruments INA3221 Driver"); >>> +MODULE_LICENSE("GPL v2"); >>> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
