Messages in this thread | | | Date | Mon, 24 Jun 2013 08:41:39 +0200 | From | Lars-Peter Clausen <> | Subject | Re: [PATCHv2 1/3] iio: Add Nuvoton NAU7802 ADC driver |
| |
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.
[...] > +}
| |