On 11 January 2017 09:17:11 GMT+00:00, Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx> wrote:
>On Wed, 11 Jan 2017, Phil Reid wrote:
>
>> Oops, title should be PATCH V2.
>>
>> On 11/01/2017 14:51, Phil Reid wrote:
>> > This adds TI's tlc4541 16-bit ADC driver. Which is a single channel
>> > ADC. Supports raw and trigger buffer access.
>> > Also supports the tlc3541 14-bit device, which has not been tested.
>> > Implementation of the tlc3541 is fairly straight forward thou.
>
>comments below
>
>>
>> > Signed-off-by: Phil Reid <preid@xxxxxxxxxxxxxxxxx>
>> > ---
>> >
>> > Notes:
>> > Changes from v1:
>> > - Add tlc3541 support and chan spec.
>> > - remove fields that where already 0 from TLC4541_V_CHAN macro
>> > - Increase rx_buf size in tlc4541_state to avoid copy in
>> > tlc4541_trigger_handle
>> > - Remove erroneous be16_to_cpu in tlc4541_trigger_handle
>> > - Docs/binding: spi -> SPI & add ti,tlc3541
>> >
>> > I haven't add Rob's Ack due to adding a new compatible string.
>> >
>> > I tried to ".index = 1" from the spec as suggested by Peter,
>but that
>> > didn't
>> > seem to work. Perhaps remove of .channel was the intended
>target.
>
>the only between index = 0/1 should be that the channel is called
>in_voltage0_raw vs in_voltage_raw in sysfs -- maybe there is an issue
>in
>iio_readdev?
>
>> > Example output from iio_readdev
>> >
>> > with ".index = 1"
>> > root@cyclone5:~# mkdir
>/sys/kernel/config/iio/triggers/hrtimer/hr1
>> > root@cyclone5:~# iio_readdev -t hr1 -b 32 -s 10 tlc4541 |
>hexdump
>> > WARNING: High-speed mode not enabled
>> > 0000000 af00 0000 0000 0000 b922 ca99 93da 1492
>> > 0000010 a800 00ff 0000 0000 b246 cb30 93da 1492
>> > 0000020 a900 0000 0000 0000 4f9c cbc9 93da 1492
>> > 0000030 aa00 00ff 0000 0000 bd2c cc61 93da 1492
>> > 0000040 aa00 00ff 0000 0000 544c ccfa 93da 1492
>> > 0000050 ab00 00ff 0000 0000 e806 cd92 93da 1492
>> > 0000060 a900 00ff 0000 0000 846c ce2b 93da 1492
>> > 0000070 ab00 0000 0000 0000 2efc cec8 93da 1492
>> > 0000080 a800 00ff 0000 0000 b090 cf5c 93da 1492
>> > 0000090 a900 00ff 0000 0000 476a cff5 93da 1492
>> >
>> > without .index
>> > root@cyclone5:~# mkdir
>/sys/kernel/config/iio/triggers/hrtimer/hr1
>> > root@cyclone5:~# iio_readdev -t hr1 -b 32 -s 10 tlc4541 |
>hexdump
>> > WARNING: High-speed mode not enabled
>> > 0000000 6db0 eeb6 93e3 1492 35e0 ef4f 93e3 1492
>> > 0000010 4b34 efe5 93e3 1492 e9f2 f07d 93e3 1492
>> > 0000020 6182 f116 93e3 1492 090a f1af 93e3 1492
>> > 0000030 409c f249 93e3 1492 6c1a f2e0 93e3 1492
>> > 0000040 cd02 f378 93e3 1492 9582 f411 93e3 1492
>> >
>> > .../devicetree/bindings/iio/adc/ti-tlc4541.txt | 17 ++
>> > drivers/iio/adc/Kconfig | 11 +
>> > drivers/iio/adc/Makefile | 1 +
>> > drivers/iio/adc/ti-tlc4541.c | 276
>> > +++++++++++++++++++++
>> > 4 files changed, 305 insertions(+)
>> > create mode 100644
>Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > create mode 100644 drivers/iio/adc/ti-tlc4541.c
>> >
>> > diff --git
>a/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > new file mode 100644
>> > index 0000000..e1de2bd
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > @@ -0,0 +1,17 @@
>> > +* Texas Instruments' TLC4541
>> > +
>> > +Required properties:
>> > + - compatible: Should be one of
>> > + * "ti,tlc4541"
>> > + * "ti,tlc3541"
>> > + - reg: SPI chip select number for the device
>> > + - vref-supply: The regulator supply for ADC reference voltage
>> > + - spi-max-frequency: Max SPI frequency to use (<= 200000)
>> > +
>> > +Example:
>> > +adc@0 {
>> > + compatible = "ti,adc0832";
>
>pasto here, should be ti,tlc4541 probably
>
>> > + reg = <0>;
>> > + vref-supply = <&vdd_supply>;
>> > + spi-max-frequency = <200000>;
>> > +};
>> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> > index 99c0514..4dda3f0 100644
>> > --- a/drivers/iio/adc/Kconfig
>> > +++ b/drivers/iio/adc/Kconfig
>> > @@ -525,6 +525,17 @@ config TI_AM335X_ADC
>> > To compile this driver as a module, choose M here: the module
>will
>> > be
>> > called ti_am335x_adc.
>> >
>> > +config TI_TLC4541
>> > + tristate "Texas Instruments TLC4541 ADC driver"
>> > + depends on SPI
>> > + select IIO_BUFFER
>> > + select IIO_TRIGGERED_BUFFER
>> > + help
>> > + Say yes here to build support for Texas Instruments TLC4541 ADC
>
>mention TLC3541 here as well?
>
>> > chip.
>> > +
>> > + This driver can also be built as a module. If so, the module
>will be
>> > + called ti-tlc4541.
>> > +
>> > config TWL4030_MADC
>> > tristate "TWL4030 MADC (Monitoring A/D Converter)"
>> > depends on TWL4030_CORE
>> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> > index 7a40c04..9bf2377 100644
>> > --- a/drivers/iio/adc/Makefile
>> > +++ b/drivers/iio/adc/Makefile
>> > @@ -49,6 +49,7 @@ obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
>> > obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>> > obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>> > obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>> > +obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
>> > obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
>> > obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
>> > obj-$(CONFIG_VF610_ADC) += vf610_adc.o
>> > diff --git a/drivers/iio/adc/ti-tlc4541.c
>b/drivers/iio/adc/ti-tlc4541.c
>> > new file mode 100644
>> > index 0000000..a0cd5e1
>> > --- /dev/null
>> > +++ b/drivers/iio/adc/ti-tlc4541.c
>> > @@ -0,0 +1,276 @@
>> > +/*
>> > + * TI tlc4541 ADC Driver
>> > + *
>> > + * Copyright (C) 2017 Phil Reid
>> > + *
>> > + * Datasheets can be found here:
>> > + * http://www.ti.com/lit/gpn/tlc3541
>> > + * http://www.ti.com/lit/gpn/tlc4541
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>modify
>> > + * it under the terms of the GNU General Public License version 2
>as
>> > + * published by the Free Software Foundation.
>> > + *
>> > + * The tlc4541 requires 24 clock cycles to start a transfer.
>> > + * Conversion then takes 2.94us to complete before data is ready
>> > + * Data is returned MSB first.
>> > + */
>> > +
>> > +#include <linux/delay.h>
>> > +#include <linux/device.h>
>> > +#include <linux/err.h>
>> > +#include <linux/interrupt.h>
>> > +#include <linux/iio/iio.h>
>> > +#include <linux/iio/sysfs.h>
>> > +#include <linux/iio/buffer.h>
>> > +#include <linux/iio/trigger_consumer.h>
>> > +#include <linux/iio/triggered_buffer.h>
>> > +#include <linux/kernel.h>
>> > +#include <linux/module.h>
>> > +#include <linux/regulator/consumer.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/spi/spi.h>
>> > +#include <linux/sysfs.h>
>> > +
>> > +struct tlc4541_state {
>> > + struct spi_device *spi;
>> > + struct regulator *reg;
>> > + struct spi_transfer scan_single_xfer[3];
>> > + struct spi_message scan_single_msg;
>> > +
>> > + /*
>> > + * DMA (thus cache coherency maintenance) requires the
>> > + * transfer buffers to live in their own cache lines.
>> > + * 2 bytes data + 6 bytes padding + 8 bytes timestamp when
>> > + * call iio_push_to_buffers_with_timestamp.
>> > + */
>> > + __be16 rx_buf[8] ____cacheline_aligned;
>> > +};
>> > +
>> > +struct tlc4541_chip_info {
>> > + const struct iio_chan_spec *channels;
>> > + unsigned int num_channels;
>> > +};
>> > +
>> > +enum tlc4541_id {
>> > + TLC3541,
>> > + TLC4541,
>> > +};
>> > +
>> > +#define TLC4541_V_CHAN(bits, bitshift) {
> \
>> > + .type = IIO_VOLTAGE, \
>> > + .indexed = 1, \
>
>shouldn't be needed
>
>> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> > + .scan_type = { \
>> > + .sign = 'u', \
>> > + .realbits = (bits), \
>> > + .storagebits = 16, \
>> > + .shift = bitshift, \
>
>(bitshift)
>
>> > + .endianness = IIO_BE, \
>> > + }, \
>> > + }
>> > +
>> > +#define DECLARE_TLC4541_CHANNELS(name, bits, bitshift) \
>> > +const struct iio_chan_spec name ## _channels[] = { \
>> > + TLC4541_V_CHAN(bits, bitshift), \
>> > + IIO_CHAN_SOFT_TIMESTAMP(1), \
>> > +}
>> > +
>> > +static DECLARE_TLC4541_CHANNELS(tlc4541, 16, 0);
>> > +static DECLARE_TLC4541_CHANNELS(tlc3541, 14, 2);
>
>maybe always keep the chip variants in the same order, the enum has
>3541
>first
>
>> > +
>> > +static const struct tlc4541_chip_info tlc4541_chip_info[] = {
>> > + [TLC4541] = {
>> > + .channels = tlc4541_channels,
>> > + .num_channels = ARRAY_SIZE(tlc4541_channels),
>> > + },
>> > + [TLC3541] = {
>> > + .channels = tlc3541_channels,
>> > + .num_channels = ARRAY_SIZE(tlc3541_channels),
>> > + },
>> > +};
>> > +
>> > +static irqreturn_t tlc4541_trigger_handler(int irq, void *p)
>> > +{
>> > + struct iio_poll_func *pf = p;
>> > + struct iio_dev *indio_dev = pf->indio_dev;
>> > + struct tlc4541_state *st = iio_priv(indio_dev);
>> > + int ret;
>> > +
>> > + ret = spi_sync(st->spi, &st->scan_single_msg);
>> > + if (ret < 0)
>> > + goto done;
>> > +
>> > + iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
>> > + iio_get_time_ns(indio_dev));
>> > +
>> > +done:
>> > + iio_trigger_notify_done(indio_dev->trig);
>> > + return IRQ_HANDLED;
>> > +}
>> > +
>> > +static int tlc4541_get_range(struct tlc4541_state *st)
>> > +{
>> > + int vref;
>> > +
>> > + vref = regulator_get_voltage(st->reg);
>> > + if (vref < 0)
>> > + return vref;
>> > +
>> > + vref /= 1000;
>> > +
>> > + return vref;
>> > +}
>> > +
>> > +static int tlc4541_read_raw(struct iio_dev *indio_dev,
>> > + struct iio_chan_spec const *chan,
>> > + int *val,
>> > + int *val2,
>> > + long m)
>> > +{
>> > + int ret = 0;
>> > + struct tlc4541_state *st = iio_priv(indio_dev);
>> > +
>> > + switch (m) {
>> > + case IIO_CHAN_INFO_RAW:
>> > + ret = iio_device_claim_direct_mode(indio_dev);
>> > + if (ret)
>> > + return ret;
>> > + ret = spi_sync(st->spi, &st->scan_single_msg);
>> > + iio_device_release_direct_mode(indio_dev);
>> > + if (ret < 0)
>> > + return ret;
>> > + *val = be16_to_cpu(st->rx_buf[0]);
>> > + *val = *val >> chan->scan_type.shift;
>> > + *val &= GENMASK(chan->scan_type.realbits - 1, 0);
>
>is the GENMASK() necessary?, the trigger handler doesn't do it
>
>> > + return IIO_VAL_INT;
>> > + case IIO_CHAN_INFO_SCALE:
>> > + ret = tlc4541_get_range(st);
>> > + if (ret < 0)
>> > + return ret;
>> > + *val = ret;
>> > + *val2 = chan->scan_type.realbits;
>> > + return IIO_VAL_FRACTIONAL_LOG2;
>> > + }
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static const struct iio_info tlc4541_info = {
>> > + .read_raw = &tlc4541_read_raw,
>> > + .driver_module = THIS_MODULE,
>> > +};
>> > +
>> > +static int tlc4541_probe(struct spi_device *spi)
>> > +{
>> > + struct tlc4541_state *st;
>> > + struct iio_dev *indio_dev;
>> > + const struct tlc4541_chip_info *info;
>> > + int ret;
>> > + int8_t device_init = 0;
>> > +
>> > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>
>wondering what happens to the cache aligned rx_buf here?
Fine as we are careful to also align the private data so this works.
>
>> > + if (indio_dev == NULL)
>> > + return -ENOMEM;
>> > +
>> > + st = iio_priv(indio_dev);
>> > +
>> > + spi_set_drvdata(spi, indio_dev);
>> > +
>> > + st->spi = spi;
>> > +
>> > + info = &tlc4541_chip_info[spi_get_device_id(spi)->driver_data];
>> > +
>> > + indio_dev->name = spi_get_device_id(spi)->name;
>> > + indio_dev->dev.parent = &spi->dev;
>> > + indio_dev->modes = INDIO_DIRECT_MODE;
>> > + indio_dev->channels = info->channels;
>> > + indio_dev->num_channels = info->num_channels;
>> > + indio_dev->info = &tlc4541_info;
>> > +
>> > + /* perform reset */
>> > + spi_write(spi, &device_init, 1);
>> > +
>> > + /* Setup default message */
>> > + st->scan_single_xfer[0].rx_buf = &st->rx_buf[0];
>> > + st->scan_single_xfer[0].len = 3;
>> > + st->scan_single_xfer[1].delay_usecs = 3;
>> > + st->scan_single_xfer[2].rx_buf = &st->rx_buf[0];
>> > + st->scan_single_xfer[2].len = 2;
>> > +
>> > + spi_message_init(&st->scan_single_msg);
>> > + spi_message_add_tail(&st->scan_single_xfer[0],
>&st->scan_single_msg);
>> > + spi_message_add_tail(&st->scan_single_xfer[1],
>&st->scan_single_msg);
>> > + spi_message_add_tail(&st->scan_single_xfer[2],
>&st->scan_single_msg);
>> > +
>> > + st->reg = devm_regulator_get(&spi->dev, "vref");
>> > + if (IS_ERR(st->reg))
>> > + return PTR_ERR(st->reg);
>> > +
>> > + ret = regulator_enable(st->reg);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> > + &tlc4541_trigger_handler, NULL);
>> > + if (ret)
>> > + goto error_disable_reg;
>> > +
>> > + ret = iio_device_register(indio_dev);
>> > + if (ret)
>> > + goto error_cleanup_buffer;
>> > +
>> > + return 0;
>> > +
>> > +error_cleanup_buffer:
>> > + iio_triggered_buffer_cleanup(indio_dev);
>> > +error_disable_reg:
>> > + regulator_disable(st->reg);
>> > +
>> > + return ret;
>> > +}
>> > +
>> > +static int tlc4541_remove(struct spi_device *spi)
>> > +{
>> > + struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> > + struct tlc4541_state *st = iio_priv(indio_dev);
>> > +
>> > + iio_device_unregister(indio_dev);
>> > + iio_triggered_buffer_cleanup(indio_dev);
>> > + regulator_disable(st->reg);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +#ifdef CONFIG_OF
>
>maybe drop the newlines here?
>
>> > +
>> > +static const struct of_device_id tlc4541_dt_ids[] = {
>> > + { .compatible = "ti,tlc3541", },
>> > + { .compatible = "ti,tlc4541", },
>> > + {}
>> > +};
>> > +MODULE_DEVICE_TABLE(of, tlc4541_dt_ids);
>> > +
>> > +#endif
>> > +
>> > +static const struct spi_device_id tlc4541_id[] = {
>> > + {"tlc3541", TLC3541},
>> > + {"tlc4541", TLC4541},
>> > + {}
>> > +};
>> > +MODULE_DEVICE_TABLE(spi, tlc4541_id);
>> > +
>> > +static struct spi_driver tlc4541_driver = {
>> > + .driver = {
>> > + .name = "tlc4541",
>> > + .of_match_table = of_match_ptr(tlc4541_dt_ids),
>> > + },
>> > + .probe = tlc4541_probe,
>> > + .remove = tlc4541_remove,
>> > + .id_table = tlc4541_id,
>> > +};
>> > +module_spi_driver(tlc4541_driver);
>> > +
>> > +MODULE_AUTHOR("Phil Reid <preid@xxxxxxxxxxxxxxxxx>");
>> > +MODULE_DESCRIPTION("Texas Instruments TLC4541 ADC");
>> > +MODULE_LICENSE("GPL v2");
>> >
>>
>>
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
