| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <51C7EA23.1040707@metafoo.de>
Date: Mon, 24 Jun 2013 08:41:39 +0200
From: Lars-Peter Clausen <lars@...afoo.de>
To: Alexandre Belloni <alexandre.belloni@...e-electrons.com>
CC: Shawn Guo <shawn.guo@...aro.org>,
Grant Likely <grant.likely@...retlab.ca>, jimwall@...om,
brian@...stalfontz.com,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
devicetree-discuss@...ts.ozlabs.org,
Jonathan Cameron <jic23@....ac.uk>,
Rob Landley <rob@...dley.net>,
Rob Herring <rob.herring@...xeda.com>
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@...e-electrons.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@...e-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.
[...]
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists