On Thu, 23 Mar 2017, michael.henner...@analog.com wrote:

> From: Michael Hennerich <michael.henner...@analog.com>

comments below
 
> Signed-off-by: Michael Hennerich <michael.henner...@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/ltc2497.txt        |  13 +
>  MAINTAINERS                                        |   1 +
>  drivers/iio/adc/Kconfig                            |  11 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/ltc2497.c                          | 263 
> +++++++++++++++++++++
>  5 files changed, 289 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt
>  create mode 100644 drivers/iio/adc/ltc2497.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt 
> b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> new file mode 100644
> index 0000000..c2829c19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> @@ -0,0 +1,13 @@
> +* Linear Technology / Analog Devices LTC2497 ADC
> +
> +Required properties:
> + - compatible: Should be "lltc,ltc2497"
> + - reg: Should contain the ADC I2C address
> + - vref-supply: The regulator supply for ADC reference voltage
> +
> +Example:
> +     ltc2497: adc@76 {
> +             compatible = "lltc,ltc2497";
> +             reg = <0x76>;
> +             vref-supply = <&ltc2497_reg>;
> +     };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a7d6f9a..173043c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -813,6 +813,7 @@ W:        http://wiki.analog.com/
>  W:   http://ez.analog.com/community/linux-device-drivers
>  S:   Supported
>  F:   drivers/iio/*/ad*
> +F:   drivers/iio/adc/ltc2497*
>  X:   drivers/iio/*/adjd*
>  F:   drivers/staging/iio/*/ad*
>  F:   drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2268a6f..141cf4a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -326,6 +326,17 @@ config LTC2485
>         To compile this driver as a module, choose M here: the module will be
>         called ltc2485.
>  
> +config LTC2497
> +     tristate "Linear Technology LTC2497 ADC driver"
> +     depends on I2C
> +     help
> +       Say yes here to build support for Linear Technology LTC2497
> +       16-Bit 8-/16-Channel Delta Sigma ADC.
> +       Provides direct access via sysfs

the line "Provides..." should be removed I think, it confuses me at least

> +
> +       To compile this driver as a module, choose M here: the module will be
> +       called ltc2497.
> +
>  config MAX1027
>       tristate "Maxim max1027 ADC driver"
>       depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 73dbe39..9d626b5 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
> +obj-$(CONFIG_LTC2497) += ltc2497.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1363) += max1363.o
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> new file mode 100644
> index 0000000..0452715
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -0,0 +1,263 @@
> +/*
> + * ltc2497.c - Driver for Linear Technology LTC2497 ADC
> + *
> + * Copyright (C) 2017 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/of.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define LTC2497_ENABLE                       0xA0
> +#define LTC2497_SGL                  (1 << 4)
> +#define LTC2497_DIFF                 (0 << 4)
> +#define LTC2497_SIGN                 (1 << 3)
> +#define LTC2497_CONFIG_DEFAULT               LTC2497_ENABLE
> +#define LTC2497_CONVERSION_TIME_MS   150ULL
 
is the ULL really needed?

> +struct ltc2497_st {
> +     struct i2c_client *client;
> +     struct regulator *ref;
> +     ktime_t time_prev;
> +     u8 addr_prev;
> +};
> +
> +static int ltc2497_wait_conv(struct ltc2497_st *st)
> +{
> +     s64 time_elapsed;
> +
> +     time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
> +
> +     if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
> +             /* delay if conversion time not passed
> +              * since last read or write
> +              */
> +             msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);
> +             return 0;
> +     }
> +
> +     if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
> +             /* We're in automatic mode -
> +              * so the last reading is stil not outdated
> +              */
> +             return 0;
> +     }
> +
> +     return -ETIMEDOUT;
> +}
> +
> +static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
> +{
> +     struct i2c_client *client = st->client;
> +     __be32 buf = 0;
> +     int ret;
> +
> +     ret = ltc2497_wait_conv(st);
> +     if (ret < 0 || st->addr_prev != address) {
> +             ret = i2c_smbus_write_byte(st->client, 0xA0 | address);

use LTC2497_ENABLE instead of 0xa0

> +             if (ret < 0)
> +                     return ret;
> +             st->addr_prev = address;
> +             msleep(LTC2497_CONVERSION_TIME_MS);
> +     }
> +     ret = i2c_master_recv(client, (char *)&buf, 3);
> +     if (ret < 0)  {
> +             dev_err(&client->dev, "i2c_master_recv failed\n");
> +             return ret;
> +     }
> +     st->time_prev = ktime_get();
> +     *val = (be32_to_cpu(buf) >> 14) - (1 << 17);

what is the purpose of - (1 << 17)? maybe add a comment?
I'd rather do it with bit operations that arithmetic

> +
> +     return ret;
> +}
> +
> +static int ltc2497_read_raw(struct iio_dev *indio_dev,
> +                         struct iio_chan_spec const *chan,
> +                         int *val, int *val2, long mask)
> +{
> +     struct ltc2497_st *st = iio_priv(indio_dev);
> +     int ret;
> +
> +     switch (mask) {
> +     case IIO_CHAN_INFO_RAW:
> +             mutex_lock(&indio_dev->mlock);
> +             ret = ltc2497_read(st, chan->address, val);
> +             mutex_unlock(&indio_dev->mlock);
> +             if (ret < 0)
> +                     return ret;
> +
> +             return IIO_VAL_INT;
> +
> +     case IIO_CHAN_INFO_SCALE:
> +             ret = regulator_get_voltage(st->ref);
> +             if (ret < 0)
> +                     return ret;
> +
> +             *val = ret / 1000;
> +             *val2 = 17;
> +
> +             return IIO_VAL_FRACTIONAL_LOG2;
> +
> +     default:
> +             return -EINVAL;
> +     }
> +}
> +
> +#define LTC2497_CHAN(_chan, _addr) { \
> +     .type = IIO_VOLTAGE, \
> +     .indexed = 1, \
> +     .channel = (_chan), \
> +     .address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \
> +     .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +     .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +#define LTC2497_CHAN_DIFF(_chan, _addr) { \
> +     .type = IIO_VOLTAGE, \
> +     .indexed = 1, \
> +     .channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \
> +     .channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\
> +     .address = (_addr | _chan), \
> +     .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +     .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +     .differential = 1, \
> +}
> +
> +static const struct iio_chan_spec ltc2497_channel[] = {
> +     LTC2497_CHAN(0, LTC2497_SGL),
> +     LTC2497_CHAN(1, LTC2497_SGL),
> +     LTC2497_CHAN(2, LTC2497_SGL),
> +     LTC2497_CHAN(3, LTC2497_SGL),
> +     LTC2497_CHAN(4, LTC2497_SGL),
> +     LTC2497_CHAN(5, LTC2497_SGL),
> +     LTC2497_CHAN(6, LTC2497_SGL),
> +     LTC2497_CHAN(7, LTC2497_SGL),
> +     LTC2497_CHAN(8, LTC2497_SGL),
> +     LTC2497_CHAN(9, LTC2497_SGL),
> +     LTC2497_CHAN(10, LTC2497_SGL),
> +     LTC2497_CHAN(11, LTC2497_SGL),
> +     LTC2497_CHAN(12, LTC2497_SGL),
> +     LTC2497_CHAN(13, LTC2497_SGL),
> +     LTC2497_CHAN(14, LTC2497_SGL),
> +     LTC2497_CHAN(15, LTC2497_SGL),
> +     LTC2497_CHAN_DIFF(0, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(1, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(2, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(3, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(4, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(5, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(6, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(7, LTC2497_DIFF),
> +     LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN),
> +     LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
> +};
> +
> +static const struct iio_info ltc2497_info = {
> +     .read_raw = ltc2497_read_raw,
> +     .driver_module = THIS_MODULE,
> +};
> +
> +static int ltc2497_probe(struct i2c_client *client,
> +                      const struct i2c_device_id *id)
> +{
> +     struct iio_dev *indio_dev;
> +     struct ltc2497_st *st;
> +     int ret;
> +
> +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
> +                                  I2C_FUNC_SMBUS_WRITE_BYTE))
> +             return -EOPNOTSUPP;
> +
> +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> +     if (!indio_dev)
> +             return -ENOMEM;
> +
> +     st = iio_priv(indio_dev);
> +     i2c_set_clientdata(client, indio_dev);
> +     st->client = client;
> +
> +     indio_dev->dev.parent = &client->dev;
> +     indio_dev->name = id->name;
> +     indio_dev->info = &ltc2497_info;
> +     indio_dev->modes = INDIO_DIRECT_MODE;
> +     indio_dev->channels = ltc2497_channel;
> +     indio_dev->num_channels = ARRAY_SIZE(ltc2497_channel);
> +
> +     st->ref = devm_regulator_get(&client->dev, "vref");
> +     if (IS_ERR(st->ref))
> +             return PTR_ERR(st->ref);
> +
> +     ret = regulator_enable(st->ref);
> +     if (ret < 0)
> +             return ret;
> +
> +     ret = i2c_smbus_write_byte(st->client, LTC2497_CONFIG_DEFAULT);
> +     if (ret < 0)
> +             goto err_regulator_disable;
> +
> +     st->addr_prev = LTC2497_CONFIG_DEFAULT;
> +     st->time_prev = ktime_get();
> +
> +     ret = devm_iio_device_register(&client->dev, indio_dev);

no devm_ since we have a non-empty _remove()?

> +     if (ret < 0)
> +             goto err_regulator_disable;
> +
> +     return 0;
> +
> +err_regulator_disable:
> +     regulator_disable(st->ref);
> +
> +     return ret;
> +}
> +
> +static int ltc2497_remove(struct i2c_client *client)
> +{
> +     struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +     struct ltc2497_st *st = iio_priv(indio_dev);
> +
> +     regulator_disable(st->ref);
> +
> +     return 0;
> +}
> +
> +static const struct i2c_device_id ltc2497_id[] = {
> +     { "ltc2497", 0 },
> +     { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc2497_id);
> +
> +static const struct of_device_id ltc2497_of_match[] = {
> +     { .compatible = "lltc,ltc2497", },
> +     {},
> +};
> +MODULE_DEVICE_TABLE(of, ltc2497_of_match);
> +
> +static struct i2c_driver ltc2497_driver = {
> +     .driver = {
> +             .name = "ltc2497",
> +             .of_match_table = of_match_ptr(ltc2497_of_match),
> +     },
> +     .probe = ltc2497_probe,
> +     .remove = ltc2497_remove,
> +     .id_table = ltc2497_id,
> +};
> +module_i2c_driver(ltc2497_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <michael.henner...@analog.com>");
> +MODULE_DESCRIPTION("Linear Technology LTC2497 ADC driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

Reply via email to