| Message ID | 20230828-ltc2309-v3-2-338b3a8fab8b@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: add LTC2309 support | expand |
Hi Liam, kernel test robot noticed the following build warnings: [auto build test WARNING on a5e505a99ca748583dbe558b691be1b26f05d678] url: https://github.com/intel-lab-lkp/linux/commits/Liam-Beguin/dt-bindings-iio-adc-add-lltc-ltc2309-bindings/20230829-104615 base: a5e505a99ca748583dbe558b691be1b26f05d678 patch link: https://lore.kernel.org/r/20230828-ltc2309-v3-2-338b3a8fab8b%40gmail.com patch subject: [PATCH v3 2/2] iio: adc: add ltc2309 support config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230829/202308291144.r2JZxP9a-lkp@intel.com/config) compiler: sparc64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230829/202308291144.r2JZxP9a-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202308291144.r2JZxP9a-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/iio/adc/ltc2309.c:42: warning: Function parameter or member 'vref_mv' not described in 'ltc2309' vim +42 drivers/iio/adc/ltc2309.c 27 28 /** 29 * struct ltc2309 - internal device data structure 30 * @dev: Device reference 31 * @client: I2C reference 32 * @vref: External reference source 33 * @lock: Lock to serialize data access 34 * @vref_mv Internal voltage reference 35 */ 36 struct ltc2309 { 37 struct device *dev; 38 struct i2c_client *client; 39 struct regulator *vref; 40 struct mutex lock; /* serialize data access */ 41 int vref_mv; > 42 }; 43
On Mon, 28 Aug 2023 22:41:35 -0400 Liam Beguin <liambeguin@gmail.com> wrote: > The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface. > > This implements support for all single-ended and differential channels, > in unipolar mode only. > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> Hi Liam, A few really small editorial bits in here. I'll fix them whilst applying. Series applied to the togreg branch of iio.git Note I will be rebasing the tree on rc1 once available and in the meantime this will only be pushed out as testing. Thanks, Jonathan > +/** > + * struct ltc2309 - internal device data structure > + * @dev: Device reference > + * @client: I2C reference > + * @vref: External reference source > + * @lock: Lock to serialize data access > + * @vref_mv Internal voltage reference Missing : which is what the bot picked up on. > + */ > +struct ltc2309 { > + struct device *dev; > + struct i2c_client *client; > + struct regulator *vref; > + struct mutex lock; /* serialize data access */ > + int vref_mv; > +}; > + > +void ltc2309_regulator_disable(void *regulator) > +{ > + struct regulator *r = (struct regulator *)regulator; Never any need to explicitly cast from a void * to any other pointer type. (C spec says it is always fine to do this :) Given type is obvious from use, can just do regulator_disable(regulator); and lose the local variable. > + > + regulator_disable(r); > +} .. > + > +static const struct of_device_id ltc2309_of_match[] = { > + { .compatible = "lltc,ltc2309" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ltc2309_of_match); > + > +static const struct i2c_device_id ltc2309_id[] = { > + { "ltc2309" }, > + {} Trivial but space between { and } for consistency.
On Sun, 3 Sep 2023 12:43:41 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Mon, 28 Aug 2023 22:41:35 -0400 > Liam Beguin <liambeguin@gmail.com> wrote: > > > The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface. > > > > This implements support for all single-ended and differential channels, > > in unipolar mode only. > > > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> > Hi Liam, > > A few really small editorial bits in here. I'll fix them whilst applying. > Series applied to the togreg branch of iio.git > > Note I will be rebasing the tree on rc1 once available and in the meantime > this will only be pushed out as testing. A couple more static analysis reports came in (unreachable return in one pace and a missing static. I've fixed up in my tree. Thanks, Jonathan > > Thanks, > > Jonathan > > > > > +/** > > + * struct ltc2309 - internal device data structure > > + * @dev: Device reference > > + * @client: I2C reference > > + * @vref: External reference source > > + * @lock: Lock to serialize data access > > + * @vref_mv Internal voltage reference > > Missing : which is what the bot picked up on. > > > + */ > > +struct ltc2309 { > > + struct device *dev; > > + struct i2c_client *client; > > + struct regulator *vref; > > + struct mutex lock; /* serialize data access */ > > + int vref_mv; > > +}; > > > + > > +void ltc2309_regulator_disable(void *regulator) > > +{ > > + struct regulator *r = (struct regulator *)regulator; > > Never any need to explicitly cast from a void * to any other pointer type. > (C spec says it is always fine to do this :) > > Given type is obvious from use, can just do > regulator_disable(regulator); > and lose the local variable. > > > + > > + regulator_disable(r); > > +} > > .. > > + > > +static const struct of_device_id ltc2309_of_match[] = { > > + { .compatible = "lltc,ltc2309" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, ltc2309_of_match); > > + > > +static const struct i2c_device_id ltc2309_id[] = { > > + { "ltc2309" }, > > + {} > > Trivial but space between { and } for consistency. > >
Hi Jonathan, On Sun, Sep 10, 2023 at 03:03:33PM +0100, Jonathan Cameron wrote: > On Sun, 3 Sep 2023 12:43:41 +0100 > Jonathan Cameron <jic23@kernel.org> wrote: > > > On Mon, 28 Aug 2023 22:41:35 -0400 > > Liam Beguin <liambeguin@gmail.com> wrote: > > > > > The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface. > > > > > > This implements support for all single-ended and differential channels, > > > in unipolar mode only. > > > > > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> > > Hi Liam, > > > > A few really small editorial bits in here. I'll fix them whilst applying. > > Series applied to the togreg branch of iio.git > > > > Note I will be rebasing the tree on rc1 once available and in the meantime > > this will only be pushed out as testing. > > A couple more static analysis reports came in (unreachable return in one pace > and a missing static. I've fixed up in my tree. My apologies for the delay, I was away for a few days. Thanks for making those changes, I'll go over the comments and the reports, and if there's anything left, I'll send an update. > Thanks, > > Jonathan Thanks again for your time, Liam > > > > > Thanks, > > > > Jonathan > > > > > > > > > +/** > > > + * struct ltc2309 - internal device data structure > > > + * @dev: Device reference > > > + * @client: I2C reference > > > + * @vref: External reference source > > > + * @lock: Lock to serialize data access > > > + * @vref_mv Internal voltage reference > > > > Missing : which is what the bot picked up on. > > > > > + */ > > > +struct ltc2309 { > > > + struct device *dev; > > > + struct i2c_client *client; > > > + struct regulator *vref; > > > + struct mutex lock; /* serialize data access */ > > > + int vref_mv; > > > +}; > > > > > + > > > +void ltc2309_regulator_disable(void *regulator) > > > +{ > > > + struct regulator *r = (struct regulator *)regulator; > > > > Never any need to explicitly cast from a void * to any other pointer type. > > (C spec says it is always fine to do this :) > > > > Given type is obvious from use, can just do > > regulator_disable(regulator); > > and lose the local variable. > > > > > + > > > + regulator_disable(r); > > > +} > > > > .. > > > + > > > +static const struct of_device_id ltc2309_of_match[] = { > > > + { .compatible = "lltc,ltc2309" }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(of, ltc2309_of_match); > > > + > > > +static const struct i2c_device_id ltc2309_id[] = { > > > + { "ltc2309" }, > > > + {} > > > > Trivial but space between { and } for consistency. > > > > >
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index dc14bde31ac1..6ec18e02faf9 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -607,6 +607,16 @@ config LPC32XX_ADC activate only one via device tree selection. Provides direct access via sysfs. +config LTC2309 + tristate "Linear Technology LTC2309 ADC driver" + depends on I2C + help + Say yes here to build support for Linear Technology LTC2309, a low + noise, low power, 8-channel, 12-bit SAR ADC + + This driver can also be built as a module. If so, the module will + be called ltc2309. + config LTC2471 tristate "Linear Technology LTC2471 and LTC2473 ADC driver" depends on I2C diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index eb6e891790fb..fbd86184ec94 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_INTEL_MRFLD_ADC) += intel_mrfld_adc.o obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o +obj-$(CONFIG_LTC2309) += ltc2309.o obj-$(CONFIG_LTC2471) += ltc2471.o obj-$(CONFIG_LTC2485) += ltc2485.o obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c new file mode 100644 index 000000000000..3f8e1d238639 --- /dev/null +++ b/drivers/iio/adc/ltc2309.c @@ -0,0 +1,249 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface. + * + * Datasheet: + * https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf + * + * Copyright (c) 2023, Liam Beguin <liambeguin@gmail.com> + */ +#include <linux/bitfield.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/regulator/consumer.h> + +#define LTC2309_ADC_RESOLUTION 12 + +#define LTC2309_DIN_CH_MASK GENMASK(7, 4) +#define LTC2309_DIN_SDN BIT(7) +#define LTC2309_DIN_OSN BIT(6) +#define LTC2309_DIN_S1 BIT(5) +#define LTC2309_DIN_S0 BIT(4) +#define LTC2309_DIN_UNI BIT(3) +#define LTC2309_DIN_SLEEP BIT(2) + +/** + * struct ltc2309 - internal device data structure + * @dev: Device reference + * @client: I2C reference + * @vref: External reference source + * @lock: Lock to serialize data access + * @vref_mv Internal voltage reference + */ +struct ltc2309 { + struct device *dev; + struct i2c_client *client; + struct regulator *vref; + struct mutex lock; /* serialize data access */ + int vref_mv; +}; + +/* Order matches expected channel address, See datasheet Table 1. */ +enum ltc2309_channels { + LTC2309_CH0_CH1 = 0, + LTC2309_CH2_CH3, + LTC2309_CH4_CH5, + LTC2309_CH6_CH7, + LTC2309_CH1_CH0, + LTC2309_CH3_CH2, + LTC2309_CH5_CH4, + LTC2309_CH7_CH6, + LTC2309_CH0, + LTC2309_CH2, + LTC2309_CH4, + LTC2309_CH6, + LTC2309_CH1, + LTC2309_CH3, + LTC2309_CH5, + LTC2309_CH7, +}; + +#define LTC2309_CHAN(_chan, _addr) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .address = _addr, \ + .channel = _chan, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ +} + +#define LTC2309_DIFF_CHAN(_chan, _chan2, _addr) { \ + .type = IIO_VOLTAGE, \ + .differential = 1, \ + .indexed = 1, \ + .address = _addr, \ + .channel = _chan, \ + .channel2 = _chan2, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ +} + +static const struct iio_chan_spec ltc2309_channels[] = { + LTC2309_CHAN(0, LTC2309_CH0), + LTC2309_CHAN(1, LTC2309_CH1), + LTC2309_CHAN(2, LTC2309_CH2), + LTC2309_CHAN(3, LTC2309_CH3), + LTC2309_CHAN(4, LTC2309_CH4), + LTC2309_CHAN(5, LTC2309_CH5), + LTC2309_CHAN(6, LTC2309_CH6), + LTC2309_CHAN(7, LTC2309_CH7), + LTC2309_DIFF_CHAN(0, 1, LTC2309_CH0_CH1), + LTC2309_DIFF_CHAN(2, 3, LTC2309_CH2_CH3), + LTC2309_DIFF_CHAN(4, 5, LTC2309_CH4_CH5), + LTC2309_DIFF_CHAN(6, 7, LTC2309_CH6_CH7), + LTC2309_DIFF_CHAN(1, 0, LTC2309_CH1_CH0), + LTC2309_DIFF_CHAN(3, 2, LTC2309_CH3_CH2), + LTC2309_DIFF_CHAN(5, 4, LTC2309_CH5_CH4), + LTC2309_DIFF_CHAN(7, 6, LTC2309_CH7_CH6), +}; + +static int ltc2309_read_raw_channel(struct ltc2309 *ltc2309, + unsigned long address, int *val) +{ + int ret; + u16 buf; + u8 din; + + din = FIELD_PREP(LTC2309_DIN_CH_MASK, address & 0x0f) | + FIELD_PREP(LTC2309_DIN_UNI, 1) | + FIELD_PREP(LTC2309_DIN_SLEEP, 0); + + ret = i2c_smbus_write_byte(ltc2309->client, din); + if (ret < 0) { + dev_err(ltc2309->dev, "i2c command failed: %pe\n", + ERR_PTR(ret)); + return ret; + } + + ret = i2c_master_recv(ltc2309->client, (char *)&buf, 2); + if (ret < 0) { + dev_err(ltc2309->dev, "i2c read failed: %pe\n", ERR_PTR(ret)); + return ret; + } + + *val = be16_to_cpu(buf) >> 4; + + return ret; +} + +static int ltc2309_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct ltc2309 *ltc2309 = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + mutex_lock(<c2309->lock); + ret = ltc2309_read_raw_channel(ltc2309, chan->address, val); + mutex_unlock(<c2309->lock); + if (ret < 0) + return -EINVAL; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = ltc2309->vref_mv; + *val2 = LTC2309_ADC_RESOLUTION; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } +} + +static const struct iio_info ltc2309_info = { + .read_raw = ltc2309_read_raw, +}; + +void ltc2309_regulator_disable(void *regulator) +{ + struct regulator *r = (struct regulator *)regulator; + + regulator_disable(r); +} + +static int ltc2309_probe(struct i2c_client *client) +{ + struct iio_dev *indio_dev; + struct ltc2309 *ltc2309; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ltc2309)); + if (!indio_dev) + return -ENOMEM; + + ltc2309 = iio_priv(indio_dev); + ltc2309->dev = &indio_dev->dev; + ltc2309->client = client; + ltc2309->vref_mv = 4096; /* Default to the internal ref */ + + indio_dev->name = "ltc2309"; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = ltc2309_channels; + indio_dev->num_channels = ARRAY_SIZE(ltc2309_channels); + indio_dev->info = <c2309_info; + + ltc2309->vref = devm_regulator_get_optional(&client->dev, "vref"); + if (IS_ERR(ltc2309->vref)) { + ret = PTR_ERR(ltc2309->vref); + if (ret == -ENODEV) + ltc2309->vref = NULL; + else + return ret; + } + + if (ltc2309->vref) { + ret = regulator_enable(ltc2309->vref); + if (ret) + return dev_err_probe(ltc2309->dev, ret, + "failed to enable vref\n"); + + ret = devm_add_action_or_reset(ltc2309->dev, + ltc2309_regulator_disable, + ltc2309->vref); + if (ret) { + return dev_err_probe(ltc2309->dev, ret, + "failed to add regulator_disable action: %d\n", + ret); + return ret; + } + + ret = regulator_get_voltage(ltc2309->vref); + if (ret < 0) + return ret; + + ltc2309->vref_mv = ret / 1000; + } + + mutex_init(<c2309->lock); + + return devm_iio_device_register(&client->dev, indio_dev); +} + +static const struct of_device_id ltc2309_of_match[] = { + { .compatible = "lltc,ltc2309" }, + { } +}; +MODULE_DEVICE_TABLE(of, ltc2309_of_match); + +static const struct i2c_device_id ltc2309_id[] = { + { "ltc2309" }, + {} +}; +MODULE_DEVICE_TABLE(i2c, ltc2309_id); + +static struct i2c_driver ltc2309_driver = { + .driver = { + .name = "ltc2309", + .of_match_table = ltc2309_of_match, + }, + .probe = ltc2309_probe, + .id_table = ltc2309_id, +}; +module_i2c_driver(ltc2309_driver); + +MODULE_AUTHOR("Liam Beguin <liambeguin@gmail.com>"); +MODULE_DESCRIPTION("Linear Technology LTC2309 ADC"); +MODULE_LICENSE("GPL v2");
The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface. This implements support for all single-ended and differential channels, in unipolar mode only. Signed-off-by: Liam Beguin <liambeguin@gmail.com> --- drivers/iio/adc/Kconfig | 10 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ltc2309.c | 249 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 260 insertions(+)