Generalize the ABI documentation for DAC. The ABI defined for toggle mode
channels:
LTC2664:
* out_voltageY_toggle_en
* out_voltageY_raw0
* out_voltageY_raw1
* out_voltageY_symbol
LTC2672:
* out_currentY_toggle_en
* out_currentY_raw0
* out_currentY_raw1
* out_currentY_symbol
Default channels won't have any of the above ABIs. A channel is toggle capable
if the devicetree 'adi,toggle-mode' flag is set.
changes in v3:
ltc2664:
* Added span sanity check for no match.
* Initialized the variable 'span' to fix build warning.
* Added Reported-by and Closes by tag.
ABI:
* Modified descriptions to make it more generalize.
* Removed MAINTAINERS file entry.
Bindings:
* Changed clr-gpios to reset-gpios.
* Added output range and reset code description for 'adi,manual-span-operation-config'
property in ltc2664 binding.
* Removed the $ref for 'adi,output-range-microamp' due to dt-schema warning
in ltc2672 binding. Added Reported-by and Closes by tag.
* Modified io-channels description and added maxItems constraint.
changes in v2:
ltc2664:
* Updated struct ltc2664_chip_info to include device-specific data for scale,
offset, measurement type, internal vref, manual span support, and rfsadj
support.
* Added a read-only extended info attribute powerdown_mode to indicate the
state that the DAC output enters when the device is powered down.
* Refactored code for setting the span into separate function and directly
returning the span.
* Adjusted memory allocation for st->iio_channels to include null terminator.
* Spaces have been added after { and before }. Each pair of values is now
placed on a separate line.
ABI:
* Generalized the ABI documentation for DAC.
* Added DAC 42kohm_to_gnd powerdown mode.
Bindings:
* Created separate bindings for ltc2664 and ltc2672.
* Added v-pos-supply and v-neg-supply regulator properties.
* Renamed vref-supply to ref-supply based on the datasheet.
* Added io-channels property and specifying the pin for multiplexer output.
* Added vdd0-vdd4 supply properties for ltc2672, although they are not
currently supported in the driver.
* Changed clr-gpios description based on the datasheet.
* Used 4 spaces for example indentation.
Kim Seer Paller (5):
iio: ABI: Generalize ABI documentation for DAC
iio: ABI: add DAC 42kohm_to_gnd powerdown mode
dt-bindings: iio: dac: Add adi,ltc2664.yaml
dt-bindings: iio: dac: Add adi,ltc2672.yaml
iio: dac: ltc2664: Add driver for LTC2664 and LTC2672
Documentation/ABI/testing/sysfs-bus-iio | 1 +
Documentation/ABI/testing/sysfs-bus-iio-dac | 61 ++
.../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 -
.../bindings/iio/dac/adi,ltc2664.yaml | 167 ++++
.../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++
MAINTAINERS | 10 +
drivers/iio/dac/Kconfig | 11 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc2664.c | 806 ++++++++++++++++++
9 files changed, 1215 insertions(+), 31 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac
create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
create mode 100644 drivers/iio/dac/ltc2664.c
base-commit: 15895709c7dc5f1a8b53b3564fc2bed724209611
--
2.34.1
Introduces a more generalized ABI documentation for DAC. Instead of
having separate ABI files for each DAC, we now have a single ABI file
that covers the common sysfs interface for all DAC.
Co-developed-by: Michael Hennerich <[email protected]>
Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Kim Seer Paller <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-iio-dac | 61 +++++++++++++++++++
.../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 ----------
2 files changed, 61 insertions(+), 31 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac
new file mode 100644
index 000000000000..36d316bb75f6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac
@@ -0,0 +1,61 @@
+What: /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en
+KernelVersion: 5.18
+Contact: [email protected]
+Description:
+ Toggle enable. Write 1 to enable toggle or 0 to disable it. This
+ is useful when one wants to change the DAC output codes. The way
+ it should be done is:
+
+ - disable toggle operation;
+ - change out_currentY_rawN, where N is the integer value of the symbol;
+ - enable toggle operation.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN
+KernelVersion: 5.18
+Contact: [email protected]
+Description:
+ This attribute has the same meaning as out_currentY_raw. It is
+ specific to toggle enabled channels and refers to the DAC output
+ code in INPUT_N (_rawN), where N is the integer value of the symbol.
+ The same scale and offset as in out_currentY_raw applies.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol
+KernelVersion: 5.18
+Contact: [email protected]
+Description:
+ Performs a SW switch to a predefined output symbol. This attribute
+ is specific to toggle enabled channels and allows switching between
+ multiple predefined symbols. Each symbol corresponds to a different
+ output, denoted as out_currentY_rawN, where N is the integer value
+ of the symbol. Writing an integer value N will select out_currentY_rawN.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
+KernelVersion: 5.18
+Contact: [email protected]
+Description:
+ Toggle enable. Write 1 to enable toggle or 0 to disable it. This
+ is useful when one wants to change the DAC output codes. The way
+ it should be done is:
+
+ - disable toggle operation;
+ - change out_voltageY_rawN, where N is the integer value of the symbol;
+ - enable toggle operation.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_rawN
+KernelVersion: 5.18
+Contact: [email protected]
+Description:
+ This attribute has the same meaning as out_currentY_raw. It is
+ specific to toggle enabled channels and refers to the DAC output
+ code in INPUT_N (_rawN), where N is the integer value of the symbol.
+ The same scale and offset as in out_currentY_raw applies.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
+KernelVersion: 5.18
+Contact: [email protected]
+Description:
+ Performs a SW switch to a predefined output symbol. This attribute
+ is specific to toggle enabled channels and allows switching between
+ multiple predefined symbols. Each symbol corresponds to a different
+ output, denoted as out_voltageY_rawN, where N is the integer value
+ of the symbol. Writing an integer value N will select out_voltageY_rawN.
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
index 1c35971277ba..ae95a5477382 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688
@@ -53,34 +53,3 @@ KernelVersion: 5.18
Contact: [email protected]
Description:
Returns the available values for the dither phase.
-
-What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
-KernelVersion: 5.18
-Contact: [email protected]
-Description:
- Toggle enable. Write 1 to enable toggle or 0 to disable it. This is
- useful when one wants to change the DAC output codes. The way it should
- be done is:
-
- - disable toggle operation;
- - change out_voltageY_raw0 and out_voltageY_raw1;
- - enable toggle operation.
-
-What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0
-What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1
-KernelVersion: 5.18
-Contact: [email protected]
-Description:
- It has the same meaning as out_voltageY_raw. This attribute is
- specific to toggle enabled channels and refers to the DAC output
- code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset
- as in out_voltageY_raw applies.
-
-What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
-KernelVersion: 5.18
-Contact: [email protected]
-Description:
- Performs a SW toggle. This attribute is specific to toggle
- enabled channels and allows to toggle between out_voltageY_raw0
- and out_voltageY_raw1 through software. Writing 0 will select
- out_voltageY_raw0 while 1 selects out_voltageY_raw1.
--
2.34.1
LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC
LTC2672 5 channel, 16 bit Current Output Softspan DAC
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Co-developed-by: Michael Hennerich <[email protected]>
Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Kim Seer Paller <[email protected]>
---
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á <[email protected]>
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 <[email protected]>");
+MODULE_AUTHOR("Kim Seer Paller <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices LTC2664 and LTC2672 DAC");
+MODULE_LICENSE("GPL");
--
2.34.1
Add documentation for ltc2664.
Co-developed-by: Michael Hennerich <[email protected]>
Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Kim Seer Paller <[email protected]>
---
.../bindings/iio/dac/adi,ltc2664.yaml | 167 ++++++++++++++++++
MAINTAINERS | 8 +
2 files changed, 175 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
new file mode 100644
index 000000000000..73b1d7ac219a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ltc2664.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2664 DAC
+
+maintainers:
+ - Michael Hennerich <[email protected]>
+ - Kim Seer Paller <[email protected]>
+
+description: |
+ Analog Devices LTC2664 4 channel, 16 bit, +-10V DAC
+ https://www.analog.com/media/en/technical-documentation/data-sheets/2664fa.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ltc2664
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 50000000
+
+ vcc-supply:
+ description: Analog Supply Voltage Input.
+
+ v-pos-supply:
+ description: Positive Supply Voltage Input.
+
+ v-neg-supply:
+ description: Negative Supply Voltage Input.
+
+ iovcc-supply:
+ description: Digital Input/Output Supply Voltage.
+
+ ref-supply:
+ description:
+ Reference Input/Output. The voltage at the REF pin sets the full-scale
+ range of all channels. If not provided the internal reference is used and
+ also provided on the VREF pin.
+
+ reset-gpios:
+ description:
+ Active-low Asynchronous Clear Input. A logic low at this level-triggered
+ input clears the part to the reset code and range determined by the
+ hardwired option chosen using the MSPAN pins. The control registers are
+ cleared to zero.
+ maxItems: 1
+
+ adi,manual-span-operation-config:
+ description:
+ This property must mimic the MSPAN pin configurations. By tying the MSPAN
+ pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can be
+ hardware-configured with different mid-scale or zero-scale reset options.
+ The hardware configuration is latched during power on reset for proper
+ operation.
+ 0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V)
+ 1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V)
+ 2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V)
+ 3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V)
+ 4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V)
+ 5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V)
+ 6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V)
+ 7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables SoftSpan)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3, 4, 5, 6, 7]
+ default: 7
+
+ io-channels:
+ description:
+ ADC channel to monitor voltages and temperature at the MUXOUT pin.
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ "^channel@[0-3]$":
+ type: object
+ additionalProperties: false
+
+ properties:
+ reg:
+ description: The channel number representing the DAC output channel.
+ maximum: 3
+
+ adi,toggle-mode:
+ description:
+ Set the channel as a toggle enabled channel. Toggle operation enables
+ fast switching of a DAC output between two different DAC codes without
+ any SPI transaction.
+ type: boolean
+
+ adi,output-range-microvolt:
+ description: Specify the channel output full scale range.
+ oneOf:
+ - items:
+ - const: 0
+ - enum: [5000000, 10000000]
+ - items:
+ - const: -5000000
+ - const: 5000000
+ - items:
+ - const: -10000000
+ - const: 10000000
+ - items:
+ - const: -2500000
+ - const: 2500000
+
+ required:
+ - reg
+ - adi,output-range-microvolt
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - vcc-supply
+ - iovcc-supply
+ - v-pos-supply
+ - v-neg-supply
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ dac@0 {
+ compatible = "adi,ltc2664";
+ reg = <0>;
+ spi-max-frequency = <10000000>;
+
+ vcc-supply = <&vcc>;
+ iovcc-supply = <&vcc>;
+ ref-supply = <&vref>;
+ v-pos-supply = <&vpos>;
+ v-neg-supply = <&vneg>;
+
+ io-channels = <&adc 0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ channel@0 {
+ reg = <0>;
+ adi,toggle-mode;
+ adi,output-range-microvolt = <(-10000000) 10000000>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ adi,output-range-microvolt = <0 10000000>;
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d6c90161c7bf..c7a102d7fd6a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13063,6 +13063,14 @@ S: Maintained
F: Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.yaml
F: drivers/iio/dac/ltc1660.c
+LTC2664 IIO DAC DRIVER
+M: Michael Hennerich <[email protected]>
+M: Kim Seer Paller <[email protected]>
+L: [email protected]
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
+
LTC2688 IIO DAC DRIVER
M: Nuno Sá <[email protected]>
L: [email protected]
--
2.34.1
Add documentation for ltc2672.
Reported-by: Rob Herring (Arm) <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Co-developed-by: Michael Hennerich <[email protected]>
Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Kim Seer Paller <[email protected]>
---
.../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 159 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
new file mode 100644
index 000000000000..d143a9db7010
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2672 DAC
+
+maintainers:
+ - Michael Hennerich <[email protected]>
+ - Kim Seer Paller <[email protected]>
+
+description: |
+ Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ltc2672
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 50000000
+
+ vcc-supply:
+ description: Analog Supply Voltage Input.
+
+ v-neg-supply:
+ description: Negative Supply Voltage Input.
+
+ vdd0-supply:
+ description: Positive Supply Voltage Input for DAC OUT0.
+
+ vdd1-supply:
+ description: Positive Supply Voltage Input for DAC OUT1.
+
+ vdd2-supply:
+ description: Positive Supply Voltage Input for DAC OUT2.
+
+ vdd3-supply:
+ description: Positive Supply Voltage Input for DAC OUT3.
+
+ vdd4-supply:
+ description: Positive Supply Voltage Input for DAC OUT4.
+
+ iovcc-supply:
+ description: Digital Input/Output Supply Voltage.
+
+ ref-supply:
+ description:
+ Reference Input/Output. The voltage at the REF pin sets the full-scale
+ range of all channels. If not provided the internal reference is used and
+ also provided on the VREF pin.
+
+ reset-gpios:
+ description:
+ Active Low Asynchronous Clear Input. A logic low at this level triggered
+ input clears the device to the default reset code and output range, which
+ is zero-scale with the outputs off. The control registers are cleared to
+ zero.
+ maxItems: 1
+
+ adi,rfsadj-ohms:
+ description:
+ If FSADJ is tied to VCC, an internal RFSADJ (20 kΩ) is selected, which
+ results in nominal output ranges. When an external resistor of 19 kΩ to
+ 41 kΩ can be used instead by connecting the resistor between FSADJ and GND
+ it controls the scaling of the ranges, and the internal resistor is
+ automatically disconnected.
+ minimum: 19000
+ maximum: 41000
+ default: 20000
+
+ io-channels:
+ description:
+ ADC channel to monitor voltages and currents at the MUX pin.
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ "^channel@[0-4]$":
+ type: object
+ additionalProperties: false
+
+ properties:
+ reg:
+ description: The channel number representing the DAC output channel.
+ maximum: 4
+
+ adi,toggle-mode:
+ description:
+ Set the channel as a toggle enabled channel. Toggle operation enables
+ fast switching of a DAC output between two different DAC codes without
+ any SPI transaction.
+ type: boolean
+
+ adi,output-range-microamp:
+ description: Specify the channel output full scale range.
+ enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000,
+ 200000000, 300000000]
+
+ required:
+ - reg
+ - adi,output-range-microamp
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - vcc-supply
+ - iovcc-supply
+ - v-neg-supply
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ dac@0 {
+ compatible = "adi,ltc2672";
+ reg = <0>;
+ spi-max-frequency = <10000000>;
+
+ vcc-supply = <&vcc>;
+ iovcc-supply = <&vcc>;
+ ref-supply = <&vref>;
+ v-neg-supply = <&vneg>;
+
+ io-channels = <&adc 0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ channel@0 {
+ reg = <0>;
+ adi,toggle-mode;
+ adi,output-range-microamp = <3125000>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ adi,output-range-microamp = <6250000>;
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index c7a102d7fd6a..ac1e29e26f31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13070,6 +13070,7 @@ L: [email protected]
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
LTC2688 IIO DAC DRIVER
M: Nuno Sá <[email protected]>
--
2.34.1
Co-developed-by: Michael Hennerich <[email protected]>
Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Kim Seer Paller <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-iio | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 7cee78ad4108..6ee58f59065b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -708,6 +708,7 @@ Description:
2.5kohm_to_gnd: connected to ground via a 2.5kOhm resistor,
6kohm_to_gnd: connected to ground via a 6kOhm resistor,
20kohm_to_gnd: connected to ground via a 20kOhm resistor,
+ 42kohm_to_gnd: connected to ground via a 42kOhm resistor,
90kohm_to_gnd: connected to ground via a 90kOhm resistor,
100kohm_to_gnd: connected to ground via an 100kOhm resistor,
125kohm_to_gnd: connected to ground via an 125kOhm resistor,
--
2.34.1
On 03/06/2024 03:21, Kim Seer Paller wrote:
> Add documentation for ltc2664.
>
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
> ---
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 03/06/2024 03:21, Kim Seer Paller wrote:
> Add documentation for ltc2672.
>
> Reported-by: Rob Herring (Arm) <[email protected]>
??? There was no bug report telling you the binding is missing. Drop.
> Closes: https://lore.kernel.org/all/[email protected]/
Drop
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
> ---
> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 159 insertions(+)
With these two fixes:
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
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 <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
The above could be dropped. This is still not merged code :)
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
> ---
LGTM... just a couple of minor points/questions that you can maybe take on if a re-
spin is needed.
Reviewed-by: Nuno Sa <[email protected]>
...
>
> +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:21 PM, Kim Seer Paller wrote:
> Add documentation for ltc2672.
>
> Reported-by: Rob Herring (Arm) <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
> ---
> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 159 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> new file mode 100644
> index 000000000000..d143a9db7010
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> @@ -0,0 +1,158 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTC2672 DAC
> +
> +maintainers:
> + - Michael Hennerich <[email protected]>
> + - Kim Seer Paller <[email protected]>
> +
> +description: |
> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ltc2672
The linked datasheet describes 12-bit and 16-bit versions, so should we have
two compatibles here? adi,ltc2672-12, adi,ltc2672-16
I don't see any ID registers where this could be read from the chip at
runtime, so seems like something that needs to be in the devicetree.
On 6/3/24 2:59 PM, David Lechner wrote:
> On 6/2/24 8:21 PM, Kim Seer Paller wrote:
>> Add documentation for ltc2672.
>>
>> Reported-by: Rob Herring (Arm) <[email protected]>
>> Closes: https://lore.kernel.org/all/[email protected]/
>> Co-developed-by: Michael Hennerich <[email protected]>
>> Signed-off-by: Michael Hennerich <[email protected]>
>> Signed-off-by: Kim Seer Paller <[email protected]>
>> ---
>> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 159 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>> new file mode 100644
>> index 000000000000..d143a9db7010
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>> @@ -0,0 +1,158 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Analog Devices LTC2672 DAC
>> +
>> +maintainers:
>> + - Michael Hennerich <[email protected]>
>> + - Kim Seer Paller <[email protected]>
>> +
>> +description: |
>> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - adi,ltc2672
>
> The linked datasheet describes 12-bit and 16-bit versions, so should we have
> two compatibles here? adi,ltc2672-12, adi,ltc2672-16
>
> I don't see any ID registers where this could be read from the chip at
> runtime, so seems like something that needs to be in the devicetree.
Hmm... I guess maybe it doesn't matter for these chips (i.e. the 4 LSBs
of the sample data register in 12-bit version will just always be ignored
but no data needs to be shifted based on the bit-ness).
I would not hurt to update the description though since it only mentions
16-bit if the compatible is meant for both versions of the chip.
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 <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
> ---
> 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á <[email protected]>
> 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 <[email protected]>");
> +MODULE_AUTHOR("Kim Seer Paller <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices LTC2664 and LTC2672 DAC");
> +MODULE_LICENSE("GPL");
On 03/06/2024 21:59, David Lechner wrote:
> On 6/2/24 8:21 PM, Kim Seer Paller wrote:
>> Add documentation for ltc2672.
>>
>> Reported-by: Rob Herring (Arm) <[email protected]>
>> Closes: https://lore.kernel.org/all/[email protected]/
>> Co-developed-by: Michael Hennerich <[email protected]>
>> Signed-off-by: Michael Hennerich <[email protected]>
>> Signed-off-by: Kim Seer Paller <[email protected]>
>> ---
>> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 159 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>> new file mode 100644
>> index 000000000000..d143a9db7010
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>> @@ -0,0 +1,158 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Analog Devices LTC2672 DAC
>> +
>> +maintainers:
>> + - Michael Hennerich <[email protected]>
>> + - Kim Seer Paller <[email protected]>
>> +
>> +description: |
>> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - adi,ltc2672
>
> The linked datasheet describes 12-bit and 16-bit versions, so should we have
> two compatibles here? adi,ltc2672-12, adi,ltc2672-16
Is their programming model different?
Best regards,
Krzysztof
On 6/4/24 1:47 AM, Krzysztof Kozlowski wrote:
> On 03/06/2024 21:59, David Lechner wrote:
>> On 6/2/24 8:21 PM, Kim Seer Paller wrote:
>>> Add documentation for ltc2672.
>>>
>>> Reported-by: Rob Herring (Arm) <[email protected]>
>>> Closes: https://lore.kernel.org/all/[email protected]/
>>> Co-developed-by: Michael Hennerich <[email protected]>
>>> Signed-off-by: Michael Hennerich <[email protected]>
>>> Signed-off-by: Kim Seer Paller <[email protected]>
>>> ---
>>> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
>>> MAINTAINERS | 1 +
>>> 2 files changed, 159 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>>> new file mode 100644
>>> index 000000000000..d143a9db7010
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
>>> @@ -0,0 +1,158 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Analog Devices LTC2672 DAC
>>> +
>>> +maintainers:
>>> + - Michael Hennerich <[email protected]>
>>> + - Kim Seer Paller <[email protected]>
>>> +
>>> +description: |
>>> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC
>>> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - adi,ltc2672
>>
>> The linked datasheet describes 12-bit and 16-bit versions, so should we have
>> two compatibles here? adi,ltc2672-12, adi,ltc2672-16
>
> Is their programming model different?
>
I replied to myself already with the answer. After looking at it more it
does not appear that is the case.
> > +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 Tue, 4 Jun 2024 08:53:27 -0500
David Lechner <[email protected]> wrote:
> On 6/4/24 1:47 AM, Krzysztof Kozlowski wrote:
> > On 03/06/2024 21:59, David Lechner wrote:
> >> On 6/2/24 8:21 PM, Kim Seer Paller wrote:
> >>> Add documentation for ltc2672.
> >>>
> >>> Reported-by: Rob Herring (Arm) <[email protected]>
> >>> Closes: https://lore.kernel.org/all/[email protected]/
> >>> Co-developed-by: Michael Hennerich <[email protected]>
> >>> Signed-off-by: Michael Hennerich <[email protected]>
> >>> Signed-off-by: Kim Seer Paller <[email protected]>
> >>> ---
> >>> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++
> >>> MAINTAINERS | 1 +
> >>> 2 files changed, 159 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> >>> new file mode 100644
> >>> index 000000000000..d143a9db7010
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml
> >>> @@ -0,0 +1,158 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Analog Devices LTC2672 DAC
> >>> +
> >>> +maintainers:
> >>> + - Michael Hennerich <[email protected]>
> >>> + - Kim Seer Paller <[email protected]>
> >>> +
> >>> +description: |
> >>> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC
> >>> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - adi,ltc2672
> >>
> >> The linked datasheet describes 12-bit and 16-bit versions, so should we have
> >> two compatibles here? adi,ltc2672-12, adi,ltc2672-16
> >
> > Is their programming model different?
> >
>
> I replied to myself already with the answer. After looking at it more it
> does not appear that is the case.
>
For a DAC, this is an interesting question. The wrong impressions of
precision might be a problem if someone is trying to tune the value.
Say they set it to +15 and look at some other sensor for the affect.
They expect to see something but get no change at all. They might
assume the circuit is broken.
So I think yes the programming model is different and that should
be discoverable (ideally from hardware, but if not from the compatible)
To take an extreme example of extending the logic of these being
the 'same' from a programming model point of view, would we consider
a regulator that did 0 and 3V only different from one that did 0V,
1V, 2V, 3V just because the second bit in the register was ignored?
I think in that case we'd consider them to have an obviously different
programming model.
We have a few cases where we do paper over similar differences in
resolution, but within one part with different settings rather than
between devices (so that's a driver limitation, not a DT thing).
So I might be persuaded no one cares, but in my view the programming
model is different in a significant way.
Jonathan
On Mon, 3 Jun 2024 09:21:56 +0800
Kim Seer Paller <[email protected]> wrote:
> Introduces a more generalized ABI documentation for DAC. Instead of
> having separate ABI files for each DAC, we now have a single ABI file
> that covers the common sysfs interface for all DAC.
>
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
A few comments inline.
I wondered if it made sense to combine voltage and current entries of each type
in single block, but I think the docs would become too complicated with lots
of wild cards etc. Hence I think the duplication is fine.
Jonathan
> ---
> Documentation/ABI/testing/sysfs-bus-iio-dac | 61 +++++++++++++++++++
> .../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 ----------
> 2 files changed, 61 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac
> new file mode 100644
> index 000000000000..36d316bb75f6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac
> @@ -0,0 +1,61 @@
> +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en
> +KernelVersion: 5.18
> +Contact: [email protected]
> +Description:
> + Toggle enable. Write 1 to enable toggle or 0 to disable it. This
Tab vs space issue - see below.
> + is useful when one wants to change the DAC output codes. The way
> + it should be done is:
> +
> + - disable toggle operation;
> + - change out_currentY_rawN, where N is the integer value of the symbol;
> + - enable toggle operation.
Same question as below on whether this is accurate - Maybe it just needs to mention
this scheme needs to be used for autonomous toggling (out of software control).
It works for software toggling but may be overkill!
> +
> +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN
> +KernelVersion: 5.18
> +Contact: [email protected]
> +Description:
> + This attribute has the same meaning as out_currentY_raw. It is
> + specific to toggle enabled channels and refers to the DAC output
> + code in INPUT_N (_rawN), where N is the integer value of the symbol.
> + The same scale and offset as in out_currentY_raw applies.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol
> +KernelVersion: 5.18
> +Contact: [email protected]
> +Description:
> + Performs a SW switch to a predefined output symbol. This attribute
> + is specific to toggle enabled channels and allows switching between
> + multiple predefined symbols. Each symbol corresponds to a different
> + output, denoted as out_currentY_rawN, where N is the integer value
> + of the symbol. Writing an integer value N will select out_currentY_rawN.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
> +KernelVersion: 5.18
> +Contact: [email protected]
> +Description:
> + Toggle enable. Write 1 to enable toggle or 0 to disable it. This
Mix of spacing and tabs is inconsistent. Hence the odd indent in this reply version.
> + is useful when one wants to change the DAC output codes. The way
> + it should be done is:
Hmm. Is this true? If we are doing autonomous toggling on a clock or similar than agreed.
If we are using the out_current_symbol software control it would be common to switch
to A, modify B, switch to B, modify A etc.
I think our interface has probably evolved and so this might need an update.
> +
> + - disable toggle operation;
> + - change out_voltageY_rawN, where N is the integer value of the symbol;
> + - enable toggle operation.
On Mon, 3 Jun 2024 09:22:00 +0800
Kim Seer Paller <[email protected]> wrote:
> LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC
> LTC2672 5 channel, 16 bit Current Output Softspan DAC
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Co-developed-by: Michael Hennerich <[email protected]>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Kim Seer Paller <[email protected]>
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: Jonathan Cameron <[email protected]>
> Sent: Saturday, June 8, 2024 10:41 PM
> To: Paller, Kim Seer <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; David Lechner <[email protected]>; Lars-
> Peter Clausen <[email protected]>; Liam Girdwood <[email protected]>;
> Mark Brown <[email protected]>; Dimitri Fedrau <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Rob Herring <[email protected]>;
> Conor Dooley <[email protected]>; Hennerich, Michael
> <[email protected]>; Nuno S? <[email protected]>
> Subject: Re: [PATCH v3 1/5] iio: ABI: Generalize ABI documentation for DAC
>
> [External]
>
> On Mon, 3 Jun 2024 09:21:56 +0800
> Kim Seer Paller <[email protected]> wrote:
>
> > Introduces a more generalized ABI documentation for DAC. Instead of
> > having separate ABI files for each DAC, we now have a single ABI file
> > that covers the common sysfs interface for all DAC.
> >
> > Co-developed-by: Michael Hennerich <[email protected]>
> > Signed-off-by: Michael Hennerich <[email protected]>
> > Signed-off-by: Kim Seer Paller <[email protected]>
>
> A few comments inline.
>
> I wondered if it made sense to combine voltage and current entries of each
> type
> in single block, but I think the docs would become too complicated with lots
> of wild cards etc. Hence I think the duplication is fine.
>
> Jonathan
>
> > ---
> > Documentation/ABI/testing/sysfs-bus-iio-dac | 61 +++++++++++++++++++
> > .../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 ----------
> > 2 files changed, 61 insertions(+), 31 deletions(-)
> > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac
> b/Documentation/ABI/testing/sysfs-bus-iio-dac
> > new file mode 100644
> > index 000000000000..36d316bb75f6
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac
> > @@ -0,0 +1,61 @@
> > +What:
> /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en
> > +KernelVersion: 5.18
> > +Contact: [email protected]
> > +Description:
> > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This
> Tab vs space issue - see below.
>
> > + is useful when one wants to change the DAC output codes. The
> way
> > + it should be done is:
> > +
> > + - disable toggle operation;
> > + - change out_currentY_rawN, where N is the integer value of the
> symbol;
> > + - enable toggle operation.
> Same question as below on whether this is accurate - Maybe it just needs to
> mention
> this scheme needs to be used for autonomous toggling (out of software
> control).
> It works for software toggling but may be overkill!
>
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN
> > +KernelVersion: 5.18
> > +Contact: [email protected]
> > +Description:
> > + This attribute has the same meaning as out_currentY_raw. It is
> > + specific to toggle enabled channels and refers to the DAC
> output
> > + code in INPUT_N (_rawN), where N is the integer value of the
> symbol.
> > + The same scale and offset as in out_currentY_raw applies.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol
> > +KernelVersion: 5.18
> > +Contact: [email protected]
> > +Description:
> > + Performs a SW switch to a predefined output symbol. This
> attribute
> > + is specific to toggle enabled channels and allows switching
> between
> > + multiple predefined symbols. Each symbol corresponds to a
> different
> > + output, denoted as out_currentY_rawN, where N is the integer
> value
> > + of the symbol. Writing an integer value N will select
> out_currentY_rawN.
> > +
> > +What:
> /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
> > +KernelVersion: 5.18
> > +Contact: [email protected]
> > +Description:
> > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This
>
> Mix of spacing and tabs is inconsistent. Hence the odd indent in this reply
> version.
>
> > + is useful when one wants to change the DAC output codes. The
> way
> > + it should be done is:
>
> Hmm. Is this true? If we are doing autonomous toggling on a clock or similar
> than agreed.
> If we are using the out_current_symbol software control it would be common
> to switch
> to A, modify B, switch to B, modify A etc.
>
> I think our interface has probably evolved and so this might need an update.
I agree that the description could be clear about the differences between
autonomous and software toggling. If we were to change the description, would
this suffice?
Description:
Toggle enable. Write 1 to enable toggle or 0 to disable it. This
is useful when one wants to change the DAC output codes. For autonomous toggling, the way
it should be done is:
- disable toggle operation;
- change out_currentY_rawN, where N is the integer value of the symbol;
- enable toggle operation.
For software toggling, one can switch to A, modify B, switch to B, modify A, etc.
>
> > +
> > + - disable toggle operation;
> > + - change out_voltageY_rawN, where N is the integer value of the
> symbol;
> > + - enable toggle operation.