| Message ID | 1371754675-12242-2-git-send-email-alexandre.belloni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/20/2013 07:57 PM, Alexandre Belloni wrote: > The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable > gain and sampling rates. > Sorry, somewhat low on time today so only a quick review. 1) Missing userspace ABI documentation. Also, perhaps min_conversions is a little vague? Not that I have a better idea! 2) A lot of i2c calls could do with error handling. > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > .../bindings/iio/adc/nuvoton-nau7802.txt | 17 + > drivers/iio/adc/Kconfig | 9 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/nau7802.c | 603 +++++++++++++++++++++ > 4 files changed, 630 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt > create mode 100644 drivers/iio/adc/nau7802.c > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt > new file mode 100644 > index 0000000..9bc4218 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt > @@ -0,0 +1,17 @@ > +* Nuvoton NAU7802 Analog to Digital Converter (ADC) > + > +Required properties: > + - compatible: Should be "nuvoton,nau7802" > + - reg: Should contain the ADC I2C address > + > +Optional properties: > + - nuvoton,vldo: Reference voltage in millivolts (integer) > + - interrupts: IRQ line for the ADC. If not used the driver will use > + polling. > + > +Example: > +adc2: nau7802@2a { > + compatible = "nuvoton,nau7802"; > + reg = <0x2a>; > + nuvoton,vldo = <3000>; > +}; > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index ab0767e6..d7f9ed8 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -133,6 +133,15 @@ config MAX1363 > max11646, max11647) Provides direct access via sysfs and buffered > data via the iio dev interface. > > +config NAU7802 > + tristate "Nuvoton NAU7802 ADC driver" > + depends on I2C > + help > + Say yes here to build support for Nuvoton NAU7802 ADC. > + > + To compile this driver as a module, choose M here: the > + module will be called nau7802. > + > config TI_ADC081C > tristate "Texas Instruments ADC081C021/027" > depends on I2C > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 0a825be..d426081 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_AT91_ADC) += at91_adc.o > obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o > obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o > obj-$(CONFIG_MAX1363) += max1363.o > +obj-$(CONFIG_NAU7802) += nau7802.o > obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o > obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o > obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o > diff --git a/drivers/iio/adc/nau7802.c b/drivers/iio/adc/nau7802.c > new file mode 100644 > index 0000000..e1b6981 > --- /dev/null > +++ b/drivers/iio/adc/nau7802.c > @@ -0,0 +1,603 @@ > +/* > + * Driver for the Nuvoton NAU7802 ADC > + * > + * Copyright 2013 Free Electrons > + * > + * Licensed under the GPLv2 or later. > + */ > + > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/module.h> > +#include <linux/wait.h> > +#include <linux/log2.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > + > +#define NAU7802_REG_PUCTRL 0x00 > +#define NAU7802_PUCTRL_RR(x) (x << 0) > +#define NAU7802_PUCTRL_RR_BIT NAU7802_PUCTRL_RR(1) > +#define NAU7802_PUCTRL_PUD(x) (x << 1) > +#define NAU7802_PUCTRL_PUD_BIT NAU7802_PUCTRL_PUD(1) > +#define NAU7802_PUCTRL_PUA(x) (x << 2) > +#define NAU7802_PUCTRL_PUA_BIT NAU7802_PUCTRL_PUA(1) > +#define NAU7802_PUCTRL_PUR(x) (x << 3) > +#define NAU7802_PUCTRL_PUR_BIT NAU7802_PUCTRL_PUR(1) > +#define NAU7802_PUCTRL_CS(x) (x << 4) > +#define NAU7802_PUCTRL_CS_BIT NAU7802_PUCTRL_CS(1) > +#define NAU7802_PUCTRL_CR(x) (x << 5) > +#define NAU7802_PUCTRL_CR_BIT NAU7802_PUCTRL_CR(1) > +#define NAU7802_PUCTRL_AVDDS(x) (x << 7) > +#define NAU7802_PUCTRL_AVDDS_BIT NAU7802_PUCTRL_AVDDS(1) > +#define NAU7802_REG_CTRL1 0x01 > +#define NAU7802_CTRL1_VLDO(x) (x << 3) > +#define NAU7802_CTRL1_GAINS(x) (x) > +#define NAU7802_CTRL1_GAINS_BITS 0x07 > +#define NAU7802_REG_CTRL2 0x02 > +#define NAU7802_CTRL2_CHS(x) (x << 7) > +#define NAU7802_CTRL2_CRS(x) (x << 4) > +#define NAU7802_SAMP_FREQ_320 0x07 > +#define NAU7802_CTRL2_CHS_BIT NAU7802_CTRL2_CHS(1) > +#define NAU7802_REG_ADC_B2 0x12 > +#define NAU7802_REG_ADC_B1 0x13 > +#define NAU7802_REG_ADC_B0 0x14 > +#define NAU7802_REG_ADC_CTRL 0x15 > + > +#define NAU7802_DEFAULT_CONVERSIONS 6 > + > +struct nau7802_state { > + struct i2c_client *client; > + s32 last_value; > + struct mutex lock; > + struct mutex data_lock; > + u32 vref_mv; > + u32 conversion_count; > + u32 min_conversions; > + u8 sample_rate; > + u32 scale_avail[8]; > + struct completion value_ok; > +}; > + > +#define NAU7802_CHANNEL(chan) { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .channel = (chan), \ > + .scan_index = (chan), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ) \ > +} > + > +static const struct iio_chan_spec nau7802_chan_array[] = { > + NAU7802_CHANNEL(0), > + NAU7802_CHANNEL(1), > +}; > + > +static const u16 nau7802_sample_freq_avail[] = {10, 20, 40, 80, > + 10, 10, 10, 320}; > + > +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("10 40 80 320"); > + > +static ssize_t nau7802_sysfs_set_min_conversions(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t len) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct nau7802_state *st = iio_priv(indio_dev); > + u32 val; > + int ret; > + > + ret = kstrtouint(buf, 10, &val); > + if (ret) > + return ret; > + > + mutex_lock(&st->lock); > + st->min_conversions = val; > + st->conversion_count = 0; > + mutex_unlock(&st->lock); > + > + return len; > +} > + > +static ssize_t nau7802_sysfs_get_min_conversions(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct nau7802_state *st = iio_priv(indio_dev); > + > + return sprintf(buf, "%d\n", st->min_conversions); > +} > + > +static IIO_DEVICE_ATTR(min_conversions, S_IWUSR | S_IRUGO, > + nau7802_sysfs_get_min_conversions, > + nau7802_sysfs_set_min_conversions, 0); > + > +static struct attribute *nau7802_attributes[] = { > + &iio_const_attr_sampling_frequency_available.dev_attr.attr, > + &iio_dev_attr_min_conversions.dev_attr.attr, A new userspace abi element so it needs documented under Documentaiton/ABI/testing/ > + NULL > +}; > + > +static const struct attribute_group nau7802_attribute_group = { > + .attrs = nau7802_attributes, > +}; > + > +static int nau7802_set_gain(struct nau7802_state *st, int gain) > +{ > + u8 data; > + int ret; > + > + mutex_lock(&st->lock); > + st->conversion_count = 0; > + > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); > + if (data < 0) > + goto nau7802_sysfs_set_gain_out; > + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, > + (data & (~NAU7802_CTRL1_GAINS_BITS)) | > + gain); > + > +nau7802_sysfs_set_gain_out: > + mutex_unlock(&st->lock); > + > + return ret ? ret : 0; > +} > + > +static int nau7802_read_conversion(struct nau7802_state *st) > +{ > + u8 data; > + > + mutex_lock(&st->data_lock); > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B2); > + if (data < 0) > + goto nau7802_read_conversion_out; > + st->last_value = data << 16; > + > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B1); > + if (data < 0) > + goto nau7802_read_conversion_out; > + st->last_value |= data << 8; > + > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B0); > + if (data < 0) > + goto nau7802_read_conversion_out; > + st->last_value |= data; > + > + st->last_value = sign_extend32(st->last_value, 23); > + > +nau7802_read_conversion_out: > + mutex_unlock(&st->data_lock); > + > + return data; > +} > + > +/* > + * Conversions are synchronised on the rising edge of NAU7802_PUCTRL_CS_BIT > + */ > +static int nau7802_sync(struct nau7802_state *st) > +{ > + int ret; > + u8 data; > + > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); > + if (data < 0) > + return data; > + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, > + data | NAU7802_PUCTRL_CS_BIT); > + > + return ret; > +} > + > +static irqreturn_t nau7802_eoc_trigger(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct nau7802_state *st = iio_priv(indio_dev); > + u8 status; > + > + status = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); > + if (status < 0) > + return IRQ_HANDLED; > + > + if (!(status & NAU7802_PUCTRL_CR_BIT)) > + return IRQ_NONE; > + > + if (nau7802_read_conversion(st) < 0) > + return IRQ_HANDLED; > + > + /* Because there is actually only one ADC for both channels, we have to > + * wait for enough conversions to happen before getting a significant > + * value when changing channels and the values are far appart. > + */ > + if (st->conversion_count < st->min_conversions) > + st->conversion_count++; > + if (st->conversion_count >= st->min_conversions) > + complete_all(&st->value_ok); > + > + return IRQ_HANDLED; > +} > + > +static int nau7802_read_irq(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val) > +{ > + struct nau7802_state *st = iio_priv(indio_dev); > + int ret; > + > + INIT_COMPLETION(st->value_ok); > + enable_irq(st->client->irq); > + > + nau7802_sync(st); > + > + /* read registers to ensure we flush everything */ > + ret = nau7802_read_conversion(st); > + if (ret < 0) > + goto read_chan_info_failure; > + > + /* Wait for a conversion to finish */ > + ret = wait_for_completion_interruptible_timeout(&st->value_ok, > + msecs_to_jiffies(1000)); > + if (ret == 0) > + ret = -ETIMEDOUT; > + > + if (ret < 0) > + goto read_chan_info_failure; > + > + disable_irq(st->client->irq); > + > + *val = st->last_value; > + > + return IIO_VAL_INT; > + > +read_chan_info_failure: > + disable_irq(st->client->irq); > + > + return ret; > +} > + > +static int nau7802_read_poll(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val) > +{ > + struct nau7802_state *st = iio_priv(indio_dev); > + int ret; > + u8 data; > + > + nau7802_sync(st); > + > + /* read registers to ensure we flush everything */ > + ret = nau7802_read_conversion(st); > + if (ret < 0) > + return ret; > + > + /* Because there is actually only one ADC for both channels, we have to > + * wait for enough conversions to happen before getting a significant > + * value when changing channels and the values are far appart. apart > + */ > + do { > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); > + if (data < 0) > + return data; > + > + while (!(data & NAU7802_PUCTRL_CR_BIT)) { > + if (st->sample_rate != NAU7802_SAMP_FREQ_320) > + msleep(20); > + else > + mdelay(4); > + data = i2c_smbus_read_byte_data(st->client, > + NAU7802_REG_PUCTRL); > + if (data < 0) > + return data; > + } > + > + nau7802_read_conversion(st); > + if (ret < 0) > + return ret; > + if (st->conversion_count < st->min_conversions) > + st->conversion_count++; > + } while (st->conversion_count < st->min_conversions); > + > + *val = st->last_value; > + > + return IIO_VAL_INT; > +} > + > +static int nau7802_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct nau7802_state *st = iio_priv(indio_dev); > + u8 data; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + mutex_lock(&st->lock); > + /* > + * Select the channel to use > + * - Channel 1 is value 0 in the CHS register > + * - Channel 2 is value 1 in the CHS register > + */ > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL2); > + if (data < 0) > + return data; > + > + if (((data & NAU7802_CTRL2_CHS_BIT) && !chan->channel) || > + (!(data & NAU7802_CTRL2_CHS_BIT) && > + chan->channel)) { > + st->conversion_count = 0; > + ret = i2c_smbus_write_byte_data(st->client, > + NAU7802_REG_CTRL2, > + NAU7802_CTRL2_CHS(chan->channel) | > + NAU7802_CTRL2_CRS(st->sample_rate)); > + > + if (ret < 0) > + return ret; > + } > + > + if (st->client->irq) > + ret = nau7802_read_irq(indio_dev, chan, val); > + else > + ret = nau7802_read_poll(indio_dev, chan, val); > + > + mutex_unlock(&st->lock); > + return ret; > + > + case IIO_CHAN_INFO_SCALE: > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); > + if (data < 0) > + return data; > + > + /* we have 24 bits of signed data, that means 23 bits of data > + * plus the sign bit */ > + *val = st->vref_mv; > + *val2 = 23 + (data & NAU7802_CTRL1_GAINS_BITS); > + > + return IIO_VAL_FRACTIONAL_LOG2; > + > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = nau7802_sample_freq_avail[st->sample_rate]; > + *val2 = 0; > + return IIO_VAL_INT; > + > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int nau7802_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) > +{ > + struct nau7802_state *st = iio_priv(indio_dev); > + int i; > + > + switch (mask) { > + case IIO_CHAN_INFO_SCALE: > + for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) > + if (val2 == st->scale_avail[i]) > + return nau7802_set_gain(st, i); > + > + break; > + > + case IIO_CHAN_INFO_SAMP_FREQ: > + for (i = 0; i < ARRAY_SIZE(nau7802_sample_freq_avail); i++) > + if (val == nau7802_sample_freq_avail[i]) { > + mutex_lock(&st->lock); > + st->sample_rate = i; > + st->conversion_count = 0; Error handling for the i2c call? > + i2c_smbus_write_byte_data(st->client, > + NAU7802_REG_CTRL2, > + NAU7802_CTRL2_CRS(st->sample_rate)); > + mutex_unlock(&st->lock); > + return 0; > + } > + > + break; > + > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int nau7802_write_raw_get_fmt(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + long mask) > +{ > + return IIO_VAL_INT_PLUS_NANO; > +} > + > +static const struct iio_info nau7802_info = { > + .driver_module = THIS_MODULE, > + .read_raw = &nau7802_read_raw, > + .write_raw = &nau7802_write_raw, > + .write_raw_get_fmt = nau7802_write_raw_get_fmt, > + .attrs = &nau7802_attribute_group, > +}; > + > +static int nau7802_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct iio_dev *indio_dev; > + struct nau7802_state *st; > + struct device_node *np = client->dev.of_node; > + int i, ret; > + u8 data; > + u32 tmp; > + > + if (!client->dev.of_node) { > + dev_err(&client->dev, "No device tree node available.\n"); > + return -EINVAL; > + } > + > + indio_dev = iio_device_alloc(sizeof(*st)); > + if (indio_dev == NULL) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + > + i2c_set_clientdata(client, indio_dev); > + > + indio_dev->dev.parent = &client->dev; > + indio_dev->name = dev_name(&client->dev); > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &nau7802_info; > + > + st->client = client; > + > + /* Reset the device */ As a general rule we'd expect any errors returned by these functions to be checked and handled. > + i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, > + NAU7802_PUCTRL_RR_BIT); > + > + /* Enter normal operation mode */ > + i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, > + NAU7802_PUCTRL_PUD_BIT); > + > + /* > + * After about 200 usecs, the device should be ready and then > + * the Power Up bit will be set to 1. If not, wait for it. > + */ > + udelay(210); > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); > + if (data < 0) > + return -ENODEV; > + if (!(data & NAU7802_PUCTRL_PUR_BIT)) > + return -ENODEV; > + > + of_property_read_u32(np, "nuvoton,vldo", &tmp); > + st->vref_mv = tmp; > + > + /* Populate available ADC input ranges */ > + for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) > + st->scale_avail[i] = (((u64)st->vref_mv) * 1000000000ULL) > + >> (23 + i); > + > + data = NAU7802_PUCTRL_PUD_BIT | NAU7802_PUCTRL_PUA_BIT | > + NAU7802_PUCTRL_CS_BIT; > + if (tmp >= 2400) > + data |= NAU7802_PUCTRL_AVDDS_BIT; > + > + i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, data); > + i2c_smbus_write_byte_data(st->client, NAU7802_REG_ADC_CTRL, 0x30); > + > + if (tmp >= 2400) { > + data = NAU7802_CTRL1_VLDO((4500 - tmp) / 300); > + i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, data); > + } > + > + st->min_conversions = NAU7802_DEFAULT_CONVERSIONS; > + > + /* > + * The ADC fires continuously and we can't do anything about > + * it. So we need to have the IRQ disabled by default, and we > + * will enable them back when we will need them.. > + */ > + if (client->irq) { > + ret = request_threaded_irq(client->irq, > + NULL, > + nau7802_eoc_trigger, > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, > + client->dev.driver->name, > + indio_dev); > + if (ret) { > + /* > + * What may happen here is that our IRQ controller is > + * not able to get level interrupt but this is required > + * by this ADC as when going over 40 sample per second, > + * the interrupt line may stay high between conversions. > + * So, we continue no matter what but we switch to > + * polling mode. > + */ > + dev_info(&client->dev, > + "Failed to allocate IRQ, using polling mode\n"); > + client->irq = 0; > + } else > + disable_irq(client->irq); > + } > + > + if (!client->irq) { > + /* > + * We are polling, use the fastest sample rate by > + * default > + */ > + st->sample_rate = NAU7802_SAMP_FREQ_320; > + i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL2, > + NAU7802_CTRL2_CRS(st->sample_rate)); > + } > + > + /* Setup the ADC channels available on the board */ > + indio_dev->num_channels = 2; > + indio_dev->channels = nau7802_chan_array; > + > + init_completion(&st->value_ok); > + mutex_init(&st->lock); > + mutex_init(&st->data_lock); > + > + ret = iio_device_register(indio_dev); > + if (ret < 0) { > + dev_err(&client->dev, "Couldn't register the device.\n"); > + goto error_device_register; > + } > + > + return 0; > + > +error_device_register: > + mutex_destroy(&st->lock); > + mutex_destroy(&st->data_lock); > + if (client->irq) > + free_irq(client->irq, indio_dev); > + iio_device_free(indio_dev); > + > + return ret; > +} > + > +static int nau7802_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + struct nau7802_state *st = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + mutex_destroy(&st->lock); > + mutex_destroy(&st->data_lock); > + if (client->irq) > + free_irq(client->irq, indio_dev); > + iio_device_free(indio_dev); > + > + return 0; > +} > + > +static const struct i2c_device_id nau7802_i2c_id[] = { > + { "nau7802", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, nau7802_i2c_id); > + > +static const struct of_device_id nau7802_dt_ids[] = { > + { .compatible = "nuvoton,nau7802" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, nau7802_dt_ids); > + > +static struct i2c_driver nau7802_driver = { > + .probe = nau7802_probe, > + .remove = nau7802_remove, > + .id_table = nau7802_i2c_id, > + .driver = { > + .name = "nau7802", > + .of_match_table = of_match_ptr(nau7802_dt_ids), > + }, > +}; > + > +module_i2c_driver(nau7802_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("Nuvoton NAU7802 ADC Driver"); > +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>"); > +MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>"); >
On 06/22/2013 01:55 PM, Jonathan Cameron wrote: > On 06/20/2013 07:57 PM, Alexandre Belloni wrote: >> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >> gain and sampling rates. >> > Sorry, somewhat low on time today so only a quick review. > > 1) Missing userspace ABI documentation. Also, perhaps min_conversions is > a little vague? Not that I have a better idea! I really don't like the name min_conversions either. Isn't this effectively a decimation filter? - Lars
On 22/06/2013 14:02, Lars-Peter Clausen wrote: > On 06/22/2013 01:55 PM, Jonathan Cameron wrote: >> On 06/20/2013 07:57 PM, Alexandre Belloni wrote: >>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >>> gain and sampling rates. >>> >> Sorry, somewhat low on time today so only a quick review. >> >> 1) Missing userspace ABI documentation. Also, perhaps min_conversions is >> a little vague? Not that I have a better idea! > I really don't like the name min_conversions either. Isn't this effectively > a decimation filter? Yeah, it could be seen like that but it is only relevant and only happens when switching between channels. I'm open to any ideas.
On 06/22/2013 03:07 PM, Alexandre Belloni wrote: > On 22/06/2013 14:02, Lars-Peter Clausen wrote: >> On 06/22/2013 01:55 PM, Jonathan Cameron wrote: >>> On 06/20/2013 07:57 PM, Alexandre Belloni wrote: >>>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >>>> gain and sampling rates. >>>> >>> Sorry, somewhat low on time today so only a quick review. >>> >>> 1) Missing userspace ABI documentation. Also, perhaps min_conversions is >>> a little vague? Not that I have a better idea! >> I really don't like the name min_conversions either. Isn't this effectively >> a decimation filter? > > Yeah, it could be seen like that but it is only relevant and only > happens when switching between channels. I'm open to any ideas. > I see. Is there anything about this in the datasheet on how many conversions you usually need? Is this really something you need to change at runtime or does moving this to platform data work? - Lars
On 22/06/2013 15:20, Lars-Peter Clausen wrote: > On 06/22/2013 03:07 PM, Alexandre Belloni wrote: >> On 22/06/2013 14:02, Lars-Peter Clausen wrote: >>> On 06/22/2013 01:55 PM, Jonathan Cameron wrote: >>>> On 06/20/2013 07:57 PM, Alexandre Belloni wrote: >>>>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >>>>> gain and sampling rates. >>>>> >>>> Sorry, somewhat low on time today so only a quick review. >>>> >>>> 1) Missing userspace ABI documentation. Also, perhaps min_conversions is >>>> a little vague? Not that I have a better idea! >>> I really don't like the name min_conversions either. Isn't this effectively >>> a decimation filter? >> Yeah, it could be seen like that but it is only relevant and only >> happens when switching between channels. I'm open to any ideas. >> > I see. Is there anything about this in the datasheet on how many conversions > you usually need? Is this really something you need to change at runtime or > does moving this to platform data work? > > There is actually nothing in the datasheet. The default value (6 conversions) was found experimentally. What I did was saturating the ADC with the higher value on one channel and the lower value on the other one and I tried to find when reading both channel sequentially was resulting in a correct value. You may not need to change it at runtime. And that value mainly depend on the precision versus speed balance you want to achieve. If you know that the values on both channels will not be to far apart, then you may not need to wait at all. Would you think that is something I should hide in the DT ? Or maybe I can drop that knob for now and see if it is needed in the future.
On 06/22/2013 03:28 PM, Alexandre Belloni wrote: > On 22/06/2013 15:20, Lars-Peter Clausen wrote: >> On 06/22/2013 03:07 PM, Alexandre Belloni wrote: >>> On 22/06/2013 14:02, Lars-Peter Clausen wrote: >>>> On 06/22/2013 01:55 PM, Jonathan Cameron wrote: >>>>> On 06/20/2013 07:57 PM, Alexandre Belloni wrote: >>>>>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >>>>>> gain and sampling rates. >>>>>> >>>>> Sorry, somewhat low on time today so only a quick review. >>>>> >>>>> 1) Missing userspace ABI documentation. Also, perhaps min_conversions is >>>>> a little vague? Not that I have a better idea! >>>> I really don't like the name min_conversions either. Isn't this effectively >>>> a decimation filter? >>> Yeah, it could be seen like that but it is only relevant and only >>> happens when switching between channels. I'm open to any ideas. >>> >> I see. Is there anything about this in the datasheet on how many conversions >> you usually need? Is this really something you need to change at runtime or >> does moving this to platform data work? >> >> > > There is actually nothing in the datasheet. The default value (6 > conversions) was found experimentally. What I did was saturating the ADC > with the higher value on one channel and the lower value on the other > one and I tried to find when reading both channel sequentially was > resulting in a correct value. > > You may not need to change it at runtime. And that value mainly depend > on the precision versus speed balance you want to achieve. If you know > that the values on both channels will not be to far apart, then you may > not need to wait at all. > > Would you think that is something I should hide in the DT ? Or maybe I > can drop that knob for now and see if it is needed in the future. > It is always a good idea to be conservative when introducing new ABI, so if you think we can get away with hardcoding this in the driver I think that's a good idea. - Lars
On 06/23/2013 02:54 PM, Lars-Peter Clausen wrote: > On 06/22/2013 03:28 PM, Alexandre Belloni wrote: >> On 22/06/2013 15:20, Lars-Peter Clausen wrote: >>> On 06/22/2013 03:07 PM, Alexandre Belloni wrote: >>>> On 22/06/2013 14:02, Lars-Peter Clausen wrote: >>>>> On 06/22/2013 01:55 PM, Jonathan Cameron wrote: >>>>>> On 06/20/2013 07:57 PM, Alexandre Belloni wrote: >>>>>>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >>>>>>> gain and sampling rates. >>>>>>> >>>>>> Sorry, somewhat low on time today so only a quick review. >>>>>> >>>>>> 1) Missing userspace ABI documentation. Also, perhaps min_conversions is >>>>>> a little vague? Not that I have a better idea! >>>>> I really don't like the name min_conversions either. Isn't this effectively >>>>> a decimation filter? >>>> Yeah, it could be seen like that but it is only relevant and only >>>> happens when switching between channels. I'm open to any ideas. >>>> >>> I see. Is there anything about this in the datasheet on how many conversions >>> you usually need? Is this really something you need to change at runtime or >>> does moving this to platform data work? >>> >>> >> >> There is actually nothing in the datasheet. The default value (6 >> conversions) was found experimentally. What I did was saturating the ADC >> with the higher value on one channel and the lower value on the other >> one and I tried to find when reading both channel sequentially was >> resulting in a correct value. >> >> You may not need to change it at runtime. And that value mainly depend >> on the precision versus speed balance you want to achieve. If you know >> that the values on both channels will not be to far apart, then you may >> not need to wait at all. >> >> Would you think that is something I should hide in the DT ? Or maybe I >> can drop that knob for now and see if it is needed in the future. >> > > It is always a good idea to be conservative when introducing new ABI, so if > you think we can get away with hardcoding this in the driver I think that's > a good idea. > I agree entirely. We can always make it controllable later as long as we keep the default the same as the value you hard code in now.
On 06/20/2013 08:57 PM, Alexandre Belloni wrote: > The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable > gain and sampling rates. > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > .../bindings/iio/adc/nuvoton-nau7802.txt | 17 + > drivers/iio/adc/Kconfig | 9 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/nau7802.c | 603 +++++++++++++++++++++ > 4 files changed, 630 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt > create mode 100644 drivers/iio/adc/nau7802.c > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt > new file mode 100644 > index 0000000..9bc4218 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt > @@ -0,0 +1,17 @@ > +* Nuvoton NAU7802 Analog to Digital Converter (ADC)use > + > +Required properties: > + - compatible: Should be "nuvoton,nau7802" > + - reg: Should contain the ADC I2C address > + > +Optional properties: > + - nuvoton,vldo: Reference voltage in millivolts (integer) > + - interrupts: IRQ line for the ADC. If not used the driver will use > + polling. > + > +Example: > +adc2: nau7802@2a { > + compatible = "nuvoton,nau7802"; > + reg = <0x2a>; > + nuvoton,vldo = <3000>; We usually use the regulator framework for specifying the reference voltage. > +}; [...] > diff --git a/drivers/iio/adc/nau7802.c b/drivers/iio/adc/nau7802.c > new file mode 100644 > index 0000000..e1b6981 > --- /dev/null > +++ b/drivers/iio/adc/nau7802.c > @@ -0,0 +1,603 @@ [...] > +static int nau7802_set_gain(struct nau7802_state *st, int gain) > +{ > + u8 data; > + int ret; > + > + mutex_lock(&st->lock); > + st->conversion_count = 0; > + > + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); > + if (data < 0) > + goto nau7802_sysfs_set_gain_out; ret will be uninitialized if the goto above is taken > + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, > + (data & (~NAU7802_CTRL1_GAINS_BITS)) | > + gain); > + > +nau7802_sysfs_set_gain_out: > + mutex_unlock(&st->lock); > + > + return ret ? ret : 0; > +} [...] > +static int nau7802_read_irq(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val) > +{ > + struct nau7802_state *st = iio_priv(indio_dev); > + int ret; > + > + INIT_COMPLETION(st->value_ok); > + enable_irq(st->client->irq); Is it really necessary to enable/disable the IRQ or could you keep it enabled all the time? > + > + nau7802_sync(st); > + > + /* read registers to ensure we flush everything */ > + ret = nau7802_read_conversion(st); > + if (ret < 0) > + goto read_chan_info_failure; > + > + /* Wait for a conversion to finish */ > + ret = wait_for_completion_interruptible_timeout(&st->value_ok, > + msecs_to_jiffies(1000)); > + if (ret == 0) > + ret = -ETIMEDOUT; > + > + if (ret < 0) > + goto read_chan_info_failure; > + > + disable_irq(st->client->irq); > + > + *val = st->last_value; > + > + return IIO_VAL_INT; > + > +read_chan_info_failure: > + disable_irq(st->client->irq); > + > + return ret; > +} [...] [...] > +static int nau7802_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct iio_dev *indio_dev; > + struct nau7802_state *st; > + struct device_node *np = client->dev.of_node; > + int i, ret; > + u8 data; > + u32 tmp; > + > + if (!client->dev.of_node) { > + dev_err(&client->dev, "No device tree node available.\n"); > + return -EINVAL; > + } Except for getting the vref the is no direct dependency on devicetree, if you switch to the regulator framework for the vref this check can be removed. [...] > + /* Setup the ADC channels available on the board */ > + indio_dev->num_channels = 2; ARRAY_SIZE(nau7802_chan_array) > + indio_dev->channels = nau7802_chan_array; > + > + init_completion(&st->value_ok); You need to initialize the completion before requesting the IRQ handler. [...] > +}
On 24/06/2013 08:41, Lars-Peter Clausen wrote: > On 06/20/2013 08:57 PM, Alexandre Belloni wrote: >> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >> gain and sampling rates. >> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> --- >> .../bindings/iio/adc/nuvoton-nau7802.txt | 17 + >> drivers/iio/adc/Kconfig | 9 + >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/nau7802.c | 603 +++++++++++++++++++++ >> 4 files changed, 630 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt >> create mode 100644 drivers/iio/adc/nau7802.c >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt >> new file mode 100644 >> index 0000000..9bc4218 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt >> @@ -0,0 +1,17 @@ >> +* Nuvoton NAU7802 Analog to Digital Converter (ADC)use >> + >> +Required properties: >> + - compatible: Should be "nuvoton,nau7802" >> + - reg: Should contain the ADC I2C address >> + >> +Optional properties: >> + - nuvoton,vldo: Reference voltage in millivolts (integer) >> + - interrupts: IRQ line for the ADC. If not used the driver will use >> + polling. >> + >> +Example: >> +adc2: nau7802@2a { >> + compatible = "nuvoton,nau7802"; >> + reg = <0x2a>; >> + nuvoton,vldo = <3000>; > We usually use the regulator framework for specifying the reference voltage. I followed what Jonathan said in his review of my first patch. Do we want to use the regulator framework to set the internal reference voltage of the ADC ? I agree that if you supply an external voltage, it will be necessary to use the regulator framework. Unfortunately, I can't test that here. >> +}; > [...] >> diff --git a/drivers/iio/adc/nau7802.c b/drivers/iio/adc/nau7802.c >> new file mode 100644 >> index 0000000..e1b6981 >> --- /dev/null >> +++ b/drivers/iio/adc/nau7802.c >> @@ -0,0 +1,603 @@ > [...] >> +static int nau7802_set_gain(struct nau7802_state *st, int gain) >> +{ >> + u8 data; >> + int ret; >> + >> + mutex_lock(&st->lock); >> + st->conversion_count = 0; >> + >> + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); >> + if (data < 0) >> + goto nau7802_sysfs_set_gain_out; > ret will be uninitialized if the goto above is taken Right, bigger issue, data is u8 so it will never be negative. I'm fixing that ! >> + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, >> + (data & (~NAU7802_CTRL1_GAINS_BITS)) | >> + gain); >> + >> +nau7802_sysfs_set_gain_out: >> + mutex_unlock(&st->lock); >> + >> + return ret ? ret : 0; >> +} > [...] >> +static int nau7802_read_irq(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int *val) >> +{ >> + struct nau7802_state *st = iio_priv(indio_dev); >> + int ret; >> + >> + INIT_COMPLETION(st->value_ok); >> + enable_irq(st->client->irq); > Is it really necessary to enable/disable the IRQ or could you keep it > enabled all the time? Fact is that the ADC doesn't really care if you are reading data or not so you will probably endd up in a situation were you will get 320 IRQ per second but not caring about the result. We have 3 ADCs on the board. so that amounts to 960 IRQ per second when we are only reading like once par second !
On 06/24/2013 12:37 PM, Alexandre Belloni wrote: > On 24/06/2013 08:41, Lars-Peter Clausen wrote: >> On 06/20/2013 08:57 PM, Alexandre Belloni wrote: >>> The Nuvoton NAU7802 ADC is a 24-bit 2-channels I2C ADC, with adjustable >>> gain and sampling rates. >>> >>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>> --- >>> .../bindings/iio/adc/nuvoton-nau7802.txt | 17 + >>> drivers/iio/adc/Kconfig | 9 + >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/nau7802.c | 603 +++++++++++++++++++++ >>> 4 files changed, 630 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt >>> create mode 100644 drivers/iio/adc/nau7802.c >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt >>> new file mode 100644 >>> index 0000000..9bc4218 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt >>> @@ -0,0 +1,17 @@ >>> +* Nuvoton NAU7802 Analog to Digital Converter (ADC)use >>> + >>> +Required properties: >>> + - compatible: Should be "nuvoton,nau7802" >>> + - reg: Should contain the ADC I2C address >>> + >>> +Optional properties: >>> + - nuvoton,vldo: Reference voltage in millivolts (integer) >>> + - interrupts: IRQ line for the ADC. If not used the driver will use >>> + polling. >>> + >>> +Example: >>> +adc2: nau7802@2a { >>> + compatible = "nuvoton,nau7802"; >>> + reg = <0x2a>; >>> + nuvoton,vldo = <3000>; >> We usually use the regulator framework for specifying the reference voltage. > > I followed what Jonathan said in his review of my first patch. Do we > want to use the regulator framework to set the internal reference > voltage of the ADC ? I agree that if you supply an external voltage, it > will be necessary to use the regulator framework. Unfortunately, I can't > test that here. > Ah, ok I missed that it is an internally generated voltage. It might makes sense to add that to the properties documentation. I guess ideally you'd also register a regulator for the internal regulator and then use that. But I think that will unnecessarily complicate the code, so I guess the current solution is fine. There is one bug in probe though, if nuvoton,vldo is not set tmp will remain uninitialized. >>> +}; >> [...] >>> diff --git a/drivers/iio/adc/nau7802.c b/drivers/iio/adc/nau7802.c >>> new file mode 100644 >>> index 0000000..e1b6981 >>> --- /dev/null >>> +++ b/drivers/iio/adc/nau7802.c >>> @@ -0,0 +1,603 @@ >> [...] >>> +static int nau7802_set_gain(struct nau7802_state *st, int gain) >>> +{ >>> + u8 data; >>> + int ret; >>> + >>> + mutex_lock(&st->lock); >>> + st->conversion_count = 0; >>> + >>> + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); >>> + if (data < 0) >>> + goto nau7802_sysfs_set_gain_out; >> ret will be uninitialized if the goto above is taken > > Right, bigger issue, data is u8 so it will never be negative. I'm fixing > that ! > > >>> + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, >>> + (data & (~NAU7802_CTRL1_GAINS_BITS)) | >>> + gain); >>> + >>> +nau7802_sysfs_set_gain_out: >>> + mutex_unlock(&st->lock); >>> + >>> + return ret ? ret : 0; >>> +} >> [...] >>> +static int nau7802_read_irq(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val) >>> +{ >>> + struct nau7802_state *st = iio_priv(indio_dev); >>> + int ret; >>> + >>> + INIT_COMPLETION(st->value_ok); >>> + enable_irq(st->client->irq); >> Is it really necessary to enable/disable the IRQ or could you keep it >> enabled all the time? > > Fact is that the ADC doesn't really care if you are reading data or not > so you will probably endd up in a situation were you will get 320 IRQ > per second but not caring about the result. We have 3 ADCs on the board. > so that amounts to 960 IRQ per second when we are only reading like once > par second ! Ah, ok, makes sense.
On 24/06/2013 18:41, Lars-Peter Clausen wrote: > Ah, ok, makes sense. > I just sent v3, thanks for your reviews !
diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt new file mode 100644 index 0000000..9bc4218 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton-nau7802.txt @@ -0,0 +1,17 @@ +* Nuvoton NAU7802 Analog to Digital Converter (ADC) + +Required properties: + - compatible: Should be "nuvoton,nau7802" + - reg: Should contain the ADC I2C address + +Optional properties: + - nuvoton,vldo: Reference voltage in millivolts (integer) + - interrupts: IRQ line for the ADC. If not used the driver will use + polling. + +Example: +adc2: nau7802@2a { + compatible = "nuvoton,nau7802"; + reg = <0x2a>; + nuvoton,vldo = <3000>; +}; diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index ab0767e6..d7f9ed8 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -133,6 +133,15 @@ config MAX1363 max11646, max11647) Provides direct access via sysfs and buffered data via the iio dev interface. +config NAU7802 + tristate "Nuvoton NAU7802 ADC driver" + depends on I2C + help + Say yes here to build support for Nuvoton NAU7802 ADC. + + To compile this driver as a module, choose M here: the + module will be called nau7802. + config TI_ADC081C tristate "Texas Instruments ADC081C021/027" depends on I2C diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index 0a825be..d426081 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_AT91_ADC) += at91_adc.o obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o obj-$(CONFIG_MAX1363) += max1363.o +obj-$(CONFIG_NAU7802) += nau7802.o obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o diff --git a/drivers/iio/adc/nau7802.c b/drivers/iio/adc/nau7802.c new file mode 100644 index 0000000..e1b6981 --- /dev/null +++ b/drivers/iio/adc/nau7802.c @@ -0,0 +1,603 @@ +/* + * Driver for the Nuvoton NAU7802 ADC + * + * Copyright 2013 Free Electrons + * + * Licensed under the GPLv2 or later. + */ + +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/wait.h> +#include <linux/log2.h> + +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> + +#define NAU7802_REG_PUCTRL 0x00 +#define NAU7802_PUCTRL_RR(x) (x << 0) +#define NAU7802_PUCTRL_RR_BIT NAU7802_PUCTRL_RR(1) +#define NAU7802_PUCTRL_PUD(x) (x << 1) +#define NAU7802_PUCTRL_PUD_BIT NAU7802_PUCTRL_PUD(1) +#define NAU7802_PUCTRL_PUA(x) (x << 2) +#define NAU7802_PUCTRL_PUA_BIT NAU7802_PUCTRL_PUA(1) +#define NAU7802_PUCTRL_PUR(x) (x << 3) +#define NAU7802_PUCTRL_PUR_BIT NAU7802_PUCTRL_PUR(1) +#define NAU7802_PUCTRL_CS(x) (x << 4) +#define NAU7802_PUCTRL_CS_BIT NAU7802_PUCTRL_CS(1) +#define NAU7802_PUCTRL_CR(x) (x << 5) +#define NAU7802_PUCTRL_CR_BIT NAU7802_PUCTRL_CR(1) +#define NAU7802_PUCTRL_AVDDS(x) (x << 7) +#define NAU7802_PUCTRL_AVDDS_BIT NAU7802_PUCTRL_AVDDS(1) +#define NAU7802_REG_CTRL1 0x01 +#define NAU7802_CTRL1_VLDO(x) (x << 3) +#define NAU7802_CTRL1_GAINS(x) (x) +#define NAU7802_CTRL1_GAINS_BITS 0x07 +#define NAU7802_REG_CTRL2 0x02 +#define NAU7802_CTRL2_CHS(x) (x << 7) +#define NAU7802_CTRL2_CRS(x) (x << 4) +#define NAU7802_SAMP_FREQ_320 0x07 +#define NAU7802_CTRL2_CHS_BIT NAU7802_CTRL2_CHS(1) +#define NAU7802_REG_ADC_B2 0x12 +#define NAU7802_REG_ADC_B1 0x13 +#define NAU7802_REG_ADC_B0 0x14 +#define NAU7802_REG_ADC_CTRL 0x15 + +#define NAU7802_DEFAULT_CONVERSIONS 6 + +struct nau7802_state { + struct i2c_client *client; + s32 last_value; + struct mutex lock; + struct mutex data_lock; + u32 vref_mv; + u32 conversion_count; + u32 min_conversions; + u8 sample_rate; + u32 scale_avail[8]; + struct completion value_ok; +}; + +#define NAU7802_CHANNEL(chan) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = (chan), \ + .scan_index = (chan), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_SAMP_FREQ) \ +} + +static const struct iio_chan_spec nau7802_chan_array[] = { + NAU7802_CHANNEL(0), + NAU7802_CHANNEL(1), +}; + +static const u16 nau7802_sample_freq_avail[] = {10, 20, 40, 80, + 10, 10, 10, 320}; + +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("10 40 80 320"); + +static ssize_t nau7802_sysfs_set_min_conversions(struct device *dev, + struct device_attribute *attr, + const char *buf, + size_t len) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct nau7802_state *st = iio_priv(indio_dev); + u32 val; + int ret; + + ret = kstrtouint(buf, 10, &val); + if (ret) + return ret; + + mutex_lock(&st->lock); + st->min_conversions = val; + st->conversion_count = 0; + mutex_unlock(&st->lock); + + return len; +} + +static ssize_t nau7802_sysfs_get_min_conversions(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct nau7802_state *st = iio_priv(indio_dev); + + return sprintf(buf, "%d\n", st->min_conversions); +} + +static IIO_DEVICE_ATTR(min_conversions, S_IWUSR | S_IRUGO, + nau7802_sysfs_get_min_conversions, + nau7802_sysfs_set_min_conversions, 0); + +static struct attribute *nau7802_attributes[] = { + &iio_const_attr_sampling_frequency_available.dev_attr.attr, + &iio_dev_attr_min_conversions.dev_attr.attr, + NULL +}; + +static const struct attribute_group nau7802_attribute_group = { + .attrs = nau7802_attributes, +}; + +static int nau7802_set_gain(struct nau7802_state *st, int gain) +{ + u8 data; + int ret; + + mutex_lock(&st->lock); + st->conversion_count = 0; + + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); + if (data < 0) + goto nau7802_sysfs_set_gain_out; + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, + (data & (~NAU7802_CTRL1_GAINS_BITS)) | + gain); + +nau7802_sysfs_set_gain_out: + mutex_unlock(&st->lock); + + return ret ? ret : 0; +} + +static int nau7802_read_conversion(struct nau7802_state *st) +{ + u8 data; + + mutex_lock(&st->data_lock); + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B2); + if (data < 0) + goto nau7802_read_conversion_out; + st->last_value = data << 16; + + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B1); + if (data < 0) + goto nau7802_read_conversion_out; + st->last_value |= data << 8; + + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_ADC_B0); + if (data < 0) + goto nau7802_read_conversion_out; + st->last_value |= data; + + st->last_value = sign_extend32(st->last_value, 23); + +nau7802_read_conversion_out: + mutex_unlock(&st->data_lock); + + return data; +} + +/* + * Conversions are synchronised on the rising edge of NAU7802_PUCTRL_CS_BIT + */ +static int nau7802_sync(struct nau7802_state *st) +{ + int ret; + u8 data; + + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); + if (data < 0) + return data; + ret = i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, + data | NAU7802_PUCTRL_CS_BIT); + + return ret; +} + +static irqreturn_t nau7802_eoc_trigger(int irq, void *private) +{ + struct iio_dev *indio_dev = private; + struct nau7802_state *st = iio_priv(indio_dev); + u8 status; + + status = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); + if (status < 0) + return IRQ_HANDLED; + + if (!(status & NAU7802_PUCTRL_CR_BIT)) + return IRQ_NONE; + + if (nau7802_read_conversion(st) < 0) + return IRQ_HANDLED; + + /* Because there is actually only one ADC for both channels, we have to + * wait for enough conversions to happen before getting a significant + * value when changing channels and the values are far appart. + */ + if (st->conversion_count < st->min_conversions) + st->conversion_count++; + if (st->conversion_count >= st->min_conversions) + complete_all(&st->value_ok); + + return IRQ_HANDLED; +} + +static int nau7802_read_irq(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val) +{ + struct nau7802_state *st = iio_priv(indio_dev); + int ret; + + INIT_COMPLETION(st->value_ok); + enable_irq(st->client->irq); + + nau7802_sync(st); + + /* read registers to ensure we flush everything */ + ret = nau7802_read_conversion(st); + if (ret < 0) + goto read_chan_info_failure; + + /* Wait for a conversion to finish */ + ret = wait_for_completion_interruptible_timeout(&st->value_ok, + msecs_to_jiffies(1000)); + if (ret == 0) + ret = -ETIMEDOUT; + + if (ret < 0) + goto read_chan_info_failure; + + disable_irq(st->client->irq); + + *val = st->last_value; + + return IIO_VAL_INT; + +read_chan_info_failure: + disable_irq(st->client->irq); + + return ret; +} + +static int nau7802_read_poll(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val) +{ + struct nau7802_state *st = iio_priv(indio_dev); + int ret; + u8 data; + + nau7802_sync(st); + + /* read registers to ensure we flush everything */ + ret = nau7802_read_conversion(st); + if (ret < 0) + return ret; + + /* Because there is actually only one ADC for both channels, we have to + * wait for enough conversions to happen before getting a significant + * value when changing channels and the values are far appart. + */ + do { + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); + if (data < 0) + return data; + + while (!(data & NAU7802_PUCTRL_CR_BIT)) { + if (st->sample_rate != NAU7802_SAMP_FREQ_320) + msleep(20); + else + mdelay(4); + data = i2c_smbus_read_byte_data(st->client, + NAU7802_REG_PUCTRL); + if (data < 0) + return data; + } + + nau7802_read_conversion(st); + if (ret < 0) + return ret; + if (st->conversion_count < st->min_conversions) + st->conversion_count++; + } while (st->conversion_count < st->min_conversions); + + *val = st->last_value; + + return IIO_VAL_INT; +} + +static int nau7802_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct nau7802_state *st = iio_priv(indio_dev); + u8 data; + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + mutex_lock(&st->lock); + /* + * Select the channel to use + * - Channel 1 is value 0 in the CHS register + * - Channel 2 is value 1 in the CHS register + */ + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL2); + if (data < 0) + return data; + + if (((data & NAU7802_CTRL2_CHS_BIT) && !chan->channel) || + (!(data & NAU7802_CTRL2_CHS_BIT) && + chan->channel)) { + st->conversion_count = 0; + ret = i2c_smbus_write_byte_data(st->client, + NAU7802_REG_CTRL2, + NAU7802_CTRL2_CHS(chan->channel) | + NAU7802_CTRL2_CRS(st->sample_rate)); + + if (ret < 0) + return ret; + } + + if (st->client->irq) + ret = nau7802_read_irq(indio_dev, chan, val); + else + ret = nau7802_read_poll(indio_dev, chan, val); + + mutex_unlock(&st->lock); + return ret; + + case IIO_CHAN_INFO_SCALE: + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_CTRL1); + if (data < 0) + return data; + + /* we have 24 bits of signed data, that means 23 bits of data + * plus the sign bit */ + *val = st->vref_mv; + *val2 = 23 + (data & NAU7802_CTRL1_GAINS_BITS); + + return IIO_VAL_FRACTIONAL_LOG2; + + case IIO_CHAN_INFO_SAMP_FREQ: + *val = nau7802_sample_freq_avail[st->sample_rate]; + *val2 = 0; + return IIO_VAL_INT; + + default: + break; + } + + return -EINVAL; +} + +static int nau7802_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct nau7802_state *st = iio_priv(indio_dev); + int i; + + switch (mask) { + case IIO_CHAN_INFO_SCALE: + for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) + if (val2 == st->scale_avail[i]) + return nau7802_set_gain(st, i); + + break; + + case IIO_CHAN_INFO_SAMP_FREQ: + for (i = 0; i < ARRAY_SIZE(nau7802_sample_freq_avail); i++) + if (val == nau7802_sample_freq_avail[i]) { + mutex_lock(&st->lock); + st->sample_rate = i; + st->conversion_count = 0; + i2c_smbus_write_byte_data(st->client, + NAU7802_REG_CTRL2, + NAU7802_CTRL2_CRS(st->sample_rate)); + mutex_unlock(&st->lock); + return 0; + } + + break; + + default: + break; + } + + return -EINVAL; +} + +static int nau7802_write_raw_get_fmt(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + long mask) +{ + return IIO_VAL_INT_PLUS_NANO; +} + +static const struct iio_info nau7802_info = { + .driver_module = THIS_MODULE, + .read_raw = &nau7802_read_raw, + .write_raw = &nau7802_write_raw, + .write_raw_get_fmt = nau7802_write_raw_get_fmt, + .attrs = &nau7802_attribute_group, +}; + +static int nau7802_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct iio_dev *indio_dev; + struct nau7802_state *st; + struct device_node *np = client->dev.of_node; + int i, ret; + u8 data; + u32 tmp; + + if (!client->dev.of_node) { + dev_err(&client->dev, "No device tree node available.\n"); + return -EINVAL; + } + + indio_dev = iio_device_alloc(sizeof(*st)); + if (indio_dev == NULL) + return -ENOMEM; + + st = iio_priv(indio_dev); + + i2c_set_clientdata(client, indio_dev); + + indio_dev->dev.parent = &client->dev; + indio_dev->name = dev_name(&client->dev); + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &nau7802_info; + + st->client = client; + + /* Reset the device */ + i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, + NAU7802_PUCTRL_RR_BIT); + + /* Enter normal operation mode */ + i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, + NAU7802_PUCTRL_PUD_BIT); + + /* + * After about 200 usecs, the device should be ready and then + * the Power Up bit will be set to 1. If not, wait for it. + */ + udelay(210); + data = i2c_smbus_read_byte_data(st->client, NAU7802_REG_PUCTRL); + if (data < 0) + return -ENODEV; + if (!(data & NAU7802_PUCTRL_PUR_BIT)) + return -ENODEV; + + of_property_read_u32(np, "nuvoton,vldo", &tmp); + st->vref_mv = tmp; + + /* Populate available ADC input ranges */ + for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) + st->scale_avail[i] = (((u64)st->vref_mv) * 1000000000ULL) + >> (23 + i); + + data = NAU7802_PUCTRL_PUD_BIT | NAU7802_PUCTRL_PUA_BIT | + NAU7802_PUCTRL_CS_BIT; + if (tmp >= 2400) + data |= NAU7802_PUCTRL_AVDDS_BIT; + + i2c_smbus_write_byte_data(st->client, NAU7802_REG_PUCTRL, data); + i2c_smbus_write_byte_data(st->client, NAU7802_REG_ADC_CTRL, 0x30); + + if (tmp >= 2400) { + data = NAU7802_CTRL1_VLDO((4500 - tmp) / 300); + i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL1, data); + } + + st->min_conversions = NAU7802_DEFAULT_CONVERSIONS; + + /* + * The ADC fires continuously and we can't do anything about + * it. So we need to have the IRQ disabled by default, and we + * will enable them back when we will need them.. + */ + if (client->irq) { + ret = request_threaded_irq(client->irq, + NULL, + nau7802_eoc_trigger, + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, + client->dev.driver->name, + indio_dev); + if (ret) { + /* + * What may happen here is that our IRQ controller is + * not able to get level interrupt but this is required + * by this ADC as when going over 40 sample per second, + * the interrupt line may stay high between conversions. + * So, we continue no matter what but we switch to + * polling mode. + */ + dev_info(&client->dev, + "Failed to allocate IRQ, using polling mode\n"); + client->irq = 0; + } else + disable_irq(client->irq); + } + + if (!client->irq) { + /* + * We are polling, use the fastest sample rate by + * default + */ + st->sample_rate = NAU7802_SAMP_FREQ_320; + i2c_smbus_write_byte_data(st->client, NAU7802_REG_CTRL2, + NAU7802_CTRL2_CRS(st->sample_rate)); + } + + /* Setup the ADC channels available on the board */ + indio_dev->num_channels = 2; + indio_dev->channels = nau7802_chan_array; + + init_completion(&st->value_ok); + mutex_init(&st->lock); + mutex_init(&st->data_lock); + + ret = iio_device_register(indio_dev); + if (ret < 0) { + dev_err(&client->dev, "Couldn't register the device.\n"); + goto error_device_register; + } + + return 0; + +error_device_register: + mutex_destroy(&st->lock); + mutex_destroy(&st->data_lock); + if (client->irq) + free_irq(client->irq, indio_dev); + iio_device_free(indio_dev); + + return ret; +} + +static int nau7802_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(client); + struct nau7802_state *st = iio_priv(indio_dev); + + iio_device_unregister(indio_dev); + mutex_destroy(&st->lock); + mutex_destroy(&st->data_lock); + if (client->irq) + free_irq(client->irq, indio_dev); + iio_device_free(indio_dev); + + return 0; +} + +static const struct i2c_device_id nau7802_i2c_id[] = { + { "nau7802", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, nau7802_i2c_id); + +static const struct of_device_id nau7802_dt_ids[] = { + { .compatible = "nuvoton,nau7802" }, + {}, +}; +MODULE_DEVICE_TABLE(of, nau7802_dt_ids); + +static struct i2c_driver nau7802_driver = { + .probe = nau7802_probe, + .remove = nau7802_remove, + .id_table = nau7802_i2c_id, + .driver = { + .name = "nau7802", + .of_match_table = of_match_ptr(nau7802_dt_ids), + }, +}; + +module_i2c_driver(nau7802_driver); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Nuvoton NAU7802 ADC Driver"); +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>"); +MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");