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.bell...@free-electrons.com> >> Signed-off-by: Maxime Ripard <maxime.rip...@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 ! -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/