* [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation
@ 2020-11-23 14:50 Cristian Pop
2020-11-23 14:50 ` [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766 Cristian Pop
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Cristian Pop @ 2020-11-23 14:50 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, devicetree, robh+dt, Cristian Pop
This adds device tree bindings for the AD5766 DAC.
Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
.../bindings/iio/dac/adi,ad5766.yaml | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
new file mode 100644
index 000000000000..aed4a0472bc4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/iio/dac/adi,ad5766.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5766 DAC device driver
+
+maintainers:
+ - Cristian Pop <cristian.pop@analog.com>
+
+description: |
+ Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
+ found here:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
+
+
+properties:
+ compatible:
+ enum:
+ - adi,ad5766
+ - adi,ad5767
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 1000000
+
+ spi-cpol: true
+
+ reset-gpios:
+ description: GPIO spec for the RESET pin. If specified, it will be
+ asserted during driver probe.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - spi-cpol
+
+examples:
+ - |
+ ad5766@0{
+ compatible = "adi,ad5766";
+ reg = <0>;
+ spi-cpol;
+ spi-max-frequency = <1000000>;
+ reset-gpios = <&gpio 22 0>;
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766
2020-11-23 14:50 [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Cristian Pop
@ 2020-11-23 14:50 ` Cristian Pop
2020-11-24 12:02 ` Jonathan Cameron
2020-11-24 11:42 ` [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Jonathan Cameron
2020-11-30 16:38 ` Rob Herring
2 siblings, 1 reply; 5+ messages in thread
From: Cristian Pop @ 2020-11-23 14:50 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, devicetree, robh+dt, Cristian Pop
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.pop@analog.com>
---
drivers/iio/dac/ad5766.c | 768 +++++++++++++++++++++++++++++++++++++++
1 file changed, 768 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..feef78960990
--- /dev/null
+++ b/drivers/iio/dac/ad5766.c
@@ -0,0 +1,768 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD5766, AD5767
+ * Digital to Analog Converters driver
+ *
+ * Copyright 2019 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>
+
+#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,
+};
+
+static ssize_t ad5766_write_ext(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len);
+static ssize_t ad5766_read_ext(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf);
+
+static int ad5766_get_dither_source(struct iio_dev *dev,
+ const struct iio_chan_spec *chan);
+
+static int ad5766_set_dither_source(struct iio_dev *dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode);
+
+static int ad5766_get_dither_scale(struct iio_dev *dev,
+ const struct iio_chan_spec *chan);
+
+static int ad5766_set_dither_scale(struct iio_dev *dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode);
+
+#define _AD5766_CHAN_EXT_INFO(_name, _what, _shared) { \
+ .name = _name, \
+ .read = ad5766_read_ext, \
+ .write = ad5766_write_ext, \
+ .private = _what, \
+ .shared = _shared, \
+}
+
+enum {
+ AD5766_DITHER_PWR,
+ AD5766_DITHER_INVERT
+};
+
+static const char * const ad5766_dither_sources[] = {
+ "NO_DITHER",
+ "N0",
+ "N1",
+};
+
+static const char * const ad5766_dither_scales[] = {
+ "NO_SCALING",
+ "75%_SCALING",
+ "50%_SCALING",
+ "25%_SCALING",
+};
+
+static const struct iio_enum ad5766_dither_source_enum = {
+ .items = ad5766_dither_sources,
+ .num_items = ARRAY_SIZE(ad5766_dither_sources),
+ .set = ad5766_set_dither_source,
+ .get = ad5766_get_dither_source,
+};
+
+static const struct iio_enum ad5766_dither_scale_enum = {
+ .items = ad5766_dither_scales,
+ .num_items = ARRAY_SIZE(ad5766_dither_scales),
+ .set = ad5766_set_dither_scale,
+ .get = ad5766_get_dither_scale,
+};
+
+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",
+ IIO_SEPARATE,
+ &ad5766_dither_scale_enum),
+ {}
+};
+
+#define AD576x_CHANNEL(_chan, _bits) { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .output = 1, \
+ .channel = (_chan), \
+ .address = (_chan), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_type_available = \
+ BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (_bits), \
+ .storagebits = 16, \
+ .shift = 16 - (_bits), \
+ }, \
+ .ext_info = ad5766_ext_info, \
+}
+
+#define DECLARE_AD576x_CHANNELS(_name, _bits) \
+const struct iio_chan_spec _name[] = { \
+ AD576x_CHANNEL(0, (_bits)), \
+ AD576x_CHANNEL(1, (_bits)), \
+ AD576x_CHANNEL(2, (_bits)), \
+ AD576x_CHANNEL(3, (_bits)), \
+ AD576x_CHANNEL(4, (_bits)), \
+ AD576x_CHANNEL(5, (_bits)), \
+ AD576x_CHANNEL(6, (_bits)), \
+ AD576x_CHANNEL(7, (_bits)), \
+ AD576x_CHANNEL(8, (_bits)), \
+ AD576x_CHANNEL(9, (_bits)), \
+ AD576x_CHANNEL(10, (_bits)), \
+ AD576x_CHANNEL(11, (_bits)), \
+ AD576x_CHANNEL(12, (_bits)), \
+ AD576x_CHANNEL(13, (_bits)), \
+ AD576x_CHANNEL(14, (_bits)), \
+ AD576x_CHANNEL(15, (_bits)), \
+}
+
+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;
+};
+
+/**
+ * struct ad5766_state - driver instance specific data
+ * @spi: Spi device
+ * @lock: Mutex lock
+ * @chip_info: Chip model specific constants
+ * @gpio_reset: Reset gpio
+ * @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: No dither, N0, N1
+ * @dither_source: Selects between 4 possible sources:
+ * No scale, 75%, 50%, 25%
+ * @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;
+ s32 scale_avail[AD5766_VOLTAGE_RANGE_MAX][2];
+ s32 offset_avail[AD5766_VOLTAGE_RANGE_MAX][2];
+ union {
+ u32 d32;
+ u16 w16[2];
+ u8 b8[4];
+ } data[3] ____cacheline_aligned;
+};
+
+struct ad5766_span_tbl {
+ int min;
+ int max;
+};
+
+static const struct ad5766_span_tbl ad5766_span_tbl[] = {
+ [AD5766_VOLTAGE_RANGE_M20V_0V] = {
+ .min = -20,
+ .max = 0,
+ },
+ [AD5766_VOLTAGE_RANGE_M16V_to_0V] = {
+ .min = -16,
+ .max = 0,
+ },
+ [AD5766_VOLTAGE_RANGE_M10V_to_0V] = {
+ .min = -10,
+ .max = 0,
+ },
+ [AD5766_VOLTAGE_RANGE_M12V_to_14V] = {
+ .min = -12,
+ .max = 14,
+ },
+ [AD5766_VOLTAGE_RANGE_M16V_to_10V] = {
+ .min = -16,
+ .max = 10,
+ },
+ [AD5766_VOLTAGE_RANGE_M10V_to_6V] = {
+ .min = -10,
+ .max = 6,
+ },
+ [AD5766_VOLTAGE_RANGE_M5V_to_5V] = {
+ .min = -5,
+ .max = 5,
+ },
+ [AD5766_VOLTAGE_RANGE_M10V_to_10V] = {
+ .min = -10,
+ .max = 10,
+ },
+};
+
+static int ad5766_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long info);
+
+static int ad5766_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long m);
+
+static int ad5766_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask);
+
+static const struct iio_info ad5766_info = {
+ .read_raw = ad5766_read_raw,
+ .write_raw = ad5766_write_raw,
+ .read_avail = ad5766_read_avail,
+};
+
+static DECLARE_AD576x_CHANNELS(ad5766_channels, 16);
+static DECLARE_AD576x_CHANNELS(ad5767_channels, 12);
+
+static const struct ad5766_chip_info ad5766_chip_infos[] = {
+ [ID_AD5766] = {
+ .num_channels = ARRAY_SIZE(ad5766_channels),
+ .channels = ad5766_channels,
+ },
+ [ID_AD5767] = {
+ .num_channels = ARRAY_SIZE(ad5767_channels),
+ .channels = ad5767_channels,
+ },
+};
+
+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;
+
+ return spi_write(st->spi, &st->data[0].b8[0], 3);
+}
+
+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);
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static void ad5766_init_scale_tables(struct ad5766_state *st)
+{
+ int i;
+ s32 denom;
+ s64 offset;
+ u64 scale;
+ u8 realbits = st->chip_info->channels[0].scan_type.realbits;
+
+ for (i = 0; i < AD5766_VOLTAGE_RANGE_MAX; i++) {
+ offset = (1 << realbits) * ad5766_span_tbl[i].min;
+ denom = ad5766_span_tbl[i].max - ad5766_span_tbl[i].min;
+ offset = div_s64(offset * 1000000, denom);
+ st->offset_avail[i][0] = div_s64(offset, 1000000);
+ div_s64_rem(offset, 1000000, &st->offset_avail[i][1]);
+
+ scale = ad5766_span_tbl[i].max - ad5766_span_tbl[i].min;
+ scale = div_u64((scale * 1000000000), (1 << realbits));
+ st->scale_avail[i][0] = (int)div_u64(scale, 1000000);
+ div_s64_rem(scale, 1000000, &st->scale_avail[i][1]);
+ }
+}
+
+static int ad5766_reset(struct ad5766_state *st)
+{
+ int ret = 0;
+
+ if (st->gpio_reset) {
+ gpiod_set_value_cansleep(st->gpio_reset, 0);
+ ndelay(100); /* t_reset >= 100ns */
+ gpiod_set_value_cansleep(st->gpio_reset, 1);
+ } else {
+ ret = _ad5766_spi_write(st, AD5766_CMD_SW_FULL_RESET,
+ AD5766_FULL_RESET_CODE);
+ if (ret < 0)
+ return ret;
+ }
+
+ /*
+ * Minimum time between a reset and the subsequent successful write is
+ * typically 25 ns
+ */
+ ndelay(25);
+
+ return 0;
+}
+
+static int ad5766_default_setup(struct ad5766_state *st,
+ enum ad5766_voltage_range range)
+{
+ int ret;
+
+ /* Always issue a software reset before writing to the span register. */
+ ret = ad5766_reset(st);
+ if (ret)
+ return ret;
+
+ ret = _ad5766_spi_write(st, AD5766_CMD_SPAN_REG, range);
+ if (ret)
+ return ret;
+
+ st->crt_range = range;
+ st->cached_offset = range;
+
+ st->dither_power_ctrl = 0;
+ ret = _ad5766_spi_write(st, AD5766_CMD_WR_PWR_DITHER,
+ st->dither_power_ctrl);
+ if (ret)
+ return ret;
+
+ st->dither_source = 0;
+ ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_1,
+ st->dither_source & 0xFFFF);
+ if (ret)
+ return ret;
+
+ ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_2,
+ (st->dither_source >> 16) & 0xFFFF);
+ if (ret)
+ return ret;
+
+ st->dither_scale = 0;
+ ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_1,
+ st->dither_scale & 0xFFFF);
+ if (ret)
+ return ret;
+
+ ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_2,
+ (st->dither_scale >> 16) & 0xFFFF);
+ if (ret)
+ return ret;
+
+ st->dither_invert = 0;
+ ret = _ad5766_spi_write(st, AD5766_CMD_INV_DITHER,
+ st->dither_invert);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ad5766_set_offset(struct ad5766_state *st, int val, int val2)
+{
+ int i;
+ s32 (*tbl)[AD5766_VOLTAGE_RANGE_MAX][2] = &(st->offset_avail);
+
+ for (i = 0; i < AD5766_VOLTAGE_RANGE_MAX; i++) {
+ if ((*tbl)[i][0] == val && (*tbl)[i][1] == val2) {
+ st->cached_offset = i;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static int ad5766_set_scale(struct ad5766_state *st, int val, int val2)
+{
+ int i;
+ enum ad5766_voltage_range offset_idx = st->cached_offset;
+ s32 (*offset_tbl)[AD5766_VOLTAGE_RANGE_MAX][2] = &(st->offset_avail);
+ s32 (*scale_tbl)[AD5766_VOLTAGE_RANGE_MAX][2] = &(st->scale_avail);
+
+ for (i = 0; i < AD5766_VOLTAGE_RANGE_MAX; i++) {
+ if ((*scale_tbl)[i][0] != val || (*scale_tbl)[i][1] != val2)
+ continue;
+
+ if ((*offset_tbl)[i][0] != (*offset_tbl)[offset_idx][0] ||
+ (*offset_tbl)[i][1] != (*offset_tbl)[offset_idx][1])
+ continue;
+
+ return ad5766_default_setup(st, i);
+ }
+
+ return -EINVAL;
+}
+
+static int ad5766_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long info)
+{
+ struct ad5766_state *st = iio_priv(indio_dev);
+ const int max_val = (1 << chan->scan_type.realbits);
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ if (val >= max_val || val < 0)
+ return -EINVAL;
+ val <<= chan->scan_type.shift;
+ return ad5766_write(indio_dev, chan->address, val);
+ case IIO_CHAN_INFO_OFFSET:
+ return ad5766_set_offset(st, val, val2);
+ case IIO_CHAN_INFO_SCALE:
+ return ad5766_set_scale(st, val, val2);
+ default:
+ return -EINVAL;
+ }
+}
+
+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_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 ssize_t ad5766_write_ext(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ bool readin;
+ int ret;
+ struct ad5766_state *st = iio_priv(indio_dev);
+
+ ret = kstrtobool(buf, &readin);
+ if (ret)
+ return ret;
+
+ switch ((u32)private) {
+ case AD5766_DITHER_PWR:
+ ret = kstrtobool(buf, &readin);
+ if (ret)
+ break;
+ readin = !readin;
+ st->dither_power_ctrl = (st->dither_power_ctrl &
+ ~BIT(chan->channel)) |
+ (readin << chan->channel);
+ ret = ad5766_write(indio_dev, AD5766_CMD_WR_PWR_DITHER,
+ st->dither_power_ctrl);
+ break;
+ case AD5766_DITHER_INVERT:
+ st->dither_invert = (st->dither_invert &
+ ~BIT(chan->channel)) |
+ (readin << chan->channel);
+ ret = ad5766_write(indio_dev, AD5766_CMD_INV_DITHER,
+ st->dither_power_ctrl);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret ? ret : len;
+}
+
+static int ad5766_get_dither_source(struct iio_dev *dev,
+ const struct iio_chan_spec *chan)
+{
+ struct ad5766_state *st = iio_priv(dev);
+
+ return (st->dither_source >> (chan->channel * 2) & 0x03);
+}
+
+static int ad5766_set_dither_source(struct iio_dev *dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode)
+{
+ int ret;
+ struct ad5766_state *st = iio_priv(dev);
+
+ st->dither_source = (st->dither_source & ~(0x03 << (chan->channel * 2)))
+ | (mode << (chan->channel * 2));
+
+ ret = ad5766_write(dev, AD5766_CMD_DITHER_SIG_1,
+ st->dither_source & 0xFFFF);
+ if (ret)
+ return ret;
+
+ return ad5766_write(dev, AD5766_CMD_DITHER_SIG_2,
+ (st->dither_source >> 16) & 0xFFFF);
+}
+
+static int ad5766_get_dither_scale(struct iio_dev *dev,
+ const struct iio_chan_spec *chan)
+{
+ struct ad5766_state *st = iio_priv(dev);
+
+ return (st->dither_scale >> (chan->channel * 2) & 0x03);
+}
+
+static int ad5766_set_dither_scale(struct iio_dev *dev,
+ const struct iio_chan_spec *chan,
+ unsigned int scale)
+{
+ int ret;
+ struct ad5766_state *st = iio_priv(dev);
+
+ st->dither_scale = (st->dither_scale & ~(0x03 << (chan->channel * 2)))
+ | (scale << (chan->channel * 2));
+
+ ret = ad5766_write(dev, AD5766_CMD_DITHER_SCALE_1,
+ st->dither_scale & 0xFFFF);
+ if (ret)
+ return ret;
+
+ return ad5766_write(dev, AD5766_CMD_DITHER_SCALE_2,
+ (st->dither_scale >> 16) & 0xFFFF);
+}
+
+static ssize_t ad5766_read_ext(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ int ret;
+ struct ad5766_state *st = iio_priv(indio_dev);
+
+ switch ((u32)private) {
+ case AD5766_DITHER_PWR:
+ return sprintf(buf, "%u\n", 0x01 &
+ ~(st->dither_power_ctrl >> chan->channel));
+ break;
+ case AD5766_DITHER_INVERT:
+ return sprintf(buf, "%u\n", 0x01 &
+ (st->dither_invert >> chan->channel));
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static int ad5766_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ struct ad5766_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (int *)st->scale_avail;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ /* Values are stored in a 2D matrix */
+ *length = AD5766_VOLTAGE_RANGE_MAX * 2;
+
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_OFFSET:
+ *vals = (int *)st->offset_avail;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ /* Values are stored in a 2D matrix */
+ *length = AD5766_VOLTAGE_RANGE_MAX * 2;
+
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5766_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long m)
+{
+ int ret;
+ struct ad5766_state *st = iio_priv(indio_dev);
+
+ switch (m) {
+ case IIO_CHAN_INFO_RAW:
+ ret = ad5766_read(indio_dev, chan->address, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SCALE:
+ *val = st->scale_avail[st->crt_range][0];
+ *val2 = st->scale_avail[st->crt_range][1];
+
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case IIO_CHAN_INFO_OFFSET:
+ *val = st->offset_avail[st->crt_range][0];
+ *val2 = st->offset_avail[st->crt_range][1];
+
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5766_probe(struct spi_device *spi)
+{
+ enum ad5766_type type = spi_get_device_id(spi)->driver_data;
+ struct iio_dev *indio_dev;
+ struct ad5766_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, indio_dev);
+
+ mutex_init(&st->lock);
+
+ st->spi = spi;
+ st->chip_info = &ad5766_chip_infos[type];
+
+ indio_dev->channels = st->chip_info->channels;
+ indio_dev->num_channels = st->chip_info->num_channels;
+ indio_dev->info = &ad5766_info;
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->dev.of_node = spi->dev.of_node;
+ indio_dev->name = spi_get_device_id(spi)->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
+ GPIOD_OUT_HIGH);
+
+ ad5766_init_scale_tables(st);
+
+ ret = ad5766_default_setup(st, AD5766_VOLTAGE_RANGE_M5V_to_5V);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad5766_dt_match[] = {
+ { .compatible = "adi,ad5766" },
+ { .compatible = "adi,ad5767" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ad5766_dt_match);
+
+static const struct spi_device_id ad5766_spi_ids[] = {
+ { "ad5766", ID_AD5766 },
+ { "ad5767", ID_AD5767 },
+ {}
+};
+MODULE_DEVICE_TABLE(spi, ad5766_spi_ids);
+
+static struct spi_driver ad5766_driver = {
+ .driver = {
+ .name = "ad5766",
+ .of_match_table = ad5766_dt_match,
+ },
+ .probe = ad5766_probe,
+ .id_table = ad5766_spi_ids,
+};
+module_spi_driver(ad5766_driver);
+
+MODULE_AUTHOR("Denis-Gabriel Gheorghescu <denis.gheorghescu@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5766/AD5767 DACs");
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation
2020-11-23 14:50 [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Cristian Pop
2020-11-23 14:50 ` [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766 Cristian Pop
@ 2020-11-24 11:42 ` Jonathan Cameron
2020-11-30 16:38 ` Rob Herring
2 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2020-11-24 11:42 UTC (permalink / raw)
To: Cristian Pop; +Cc: linux-iio, linux-kernel, jic23, devicetree, robh+dt
On Mon, 23 Nov 2020 16:50:41 +0200
Cristian Pop <cristian.pop@analog.com> wrote:
> This adds device tree bindings for the AD5766 DAC.
>
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
One trivial point inline. If that's all we have in the series I can tidy up
whilst applying.
Jonathan
> ---
> .../bindings/iio/dac/adi,ad5766.yaml | 52 +++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> new file mode 100644
> index 000000000000..aed4a0472bc4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/iio/dac/adi,ad5766.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5766 DAC device driver
> +
> +maintainers:
> + - Cristian Pop <cristian.pop@analog.com>
> +
> +description: |
> + Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
> + found here:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
> +
Just one blank line.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad5766
> + - adi,ad5767
> +
> + reg:
> + maxItems: 1
> +
> + spi-max-frequency:
> + maximum: 1000000
> +
> + spi-cpol: true
> +
> + reset-gpios:
> + description: GPIO spec for the RESET pin. If specified, it will be
> + asserted during driver probe.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - spi-max-frequency
> + - spi-cpol
> +
> +examples:
> + - |
> + ad5766@0{
> + compatible = "adi,ad5766";
> + reg = <0>;
> + spi-cpol;
> + spi-max-frequency = <1000000>;
> + reset-gpios = <&gpio 22 0>;
> + };
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766
2020-11-23 14:50 ` [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766 Cristian Pop
@ 2020-11-24 12:02 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2020-11-24 12:02 UTC (permalink / raw)
To: Cristian Pop; +Cc: linux-iio, linux-kernel, jic23, devicetree, robh+dt
On Mon, 23 Nov 2020 16:50:42 +0200
Cristian Pop <cristian.pop@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
Biggest thing in here is you are proposing new ABI, but I'm not seeing
the docs we'd need to properly discuss that.
Otherwise, some more minor bits inline.
Thanks,
Jonathan
>
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> ---
> drivers/iio/dac/ad5766.c | 768 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 768 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..feef78960990
> --- /dev/null
> +++ b/drivers/iio/dac/ad5766.c
> @@ -0,0 +1,768 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD5766, AD5767
> + * Digital to Analog Converters driver
> + *
> + * Copyright 2019 Analog Devices Inc.
2019-2020 perhaps given making changes?
> + */
> +
> +#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>
> +
> +#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,
> +};
> +
> +static ssize_t ad5766_write_ext(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len);
> +static ssize_t ad5766_read_ext(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf);
> +
> +static int ad5766_get_dither_source(struct iio_dev *dev,
> + const struct iio_chan_spec *chan);
> +
> +static int ad5766_set_dither_source(struct iio_dev *dev,
> + const struct iio_chan_spec *chan,
> + unsigned int mode);
> +
> +static int ad5766_get_dither_scale(struct iio_dev *dev,
> + const struct iio_chan_spec *chan);
> +
> +static int ad5766_set_dither_scale(struct iio_dev *dev,
> + const struct iio_chan_spec *chan,
> + unsigned int mode);
I'd be looking to reorganize the code to avoid the need for
these forward definitions. It's fairly rare that we actually need to do this.
> +
> +#define _AD5766_CHAN_EXT_INFO(_name, _what, _shared) { \
> + .name = _name, \
> + .read = ad5766_read_ext, \
> + .write = ad5766_write_ext, \
> + .private = _what, \
> + .shared = _shared, \
> +}
> +
> +enum {
> + AD5766_DITHER_PWR,
> + AD5766_DITHER_INVERT
> +};
> +
> +static const char * const ad5766_dither_sources[] = {
> + "NO_DITHER",
Definitely needs docs as I have no idea what this is!
> + "N0",
> + "N1",
> +};
> +
> +static const char * const ad5766_dither_scales[] = {
> + "NO_SCALING",
> + "75%_SCALING",
> + "50%_SCALING",
> + "25%_SCALING",
New ABI.... Docs?
This looks like it should be a well defined nummeric ABI, though
I don't understand it enough yet to know if there is a number that
corresponds to NO_SCALING? If there isn't then split it into two
ABI elements, one to enable / disable and one for the scaling.
> +};
> +
> +static const struct iio_enum ad5766_dither_source_enum = {
> + .items = ad5766_dither_sources,
> + .num_items = ARRAY_SIZE(ad5766_dither_sources),
> + .set = ad5766_set_dither_source,
> + .get = ad5766_get_dither_source,
> +};
> +
> +static const struct iio_enum ad5766_dither_scale_enum = {
> + .items = ad5766_dither_scales,
> + .num_items = ARRAY_SIZE(ad5766_dither_scales),
> + .set = ad5766_set_dither_scale,
> + .get = ad5766_get_dither_scale,
> +};
> +
> +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",
> + IIO_SEPARATE,
> + &ad5766_dither_scale_enum),
> + {}
> +};
> +
> +#define AD576x_CHANNEL(_chan, _bits) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = (_chan), \
> + .address = (_chan), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_type_available = \
> + BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = (_bits), \
> + .storagebits = 16, \
> + .shift = 16 - (_bits), \
> + }, \
> + .ext_info = ad5766_ext_info, \
> +}
> +
> +#define DECLARE_AD576x_CHANNELS(_name, _bits) \
> +const struct iio_chan_spec _name[] = { \
> + AD576x_CHANNEL(0, (_bits)), \
> + AD576x_CHANNEL(1, (_bits)), \
> + AD576x_CHANNEL(2, (_bits)), \
> + AD576x_CHANNEL(3, (_bits)), \
> + AD576x_CHANNEL(4, (_bits)), \
> + AD576x_CHANNEL(5, (_bits)), \
> + AD576x_CHANNEL(6, (_bits)), \
> + AD576x_CHANNEL(7, (_bits)), \
> + AD576x_CHANNEL(8, (_bits)), \
> + AD576x_CHANNEL(9, (_bits)), \
> + AD576x_CHANNEL(10, (_bits)), \
> + AD576x_CHANNEL(11, (_bits)), \
> + AD576x_CHANNEL(12, (_bits)), \
> + AD576x_CHANNEL(13, (_bits)), \
> + AD576x_CHANNEL(14, (_bits)), \
> + AD576x_CHANNEL(15, (_bits)), \
> +}
> +
> +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;
> +};
> +
> +/**
> + * struct ad5766_state - driver instance specific data
> + * @spi: Spi device
SPI device
> + * @lock: Mutex lock
> + * @chip_info: Chip model specific constants
> + * @gpio_reset: Reset gpio
Reset GPIO etc.
> + * @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: No dither, N0, N1
> + * @dither_source: Selects between 4 possible sources:
> + * No scale, 75%, 50%, 25%
> + * @scale_avail: Scale available table
> + * @offset_avail: Offest available table
> + * @data: Spi transfer buffers
> + */
> +
I would drop the blank line between comments for structures and the structure
starting. Helps the eye to group them when reading (slightly!)
> +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;
> + s32 scale_avail[AD5766_VOLTAGE_RANGE_MAX][2];
> + s32 offset_avail[AD5766_VOLTAGE_RANGE_MAX][2];
> + union {
> + u32 d32;
> + u16 w16[2];
> + u8 b8[4];
> + } data[3] ____cacheline_aligned;
> +};
> +
> +struct ad5766_span_tbl {
> + int min;
> + int max;
> +};
> +
> +static const struct ad5766_span_tbl ad5766_span_tbl[] = {
> + [AD5766_VOLTAGE_RANGE_M20V_0V] = {
> + .min = -20,
> + .max = 0,
> + },
> + [AD5766_VOLTAGE_RANGE_M16V_to_0V] = {
> + .min = -16,
> + .max = 0,
> + },
> + [AD5766_VOLTAGE_RANGE_M10V_to_0V] = {
> + .min = -10,
> + .max = 0,
> + },
> + [AD5766_VOLTAGE_RANGE_M12V_to_14V] = {
> + .min = -12,
> + .max = 14,
> + },
> + [AD5766_VOLTAGE_RANGE_M16V_to_10V] = {
> + .min = -16,
> + .max = 10,
> + },
> + [AD5766_VOLTAGE_RANGE_M10V_to_6V] = {
> + .min = -10,
> + .max = 6,
> + },
> + [AD5766_VOLTAGE_RANGE_M5V_to_5V] = {
> + .min = -5,
> + .max = 5,
> + },
> + [AD5766_VOLTAGE_RANGE_M10V_to_10V] = {
> + .min = -10,
> + .max = 10,
> + },
> +};
> +
> +static int ad5766_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long info);
> +
> +static int ad5766_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long m);
> +
> +static int ad5766_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask);
> +
> +static const struct iio_info ad5766_info = {
> + .read_raw = ad5766_read_raw,
> + .write_raw = ad5766_write_raw,
> + .read_avail = ad5766_read_avail,
> +};
I'd move this down so you don't need the forward declarations.
> +
> +static DECLARE_AD576x_CHANNELS(ad5766_channels, 16);
> +static DECLARE_AD576x_CHANNELS(ad5767_channels, 12);
> +
> +static const struct ad5766_chip_info ad5766_chip_infos[] = {
> + [ID_AD5766] = {
> + .num_channels = ARRAY_SIZE(ad5766_channels),
> + .channels = ad5766_channels,
> + },
> + [ID_AD5767] = {
> + .num_channels = ARRAY_SIZE(ad5767_channels),
> + .channels = ad5767_channels,
> + },
> +};
> +
> +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;
Maybe an unaligned write would be easier to read?
> +
> + return spi_write(st->spi, &st->data[0].b8[0], 3);
> +}
> +
> +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);
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static void ad5766_init_scale_tables(struct ad5766_state *st)
> +{
> + int i;
> + s32 denom;
> + s64 offset;
> + u64 scale;
> + u8 realbits = st->chip_info->channels[0].scan_type.realbits;
> +
> + for (i = 0; i < AD5766_VOLTAGE_RANGE_MAX; i++) {
> + offset = (1 << realbits) * ad5766_span_tbl[i].min;
> + denom = ad5766_span_tbl[i].max - ad5766_span_tbl[i].min;
> + offset = div_s64(offset * 1000000, denom);
> + st->offset_avail[i][0] = div_s64(offset, 1000000);
> + div_s64_rem(offset, 1000000, &st->offset_avail[i][1]);
> +
> + scale = ad5766_span_tbl[i].max - ad5766_span_tbl[i].min;
> + scale = div_u64((scale * 1000000000), (1 << realbits));
> + st->scale_avail[i][0] = (int)div_u64(scale, 1000000);
> + div_s64_rem(scale, 1000000, &st->scale_avail[i][1]);
> + }
> +}
> +
> +static int ad5766_reset(struct ad5766_state *st)
> +{
> + int ret = 0;
> +
> + if (st->gpio_reset) {
> + gpiod_set_value_cansleep(st->gpio_reset, 0);
> + ndelay(100); /* t_reset >= 100ns */
> + gpiod_set_value_cansleep(st->gpio_reset, 1);
> + } else {
> + ret = _ad5766_spi_write(st, AD5766_CMD_SW_FULL_RESET,
> + AD5766_FULL_RESET_CODE);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /*
> + * Minimum time between a reset and the subsequent successful write is
> + * typically 25 ns
> + */
> + ndelay(25);
> +
> + return 0;
> +}
> +
> +static int ad5766_default_setup(struct ad5766_state *st,
> + enum ad5766_voltage_range range)
> +{
> + int ret;
> +
> + /* Always issue a software reset before writing to the span register. */
> + ret = ad5766_reset(st);
> + if (ret)
> + return ret;
> +
> + ret = _ad5766_spi_write(st, AD5766_CMD_SPAN_REG, range);
> + if (ret)
> + return ret;
> +
> + st->crt_range = range;
> + st->cached_offset = range;
> +
> + st->dither_power_ctrl = 0;
> + ret = _ad5766_spi_write(st, AD5766_CMD_WR_PWR_DITHER,
> + st->dither_power_ctrl);
> + if (ret)
> + return ret;
> +
> + st->dither_source = 0;
> + ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_1,
> + st->dither_source & 0xFFFF);
> + if (ret)
> + return ret;
> +
> + ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_2,
> + (st->dither_source >> 16) & 0xFFFF);
> + if (ret)
> + return ret;
> +
> + st->dither_scale = 0;
> + ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_1,
> + st->dither_scale & 0xFFFF);
> + if (ret)
> + return ret;
> +
> + ret = _ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_2,
> + (st->dither_scale >> 16) & 0xFFFF);
> + if (ret)
> + return ret;
> +
> + st->dither_invert = 0;
> + ret = _ad5766_spi_write(st, AD5766_CMD_INV_DITHER,
> + st->dither_invert);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int ad5766_set_offset(struct ad5766_state *st, int val, int val2)
> +{
> + int i;
> + s32 (*tbl)[AD5766_VOLTAGE_RANGE_MAX][2] = &(st->offset_avail);
> +
> + for (i = 0; i < AD5766_VOLTAGE_RANGE_MAX; i++) {
> + if ((*tbl)[i][0] == val && (*tbl)[i][1] == val2) {
> + st->cached_offset = i;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int ad5766_set_scale(struct ad5766_state *st, int val, int val2)
> +{
> + int i;
> + enum ad5766_voltage_range offset_idx = st->cached_offset;
> + s32 (*offset_tbl)[AD5766_VOLTAGE_RANGE_MAX][2] = &(st->offset_avail);
> + s32 (*scale_tbl)[AD5766_VOLTAGE_RANGE_MAX][2] = &(st->scale_avail);
> +
> + for (i = 0; i < AD5766_VOLTAGE_RANGE_MAX; i++) {
> + if ((*scale_tbl)[i][0] != val || (*scale_tbl)[i][1] != val2)
> + continue;
> +
> + if ((*offset_tbl)[i][0] != (*offset_tbl)[offset_idx][0] ||
> + (*offset_tbl)[i][1] != (*offset_tbl)[offset_idx][1])
> + continue;
> +
> + return ad5766_default_setup(st, i);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int ad5766_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long info)
> +{
> + struct ad5766_state *st = iio_priv(indio_dev);
> + const int max_val = (1 << chan->scan_type.realbits);
Only used in the INFO_RAW path so move it down there.
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + if (val >= max_val || val < 0)
> + return -EINVAL;
> + val <<= chan->scan_type.shift;
> + return ad5766_write(indio_dev, chan->address, val);
> + case IIO_CHAN_INFO_OFFSET:
> + return ad5766_set_offset(st, val, val2);
> + case IIO_CHAN_INFO_SCALE:
> + return ad5766_set_scale(st, val, val2);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +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));
> +
no blank line here. Keep the error handling in the same visual block
as the call that returned the error.
> + if (ret)
> + return ret;
> +
> + *val = st->data[2].w16[1];
> +
> + return ret;
> +}
> +
> +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 ssize_t ad5766_write_ext(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + bool readin;
> + int ret;
> + struct ad5766_state *st = iio_priv(indio_dev);
> +
> + ret = kstrtobool(buf, &readin);
> + if (ret)
> + return ret;
> +
> + switch ((u32)private) {
> + case AD5766_DITHER_PWR:
> + ret = kstrtobool(buf, &readin);
> + if (ret)
> + break;
> + readin = !readin;
> + st->dither_power_ctrl = (st->dither_power_ctrl &
> + ~BIT(chan->channel)) |
> + (readin << chan->channel);
> + ret = ad5766_write(indio_dev, AD5766_CMD_WR_PWR_DITHER,
> + st->dither_power_ctrl);
> + break;
> + case AD5766_DITHER_INVERT:
> + st->dither_invert = (st->dither_invert &
> + ~BIT(chan->channel)) |
> + (readin << chan->channel);
> + ret = ad5766_write(indio_dev, AD5766_CMD_INV_DITHER,
> + st->dither_power_ctrl);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret ? ret : len;
> +}
> +
> +static int ad5766_get_dither_source(struct iio_dev *dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct ad5766_state *st = iio_priv(dev);
> +
> + return (st->dither_source >> (chan->channel * 2) & 0x03);
> +}
> +
> +static int ad5766_set_dither_source(struct iio_dev *dev,
> + const struct iio_chan_spec *chan,
> + unsigned int mode)
> +{
> + int ret;
> + struct ad5766_state *st = iio_priv(dev);
> +
> + st->dither_source = (st->dither_source & ~(0x03 << (chan->channel * 2)))
> + | (mode << (chan->channel * 2));
> +
> + ret = ad5766_write(dev, AD5766_CMD_DITHER_SIG_1,
> + st->dither_source & 0xFFFF);
> + if (ret)
> + return ret;
> +
> + return ad5766_write(dev, AD5766_CMD_DITHER_SIG_2,
> + (st->dither_source >> 16) & 0xFFFF);
> +}
> +
> +static int ad5766_get_dither_scale(struct iio_dev *dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct ad5766_state *st = iio_priv(dev);
> +
> + return (st->dither_scale >> (chan->channel * 2) & 0x03);
> +}
> +
> +static int ad5766_set_dither_scale(struct iio_dev *dev,
> + const struct iio_chan_spec *chan,
> + unsigned int scale)
> +{
> + int ret;
> + struct ad5766_state *st = iio_priv(dev);
> +
> + st->dither_scale = (st->dither_scale & ~(0x03 << (chan->channel * 2)))
0x03 looks like a mask? If so define it as such with GENMASK()
> + | (scale << (chan->channel * 2));
> +
> + ret = ad5766_write(dev, AD5766_CMD_DITHER_SCALE_1,
> + st->dither_scale & 0xFFFF);
Another mask, define it clearly with GENMASK() so that's self documenting.
Same in other places.
Looks like good places to use FIELD_PREP() etc.
> + if (ret)
> + return ret;
> +
> + return ad5766_write(dev, AD5766_CMD_DITHER_SCALE_2,
> + (st->dither_scale >> 16) & 0xFFFF);
> +}
> +
> +static ssize_t ad5766_read_ext(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + int ret;
> + struct ad5766_state *st = iio_priv(indio_dev);
> +
> + switch ((u32)private) {
> + case AD5766_DITHER_PWR:
> + return sprintf(buf, "%u\n", 0x01 &
> + ~(st->dither_power_ctrl >> chan->channel));
> + break;
> + case AD5766_DITHER_INVERT:
> + return sprintf(buf, "%u\n", 0x01 &
> + (st->dither_invert >> chan->channel));
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int ad5766_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + struct ad5766_state *st = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (int *)st->scale_avail;
int might not have the same size as s32 which would make this
exciting. I'd change the definition of scale_avail to int [][]
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + /* Values are stored in a 2D matrix */
> + *length = AD5766_VOLTAGE_RANGE_MAX * 2;
> +
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_OFFSET:
> + *vals = (int *)st->offset_avail;
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + /* Values are stored in a 2D matrix */
> + *length = AD5766_VOLTAGE_RANGE_MAX * 2;
> +
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad5766_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long m)
> +{
> + int ret;
> + struct ad5766_state *st = iio_priv(indio_dev);
> +
> + switch (m) {
> + case IIO_CHAN_INFO_RAW:
> + ret = ad5766_read(indio_dev, chan->address, val);
> + if (ret)
> + return ret;
> +
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + *val = st->scale_avail[st->crt_range][0];
> + *val2 = st->scale_avail[st->crt_range][1];
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case IIO_CHAN_INFO_OFFSET:
> + *val = st->offset_avail[st->crt_range][0];
> + *val2 = st->offset_avail[st->crt_range][1];
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad5766_probe(struct spi_device *spi)
> +{
> + enum ad5766_type type = spi_get_device_id(spi)->driver_data;
That spi_get_device_id is non trivial enough I'd prefer to see this
not hidden away in the declarations block. Move it down to just above
where it is used perhaps?
> + struct iio_dev *indio_dev;
> + struct ad5766_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + spi_set_drvdata(spi, indio_dev);
Can't see where this is used.
> +
> + mutex_init(&st->lock);
> +
> + st->spi = spi;
> + st->chip_info = &ad5766_chip_infos[type];
> +
> + indio_dev->channels = st->chip_info->channels;
> + indio_dev->num_channels = st->chip_info->num_channels;
> + indio_dev->info = &ad5766_info;
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->dev.of_node = spi->dev.of_node;
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> + GPIOD_OUT_HIGH);
> +
> + ad5766_init_scale_tables(st);
> +
> + ret = ad5766_default_setup(st, AD5766_VOLTAGE_RANGE_M5V_to_5V);
> + if (ret)
> + return ret;
> +
> + return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct of_device_id ad5766_dt_match[] = {
> + { .compatible = "adi,ad5766" },
> + { .compatible = "adi,ad5767" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ad5766_dt_match);
> +
> +static const struct spi_device_id ad5766_spi_ids[] = {
> + { "ad5766", ID_AD5766 },
> + { "ad5767", ID_AD5767 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5766_spi_ids);
> +
> +static struct spi_driver ad5766_driver = {
> + .driver = {
> + .name = "ad5766",
> + .of_match_table = ad5766_dt_match,
> + },
> + .probe = ad5766_probe,
> + .id_table = ad5766_spi_ids,
> +};
> +module_spi_driver(ad5766_driver);
> +
> +MODULE_AUTHOR("Denis-Gabriel Gheorghescu <denis.gheorghescu@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5766/AD5767 DACs");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation
2020-11-23 14:50 [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Cristian Pop
2020-11-23 14:50 ` [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766 Cristian Pop
2020-11-24 11:42 ` [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Jonathan Cameron
@ 2020-11-30 16:38 ` Rob Herring
2 siblings, 0 replies; 5+ messages in thread
From: Rob Herring @ 2020-11-30 16:38 UTC (permalink / raw)
To: Cristian Pop; +Cc: linux-iio, jic23, devicetree, robh+dt, linux-kernel
On Mon, 23 Nov 2020 16:50:41 +0200, Cristian Pop wrote:
> This adds device tree bindings for the AD5766 DAC.
>
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> ---
> .../bindings/iio/dac/adi,ad5766.yaml | 52 +++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml: 'additionalProperties' is a required property
./Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/iio/dac/adi,ad5766.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dts:21.13-23: Warning (reg_format): /example-0/ad5766@0:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/dac/adi,ad5766.example.dt.yaml: example-0: ad5766@0:reg:0: [0] is too short
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
See https://patchwork.ozlabs.org/patch/1404863
The base for the patch is generally the last rc1. Any dependencies
should be noted.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-30 16:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 14:50 [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Cristian Pop
2020-11-23 14:50 ` [PATCH 2/2] iio: dac: ad5766: add driver support for AD5766 Cristian Pop
2020-11-24 12:02 ` Jonathan Cameron
2020-11-24 11:42 ` [PATCH 1/2] dt-bindings: iio: dac: AD5766 yaml documentation Jonathan Cameron
2020-11-30 16:38 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).