| Message ID | 20240603012200.16589-6-kimseer.paller@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add driver for LTC2664 and LTC2672 | expand |
On Mon, 2024-06-03 at 09:22 +0800, Kim Seer Paller wrote: > LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC > LTC2672 5 channel, 16 bit Current Output Softspan DAC > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202405241141.kYcxrSem-lkp@intel.com/ The above could be dropped. This is still not merged code :) > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > --- LGTM... just a couple of minor points/questions that you can maybe take on if a re- spin is needed. Reviewed-by: Nuno Sa <nuno.sa@analog.com> ... > > +static int ltc2664_scale_get(const struct ltc2664_state *st, int c) > +{ > + const struct ltc2664_chan *chan = &st->channels[c]; > + const int (*span_helper)[2] = st->chip_info->span_helper; > + int span, fs; > + > + span = chan->span; > + if (span < 0) > + return span; > + > + fs = span_helper[span][1] - span_helper[span][0]; > + > + return (fs / 2500) * st->vref; no need for () ... > > +static int ltc2664_channel_config(struct ltc2664_state *st) > +{ > + const struct ltc2664_chip_info *chip_info = st->chip_info; > + struct device *dev = &st->spi->dev; > + u32 reg, tmp[2], mspan; > + int ret, span = 0; > + > + mspan = LTC2664_MSPAN_SOFTSPAN; > + ret = device_property_read_u32(dev, "adi,manual-span-operation-config", > + &mspan); > + if (!ret) { > + if (!chip_info->manual_span_support) > + return dev_err_probe(dev, -EINVAL, > + "adi,manual-span-operation-config not > supported\n"); > + > + if (mspan > ARRAY_SIZE(ltc2664_mspan_lut)) > + return dev_err_probe(dev, -EINVAL, > + "adi,manual-span-operation-config not in range\n"); > + } > + > + st->rfsadj = 20000; > + ret = device_property_read_u32(dev, "adi,rfsadj-ohms", &st->rfsadj); > + if (!ret) { > + if (!chip_info->rfsadj_support) > + return dev_err_probe(dev, -EINVAL, > + "adi,rfsadj-ohms not supported\n"); > + > + if (st->rfsadj < 19000 || st->rfsadj > 41000) > + return dev_err_probe(dev, -EINVAL, > + "adi,rfsadj-ohms not in range\n"); > + } > + > + device_for_each_child_node_scoped(dev, child) { > + struct ltc2664_chan *chan; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to get reg property\n"); > + > + if (reg >= chip_info->num_channels) > + return dev_err_probe(dev, -EINVAL, > + "reg bigger than: %d\n", > + chip_info->num_channels); > + > + chan = &st->channels[reg]; > + > + if (fwnode_property_read_bool(child, "adi,toggle-mode")) { > + chan->toggle_chan = true; > + /* assume sw toggle ABI */ Do we have any other option :)? For the ltc2668 driver (where this code came from), we do have another way (driven by clocks) to toggle between outputs and hence the comment. BTW, there's a fair amount of duplicated code between this and ltc2668. At some point we may see if it makes sense to add some common module. Anyways, fine for now. - Nuno Sá
On 6/2/24 8:22 PM, Kim Seer Paller wrote: > LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC > LTC2672 5 channel, 16 bit Current Output Softspan DAC > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202405241141.kYcxrSem-lkp@intel.com/ > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > --- > MAINTAINERS | 1 + > drivers/iio/dac/Kconfig | 11 + > drivers/iio/dac/Makefile | 1 + > drivers/iio/dac/ltc2664.c | 806 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 819 insertions(+) > create mode 100644 drivers/iio/dac/ltc2664.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ac1e29e26f31..1262e1231923 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13071,6 +13071,7 @@ S: Supported > W: https://ez.analog.com/linux-software-drivers > F: Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml > F: Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > +F: drivers/iio/dac/ltc2664.c > > LTC2688 IIO DAC DRIVER > M: Nuno Sá <nuno.sa@analog.com> > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > index 3c2bf620f00f..3d065c157605 100644 > --- a/drivers/iio/dac/Kconfig > +++ b/drivers/iio/dac/Kconfig > @@ -370,6 +370,17 @@ config LTC2632 > To compile this driver as a module, choose M here: the > module will be called ltc2632. > > +config LTC2664 > + tristate "Analog Devices LTC2664 and LTC2672 DAC SPI driver" > + depends on SPI > + select REGMAP > + help > + Say yes here to build support for Analog Devices > + LTC2664 and LTC2672 converters (DAC). > + > + To compile this driver as a module, choose M here: the > + module will be called ltc2664. > + > config M62332 > tristate "Mitsubishi M62332 DAC driver" > depends on I2C > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile > index 8432a81a19dc..2cf148f16306 100644 > --- a/drivers/iio/dac/Makefile > +++ b/drivers/iio/dac/Makefile > @@ -37,6 +37,7 @@ obj-$(CONFIG_DS4424) += ds4424.o > obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o > obj-$(CONFIG_LTC1660) += ltc1660.o > obj-$(CONFIG_LTC2632) += ltc2632.o > +obj-$(CONFIG_LTC2664) += ltc2664.o > obj-$(CONFIG_LTC2688) += ltc2688.o > obj-$(CONFIG_M62332) += m62332.o > obj-$(CONFIG_MAX517) += max517.o > diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c > new file mode 100644 > index 000000000000..ef5d7d6fec5a > --- /dev/null > +++ b/drivers/iio/dac/ltc2664.c > @@ -0,0 +1,806 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC driver > + * LTC2672 5 channel, 16 bit Current Output Softspan DAC driver > + * > + * Copyright 2024 Analog Devices Inc. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/cleanup.h> > +#include <linux/device.h> > +#include <linux/gpio/consumer.h> > +#include <linux/iio/iio.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/mutex.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > + > +#define LTC2664_CMD_WRITE_N(n) (0x00 + (n)) > +#define LTC2664_CMD_UPDATE_N(n) (0x10 + (n)) > +#define LTC2664_CMD_WRITE_N_UPDATE_ALL 0x20 > +#define LTC2664_CMD_WRITE_N_UPDATE_N(n) (0x30 + (n)) > +#define LTC2664_CMD_POWER_DOWN_N(n) (0x40 + (n)) > +#define LTC2664_CMD_POWER_DOWN_ALL 0x50 > +#define LTC2664_CMD_SPAN_N(n) (0x60 + (n)) > +#define LTC2664_CMD_CONFIG 0x70 > +#define LTC2664_CMD_MUX 0xB0 > +#define LTC2664_CMD_TOGGLE_SEL 0xC0 > +#define LTC2664_CMD_GLOBAL_TOGGLE 0xD0 > +#define LTC2664_CMD_NO_OPERATION 0xF0 > +#define LTC2664_REF_DISABLE 0x0001 > +#define LTC2664_MSPAN_SOFTSPAN 7 > + > +#define LTC2672_MAX_CHANNEL 5 > +#define LTC2672_MAX_SPAN 7 > +#define LTC2672_SCALE_MULTIPLIER(n) (50 * BIT(n)) > + > +enum ltc2664_ids { > + LTC2664, > + LTC2672, > +}; > + > +enum { > + LTC2664_SPAN_RANGE_0V_5V, > + LTC2664_SPAN_RANGE_0V_10V, > + LTC2664_SPAN_RANGE_M5V_5V, > + LTC2664_SPAN_RANGE_M10V_10V, > + LTC2664_SPAN_RANGE_M2V5_2V5, > +}; > + > +enum { > + LTC2664_INPUT_A, > + LTC2664_INPUT_B, > + LTC2664_INPUT_B_AVAIL, > + LTC2664_POWERDOWN, > + LTC2664_POWERDOWN_MODE, > + LTC2664_TOGGLE_EN, > + LTC2664_GLOBAL_TOGGLE, > +}; > + > +static const u16 ltc2664_mspan_lut[8][2] = { > + { LTC2664_SPAN_RANGE_M10V_10V, 32768 }, /* MPS2=0, MPS1=0, MSP0=0 (0)*/ > + { LTC2664_SPAN_RANGE_M5V_5V, 32768 }, /* MPS2=0, MPS1=0, MSP0=1 (1)*/ > + { LTC2664_SPAN_RANGE_M2V5_2V5, 32768 }, /* MPS2=0, MPS1=1, MSP0=0 (2)*/ > + { LTC2664_SPAN_RANGE_0V_10V, 0 }, /* MPS2=0, MPS1=1, MSP0=1 (3)*/ > + { LTC2664_SPAN_RANGE_0V_10V, 32768 }, /* MPS2=1, MPS1=0, MSP0=0 (4)*/ > + { LTC2664_SPAN_RANGE_0V_5V, 0 }, /* MPS2=1, MPS1=0, MSP0=1 (5)*/ > + { LTC2664_SPAN_RANGE_0V_5V, 32768 }, /* MPS2=1, MPS1=1, MSP0=0 (6)*/ > + { LTC2664_SPAN_RANGE_0V_5V, 0 } /* MPS2=1, MPS1=1, MSP0=1 (7)*/ > +}; > + > +struct ltc2664_state; > + > +struct ltc2664_chip_info { > + enum ltc2664_ids id; > + const char *name; > + int (*scale_get)(const struct ltc2664_state *st, int c); > + int (*offset_get)(const struct ltc2664_state *st, int c); > + int measurement_type; > + unsigned int num_channels; > + const struct iio_chan_spec *iio_chan; > + const int (*span_helper)[2]; > + unsigned int num_span; > + unsigned int internal_vref; > + bool manual_span_support; > + bool rfsadj_support; > +}; > + > +struct ltc2664_chan { > + bool toggle_chan; > + bool powerdown; > + u8 span; > + u16 raw[2]; /* A/B */ > +}; I would find it helpful to have more comments explainging what the various fields are for. For example, raw to be used to supply data to a SPI xfer but actually it is just a shadow copy of the current state of the chip registers. > + > +struct ltc2664_state { > + struct spi_device *spi; > + struct regmap *regmap; > + struct ltc2664_chan channels[LTC2672_MAX_CHANNEL]; > + /* lock to protect against multiple access to the device and shared data */ > + struct mutex lock; > + const struct ltc2664_chip_info *chip_info; > + struct iio_chan_spec *iio_channels; > + int vref; vref_mv Always nice to have the units since regulators use µV and IIO uses mV. Otherwise we have to guess. > + u32 toggle_sel; > + u32 global_toggle; Should this be bool? > + u32 rfsadj; rfsadj_ohms > +}; > + > +static const int ltc2664_span_helper[][2] = { > + { 0, 5000 }, > + { 0, 10000 }, > + { -5000, 5000 }, > + { -10000, 10000 }, > + { -2500, 2500 }, > +}; > + > +static const int ltc2672_span_helper[][2] = { > + { 0, 3125 }, > + { 0, 6250 }, > + { 0, 12500 }, > + { 0, 25000 }, > + { 0, 50000 }, > + { 0, 100000 }, > + { 0, 200000 }, > + { 0, 300000 }, > +}; > + > +static int ltc2664_scale_get(const struct ltc2664_state *st, int c) > +{ > + const struct ltc2664_chan *chan = &st->channels[c]; > + const int (*span_helper)[2] = st->chip_info->span_helper; > + int span, fs; > + > + span = chan->span; > + if (span < 0) > + return span; > + > + fs = span_helper[span][1] - span_helper[span][0]; > + > + return (fs / 2500) * st->vref; Should we multiply first and then divide? 3125 isn't divisible by 2500 so there may be unwanted rounding otherwise. > +} > + > +static int ltc2672_scale_get(const struct ltc2664_state *st, int c) > +{ > + const struct ltc2664_chan *chan = &st->channels[c]; > + int span, fs; > + > + span = chan->span; > + if (span < 0) > + return span; > + > + fs = 1000 * st->vref / st->rfsadj; > + > + if (span == LTC2672_MAX_SPAN) > + return 4800 * fs; > + > + return LTC2672_SCALE_MULTIPLIER(span) * fs; Are we losing accuracy by multiplying after dividing here as well? > +} > + > +static int ltc2664_offset_get(const struct ltc2664_state *st, int c) > +{ > + const struct ltc2664_chan *chan = &st->channels[c]; > + int span; > + > + span = chan->span; > + if (span < 0) > + return span; > + > + if (st->chip_info->span_helper[span][0] < 0) > + return -32768; > + > + return 0; > +} > + > +static int ltc2672_offset_get(const struct ltc2664_state *st, int c) > +{ > + const struct ltc2664_chan *chan = &st->channels[c]; > + int span; > + > + span = chan->span; > + if (span < 0) > + return span; > + > + if (st->chip_info->span_helper[span][1] < 0) Should this be span_helper[span][0]? [span][1] is always > 0. And for that matter, [span][0] is always 0 for ltc2672, so maybe we don't need this check at all? > + return -32768; > + > + return st->chip_info->span_helper[span][1] / 250; Why is this one not return 0 like the other chip? Figure 24 and 25 in the datasheet don't show an offset in the tranfer function. > +} > + > +static int ltc2664_dac_code_write(struct ltc2664_state *st, u32 chan, u32 input, > + u16 code) > +{ > + struct ltc2664_chan *c = &st->channels[chan]; > + int ret, reg; > + > + guard(mutex)(&st->lock); > + /* select the correct input register to write to */ > + if (c->toggle_chan) { > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > + input << chan); > + if (ret) > + return ret; > + } > + /* > + * If in toggle mode the dac should be updated by an > + * external signal (or sw toggle) and not here. > + */ > + if (st->toggle_sel & BIT(chan)) > + reg = LTC2664_CMD_WRITE_N(chan); > + else > + reg = LTC2664_CMD_WRITE_N_UPDATE_N(chan); > + > + ret = regmap_write(st->regmap, reg, code); > + if (ret) > + return ret; > + > + c->raw[input] = code; > + > + if (c->toggle_chan) { > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > + st->toggle_sel); > + if (ret) > + return ret; > + } > + > + return ret; > +} > + > +static int ltc2664_dac_code_read(struct ltc2664_state *st, u32 chan, u32 input, > + u32 *code) > +{ > + guard(mutex)(&st->lock); > + *code = st->channels[chan].raw[input]; > + > + return 0; > +} > + > +static const int ltc2664_raw_range[] = {0, 1, U16_MAX}; > + > +static int ltc2664_read_avail(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + const int **vals, int *type, int *length, > + long info) > +{ > + switch (info) { > + case IIO_CHAN_INFO_RAW: > + *vals = ltc2664_raw_range; > + *type = IIO_VAL_INT; > + > + return IIO_AVAIL_RANGE; > + default: > + return -EINVAL; > + } > +} > + > +static int ltc2664_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long info) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + int ret; > + > + switch (info) { > + case IIO_CHAN_INFO_RAW: > + ret = ltc2664_dac_code_read(st, chan->channel, > + LTC2664_INPUT_A, val); > + if (ret) > + return ret; > + > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_OFFSET: > + *val = st->chip_info->offset_get(st, chan->channel); > + > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + *val = st->chip_info->scale_get(st, chan->channel); > + > + *val2 = 16; > + return IIO_VAL_FRACTIONAL_LOG2; > + default: > + return -EINVAL; > + } > +} > + > +static int ltc2664_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int val, > + int val2, long info) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_CHAN_INFO_RAW: > + if (val > U16_MAX || val < 0) > + return -EINVAL; > + > + return ltc2664_dac_code_write(st, chan->channel, > + LTC2664_INPUT_A, val); > + default: > + return -EINVAL; > + } > +} > + > +static ssize_t ltc2664_reg_bool_get(struct iio_dev *indio_dev, > + uintptr_t private, > + const struct iio_chan_spec *chan, > + char *buf) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + u32 val; > + > + guard(mutex)(&st->lock); > + switch (private) { > + case LTC2664_POWERDOWN: > + val = st->channels[chan->channel].powerdown; > + > + return sysfs_emit(buf, "%u\n", val); > + case LTC2664_POWERDOWN_MODE: > + return sysfs_emit(buf, "42kohm_to_gnd\n");> + case LTC2664_TOGGLE_EN: > + val = !!(st->toggle_sel & BIT(chan->channel)); > + > + return sysfs_emit(buf, "%u\n", val); > + case LTC2664_GLOBAL_TOGGLE: > + val = st->global_toggle; > + > + return sysfs_emit(buf, "%u\n", val); > + default: > + return -EINVAL; > + } > +} > + > +static ssize_t ltc2664_reg_bool_set(struct iio_dev *indio_dev, > + uintptr_t private, > + const struct iio_chan_spec *chan, > + const char *buf, size_t len) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + int ret; > + bool en; > + > + ret = kstrtobool(buf, &en); > + if (ret) > + return ret; > + > + guard(mutex)(&st->lock); > + switch (private) { > + case LTC2664_POWERDOWN: > + ret = regmap_write(st->regmap, > + en ? LTC2664_CMD_POWER_DOWN_N(chan->channel) : > + LTC2664_CMD_UPDATE_N(chan->channel), en); > + if (ret) > + return ret; > + > + st->channels[chan->channel].powerdown = en; > + > + return len; > + case LTC2664_TOGGLE_EN: > + if (en) > + st->toggle_sel |= BIT(chan->channel); > + else > + st->toggle_sel &= ~BIT(chan->channel); > + > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > + st->toggle_sel); > + if (ret) > + return ret; > + > + return len; > + case LTC2664_GLOBAL_TOGGLE: > + ret = regmap_write(st->regmap, LTC2664_CMD_GLOBAL_TOGGLE, en); > + if (ret) > + return ret; > + > + st->global_toggle = en; > + > + return len; > + default: > + return -EINVAL; > + } > +} > + > +static ssize_t ltc2664_dac_input_read(struct iio_dev *indio_dev, > + uintptr_t private, > + const struct iio_chan_spec *chan, > + char *buf) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + int ret; > + u32 val; > + > + if (private == LTC2664_INPUT_B_AVAIL) > + return sysfs_emit(buf, "[%u %u %u]\n", ltc2664_raw_range[0], > + ltc2664_raw_range[1], > + ltc2664_raw_range[2] / 4); > + > + ret = ltc2664_dac_code_read(st, chan->channel, private, &val); > + if (ret) > + return ret; > + > + return sysfs_emit(buf, "%u\n", val); > +} > + > +static ssize_t ltc2664_dac_input_write(struct iio_dev *indio_dev, > + uintptr_t private, > + const struct iio_chan_spec *chan, > + const char *buf, size_t len) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + int ret; > + u16 val; > + > + if (private == LTC2664_INPUT_B_AVAIL) > + return -EINVAL; > + > + ret = kstrtou16(buf, 10, &val); > + if (ret) > + return ret; > + > + ret = ltc2664_dac_code_write(st, chan->channel, private, val); > + if (ret) > + return ret; > + > + return len; > +} > + > +static int ltc2664_reg_access(struct iio_dev *indio_dev, > + unsigned int reg, > + unsigned int writeval, > + unsigned int *readval) > +{ > + struct ltc2664_state *st = iio_priv(indio_dev); > + > + if (readval) > + return -EOPNOTSUPP; > + > + return regmap_write(st->regmap, reg, writeval); > +} > + > +#define LTC2664_CHAN_EXT_INFO(_name, _what, _shared, _read, _write) { \ > + .name = _name, \ > + .read = (_read), \ > + .write = (_write), \ > + .private = (_what), \ > + .shared = (_shared), \ > +} > + > +/* > + * For toggle mode we only expose the symbol attr (sw_toggle) in case a TGPx is > + * not provided in dts. > + */ > +static const struct iio_chan_spec_ext_info ltc2664_toggle_sym_ext_info[] = { > + LTC2664_CHAN_EXT_INFO("raw0", LTC2664_INPUT_A, IIO_SEPARATE, > + ltc2664_dac_input_read, ltc2664_dac_input_write), > + LTC2664_CHAN_EXT_INFO("raw1", LTC2664_INPUT_B, IIO_SEPARATE, > + ltc2664_dac_input_read, ltc2664_dac_input_write), > + LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, IIO_SEPARATE, > + ltc2664_reg_bool_get, ltc2664_reg_bool_set), > + LTC2664_CHAN_EXT_INFO("powerdown_mode", LTC2664_POWERDOWN_MODE, > + IIO_SEPARATE, ltc2664_reg_bool_get, NULL), > + LTC2664_CHAN_EXT_INFO("symbol", LTC2664_GLOBAL_TOGGLE, IIO_SEPARATE, > + ltc2664_reg_bool_get, ltc2664_reg_bool_set), > + LTC2664_CHAN_EXT_INFO("toggle_en", LTC2664_TOGGLE_EN, > + IIO_SEPARATE, ltc2664_reg_bool_get, > + ltc2664_reg_bool_set), > + { } > +}; > + > +static const struct iio_chan_spec_ext_info ltc2664_ext_info[] = { > + LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, IIO_SEPARATE, > + ltc2664_reg_bool_get, ltc2664_reg_bool_set), > + LTC2664_CHAN_EXT_INFO("powerdown_mode", LTC2664_POWERDOWN_MODE, > + IIO_SEPARATE, ltc2664_reg_bool_get, NULL), > + { } > +}; > + > +#define LTC2664_CHANNEL(_chan) { \ > + .indexed = 1, \ > + .output = 1, \ > + .channel = (_chan), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), \ > + .ext_info = ltc2664_ext_info, \ > +} > + > +static const struct iio_chan_spec ltc2664_channels[] = { > + LTC2664_CHANNEL(0), > + LTC2664_CHANNEL(1), > + LTC2664_CHANNEL(2), > + LTC2664_CHANNEL(3), > +}; > + > +static const struct iio_chan_spec ltc2672_channels[] = { > + LTC2664_CHANNEL(0), > + LTC2664_CHANNEL(1), > + LTC2664_CHANNEL(2), > + LTC2664_CHANNEL(3), > + LTC2664_CHANNEL(4), > +}; Do we really need these since they are only used as a template anyway? We could just have a single template for one channel and copy it as manay times as needed. > + > +static const struct ltc2664_chip_info ltc2664_chip = { > + .id = LTC2664, > + .name = "ltc2664", > + .scale_get = ltc2664_scale_get, > + .offset_get = ltc2664_offset_get, > + .measurement_type = IIO_VOLTAGE, > + .num_channels = ARRAY_SIZE(ltc2664_channels), > + .iio_chan = ltc2664_channels, > + .span_helper = ltc2664_span_helper, > + .num_span = ARRAY_SIZE(ltc2664_span_helper), > + .internal_vref = 2500, > + .manual_span_support = true, > + .rfsadj_support = false, > +}; > + > +static const struct ltc2664_chip_info ltc2672_chip = { > + .id = LTC2672, > + .name = "ltc2672", > + .scale_get = ltc2672_scale_get, > + .offset_get = ltc2672_offset_get, > + .measurement_type = IIO_CURRENT, > + .num_channels = ARRAY_SIZE(ltc2672_channels), > + .iio_chan = ltc2672_channels, > + .span_helper = ltc2672_span_helper, > + .num_span = ARRAY_SIZE(ltc2672_span_helper), > + .internal_vref = 1250, > + .manual_span_support = false, > + .rfsadj_support = true, > +}; > + > +static int ltc2664_set_span(const struct ltc2664_state *st, int min, int max, > + int chan) > +{ > + const struct ltc2664_chip_info *chip_info = st->chip_info; > + const int (*span_helper)[2] = chip_info->span_helper; > + int span, ret; > + > + st->iio_channels[chan].type = chip_info->measurement_type; > + > + for (span = 0; span < chip_info->num_span; span++) { > + if (min == span_helper[span][0] && max == span_helper[span][1]) > + break; > + } > + > + if (span == chip_info->num_span) > + return -EINVAL; > + > + ret = regmap_write(st->regmap, LTC2664_CMD_SPAN_N(chan), > + (chip_info->id == LTC2672) ? span + 1 : span); > + if (ret) > + return ret; > + > + return span; > +} > + > +static int ltc2664_channel_config(struct ltc2664_state *st) > +{ > + const struct ltc2664_chip_info *chip_info = st->chip_info; > + struct device *dev = &st->spi->dev; > + u32 reg, tmp[2], mspan; > + int ret, span = 0; > + > + mspan = LTC2664_MSPAN_SOFTSPAN; > + ret = device_property_read_u32(dev, "adi,manual-span-operation-config", > + &mspan); > + if (!ret) { > + if (!chip_info->manual_span_support) > + return dev_err_probe(dev, -EINVAL, > + "adi,manual-span-operation-config not supported\n"); > + > + if (mspan > ARRAY_SIZE(ltc2664_mspan_lut)) > + return dev_err_probe(dev, -EINVAL, > + "adi,manual-span-operation-config not in range\n"); > + } > + > + st->rfsadj = 20000; > + ret = device_property_read_u32(dev, "adi,rfsadj-ohms", &st->rfsadj); > + if (!ret) { > + if (!chip_info->rfsadj_support) > + return dev_err_probe(dev, -EINVAL, > + "adi,rfsadj-ohms not supported\n"); > + > + if (st->rfsadj < 19000 || st->rfsadj > 41000) > + return dev_err_probe(dev, -EINVAL, > + "adi,rfsadj-ohms not in range\n"); > + } > + > + device_for_each_child_node_scoped(dev, child) { > + struct ltc2664_chan *chan; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to get reg property\n"); > + > + if (reg >= chip_info->num_channels) > + return dev_err_probe(dev, -EINVAL, > + "reg bigger than: %d\n", > + chip_info->num_channels); > + > + chan = &st->channels[reg]; > + > + if (fwnode_property_read_bool(child, "adi,toggle-mode")) { > + chan->toggle_chan = true; > + /* assume sw toggle ABI */ > + st->iio_channels[reg].ext_info = ltc2664_toggle_sym_ext_info; > + > + /* > + * Clear IIO_CHAN_INFO_RAW bit as toggle channels expose > + * out_voltage/current_raw{0|1} files. > + */ > + __clear_bit(IIO_CHAN_INFO_RAW, > + &st->iio_channels[reg].info_mask_separate); > + } > + > + chan->raw[0] = ltc2664_mspan_lut[mspan][1]; > + chan->raw[1] = ltc2664_mspan_lut[mspan][1]; > + > + chan->span = ltc2664_mspan_lut[mspan][0]; > + > + ret = fwnode_property_read_u32_array(child, "adi,output-range-microvolt", > + tmp, ARRAY_SIZE(tmp)); > + if (!ret && mspan == LTC2664_MSPAN_SOFTSPAN) { > + chan->span = ltc2664_set_span(st, tmp[0] / 1000, > + tmp[1] / 1000, reg); > + if (span < 0) > + return dev_err_probe(dev, span, > + "Failed to set span\n"); > + > + } > + > + ret = fwnode_property_read_u32(child, > + "adi,output-range-microamp", > + &tmp[0]); > + if (!ret) { > + chan->span = ltc2664_set_span(st, 0, tmp[0] / 1000, reg); > + if (span < 0) > + return dev_err_probe(dev, span, > + "Failed to set span\n"); > + } > + } > + > + return 0; > +} > + > +static int ltc2664_setup(struct ltc2664_state *st, struct regulator *vref) > +{ > + const struct ltc2664_chip_info *chip_info = st->chip_info; > + struct gpio_desc *gpio; > + int ret; > + > + /* If we have a clr/reset pin, use that to reset the chip. */ > + gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(gpio)) > + return dev_err_probe(&st->spi->dev, PTR_ERR(gpio), > + "Failed to get reset gpio"); > + if (gpio) { > + usleep_range(1000, 1200); fsleep(1000) > + gpiod_set_value_cansleep(gpio, 0); > + } > + > + /* > + * Duplicate the default channel configuration as it can change during > + * @ltc2664_channel_config() > + */ > + st->iio_channels = devm_kmemdup(&st->spi->dev, chip_info->iio_chan, > + (chip_info->num_channels + 1) * > + sizeof(*chip_info->iio_chan), > + GFP_KERNEL); > + > + ret = ltc2664_channel_config(st); > + if (ret) > + return ret; > + > + if (!vref) > + return 0; > + > + return regmap_set_bits(st->regmap, LTC2664_CMD_CONFIG, LTC2664_REF_DISABLE); > +} > + > +static void ltc2664_disable_regulator(void *regulator) > +{ > + regulator_disable(regulator); > +} > + > +static const struct regmap_config ltc2664_regmap_config = { > + .reg_bits = 8, > + .val_bits = 16, > + .max_register = LTC2664_CMD_NO_OPERATION, > +}; > + > +static const struct iio_info ltc2664_info = { > + .write_raw = ltc2664_write_raw, > + .read_raw = ltc2664_read_raw, > + .read_avail = ltc2664_read_avail, > + .debugfs_reg_access = ltc2664_reg_access, > +}; > + > +static int ltc2664_probe(struct spi_device *spi) > +{ > + static const char * const regulators[] = { "vcc", "iovcc", "v-neg" }; > + const struct ltc2664_chip_info *chip_info; > + struct device *dev = &spi->dev; > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ltc2664_state *st; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + > + chip_info = spi_get_device_match_data(spi); > + if (!chip_info) > + return -ENOMEM; ENOMEM? Usually, this is EINVAL and sometimes ENODEV. Not sure what should be preferred. > + > + st->chip_info = chip_info; > + > + mutex_init(&st->lock); > + > + st->regmap = devm_regmap_init_spi(spi, <c2664_regmap_config); > + if (IS_ERR(st->regmap)) > + return dev_err_probe(dev, PTR_ERR(st->regmap), > + "Failed to init regmap"); > + > + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), > + regulators); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to enable regulators\n"); > + > + vref_reg = devm_regulator_get_optional(dev, "ref"); > + if (IS_ERR(vref_reg)) { > + if (PTR_ERR(vref_reg) != -ENODEV) > + return dev_err_probe(dev, PTR_ERR(vref_reg), > + "Failed to get ref regulator"); > + > + vref_reg = NULL; > + > + st->vref = chip_info->internal_vref; > + } else { > + ret = regulator_enable(vref_reg); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to enable ref regulators\n"); > + > + ret = devm_add_action_or_reset(dev, ltc2664_disable_regulator, > + vref_reg); > + if (ret) > + return ret; > + > + ret = regulator_get_voltage(vref_reg); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to get ref\n"); > + > + st->vref = ret / 1000; > + } There is a new API to allow simplifying this: ret = devm_regulator_get_enable_read_voltage(dev, "ref"); if (ret == -ENODEV) st->vref_mv = chip_info->internal_vref_mv; else if (ret < 0) return dev_err_probe(dev, ret, "Failed to get vref voltage\n"); else st->vref_mv = ret / 1000; And ltc2664_disable_regulator and vref_reg are removed too. > + > + ret = ltc2664_setup(st, vref_reg); > + if (ret) > + return ret; > + > + indio_dev->name = chip_info->name; > + indio_dev->info = <c2664_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = st->iio_channels; > + indio_dev->num_channels = chip_info->num_channels; > + > + return devm_iio_device_register(dev, indio_dev); > +} > + > +static const struct spi_device_id ltc2664_id[] = { > + { "ltc2664", (kernel_ulong_t)<c2664_chip }, > + { "ltc2672", (kernel_ulong_t)<c2672_chip }, > + { } > +}; > +MODULE_DEVICE_TABLE(spi, ltc2664_id); > + > +static const struct of_device_id ltc2664_of_id[] = { > + { .compatible = "adi,ltc2664", .data = <c2664_chip }, > + { .compatible = "adi,ltc2672", .data = <c2672_chip }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ltc2664_of_id); > + > +static struct spi_driver ltc2664_driver = { > + .driver = { > + .name = "ltc2664", > + .of_match_table = ltc2664_of_id, > + }, > + .probe = ltc2664_probe, > + .id_table = ltc2664_id, > +}; > +module_spi_driver(ltc2664_driver); > + > +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); > +MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>"); > +MODULE_DESCRIPTION("Analog Devices LTC2664 and LTC2672 DAC"); > +MODULE_LICENSE("GPL");
> > +struct ltc2664_state { > > + struct spi_device *spi; > > + struct regmap *regmap; > > + struct ltc2664_chan channels[LTC2672_MAX_CHANNEL]; > > + /* lock to protect against multiple access to the device and shared data > */ > > + struct mutex lock; > > + const struct ltc2664_chip_info *chip_info; > > + struct iio_chan_spec *iio_channels; > > + int vref; > > vref_mv > > Always nice to have the units since regulators use µV and IIO uses mV. > Otherwise we have to guess. > > > + u32 toggle_sel; > > + u32 global_toggle; > > Should this be bool? > > > + u32 rfsadj; > > rfsadj_ohms > > > +}; > > + > > +static const int ltc2664_span_helper[][2] = { > > + { 0, 5000 }, > > + { 0, 10000 }, > > + { -5000, 5000 }, > > + { -10000, 10000 }, > > + { -2500, 2500 }, > > +}; > > + > > +static const int ltc2672_span_helper[][2] = { > > + { 0, 3125 }, > > + { 0, 6250 }, > > + { 0, 12500 }, > > + { 0, 25000 }, > > + { 0, 50000 }, > > + { 0, 100000 }, > > + { 0, 200000 }, > > + { 0, 300000 }, > > +}; > > + > > +static int ltc2664_scale_get(const struct ltc2664_state *st, int c) > > +{ > > + const struct ltc2664_chan *chan = &st->channels[c]; > > + const int (*span_helper)[2] = st->chip_info->span_helper; > > + int span, fs; > > + > > + span = chan->span; > > + if (span < 0) > > + return span; > > + > > + fs = span_helper[span][1] - span_helper[span][0]; > > + > > + return (fs / 2500) * st->vref; > > Should we multiply first and then divide? 3125 isn't divisible by 2500 > so there may be unwanted rounding otherwise. Yes that would make sense, should perform the multiplication first, then the division. > > +} > > + > > +static int ltc2672_scale_get(const struct ltc2664_state *st, int c) > > +{ > > + const struct ltc2664_chan *chan = &st->channels[c]; > > + int span, fs; > > + > > + span = chan->span; > > + if (span < 0) > > + return span; > > + > > + fs = 1000 * st->vref / st->rfsadj; > > + > > + if (span == LTC2672_MAX_SPAN) > > + return 4800 * fs; > > + > > + return LTC2672_SCALE_MULTIPLIER(span) * fs; > > Are we losing accuracy by multiplying after dividing here as well? > > > +} > > + > > +static int ltc2664_offset_get(const struct ltc2664_state *st, int c) > > +{ > > + const struct ltc2664_chan *chan = &st->channels[c]; > > + int span; > > + > > + span = chan->span; > > + if (span < 0) > > + return span; > > + > > + if (st->chip_info->span_helper[span][0] < 0) > > + return -32768; > > + > > + return 0; > > +} > > + > > +static int ltc2672_offset_get(const struct ltc2664_state *st, int c) > > +{ > > + const struct ltc2664_chan *chan = &st->channels[c]; > > + int span; > > + > > + span = chan->span; > > + if (span < 0) > > + return span; > > + > > + if (st->chip_info->span_helper[span][1] < 0) > > Should this be span_helper[span][0]? [span][1] is always > 0. > > And for that matter, [span][0] is always 0 for ltc2672, so > maybe we don't need this check at all? > > > + return -32768; > > + > > + return st->chip_info->span_helper[span][1] / 250; > > Why is this one not return 0 like the other chip? > > Figure 24 and 25 in the datasheet don't show an offset in > the tranfer function. I think I misinterpret page 25 of the datasheet about the offset. We can make it return 0. > > +} > > + > > +static int ltc2664_dac_code_write(struct ltc2664_state *st, u32 chan, u32 > input, > > + u16 code) > > +{ > > + struct ltc2664_chan *c = &st->channels[chan]; > > + int ret, reg; > > + > > + guard(mutex)(&st->lock); > > + /* select the correct input register to write to */ > > + if (c->toggle_chan) { > > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > > + input << chan); > > + if (ret) > > + return ret; > > + } > > + /* > > + * If in toggle mode the dac should be updated by an > > + * external signal (or sw toggle) and not here. > > + */ > > + if (st->toggle_sel & BIT(chan)) > > + reg = LTC2664_CMD_WRITE_N(chan); > > + else > > + reg = LTC2664_CMD_WRITE_N_UPDATE_N(chan); > > + > > + ret = regmap_write(st->regmap, reg, code); > > + if (ret) > > + return ret; > > + > > + c->raw[input] = code; > > + > > + if (c->toggle_chan) { > > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > > + st->toggle_sel); > > + if (ret) > > + return ret; > > + } > > + > > + return ret; > > +} > > + > > +static int ltc2664_dac_code_read(struct ltc2664_state *st, u32 chan, u32 > input, > > + u32 *code) > > +{ > > + guard(mutex)(&st->lock); > > + *code = st->channels[chan].raw[input]; > > + > > + return 0; > > +} > > + > > +static const int ltc2664_raw_range[] = {0, 1, U16_MAX}; > > + > > +static int ltc2664_read_avail(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + const int **vals, int *type, int *length, > > + long info) > > +{ > > + switch (info) { > > + case IIO_CHAN_INFO_RAW: > > + *vals = ltc2664_raw_range; > > + *type = IIO_VAL_INT; > > + > > + return IIO_AVAIL_RANGE; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static int ltc2664_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, int *val, > > + int *val2, long info) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + int ret; > > + > > + switch (info) { > > + case IIO_CHAN_INFO_RAW: > > + ret = ltc2664_dac_code_read(st, chan->channel, > > + LTC2664_INPUT_A, val); > > + if (ret) > > + return ret; > > + > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_OFFSET: > > + *val = st->chip_info->offset_get(st, chan->channel); > > + > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_SCALE: > > + *val = st->chip_info->scale_get(st, chan->channel); > > + > > + *val2 = 16; > > + return IIO_VAL_FRACTIONAL_LOG2; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static int ltc2664_write_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, int val, > > + int val2, long info) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + > > + switch (info) { > > + case IIO_CHAN_INFO_RAW: > > + if (val > U16_MAX || val < 0) > > + return -EINVAL; > > + > > + return ltc2664_dac_code_write(st, chan->channel, > > + LTC2664_INPUT_A, val); > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static ssize_t ltc2664_reg_bool_get(struct iio_dev *indio_dev, > > + uintptr_t private, > > + const struct iio_chan_spec *chan, > > + char *buf) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + u32 val; > > + > > + guard(mutex)(&st->lock); > > + switch (private) { > > + case LTC2664_POWERDOWN: > > + val = st->channels[chan->channel].powerdown; > > + > > + return sysfs_emit(buf, "%u\n", val); > > + case LTC2664_POWERDOWN_MODE: > > + return sysfs_emit(buf, "42kohm_to_gnd\n");> + case > LTC2664_TOGGLE_EN: > > + val = !!(st->toggle_sel & BIT(chan->channel)); > > + > > + return sysfs_emit(buf, "%u\n", val); > > + case LTC2664_GLOBAL_TOGGLE: > > + val = st->global_toggle; > > + > > + return sysfs_emit(buf, "%u\n", val); > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static ssize_t ltc2664_reg_bool_set(struct iio_dev *indio_dev, > > + uintptr_t private, > > + const struct iio_chan_spec *chan, > > + const char *buf, size_t len) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + int ret; > > + bool en; > > + > > + ret = kstrtobool(buf, &en); > > + if (ret) > > + return ret; > > + > > + guard(mutex)(&st->lock); > > + switch (private) { > > + case LTC2664_POWERDOWN: > > + ret = regmap_write(st->regmap, > > + en ? > LTC2664_CMD_POWER_DOWN_N(chan->channel) : > > + LTC2664_CMD_UPDATE_N(chan->channel), > en); > > + if (ret) > > + return ret; > > + > > + st->channels[chan->channel].powerdown = en; > > + > > + return len; > > + case LTC2664_TOGGLE_EN: > > + if (en) > > + st->toggle_sel |= BIT(chan->channel); > > + else > > + st->toggle_sel &= ~BIT(chan->channel); > > + > > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > > + st->toggle_sel); > > + if (ret) > > + return ret; > > + > > + return len; > > + case LTC2664_GLOBAL_TOGGLE: > > + ret = regmap_write(st->regmap, > LTC2664_CMD_GLOBAL_TOGGLE, en); > > + if (ret) > > + return ret; > > + > > + st->global_toggle = en; > > + > > + return len; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static ssize_t ltc2664_dac_input_read(struct iio_dev *indio_dev, > > + uintptr_t private, > > + const struct iio_chan_spec *chan, > > + char *buf) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + int ret; > > + u32 val; > > + > > + if (private == LTC2664_INPUT_B_AVAIL) > > + return sysfs_emit(buf, "[%u %u %u]\n", ltc2664_raw_range[0], > > + ltc2664_raw_range[1], > > + ltc2664_raw_range[2] / 4); > > + > > + ret = ltc2664_dac_code_read(st, chan->channel, private, &val); > > + if (ret) > > + return ret; > > + > > + return sysfs_emit(buf, "%u\n", val); > > +} > > + > > +static ssize_t ltc2664_dac_input_write(struct iio_dev *indio_dev, > > + uintptr_t private, > > + const struct iio_chan_spec *chan, > > + const char *buf, size_t len) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + int ret; > > + u16 val; > > + > > + if (private == LTC2664_INPUT_B_AVAIL) > > + return -EINVAL; > > + > > + ret = kstrtou16(buf, 10, &val); > > + if (ret) > > + return ret; > > + > > + ret = ltc2664_dac_code_write(st, chan->channel, private, val); > > + if (ret) > > + return ret; > > + > > + return len; > > +} > > + > > +static int ltc2664_reg_access(struct iio_dev *indio_dev, > > + unsigned int reg, > > + unsigned int writeval, > > + unsigned int *readval) > > +{ > > + struct ltc2664_state *st = iio_priv(indio_dev); > > + > > + if (readval) > > + return -EOPNOTSUPP; > > + > > + return regmap_write(st->regmap, reg, writeval); > > +} > > + > > +#define LTC2664_CHAN_EXT_INFO(_name, _what, _shared, _read, _write) { > \ > > + .name = _name, \ > > + .read = (_read), \ > > + .write = (_write), \ > > + .private = (_what), \ > > + .shared = (_shared), \ > > +} > > + > > +/* > > + * For toggle mode we only expose the symbol attr (sw_toggle) in case a > TGPx is > > + * not provided in dts. > > + */ > > +static const struct iio_chan_spec_ext_info ltc2664_toggle_sym_ext_info[] = { > > + LTC2664_CHAN_EXT_INFO("raw0", LTC2664_INPUT_A, IIO_SEPARATE, > > + ltc2664_dac_input_read, > ltc2664_dac_input_write), > > + LTC2664_CHAN_EXT_INFO("raw1", LTC2664_INPUT_B, IIO_SEPARATE, > > + ltc2664_dac_input_read, > ltc2664_dac_input_write), > > + LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, > IIO_SEPARATE, > > + ltc2664_reg_bool_get, ltc2664_reg_bool_set), > > + LTC2664_CHAN_EXT_INFO("powerdown_mode", > LTC2664_POWERDOWN_MODE, > > + IIO_SEPARATE, ltc2664_reg_bool_get, NULL), > > + LTC2664_CHAN_EXT_INFO("symbol", LTC2664_GLOBAL_TOGGLE, > IIO_SEPARATE, > > + ltc2664_reg_bool_get, ltc2664_reg_bool_set), > > + LTC2664_CHAN_EXT_INFO("toggle_en", LTC2664_TOGGLE_EN, > > + IIO_SEPARATE, ltc2664_reg_bool_get, > > + ltc2664_reg_bool_set), > > + { } > > +}; > > + > > +static const struct iio_chan_spec_ext_info ltc2664_ext_info[] = { > > + LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, > IIO_SEPARATE, > > + ltc2664_reg_bool_get, ltc2664_reg_bool_set), > > + LTC2664_CHAN_EXT_INFO("powerdown_mode", > LTC2664_POWERDOWN_MODE, > > + IIO_SEPARATE, ltc2664_reg_bool_get, NULL), > > + { } > > +}; > > + > > +#define LTC2664_CHANNEL(_chan) { \ > > + .indexed = 1, \ > > + .output = 1, \ > > + .channel = (_chan), \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) | \ > > + BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_RAW), > \ > > + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), > \ > > + .ext_info = ltc2664_ext_info, \ > > +} > > + > > +static const struct iio_chan_spec ltc2664_channels[] = { > > + LTC2664_CHANNEL(0), > > + LTC2664_CHANNEL(1), > > + LTC2664_CHANNEL(2), > > + LTC2664_CHANNEL(3), > > +}; > > + > > +static const struct iio_chan_spec ltc2672_channels[] = { > > + LTC2664_CHANNEL(0), > > + LTC2664_CHANNEL(1), > > + LTC2664_CHANNEL(2), > > + LTC2664_CHANNEL(3), > > + LTC2664_CHANNEL(4), > > +}; > > Do we really need these since they are only used as a template anyway? > We could just have a single template for one channel and copy it as > manay times as needed. Yes, from what I can see we need separate channel specs for both devices since they have a differing number of channels. As for your suggestion about having a single template for one channel and copying it as many times as needed, I'm not entirely sure how to implement it in this context. Could you provide something like a code snippet to illustrate this? > > + > > +static const struct ltc2664_chip_info ltc2664_chip = { > > + .id = LTC2664, > > + .name = "ltc2664", > > + .scale_get = ltc2664_scale_get, > > + .offset_get = ltc2664_offset_get, > > + .measurement_type = IIO_VOLTAGE, > > + .num_channels = ARRAY_SIZE(ltc2664_channels), > > + .iio_chan = ltc2664_channels, > > + .span_helper = ltc2664_span_helper, > > + .num_span = ARRAY_SIZE(ltc2664_span_helper), > > + .internal_vref = 2500, > > + .manual_span_support = true, > > + .rfsadj_support = false, > > +}; > > +
On 6/6/24 10:49 AM, Paller, Kim Seer wrote: >>> +#define LTC2664_CHANNEL(_chan) { \ >>> + .indexed = 1, \ >>> + .output = 1, \ >>> + .channel = (_chan), \ >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) | \ >>> + BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_RAW), >> \ >>> + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), >> \ >>> + .ext_info = ltc2664_ext_info, \ >>> +} >>> + >>> +static const struct iio_chan_spec ltc2664_channels[] = { >>> + LTC2664_CHANNEL(0), >>> + LTC2664_CHANNEL(1), >>> + LTC2664_CHANNEL(2), >>> + LTC2664_CHANNEL(3), >>> +}; >>> + >>> +static const struct iio_chan_spec ltc2672_channels[] = { >>> + LTC2664_CHANNEL(0), >>> + LTC2664_CHANNEL(1), >>> + LTC2664_CHANNEL(2), >>> + LTC2664_CHANNEL(3), >>> + LTC2664_CHANNEL(4), >>> +}; >> >> Do we really need these since they are only used as a template anyway? >> We could just have a single template for one channel and copy it as >> manay times as needed. > > Yes, from what I can see we need separate channel specs for both devices > since they have a differing number of channels. As for your suggestion about > having a single template for one channel and copying it as many times as > needed, I'm not entirely sure how to implement it in this context. Could you > provide something like a code snippet to illustrate this? > Instead of the #define and arrays above, just have a single static struct: static const struct iio_chan_spec ltc2664_channel_template = { .indexed = 1, .output = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_RAW), .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), .ext_info = ltc2664_ext_info, }; >>> +static int ltc2664_setup(struct ltc2664_state *st, struct regulator *vref) >>> +{ >>> + const struct ltc2664_chip_info *chip_info = st->chip_info; >>> + struct gpio_desc *gpio; >>> + int ret; >>> + >>> + /* If we have a clr/reset pin, use that to reset the chip. */ >>> + gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH); >>> + if (IS_ERR(gpio)) >>> + return dev_err_probe(&st->spi->dev, PTR_ERR(gpio), >>> + "Failed to get reset gpio"); >>> + if (gpio) { >>> + usleep_range(1000, 1200); >>> + gpiod_set_value_cansleep(gpio, 0); >>> + } >>> + >>> + /* >>> + * Duplicate the default channel configuration as it can change during >>> + * @ltc2664_channel_config() >>> + */ >>> + st->iio_channels = devm_kmemdup(&st->spi->dev, chip_info->iio_chan, >>> + (chip_info->num_channels + 1) * >>> + sizeof(*chip_info->iio_chan), >>> + GFP_KERNEL); Then here, instead of devm_kmemdup(): st->iio_channels = devm_kcalloc(&st->spi->dev, chip_info->num_channels, sizeof(struct iio_chan_spec), GFP_KERNEL); if (!st->iio_channels) return -ENOMEM; for (i = 0; i < chip_info->num_channels; i++) { st->iio_channels[i] = ltc2664_channel_template; st->iio_channels[i].type = chip_info->measurement_type; st->iio_channels[i].channel = i; } Note: the original code was missing the error check and I think num_channels + 1 was 1 too many, so I fixed both of those in the example as well. This also replaces: st->iio_channels[chan].type = chip_info->measurement_type; from ltc2664_set_span() as it seems a bit out of place there. >>> + >>> + ret = ltc2664_channel_config(st); >>> + if (ret) >>> + return ret; >>> + >>> + if (!vref) >>> + return 0; >>> + >>> + return regmap_set_bits(st->regmap, LTC2664_CMD_CONFIG, LTC2664_REF_DISABLE); >>> +}
On Mon, 3 Jun 2024 09:22:00 +0800 Kim Seer Paller <kimseer.paller@analog.com> wrote: > LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC > LTC2672 5 channel, 16 bit Current Output Softspan DAC > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202405241141.kYcxrSem-lkp@intel.com/ > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> A few minor things from me to add to the more detailed comments from Nuno and David. > +static int ltc2664_dac_code_write(struct ltc2664_state *st, u32 chan, u32 input, > + u16 code) > +{ > + struct ltc2664_chan *c = &st->channels[chan]; > + int ret, reg; > + > + guard(mutex)(&st->lock); > + /* select the correct input register to write to */ > + if (c->toggle_chan) { > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > + input << chan); > + if (ret) > + return ret; > + } > + /* > + * If in toggle mode the dac should be updated by an > + * external signal (or sw toggle) and not here. > + */ > + if (st->toggle_sel & BIT(chan)) > + reg = LTC2664_CMD_WRITE_N(chan); > + else > + reg = LTC2664_CMD_WRITE_N_UPDATE_N(chan); > + > + ret = regmap_write(st->regmap, reg, code); > + if (ret) > + return ret; > + > + c->raw[input] = code; > + > + if (c->toggle_chan) { > + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, > + st->toggle_sel); > + if (ret) > + return ret; > + } > + > + return ret; return 0; you won't get here otherwise, and making that explicit gives more readable code. > +} > + > +static int ltc2664_dac_code_read(struct ltc2664_state *st, u32 chan, u32 input, > + u32 *code) > +{ > + guard(mutex)(&st->lock); > + *code = st->channels[chan].raw[input]; > + > + return 0; > +} > + > +static const int ltc2664_raw_range[] = {0, 1, U16_MAX}; { 0, 1, U16_MAX }; preferred (extra spaces) > + > +static const struct ltc2664_chip_info ltc2664_chip = { > + .id = LTC2664, > + .name = "ltc2664", > + .scale_get = ltc2664_scale_get, > + .offset_get = ltc2664_offset_get, > + .measurement_type = IIO_VOLTAGE, > + .num_channels = ARRAY_SIZE(ltc2664_channels), > + .iio_chan = ltc2664_channels, > + .span_helper = ltc2664_span_helper, > + .num_span = ARRAY_SIZE(ltc2664_span_helper), > + .internal_vref = 2500, > + .manual_span_support = true, > + .rfsadj_support = false, > +}; > + > +static const struct ltc2664_chip_info ltc2672_chip = { > + .id = LTC2672, As below. Seeing an id in here made me wonder what was going on given we don't have a whoami register on these. Please remove it as that model of handling chip specific stuff always bites us in complexity and lack of flexibility as we expand the parts supported by a driver. > + .name = "ltc2672", > + .scale_get = ltc2672_scale_get, > + .offset_get = ltc2672_offset_get, > + .measurement_type = IIO_CURRENT, > + .num_channels = ARRAY_SIZE(ltc2672_channels), > + .iio_chan = ltc2672_channels, > + .span_helper = ltc2672_span_helper, > + .num_span = ARRAY_SIZE(ltc2672_span_helper), > + .internal_vref = 1250, > + .manual_span_support = false, > + .rfsadj_support = true, > +}; > + > +static int ltc2664_set_span(const struct ltc2664_state *st, int min, int max, > + int chan) > +{ > + const struct ltc2664_chip_info *chip_info = st->chip_info; > + const int (*span_helper)[2] = chip_info->span_helper; > + int span, ret; > + > + st->iio_channels[chan].type = chip_info->measurement_type; > + > + for (span = 0; span < chip_info->num_span; span++) { > + if (min == span_helper[span][0] && max == span_helper[span][1]) > + break; > + } > + > + if (span == chip_info->num_span) > + return -EINVAL; > + > + ret = regmap_write(st->regmap, LTC2664_CMD_SPAN_N(chan), > + (chip_info->id == LTC2672) ? span + 1 : span); Make this specific data, not id based. The reasoning of there being a magic value (offmode) as 0 is a bit obscure so maybe a callback is best plan. Or... put a 0,0 entry in span_helper[] and check for that + ignore it or error out if anyone tries to use it. Drop that id in the chip_info structure as well as having it there will make people not consider if their 'code' should be 'data' in future cases similar to this. > + if (ret) > + return ret; > + > + return span; > +} > +
> -----Original Message----- > From: David Lechner <dlechner@baylibre.com> > Sent: Tuesday, June 4, 2024 4:43 AM > To: Paller, Kim Seer <KimSeer.Paller@analog.com>; linux- > kernel@vger.kernel.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org > Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen > <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown > <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof > Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor > Dooley <conor+dt@kernel.org>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com>; > kernel test robot <lkp@intel.com> > Subject: Re: [PATCH v3 5/5] iio: dac: ltc2664: Add driver for LTC2664 and > LTC2672 > > [External] > > On 6/2/24 8:22 PM, Kim Seer Paller wrote: > > LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC > > LTC2672 5 channel, 16 bit Current Output Softspan DAC > > > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-kbuild- > all/202405241141.kYcxrSem- > lkp@intel.com/__;!!A3Ni8CS0y2Y!65MPSYKyqFizjs_tSxpABl45BNKqWutx4NOBi > EkmmmY8kJkwep-27ON2rqne-XnUId2M3KsqgGbQy7GI_aYi9Tg$ > > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > > --- > > MAINTAINERS | 1 + > > drivers/iio/dac/Kconfig | 11 + > > drivers/iio/dac/Makefile | 1 + > > drivers/iio/dac/ltc2664.c | 806 ++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 819 insertions(+) > > create mode 100644 drivers/iio/dac/ltc2664.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ac1e29e26f31..1262e1231923 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -13071,6 +13071,7 @@ S: Supported > > W: https://ez.analog.com/linux-software-drivers > > F: Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml > > F: Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > > +F: drivers/iio/dac/ltc2664.c > > > > LTC2688 IIO DAC DRIVER > > M: Nuno Sá <nuno.sa@analog.com> > > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > > index 3c2bf620f00f..3d065c157605 100644 > > --- a/drivers/iio/dac/Kconfig > > +++ b/drivers/iio/dac/Kconfig > > @@ -370,6 +370,17 @@ config LTC2632 > > To compile this driver as a module, choose M here: the > > module will be called ltc2632. > > > > +config LTC2664 > > + tristate "Analog Devices LTC2664 and LTC2672 DAC SPI driver" > > + depends on SPI > > + select REGMAP > > + help > > + Say yes here to build support for Analog Devices > > + LTC2664 and LTC2672 converters (DAC). > > + > > + To compile this driver as a module, choose M here: the > > + module will be called ltc2664. > > + > > config M62332 > > tristate "Mitsubishi M62332 DAC driver" > > depends on I2C > > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile > > index 8432a81a19dc..2cf148f16306 100644 > > --- a/drivers/iio/dac/Makefile > > +++ b/drivers/iio/dac/Makefile > > @@ -37,6 +37,7 @@ obj-$(CONFIG_DS4424) += ds4424.o > > obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o > > obj-$(CONFIG_LTC1660) += ltc1660.o > > obj-$(CONFIG_LTC2632) += ltc2632.o > > +obj-$(CONFIG_LTC2664) += ltc2664.o > > obj-$(CONFIG_LTC2688) += ltc2688.o > > obj-$(CONFIG_M62332) += m62332.o > > obj-$(CONFIG_MAX517) += max517.o > > diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c > > new file mode 100644 > > index 000000000000..ef5d7d6fec5a > > --- /dev/null > > +++ b/drivers/iio/dac/ltc2664.c > > @@ -0,0 +1,806 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC driver > > + * LTC2672 5 channel, 16 bit Current Output Softspan DAC driver > > + * > > + * Copyright 2024 Analog Devices Inc. > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/cleanup.h> > > +#include <linux/device.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/iio/iio.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/mutex.h> > > +#include <linux/property.h> > > +#include <linux/regmap.h> > > +#include <linux/regulator/consumer.h> > > +#include <linux/spi/spi.h> > > + > > +#define LTC2664_CMD_WRITE_N(n) (0x00 + (n)) > > +#define LTC2664_CMD_UPDATE_N(n) (0x10 + (n)) > > +#define LTC2664_CMD_WRITE_N_UPDATE_ALL 0x20 > > +#define LTC2664_CMD_WRITE_N_UPDATE_N(n) (0x30 + (n)) > > +#define LTC2664_CMD_POWER_DOWN_N(n) (0x40 + (n)) > > +#define LTC2664_CMD_POWER_DOWN_ALL 0x50 > > +#define LTC2664_CMD_SPAN_N(n) (0x60 + (n)) > > +#define LTC2664_CMD_CONFIG 0x70 > > +#define LTC2664_CMD_MUX 0xB0 > > +#define LTC2664_CMD_TOGGLE_SEL 0xC0 > > +#define LTC2664_CMD_GLOBAL_TOGGLE 0xD0 > > +#define LTC2664_CMD_NO_OPERATION 0xF0 > > +#define LTC2664_REF_DISABLE 0x0001 > > +#define LTC2664_MSPAN_SOFTSPAN 7 > > + > > +#define LTC2672_MAX_CHANNEL 5 > > +#define LTC2672_MAX_SPAN 7 > > +#define LTC2672_SCALE_MULTIPLIER(n) (50 * BIT(n)) > > + > > +enum ltc2664_ids { > > + LTC2664, > > + LTC2672, > > +}; > > + > > +enum { > > + LTC2664_SPAN_RANGE_0V_5V, > > + LTC2664_SPAN_RANGE_0V_10V, > > + LTC2664_SPAN_RANGE_M5V_5V, > > + LTC2664_SPAN_RANGE_M10V_10V, > > + LTC2664_SPAN_RANGE_M2V5_2V5, > > +}; > > + > > +enum { > > + LTC2664_INPUT_A, > > + LTC2664_INPUT_B, > > + LTC2664_INPUT_B_AVAIL, > > + LTC2664_POWERDOWN, > > + LTC2664_POWERDOWN_MODE, > > + LTC2664_TOGGLE_EN, > > + LTC2664_GLOBAL_TOGGLE, > > +}; > > + > > +static const u16 ltc2664_mspan_lut[8][2] = { > > + { LTC2664_SPAN_RANGE_M10V_10V, 32768 }, /* MPS2=0, MPS1=0, > MSP0=0 (0)*/ > > + { LTC2664_SPAN_RANGE_M5V_5V, 32768 }, /* MPS2=0, MPS1=0, > MSP0=1 (1)*/ > > + { LTC2664_SPAN_RANGE_M2V5_2V5, 32768 }, /* MPS2=0, MPS1=1, > MSP0=0 (2)*/ > > + { LTC2664_SPAN_RANGE_0V_10V, 0 }, /* MPS2=0, MPS1=1, MSP0=1 > (3)*/ > > + { LTC2664_SPAN_RANGE_0V_10V, 32768 }, /* MPS2=1, MPS1=0, > MSP0=0 (4)*/ > > + { LTC2664_SPAN_RANGE_0V_5V, 0 }, /* MPS2=1, MPS1=0, MSP0=1 > (5)*/ > > + { LTC2664_SPAN_RANGE_0V_5V, 32768 }, /* MPS2=1, MPS1=1, > MSP0=0 (6)*/ > > + { LTC2664_SPAN_RANGE_0V_5V, 0 } /* MPS2=1, MPS1=1, MSP0=1 > (7)*/ > > +}; > > + > > +struct ltc2664_state; > > + > > +struct ltc2664_chip_info { > > + enum ltc2664_ids id; > > + const char *name; > > + int (*scale_get)(const struct ltc2664_state *st, int c); > > + int (*offset_get)(const struct ltc2664_state *st, int c); > > + int measurement_type; > > + unsigned int num_channels; > > + const struct iio_chan_spec *iio_chan; > > + const int (*span_helper)[2]; > > + unsigned int num_span; > > + unsigned int internal_vref; > > + bool manual_span_support; > > + bool rfsadj_support; > > +}; > > + > > +struct ltc2664_chan { > > + bool toggle_chan; > > + bool powerdown; > > + u8 span; > > + u16 raw[2]; /* A/B */ > > +}; > > I would find it helpful to have more comments explainging what the various > fields are for. For example, raw to be used to supply data to a SPI xfer > but actually it is just a shadow copy of the current state of the chip > registers. > > > + > > +struct ltc2664_state { > > + struct spi_device *spi; > > + struct regmap *regmap; > > + struct ltc2664_chan channels[LTC2672_MAX_CHANNEL]; > > + /* lock to protect against multiple access to the device and shared data > */ > > + struct mutex lock; > > + const struct ltc2664_chip_info *chip_info; > > + struct iio_chan_spec *iio_channels; > > + int vref; > > vref_mv > > Always nice to have the units since regulators use µV and IIO uses mV. > Otherwise we have to guess. > > > + u32 toggle_sel; > > + u32 global_toggle; > > Should this be bool? > > > + u32 rfsadj; > > rfsadj_ohms > > > +}; > > + > > +static const int ltc2664_span_helper[][2] = { > > + { 0, 5000 }, > > + { 0, 10000 }, > > + { -5000, 5000 }, > > + { -10000, 10000 }, > > + { -2500, 2500 }, > > +}; > > + > > +static const int ltc2672_span_helper[][2] = { > > + { 0, 3125 }, > > + { 0, 6250 }, > > + { 0, 12500 }, > > + { 0, 25000 }, > > + { 0, 50000 }, > > + { 0, 100000 }, > > + { 0, 200000 }, > > + { 0, 300000 }, > > +}; > > + > > +static int ltc2664_scale_get(const struct ltc2664_state *st, int c) > > +{ > > + const struct ltc2664_chan *chan = &st->channels[c]; > > + const int (*span_helper)[2] = st->chip_info->span_helper; > > + int span, fs; > > + > > + span = chan->span; > > + if (span < 0) > > + return span; > > + > > + fs = span_helper[span][1] - span_helper[span][0]; > > + > > + return (fs / 2500) * st->vref; > > Should we multiply first and then divide? 3125 isn't divisible by 2500 > so there may be unwanted rounding otherwise. > > > +} > > + > > +static int ltc2672_scale_get(const struct ltc2664_state *st, int c) > > +{ > > + const struct ltc2664_chan *chan = &st->channels[c]; > > + int span, fs; > > + > > + span = chan->span; > > + if (span < 0) > > + return span; > > + > > + fs = 1000 * st->vref / st->rfsadj; > > + > > + if (span == LTC2672_MAX_SPAN) > > + return 4800 * fs; > > + > > + return LTC2672_SCALE_MULTIPLIER(span) * fs; > > Are we losing accuracy by multiplying after dividing here as well? Hi, In the case of max span for ltc2672, I found that performing multiplication before division causes an integer overflow during testing. I was wondering how the upstream handles this case. Could you provide some advice? Thanks, Kim
On 6/18/24 5:32 AM, Paller, Kim Seer wrote: > > >>> +} >>> + >>> +static int ltc2672_scale_get(const struct ltc2664_state *st, int c) >>> +{ >>> + const struct ltc2664_chan *chan = &st->channels[c]; >>> + int span, fs; >>> + >>> + span = chan->span; >>> + if (span < 0) >>> + return span; >>> + >>> + fs = 1000 * st->vref / st->rfsadj; >>> + >>> + if (span == LTC2672_MAX_SPAN) >>> + return 4800 * fs; >>> + >>> + return LTC2672_SCALE_MULTIPLIER(span) * fs; >> >> Are we losing accuracy by multiplying after dividing here as well? > > Hi, > > In the case of max span for ltc2672, I found that performing multiplication > before division causes an integer overflow during testing. I was wondering > how the upstream handles this case. Could you provide some advice? > > Thanks, > Kim > > In cases like this, we usually do 64-bit multiplication to avoid the overflow. There are helper functions for this sort of thing in linux/math64.h. For example, if LTC2672_SCALE_MULTIPLIER(span) is unsigned, you could probably do something like this: mul_u64_u32_div(LTC2672_SCALE_MULTIPLIER(span), 1000 * st->vref, st->rfsadj);
diff --git a/MAINTAINERS b/MAINTAINERS index ac1e29e26f31..1262e1231923 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13071,6 +13071,7 @@ S: Supported W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml F: Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml +F: drivers/iio/dac/ltc2664.c LTC2688 IIO DAC DRIVER M: Nuno Sá <nuno.sa@analog.com> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index 3c2bf620f00f..3d065c157605 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -370,6 +370,17 @@ config LTC2632 To compile this driver as a module, choose M here: the module will be called ltc2632. +config LTC2664 + tristate "Analog Devices LTC2664 and LTC2672 DAC SPI driver" + depends on SPI + select REGMAP + help + Say yes here to build support for Analog Devices + LTC2664 and LTC2672 converters (DAC). + + To compile this driver as a module, choose M here: the + module will be called ltc2664. + config M62332 tristate "Mitsubishi M62332 DAC driver" depends on I2C diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 8432a81a19dc..2cf148f16306 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_DS4424) += ds4424.o obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o obj-$(CONFIG_LTC1660) += ltc1660.o obj-$(CONFIG_LTC2632) += ltc2632.o +obj-$(CONFIG_LTC2664) += ltc2664.o obj-$(CONFIG_LTC2688) += ltc2688.o obj-$(CONFIG_M62332) += m62332.o obj-$(CONFIG_MAX517) += max517.o diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c new file mode 100644 index 000000000000..ef5d7d6fec5a --- /dev/null +++ b/drivers/iio/dac/ltc2664.c @@ -0,0 +1,806 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC driver + * LTC2672 5 channel, 16 bit Current Output Softspan DAC driver + * + * Copyright 2024 Analog Devices Inc. + */ + +#include <linux/bitfield.h> +#include <linux/cleanup.h> +#include <linux/device.h> +#include <linux/gpio/consumer.h> +#include <linux/iio/iio.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/mutex.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#define LTC2664_CMD_WRITE_N(n) (0x00 + (n)) +#define LTC2664_CMD_UPDATE_N(n) (0x10 + (n)) +#define LTC2664_CMD_WRITE_N_UPDATE_ALL 0x20 +#define LTC2664_CMD_WRITE_N_UPDATE_N(n) (0x30 + (n)) +#define LTC2664_CMD_POWER_DOWN_N(n) (0x40 + (n)) +#define LTC2664_CMD_POWER_DOWN_ALL 0x50 +#define LTC2664_CMD_SPAN_N(n) (0x60 + (n)) +#define LTC2664_CMD_CONFIG 0x70 +#define LTC2664_CMD_MUX 0xB0 +#define LTC2664_CMD_TOGGLE_SEL 0xC0 +#define LTC2664_CMD_GLOBAL_TOGGLE 0xD0 +#define LTC2664_CMD_NO_OPERATION 0xF0 +#define LTC2664_REF_DISABLE 0x0001 +#define LTC2664_MSPAN_SOFTSPAN 7 + +#define LTC2672_MAX_CHANNEL 5 +#define LTC2672_MAX_SPAN 7 +#define LTC2672_SCALE_MULTIPLIER(n) (50 * BIT(n)) + +enum ltc2664_ids { + LTC2664, + LTC2672, +}; + +enum { + LTC2664_SPAN_RANGE_0V_5V, + LTC2664_SPAN_RANGE_0V_10V, + LTC2664_SPAN_RANGE_M5V_5V, + LTC2664_SPAN_RANGE_M10V_10V, + LTC2664_SPAN_RANGE_M2V5_2V5, +}; + +enum { + LTC2664_INPUT_A, + LTC2664_INPUT_B, + LTC2664_INPUT_B_AVAIL, + LTC2664_POWERDOWN, + LTC2664_POWERDOWN_MODE, + LTC2664_TOGGLE_EN, + LTC2664_GLOBAL_TOGGLE, +}; + +static const u16 ltc2664_mspan_lut[8][2] = { + { LTC2664_SPAN_RANGE_M10V_10V, 32768 }, /* MPS2=0, MPS1=0, MSP0=0 (0)*/ + { LTC2664_SPAN_RANGE_M5V_5V, 32768 }, /* MPS2=0, MPS1=0, MSP0=1 (1)*/ + { LTC2664_SPAN_RANGE_M2V5_2V5, 32768 }, /* MPS2=0, MPS1=1, MSP0=0 (2)*/ + { LTC2664_SPAN_RANGE_0V_10V, 0 }, /* MPS2=0, MPS1=1, MSP0=1 (3)*/ + { LTC2664_SPAN_RANGE_0V_10V, 32768 }, /* MPS2=1, MPS1=0, MSP0=0 (4)*/ + { LTC2664_SPAN_RANGE_0V_5V, 0 }, /* MPS2=1, MPS1=0, MSP0=1 (5)*/ + { LTC2664_SPAN_RANGE_0V_5V, 32768 }, /* MPS2=1, MPS1=1, MSP0=0 (6)*/ + { LTC2664_SPAN_RANGE_0V_5V, 0 } /* MPS2=1, MPS1=1, MSP0=1 (7)*/ +}; + +struct ltc2664_state; + +struct ltc2664_chip_info { + enum ltc2664_ids id; + const char *name; + int (*scale_get)(const struct ltc2664_state *st, int c); + int (*offset_get)(const struct ltc2664_state *st, int c); + int measurement_type; + unsigned int num_channels; + const struct iio_chan_spec *iio_chan; + const int (*span_helper)[2]; + unsigned int num_span; + unsigned int internal_vref; + bool manual_span_support; + bool rfsadj_support; +}; + +struct ltc2664_chan { + bool toggle_chan; + bool powerdown; + u8 span; + u16 raw[2]; /* A/B */ +}; + +struct ltc2664_state { + struct spi_device *spi; + struct regmap *regmap; + struct ltc2664_chan channels[LTC2672_MAX_CHANNEL]; + /* lock to protect against multiple access to the device and shared data */ + struct mutex lock; + const struct ltc2664_chip_info *chip_info; + struct iio_chan_spec *iio_channels; + int vref; + u32 toggle_sel; + u32 global_toggle; + u32 rfsadj; +}; + +static const int ltc2664_span_helper[][2] = { + { 0, 5000 }, + { 0, 10000 }, + { -5000, 5000 }, + { -10000, 10000 }, + { -2500, 2500 }, +}; + +static const int ltc2672_span_helper[][2] = { + { 0, 3125 }, + { 0, 6250 }, + { 0, 12500 }, + { 0, 25000 }, + { 0, 50000 }, + { 0, 100000 }, + { 0, 200000 }, + { 0, 300000 }, +}; + +static int ltc2664_scale_get(const struct ltc2664_state *st, int c) +{ + const struct ltc2664_chan *chan = &st->channels[c]; + const int (*span_helper)[2] = st->chip_info->span_helper; + int span, fs; + + span = chan->span; + if (span < 0) + return span; + + fs = span_helper[span][1] - span_helper[span][0]; + + return (fs / 2500) * st->vref; +} + +static int ltc2672_scale_get(const struct ltc2664_state *st, int c) +{ + const struct ltc2664_chan *chan = &st->channels[c]; + int span, fs; + + span = chan->span; + if (span < 0) + return span; + + fs = 1000 * st->vref / st->rfsadj; + + if (span == LTC2672_MAX_SPAN) + return 4800 * fs; + + return LTC2672_SCALE_MULTIPLIER(span) * fs; +} + +static int ltc2664_offset_get(const struct ltc2664_state *st, int c) +{ + const struct ltc2664_chan *chan = &st->channels[c]; + int span; + + span = chan->span; + if (span < 0) + return span; + + if (st->chip_info->span_helper[span][0] < 0) + return -32768; + + return 0; +} + +static int ltc2672_offset_get(const struct ltc2664_state *st, int c) +{ + const struct ltc2664_chan *chan = &st->channels[c]; + int span; + + span = chan->span; + if (span < 0) + return span; + + if (st->chip_info->span_helper[span][1] < 0) + return -32768; + + return st->chip_info->span_helper[span][1] / 250; +} + +static int ltc2664_dac_code_write(struct ltc2664_state *st, u32 chan, u32 input, + u16 code) +{ + struct ltc2664_chan *c = &st->channels[chan]; + int ret, reg; + + guard(mutex)(&st->lock); + /* select the correct input register to write to */ + if (c->toggle_chan) { + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, + input << chan); + if (ret) + return ret; + } + /* + * If in toggle mode the dac should be updated by an + * external signal (or sw toggle) and not here. + */ + if (st->toggle_sel & BIT(chan)) + reg = LTC2664_CMD_WRITE_N(chan); + else + reg = LTC2664_CMD_WRITE_N_UPDATE_N(chan); + + ret = regmap_write(st->regmap, reg, code); + if (ret) + return ret; + + c->raw[input] = code; + + if (c->toggle_chan) { + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, + st->toggle_sel); + if (ret) + return ret; + } + + return ret; +} + +static int ltc2664_dac_code_read(struct ltc2664_state *st, u32 chan, u32 input, + u32 *code) +{ + guard(mutex)(&st->lock); + *code = st->channels[chan].raw[input]; + + return 0; +} + +static const int ltc2664_raw_range[] = {0, 1, U16_MAX}; + +static int ltc2664_read_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, + long info) +{ + switch (info) { + case IIO_CHAN_INFO_RAW: + *vals = ltc2664_raw_range; + *type = IIO_VAL_INT; + + return IIO_AVAIL_RANGE; + default: + return -EINVAL; + } +} + +static int ltc2664_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long info) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + int ret; + + switch (info) { + case IIO_CHAN_INFO_RAW: + ret = ltc2664_dac_code_read(st, chan->channel, + LTC2664_INPUT_A, val); + if (ret) + return ret; + + return IIO_VAL_INT; + case IIO_CHAN_INFO_OFFSET: + *val = st->chip_info->offset_get(st, chan->channel); + + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = st->chip_info->scale_get(st, chan->channel); + + *val2 = 16; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } +} + +static int ltc2664_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int val, + int val2, long info) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_RAW: + if (val > U16_MAX || val < 0) + return -EINVAL; + + return ltc2664_dac_code_write(st, chan->channel, + LTC2664_INPUT_A, val); + default: + return -EINVAL; + } +} + +static ssize_t ltc2664_reg_bool_get(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + char *buf) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + u32 val; + + guard(mutex)(&st->lock); + switch (private) { + case LTC2664_POWERDOWN: + val = st->channels[chan->channel].powerdown; + + return sysfs_emit(buf, "%u\n", val); + case LTC2664_POWERDOWN_MODE: + return sysfs_emit(buf, "42kohm_to_gnd\n"); + case LTC2664_TOGGLE_EN: + val = !!(st->toggle_sel & BIT(chan->channel)); + + return sysfs_emit(buf, "%u\n", val); + case LTC2664_GLOBAL_TOGGLE: + val = st->global_toggle; + + return sysfs_emit(buf, "%u\n", val); + default: + return -EINVAL; + } +} + +static ssize_t ltc2664_reg_bool_set(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + int ret; + bool en; + + ret = kstrtobool(buf, &en); + if (ret) + return ret; + + guard(mutex)(&st->lock); + switch (private) { + case LTC2664_POWERDOWN: + ret = regmap_write(st->regmap, + en ? LTC2664_CMD_POWER_DOWN_N(chan->channel) : + LTC2664_CMD_UPDATE_N(chan->channel), en); + if (ret) + return ret; + + st->channels[chan->channel].powerdown = en; + + return len; + case LTC2664_TOGGLE_EN: + if (en) + st->toggle_sel |= BIT(chan->channel); + else + st->toggle_sel &= ~BIT(chan->channel); + + ret = regmap_write(st->regmap, LTC2664_CMD_TOGGLE_SEL, + st->toggle_sel); + if (ret) + return ret; + + return len; + case LTC2664_GLOBAL_TOGGLE: + ret = regmap_write(st->regmap, LTC2664_CMD_GLOBAL_TOGGLE, en); + if (ret) + return ret; + + st->global_toggle = en; + + return len; + default: + return -EINVAL; + } +} + +static ssize_t ltc2664_dac_input_read(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + char *buf) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + int ret; + u32 val; + + if (private == LTC2664_INPUT_B_AVAIL) + return sysfs_emit(buf, "[%u %u %u]\n", ltc2664_raw_range[0], + ltc2664_raw_range[1], + ltc2664_raw_range[2] / 4); + + ret = ltc2664_dac_code_read(st, chan->channel, private, &val); + if (ret) + return ret; + + return sysfs_emit(buf, "%u\n", val); +} + +static ssize_t ltc2664_dac_input_write(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + int ret; + u16 val; + + if (private == LTC2664_INPUT_B_AVAIL) + return -EINVAL; + + ret = kstrtou16(buf, 10, &val); + if (ret) + return ret; + + ret = ltc2664_dac_code_write(st, chan->channel, private, val); + if (ret) + return ret; + + return len; +} + +static int ltc2664_reg_access(struct iio_dev *indio_dev, + unsigned int reg, + unsigned int writeval, + unsigned int *readval) +{ + struct ltc2664_state *st = iio_priv(indio_dev); + + if (readval) + return -EOPNOTSUPP; + + return regmap_write(st->regmap, reg, writeval); +} + +#define LTC2664_CHAN_EXT_INFO(_name, _what, _shared, _read, _write) { \ + .name = _name, \ + .read = (_read), \ + .write = (_write), \ + .private = (_what), \ + .shared = (_shared), \ +} + +/* + * For toggle mode we only expose the symbol attr (sw_toggle) in case a TGPx is + * not provided in dts. + */ +static const struct iio_chan_spec_ext_info ltc2664_toggle_sym_ext_info[] = { + LTC2664_CHAN_EXT_INFO("raw0", LTC2664_INPUT_A, IIO_SEPARATE, + ltc2664_dac_input_read, ltc2664_dac_input_write), + LTC2664_CHAN_EXT_INFO("raw1", LTC2664_INPUT_B, IIO_SEPARATE, + ltc2664_dac_input_read, ltc2664_dac_input_write), + LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, IIO_SEPARATE, + ltc2664_reg_bool_get, ltc2664_reg_bool_set), + LTC2664_CHAN_EXT_INFO("powerdown_mode", LTC2664_POWERDOWN_MODE, + IIO_SEPARATE, ltc2664_reg_bool_get, NULL), + LTC2664_CHAN_EXT_INFO("symbol", LTC2664_GLOBAL_TOGGLE, IIO_SEPARATE, + ltc2664_reg_bool_get, ltc2664_reg_bool_set), + LTC2664_CHAN_EXT_INFO("toggle_en", LTC2664_TOGGLE_EN, + IIO_SEPARATE, ltc2664_reg_bool_get, + ltc2664_reg_bool_set), + { } +}; + +static const struct iio_chan_spec_ext_info ltc2664_ext_info[] = { + LTC2664_CHAN_EXT_INFO("powerdown", LTC2664_POWERDOWN, IIO_SEPARATE, + ltc2664_reg_bool_get, ltc2664_reg_bool_set), + LTC2664_CHAN_EXT_INFO("powerdown_mode", LTC2664_POWERDOWN_MODE, + IIO_SEPARATE, ltc2664_reg_bool_get, NULL), + { } +}; + +#define LTC2664_CHANNEL(_chan) { \ + .indexed = 1, \ + .output = 1, \ + .channel = (_chan), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW), \ + .ext_info = ltc2664_ext_info, \ +} + +static const struct iio_chan_spec ltc2664_channels[] = { + LTC2664_CHANNEL(0), + LTC2664_CHANNEL(1), + LTC2664_CHANNEL(2), + LTC2664_CHANNEL(3), +}; + +static const struct iio_chan_spec ltc2672_channels[] = { + LTC2664_CHANNEL(0), + LTC2664_CHANNEL(1), + LTC2664_CHANNEL(2), + LTC2664_CHANNEL(3), + LTC2664_CHANNEL(4), +}; + +static const struct ltc2664_chip_info ltc2664_chip = { + .id = LTC2664, + .name = "ltc2664", + .scale_get = ltc2664_scale_get, + .offset_get = ltc2664_offset_get, + .measurement_type = IIO_VOLTAGE, + .num_channels = ARRAY_SIZE(ltc2664_channels), + .iio_chan = ltc2664_channels, + .span_helper = ltc2664_span_helper, + .num_span = ARRAY_SIZE(ltc2664_span_helper), + .internal_vref = 2500, + .manual_span_support = true, + .rfsadj_support = false, +}; + +static const struct ltc2664_chip_info ltc2672_chip = { + .id = LTC2672, + .name = "ltc2672", + .scale_get = ltc2672_scale_get, + .offset_get = ltc2672_offset_get, + .measurement_type = IIO_CURRENT, + .num_channels = ARRAY_SIZE(ltc2672_channels), + .iio_chan = ltc2672_channels, + .span_helper = ltc2672_span_helper, + .num_span = ARRAY_SIZE(ltc2672_span_helper), + .internal_vref = 1250, + .manual_span_support = false, + .rfsadj_support = true, +}; + +static int ltc2664_set_span(const struct ltc2664_state *st, int min, int max, + int chan) +{ + const struct ltc2664_chip_info *chip_info = st->chip_info; + const int (*span_helper)[2] = chip_info->span_helper; + int span, ret; + + st->iio_channels[chan].type = chip_info->measurement_type; + + for (span = 0; span < chip_info->num_span; span++) { + if (min == span_helper[span][0] && max == span_helper[span][1]) + break; + } + + if (span == chip_info->num_span) + return -EINVAL; + + ret = regmap_write(st->regmap, LTC2664_CMD_SPAN_N(chan), + (chip_info->id == LTC2672) ? span + 1 : span); + if (ret) + return ret; + + return span; +} + +static int ltc2664_channel_config(struct ltc2664_state *st) +{ + const struct ltc2664_chip_info *chip_info = st->chip_info; + struct device *dev = &st->spi->dev; + u32 reg, tmp[2], mspan; + int ret, span = 0; + + mspan = LTC2664_MSPAN_SOFTSPAN; + ret = device_property_read_u32(dev, "adi,manual-span-operation-config", + &mspan); + if (!ret) { + if (!chip_info->manual_span_support) + return dev_err_probe(dev, -EINVAL, + "adi,manual-span-operation-config not supported\n"); + + if (mspan > ARRAY_SIZE(ltc2664_mspan_lut)) + return dev_err_probe(dev, -EINVAL, + "adi,manual-span-operation-config not in range\n"); + } + + st->rfsadj = 20000; + ret = device_property_read_u32(dev, "adi,rfsadj-ohms", &st->rfsadj); + if (!ret) { + if (!chip_info->rfsadj_support) + return dev_err_probe(dev, -EINVAL, + "adi,rfsadj-ohms not supported\n"); + + if (st->rfsadj < 19000 || st->rfsadj > 41000) + return dev_err_probe(dev, -EINVAL, + "adi,rfsadj-ohms not in range\n"); + } + + device_for_each_child_node_scoped(dev, child) { + struct ltc2664_chan *chan; + + ret = fwnode_property_read_u32(child, "reg", ®); + if (ret) + return dev_err_probe(dev, ret, + "Failed to get reg property\n"); + + if (reg >= chip_info->num_channels) + return dev_err_probe(dev, -EINVAL, + "reg bigger than: %d\n", + chip_info->num_channels); + + chan = &st->channels[reg]; + + if (fwnode_property_read_bool(child, "adi,toggle-mode")) { + chan->toggle_chan = true; + /* assume sw toggle ABI */ + st->iio_channels[reg].ext_info = ltc2664_toggle_sym_ext_info; + + /* + * Clear IIO_CHAN_INFO_RAW bit as toggle channels expose + * out_voltage/current_raw{0|1} files. + */ + __clear_bit(IIO_CHAN_INFO_RAW, + &st->iio_channels[reg].info_mask_separate); + } + + chan->raw[0] = ltc2664_mspan_lut[mspan][1]; + chan->raw[1] = ltc2664_mspan_lut[mspan][1]; + + chan->span = ltc2664_mspan_lut[mspan][0]; + + ret = fwnode_property_read_u32_array(child, "adi,output-range-microvolt", + tmp, ARRAY_SIZE(tmp)); + if (!ret && mspan == LTC2664_MSPAN_SOFTSPAN) { + chan->span = ltc2664_set_span(st, tmp[0] / 1000, + tmp[1] / 1000, reg); + if (span < 0) + return dev_err_probe(dev, span, + "Failed to set span\n"); + + } + + ret = fwnode_property_read_u32(child, + "adi,output-range-microamp", + &tmp[0]); + if (!ret) { + chan->span = ltc2664_set_span(st, 0, tmp[0] / 1000, reg); + if (span < 0) + return dev_err_probe(dev, span, + "Failed to set span\n"); + } + } + + return 0; +} + +static int ltc2664_setup(struct ltc2664_state *st, struct regulator *vref) +{ + const struct ltc2664_chip_info *chip_info = st->chip_info; + struct gpio_desc *gpio; + int ret; + + /* If we have a clr/reset pin, use that to reset the chip. */ + gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(gpio)) + return dev_err_probe(&st->spi->dev, PTR_ERR(gpio), + "Failed to get reset gpio"); + if (gpio) { + usleep_range(1000, 1200); + gpiod_set_value_cansleep(gpio, 0); + } + + /* + * Duplicate the default channel configuration as it can change during + * @ltc2664_channel_config() + */ + st->iio_channels = devm_kmemdup(&st->spi->dev, chip_info->iio_chan, + (chip_info->num_channels + 1) * + sizeof(*chip_info->iio_chan), + GFP_KERNEL); + + ret = ltc2664_channel_config(st); + if (ret) + return ret; + + if (!vref) + return 0; + + return regmap_set_bits(st->regmap, LTC2664_CMD_CONFIG, LTC2664_REF_DISABLE); +} + +static void ltc2664_disable_regulator(void *regulator) +{ + regulator_disable(regulator); +} + +static const struct regmap_config ltc2664_regmap_config = { + .reg_bits = 8, + .val_bits = 16, + .max_register = LTC2664_CMD_NO_OPERATION, +}; + +static const struct iio_info ltc2664_info = { + .write_raw = ltc2664_write_raw, + .read_raw = ltc2664_read_raw, + .read_avail = ltc2664_read_avail, + .debugfs_reg_access = ltc2664_reg_access, +}; + +static int ltc2664_probe(struct spi_device *spi) +{ + static const char * const regulators[] = { "vcc", "iovcc", "v-neg" }; + const struct ltc2664_chip_info *chip_info; + struct device *dev = &spi->dev; + struct regulator *vref_reg; + struct iio_dev *indio_dev; + struct ltc2664_state *st; + int ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + + chip_info = spi_get_device_match_data(spi); + if (!chip_info) + return -ENOMEM; + + st->chip_info = chip_info; + + mutex_init(&st->lock); + + st->regmap = devm_regmap_init_spi(spi, <c2664_regmap_config); + if (IS_ERR(st->regmap)) + return dev_err_probe(dev, PTR_ERR(st->regmap), + "Failed to init regmap"); + + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), + regulators); + if (ret) + return dev_err_probe(dev, ret, "Failed to enable regulators\n"); + + vref_reg = devm_regulator_get_optional(dev, "ref"); + if (IS_ERR(vref_reg)) { + if (PTR_ERR(vref_reg) != -ENODEV) + return dev_err_probe(dev, PTR_ERR(vref_reg), + "Failed to get ref regulator"); + + vref_reg = NULL; + + st->vref = chip_info->internal_vref; + } else { + ret = regulator_enable(vref_reg); + if (ret) + return dev_err_probe(dev, ret, + "Failed to enable ref regulators\n"); + + ret = devm_add_action_or_reset(dev, ltc2664_disable_regulator, + vref_reg); + if (ret) + return ret; + + ret = regulator_get_voltage(vref_reg); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to get ref\n"); + + st->vref = ret / 1000; + } + + ret = ltc2664_setup(st, vref_reg); + if (ret) + return ret; + + indio_dev->name = chip_info->name; + indio_dev->info = <c2664_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = st->iio_channels; + indio_dev->num_channels = chip_info->num_channels; + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct spi_device_id ltc2664_id[] = { + { "ltc2664", (kernel_ulong_t)<c2664_chip }, + { "ltc2672", (kernel_ulong_t)<c2672_chip }, + { } +}; +MODULE_DEVICE_TABLE(spi, ltc2664_id); + +static const struct of_device_id ltc2664_of_id[] = { + { .compatible = "adi,ltc2664", .data = <c2664_chip }, + { .compatible = "adi,ltc2672", .data = <c2672_chip }, + { } +}; +MODULE_DEVICE_TABLE(of, ltc2664_of_id); + +static struct spi_driver ltc2664_driver = { + .driver = { + .name = "ltc2664", + .of_match_table = ltc2664_of_id, + }, + .probe = ltc2664_probe, + .id_table = ltc2664_id, +}; +module_spi_driver(ltc2664_driver); + +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); +MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>"); +MODULE_DESCRIPTION("Analog Devices LTC2664 and LTC2672 DAC"); +MODULE_LICENSE("GPL");