On Fri, 4 Dec 2020 20:20:43 +0200 Cristian Pop <cristian....@analog.com> wrote:
> The AD5766/AD5767 are 16-channel, 16-bit/12-bit, voltage output dense DACs > Digital-to-Analog converters. > > This change adds support for these DACs. > > Link: > https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf > > Signed-off-by: Cristian Pop <cristian....@analog.com> Missing build files + docs for the new ABI. Note it doesn't build so a few things to fix on that front! Docs in appropriate file under Documentation/ABI/testing/sysfs-bus-iio-* I'm a bit curious about the range being entirely controllable from userspace as well. Seems like something that might be dangerous in some systems. Perhaps we need some sort of dt binding restriction mechanism? > --- > Changes in v2: > -Remove forward declarations, arrange code > -New ABI docs > -Move "max_val" scope in case > -Remove blank line > -Use bitfield operations, where posible > -Change declaration type to int of: > int scale_avail[AD5766_VOLTAGE_RANGE_MAX][2]; > int offset_avail[AD5766_VOLTAGE_RANGE_MAX][2]; > -Move initialization down to just above where it is used: > "type = spi_get_device_id(spi)->driver_data;" > > drivers/iio/dac/ad5766.c | 758 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 758 insertions(+) > create mode 100644 drivers/iio/dac/ad5766.c > > diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c > new file mode 100644 > index 000000000000..e6d24a41bd4e > --- /dev/null > +++ b/drivers/iio/dac/ad5766.c > @@ -0,0 +1,758 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Analog Devices AD5766, AD5767 > + * Digital to Analog Converters driver > + * > + * Copyright 2019-2020 Analog Devices Inc. > + */ > + > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/spi/spi.h> > +#include <linux/iio/iio.h> > +#include <linux/bitfield.h> > + > +#define AD5766_UPPER_WORD_SPI_MASK GENMASK(31, 16) > +#define AD5766_LOWER_WORD_SPI_MASK GENMASK(15, 0) > +#define AD5766_DITHER_SOURCE_MASK(x) GENMASK(((2 * x) + 1), (2 * x)) > +#define AD5766_DITHER_SCALE_MASK(x) AD5766_DITHER_SOURCE_MASK(x) > + > +#define AD5766_CMD_NOP_MUX_OUT 0x00 > +#define AD5766_CMD_SDO_CNTRL 0x01 > +#define AD5766_CMD_WR_IN_REG(x) (0x10 | ((x) & 0xF)) > +#define AD5766_CMD_WR_DAC_REG(x) (0x20 | ((x) & 0xF)) > +#define AD5766_CMD_SW_LDAC 0x30 > +#define AD5766_CMD_SPAN_REG 0x40 > +#define AD5766_CMD_WR_PWR_DITHER 0x51 > +#define AD5766_CMD_WR_DAC_REG_ALL 0x60 > +#define AD5766_CMD_SW_FULL_RESET 0x70 > +#define AD5766_CMD_READBACK_REG(x) (0x80 | ((x) & 0xF)) > +#define AD5766_CMD_DITHER_SIG_1 0x90 > +#define AD5766_CMD_DITHER_SIG_2 0xA0 > +#define AD5766_CMD_INV_DITHER 0xB0 > +#define AD5766_CMD_DITHER_SCALE_1 0xC0 > +#define AD5766_CMD_DITHER_SCALE_2 0xD0 > + > +#define AD5766_FULL_RESET_CODE 0x1234 > + > +enum ad5766_type { > + ID_AD5766, > + ID_AD5767, > +}; > + > +enum ad5766_voltage_range { > + AD5766_VOLTAGE_RANGE_M20V_0V, > + AD5766_VOLTAGE_RANGE_M16V_to_0V, > + AD5766_VOLTAGE_RANGE_M10V_to_0V, > + AD5766_VOLTAGE_RANGE_M12V_to_14V, > + AD5766_VOLTAGE_RANGE_M16V_to_10V, > + AD5766_VOLTAGE_RANGE_M10V_to_6V, > + AD5766_VOLTAGE_RANGE_M5V_to_5V, > + AD5766_VOLTAGE_RANGE_M10V_to_10V, > + AD5766_VOLTAGE_RANGE_MAX, > +}; > + > +/** > + * struct ad5766_chip_info - chip specific information > + * @num_channels: number of channels > + * @channel: channel specification > + */ > +struct ad5766_chip_info { > + unsigned int num_channels; > + const struct iio_chan_spec *channels; > +}; > + > +enum { > + AD5766_DITHER_PWR, > + AD5766_DITHER_INVERT > +}; > + > +/* > + * External dither signal can be composed with the DAC output, if activated. > + * The dither signals are applied to the N0 and N1 input pins. > + * Dither source for each of the channel can be selected by writing to > + * "dither_source",a 32 bit variable and two bits are used for each of the 16 > + * channels: 0: NO_DITHER, 1: N0, 2: N1. > + * This variable holds available dither source strings. > + */ > +static const char * const ad5766_dither_sources[] = { > + "NO_DITHER", > + "N0", > + "N1", > +}; > + > +/* > + * Dither signal can also be scaled. > + * Available dither scale strings coresponding to "dither_scale" field in > + * "struct ad5766_state". > + * "dither_scale" is a 32 bit variable and two bits are used for each of the > 16 > + * channels: 0: NO_SCALING, 1: 75%_SCALING, 2: 50%_SCALING, 3: 25%_SCALING. Needs explicit ABI docs for a proper discussion. My gut feeling is it should be two controls. On/off + a scaling control that takes integer values. > + */ > +static const char * const ad5766_dither_scales[] = { > + "NO_SCALING", > + "75%_SCALING", > + "50%_SCALING", > + "25%_SCALING", > +}; > + > +/** > + * struct ad5766_state - driver instance specific data > + * @spi: SPI device > + * @lock: Mutex lock Say what exactly the scope of the lock is. No interest at all to tell us what is clear from the type of the structure element. > + * @chip_info: Chip model specific constants > + * @gpio_reset: Reset GPIO, used to reset the device > + * @crt_range: Current selected output range > + * @cached_offset: Cached range coresponding to the selected offset > + * @dither_power_ctrl: Power-down bit for each channel dither block > (for > + * example, D15 = DAC 15,D8 = DAC 8, and D0 = DAC 0) > + * 0 - Normal operation, 1 - Power down > + * @dither_invert: Inverts the dither signal applied to the selected DAC > + * outputs > + * @dither_source: Selects between 3 possible sources: > + * 0: No dither, 1: N0, 2: N1 > + * Two bits are used for each channel > + * @dither_scale: Selects between 4 possible scales: > + * 0: No scale, 1: 75%, 2: 50%, 3: 25% > + * Two bits are used for each channel > + * @scale_avail: Scale available table > + * @offset_avail: Offest available table > + * @data: SPI transfer buffers > + */ > +struct ad5766_state { > + struct spi_device *spi; > + struct mutex lock; > + const struct ad5766_chip_info *chip_info; > + struct gpio_desc *gpio_reset; > + enum ad5766_voltage_range crt_range; > + enum ad5766_voltage_range cached_offset; > + u16 dither_power_ctrl; > + u16 dither_invert; > + u32 dither_source; > + u32 dither_scale; > + int scale_avail[AD5766_VOLTAGE_RANGE_MAX][2]; > + int offset_avail[AD5766_VOLTAGE_RANGE_MAX][2]; > + union { > + u32 d32; > + u16 w16[2]; > + u8 b8[4]; > + } data[3] ____cacheline_aligned; > +}; > + ... > + > +static int _ad5766_spi_read(struct ad5766_state *st, u8 dac, int *val) > +{ > + int ret; > + struct spi_transfer xfers[] = { > + { > + .tx_buf = &st->data[0].d32, > + .bits_per_word = 8, > + .len = 3, > + .cs_change = 1, > + }, { > + .tx_buf = &st->data[1].d32, > + .rx_buf = &st->data[2].d32, > + .bits_per_word = 8, > + .len = 3, > + }, > + }; > + > + st->data[0].d32 = AD5766_CMD_READBACK_REG(dac); > + st->data[1].d32 = AD5766_CMD_NOP_MUX_OUT; > + > + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); > + if (ret) > + return ret; > + > + *val = st->data[2].w16[1]; > + > + return ret; > +} > + > +static int _ad5766_spi_write(struct ad5766_state *st, u8 command, u16 data) > +{ > + st->data[0].b8[0] = command; > + st->data[0].b8[1] = (data & 0xFF00) >> 8; > + st->data[0].b8[2] = (data & 0x00FF) >> 0; That's an unaligned put so ideally use put_unaligned_xx16 and friends to make that clear. > + > + return spi_write(st->spi, &st->data[0].b8[0], 3); > +} > + > +static int ad5766_read(struct iio_dev *indio_dev, u8 dac, int *val) > +{ > + struct ad5766_state *st = iio_priv(indio_dev); > + int ret; > + > + mutex_lock(&st->lock); > + ret = _ad5766_spi_read(st, dac, val); > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static int ad5766_write(struct iio_dev *indio_dev, u8 dac, u16 data) > +{ > + struct ad5766_state *st = iio_priv(indio_dev); > + int ret; > + > + mutex_lock(&st->lock); > + ret = _ad5766_spi_write(st, AD5766_CMD_WR_DAC_REG(dac), data); Normal convention for this sort of function would be __ rather than _ Looks more deliberate. > + mutex_unlock(&st->lock); > + > + return ret; > +} > + ... > + > +#define _AD5766_CHAN_EXT_INFO(_name, _what, _shared) { \ > + .name = _name, \ > + .read = ad5766_read_ext, \ > + .write = ad5766_write_ext, \ > + .private = _what, \ > + .shared = _shared, \ > +} > + > +static const struct iio_chan_spec_ext_info ad5766_ext_info[] = { > + > + _AD5766_CHAN_EXT_INFO("dither_pwr", AD5766_DITHER_PWR, IIO_SEPARATE), > + _AD5766_CHAN_EXT_INFO("dither_invert", AD5766_DITHER_INVERT, > + IIO_SEPARATE), > + IIO_ENUM("dither_source", IIO_SEPARATE, &ad5766_dither_source_enum), > + IIO_ENUM_AVAILABLE_SHARED("dither_source", > + IIO_SEPARATE, > + &ad5766_dither_source_enum), > + IIO_ENUM("dither_scale", IIO_SEPARATE, &ad5766_dither_scale_enum), > + IIO_ENUM_AVAILABLE_SHARED("dither_scale", That macro doesn't exist in mainline. > + IIO_SEPARATE, > + &ad5766_dither_scale_enum), > + {} > +}; All the above need ABI docs so we can talk about them without having to read data sheets. ...
