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@xxxxxxxxxxxxxxxxxx>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
>>> ---
>>> .../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.
--
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
