All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] iio: dac: Add AD5758 support
@ 2018-06-29  8:38 Stefan Popa
  2018-06-29  8:55 ` Sean Nyekjaer
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Stefan Popa @ 2018-06-29  8:38 UTC (permalink / raw)
  To: jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, mchehab, davem, gregkh, akpm, linus.walleij,
	rdunlap, lukas, Ismail.Kose, vilhelm.gray, sean.nyekjaer,
	pombredanne, linux-iio, linux-kernel, stefan.popa

The AD5758 is a single channel DAC with 16-bit precision which uses the
SPI interface that operates at clock rates up to 50MHz.

The output can be configured as voltage or current and is available on a
single terminal.

Datasheet:
http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758.pdf

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
Changes in v4:
	- fixed kbuild test robot warnings.
Changes in v3:
	- AD5758 can be both a current and voltage output DAC. The
	  decision is made based on the DT and the channel type is set
	  during probe.
	- dc-dc-mode, range-microvolt and range-microamp are required
	  properties.
	- Introduced a slew-time-us property from which slew rate clock
	  and slew rate step are calculated using a best match algorithm.
	- Dropped the union from ad5758_state struct.
	- Introduced a IIO_CHAN_INFO_OFFSET case part of ad5758_read_raw().
	- Added a TODO comment which specifies that CRC is not supported.
	- Kept the includes in order and removed the unused ones.
	- Removed unused macros and shortened the lengthy ones.
	- Renamed AD5758_REG_WRITE to AD5758_WR_FLAG_MSK.
	- Added an explanation for enum ad5758_output_range.
	- Used bsearch() instead of ad5758_get_array_index().
	- Reduced the delays.
	- strtobool() -> kstrtobool().

Changes in v2:
	- removed unnecessary parenthesis in AD5758_REG_WRITE macro.
	- added missing documentation fields of ad5758_state struct.
	- changed the type of pwr_down attribute to bool.
	- changed ad5758_dc_dc_ilimt[] to ad5758_dc_dc_ilim[].
	- ad5758_spi_reg_write() now returns spi_write(st->spi,
	  &st->data[0].d32, sizeof(st->data[0].d32));
	- removed unnecessary new line in ad5758_calib_mem_refresh().
	- changed the type of the mode parameter in
	  ad5758_set_dc_dc_conv_mode() from unsigned int to enum
	  ad5758_dc_dc_mode.
	- removed unnecessary parenthesis in ad5758_slew_rate_config().
	- changed the type of the enable parameter in
	  ad5758_fault_prot_switch_en() from unsigned char to bool.
	- the same as above, but for ad5758_internal_buffers_en().
	- added a missing mutex_unlock() in ad5758_reg_access().
	- moved the mutex_unlock() in ad5758_read_raw() and removed the
	  unreachable return.
	- returned directly where it was possible in ad5758_write_raw().
	- removed the channel, scan_type and scan_index fields.
	- in ad5758_parse_dt(), added missing "\n", and specified what the
	  default mode actually is.
	- returned directly at the end of ad5758_init().
	- in ad5758_probe() used device managed for registering the device
	  and returned directly without the error message.

 MAINTAINERS              |   7 +
 drivers/iio/dac/Kconfig  |  10 +
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/ad5758.c | 899 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 917 insertions(+)
 create mode 100644 drivers/iio/dac/ad5758.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 00e9670..12d102d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -796,6 +796,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
 S:	Supported
 F:	drivers/macintosh/ams/
 
+ANALOG DEVICES INC AD5758 DRIVER
+M:	Stefan Popa <stefan.popa@analog.com>
+L:	linux-iio@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+S:	Supported
+F:	drivers/iio/dac/ad5758.c
+
 ANALOG DEVICES INC AD5686 DRIVER
 M:	Stefan Popa <stefan.popa@analog.com>
 L:	linux-pm@vger.kernel.org
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 06e90de..80beb64 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -167,6 +167,16 @@ config AD5755
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5755.
 
+config AD5758
+	tristate "Analog Devices AD5758 DAC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Analog Devices AD5758 single channel
+	  Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5758.
+
 config AD5761
 	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 57aa230..a1b37cf 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
 obj-$(CONFIG_AD5592R) += ad5592r.o
 obj-$(CONFIG_AD5593R) += ad5593r.o
 obj-$(CONFIG_AD5755) += ad5755.o
+obj-$(CONFIG_AD5755) += ad5758.o
 obj-$(CONFIG_AD5761) += ad5761.o
 obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
diff --git a/drivers/iio/dac/ad5758.c b/drivers/iio/dac/ad5758.c
new file mode 100644
index 0000000..bd1bed7
--- /dev/null
+++ b/drivers/iio/dac/ad5758.c
@@ -0,0 +1,899 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD5758 Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ *
+ * TODO: Currently CRC is not supported in this driver
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/property.h>
+#include <linux/delay.h>
+#include <linux/bsearch.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <asm/div64.h>
+
+/* AD5758 registers definition */
+#define AD5758_NOP				0x00
+#define AD5758_DAC_INPUT			0x01
+#define AD5758_DAC_OUTPUT			0x02
+#define AD5758_CLEAR_CODE			0x03
+#define AD5758_USER_GAIN			0x04
+#define AD5758_USER_OFFSET			0x05
+#define AD5758_DAC_CONFIG			0x06
+#define AD5758_SW_LDAC				0x07
+#define AD5758_KEY				0x08
+#define AD5758_GP_CONFIG1			0x09
+#define AD5758_GP_CONFIG2			0x0A
+#define AD5758_DCDC_CONFIG1			0x0B
+#define AD5758_DCDC_CONFIG2			0x0C
+#define AD5758_WDT_CONFIG			0x0F
+#define AD5758_DIGITAL_DIAG_CONFIG		0x10
+#define AD5758_ADC_CONFIG			0x11
+#define AD5758_FAULT_PIN_CONFIG			0x12
+#define AD5758_TWO_STAGE_READBACK_SELECT	0x13
+#define AD5758_DIGITAL_DIAG_RESULTS		0x14
+#define AD5758_ANALOG_DIAG_RESULTS		0x15
+#define AD5758_STATUS				0x16
+#define AD5758_CHIP_ID				0x17
+#define AD5758_FREQ_MONITOR			0x18
+#define AD5758_DEVICE_ID_0			0x19
+#define AD5758_DEVICE_ID_1			0x1A
+#define AD5758_DEVICE_ID_2			0x1B
+#define AD5758_DEVICE_ID_3			0x1C
+
+/* AD5758_DAC_CONFIG */
+#define AD5758_DAC_CONFIG_RANGE_MSK		GENMASK(3, 0)
+#define AD5758_DAC_CONFIG_RANGE_MODE(x)		(((x) & 0xF) << 0)
+#define AD5758_DAC_CONFIG_INT_EN_MSK		BIT(5)
+#define AD5758_DAC_CONFIG_INT_EN_MODE(x)	(((x) & 0x1) << 5)
+#define AD5758_DAC_CONFIG_OUT_EN_MSK		BIT(6)
+#define AD5758_DAC_CONFIG_OUT_EN_MODE(x)	(((x) & 0x1) << 6)
+#define AD5758_DAC_CONFIG_SR_EN_MSK		BIT(8)
+#define AD5758_DAC_CONFIG_SR_EN_MODE(x)		(((x) & 0x1) << 8)
+#define AD5758_DAC_CONFIG_SR_CLOCK_MSK		GENMASK(12, 9)
+#define AD5758_DAC_CONFIG_SR_CLOCK_MODE(x)	(((x) & 0xF) << 9)
+#define AD5758_DAC_CONFIG_SR_STEP_MSK		GENMASK(15, 13)
+#define AD5758_DAC_CONFIG_SR_STEP_MODE(x)	(((x) & 0x7) << 13)
+
+/* AD5758_KEY */
+#define AD5758_KEY_CODE_RESET_1			0x15FA
+#define AD5758_KEY_CODE_RESET_2			0xAF51
+#define AD5758_KEY_CODE_SINGLE_ADC_CONV		0x1ADC
+#define AD5758_KEY_CODE_RESET_WDT		0x0D06
+#define AD5758_KEY_CODE_CALIB_MEM_REFRESH	0xFCBA
+
+/* AD5758_DCDC_CONFIG1 */
+#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MSK	GENMASK(4, 0)
+#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MODE(x)	(((x) & 0x1F) << 0)
+#define AD5758_DCDC_CONFIG1_DCDC_MODE_MSK	GENMASK(6, 5)
+#define AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(x)	(((x) & 0x3) << 5)
+#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK	BIT(7)
+#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(x)	(((x) & 0x1) << 7)
+
+/* AD5758_DCDC_CONFIG2 */
+#define AD5758_DCDC_CONFIG2_ILIMIT_MSK		GENMASK(3, 1)
+#define AD5758_DCDC_CONFIG2_ILIMIT_MODE(x)	(((x) & 0x7) << 1)
+#define AD5758_DCDC_CONFIG2_INTR_SAT_3WI_MSK	BIT(11)
+#define AD5758_DCDC_CONFIG2_BUSY_3WI_MSK	BIT(12)
+
+/* AD5758_DIGITAL_DIAG_RESULTS */
+#define AD5758_CAL_MEM_UNREFRESHED_MSK		BIT(15)
+
+#define AD5758_WR_FLAG_MSK(x)		(0x80 | ((x) & 0x1F))
+
+#define AD5758_FULL_SCALE_MICRO	65535000000ULL
+
+/**
+ * struct ad5758_state - driver instance specific data
+ * @spi:	spi_device
+ * @lock:	mutex lock
+ * @out_range:	struct which stores the output range
+ * @dc_dc_mode:	variable which stores the mode of operation
+ * @dc_dc_ilim:	variable which stores the dc-to-dc converter current limit
+ * @slew_time:	variable which stores the target slew time
+ * @pwr_down:	variable which contains whether a channel is powered down or not
+ * @data:	spi transfer buffers
+ */
+
+struct ad5758_range {
+	int reg;
+	int min;
+	int max;
+};
+
+struct ad5758_state {
+	struct spi_device *spi;
+	struct mutex lock;
+	struct ad5758_range out_range;
+	unsigned int dc_dc_mode;
+	unsigned int dc_dc_ilim;
+	unsigned int slew_time;
+	bool pwr_down;
+	__be32 d32[3];
+};
+
+/**
+ * Output ranges corresponding to bits [3:0] from DAC_CONFIG register
+ * 0000: 0 V to 5 V voltage range
+ * 0001: 0 V to 10 V voltage range
+ * 0010: ±5 V voltage range
+ * 0011: ±10 V voltage range
+ * 1000: 0 mA to 20 mA current range
+ * 1001: 0 mA to 24 mA current range
+ * 1010: 4 mA to 20 mA current range
+ * 1011: ±20 mA current range
+ * 1100: ±24 mA current range
+ * 1101: -1 mA to +22 mA current range
+ */
+enum ad5758_output_range {
+	AD5758_RANGE_0V_5V,
+	AD5758_RANGE_0V_10V,
+	AD5758_RANGE_PLUSMINUS_5V,
+	AD5758_RANGE_PLUSMINUS_10V,
+	AD5758_RANGE_0mA_20mA = 8,
+	AD5758_RANGE_0mA_24mA,
+	AD5758_RANGE_4mA_24mA,
+	AD5758_RANGE_PLUSMINUS_20mA,
+	AD5758_RANGE_PLUSMINUS_24mA,
+	AD5758_RANGE_MINUS_1mA_PLUS_22mA,
+};
+
+enum ad5758_dc_dc_mode {
+	AD5758_DCDC_MODE_POWER_OFF,
+	AD5758_DCDC_MODE_DPC_CURRENT,
+	AD5758_DCDC_MODE_DPC_VOLTAGE,
+	AD5758_DCDC_MODE_PPC_CURRENT,
+};
+
+static const struct ad5758_range ad5758_voltage_range[] = {
+	{ AD5758_RANGE_0V_5V, 0, 5000000 },
+	{ AD5758_RANGE_0V_10V, 0, 10000000 },
+	{ AD5758_RANGE_PLUSMINUS_5V, -5000000, 5000000 },
+	{ AD5758_RANGE_PLUSMINUS_10V, -10000000, 10000000 }
+};
+
+static const struct ad5758_range ad5758_current_range[] = {
+	{ AD5758_RANGE_0mA_20mA, 0, 20000},
+	{ AD5758_RANGE_0mA_24mA, 0, 24000 },
+	{ AD5758_RANGE_4mA_24mA, 4, 24000 },
+	{ AD5758_RANGE_PLUSMINUS_20mA, -20000, 20000 },
+	{ AD5758_RANGE_PLUSMINUS_24mA, -24000, 24000 },
+	{ AD5758_RANGE_MINUS_1mA_PLUS_22mA, -1000, 22000 },
+};
+
+static const int ad5758_sr_clk[16] = {
+	240000, 200000, 150000, 128000, 64000, 32000, 16000, 8000, 4000, 2000,
+	1000, 512, 256, 128, 64, 16
+};
+
+static const int ad5758_sr_step[8] = {
+	4, 12, 64, 120, 256, 500, 1820, 2048
+};
+
+static const int ad5758_dc_dc_ilim[6] = {
+	150000, 200000, 250000, 300000, 350000, 400000
+};
+
+static int ad5758_spi_reg_read(struct ad5758_state *st, unsigned int addr)
+{
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->d32[0],
+			.len = 4,
+			.cs_change = 1,
+		}, {
+			.tx_buf = &st->d32[1],
+			.rx_buf = &st->d32[2],
+			.len = 4,
+		},
+	};
+	int ret;
+
+	st->d32[0] = cpu_to_be32(
+		(AD5758_WR_FLAG_MSK(AD5758_TWO_STAGE_READBACK_SELECT) << 24) |
+		(addr << 8));
+	st->d32[1] = cpu_to_be32(AD5758_WR_FLAG_MSK(AD5758_NOP) << 24);
+
+	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	return (be32_to_cpu(st->d32[2]) >> 8) & 0xFFFF;
+}
+
+static int ad5758_spi_reg_write(struct ad5758_state *st,
+				unsigned int addr,
+				unsigned int val)
+{
+	st->d32[0] = cpu_to_be32((AD5758_WR_FLAG_MSK(addr) << 24) |
+				 ((val & 0xFFFF) << 8));
+
+	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
+}
+
+static int ad5758_spi_write_mask(struct ad5758_state *st,
+				 unsigned int addr,
+				 unsigned long int mask,
+				 unsigned int val)
+{
+	int regval;
+
+	regval = ad5758_spi_reg_read(st, addr);
+	if (regval < 0)
+		return regval;
+
+	regval &= ~mask;
+	regval |= val;
+
+	return ad5758_spi_reg_write(st, addr, regval);
+}
+
+static int cmpfunc(const void *a, const void *b)
+{
+	return (*(int *)a - *(int *)b);
+}
+
+static int ad5758_find_closest_match(const int *array,
+				     unsigned int size, int val)
+{
+	int i;
+
+	for (i = 0; i < size; i++) {
+		if (val <= array[i])
+			return i;
+	}
+
+	return size - 1;
+}
+
+static int ad5758_wait_for_task_complete(struct ad5758_state *st,
+					 unsigned int reg,
+					 unsigned int mask)
+{
+	unsigned int timeout;
+	int ret;
+
+	timeout = 10;
+	do {
+		ret = ad5758_spi_reg_read(st, reg);
+		if (ret < 0)
+			return ret;
+
+		if (!(ret & mask))
+			return 0;
+
+		udelay(100);
+	} while (--timeout);
+
+	dev_err(&st->spi->dev,
+		"Error reading bit 0x%x in 0x%x register\n", mask, reg);
+
+	return -EIO;
+}
+
+static int ad5758_calib_mem_refresh(struct ad5758_state *st)
+{
+	int ret;
+
+	ret = ad5758_spi_reg_write(st, AD5758_KEY,
+				   AD5758_KEY_CODE_CALIB_MEM_REFRESH);
+	if (ret < 0) {
+		dev_err(&st->spi->dev,
+			"Failed to initiate a calibration memory refresh\n");
+		return ret;
+	}
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
+					     AD5758_CAL_MEM_UNREFRESHED_MSK);
+}
+
+static int ad5758_soft_reset(struct ad5758_state *st)
+{
+	int ret;
+
+	ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
+	if (ret < 0)
+		return ret;
+
+	ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
+
+	/* Perform a software reset and wait 100us */
+	udelay(100);
+
+	return ret;
+}
+
+static int ad5758_set_dc_dc_conv_mode(struct ad5758_state *st,
+				      enum ad5758_dc_dc_mode mode)
+{
+	int ret;
+
+	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
+				    AD5758_DCDC_CONFIG1_DCDC_MODE_MSK,
+				    AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(mode));
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
+	 * This allows the 3-wire interface communication to complete.
+	 */
+	ret = ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
+					    AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
+	if (ret < 0)
+		return ret;
+
+	st->dc_dc_mode = mode;
+
+	return ret;
+}
+
+static int ad5758_set_dc_dc_ilim(struct ad5758_state *st, unsigned int ilim)
+{
+	int ret;
+
+	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG2,
+				    AD5758_DCDC_CONFIG2_ILIMIT_MSK,
+				    AD5758_DCDC_CONFIG2_ILIMIT_MODE(ilim));
+	if (ret < 0)
+		return ret;
+	/*
+	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
+	 * This allows the 3-wire interface communication to complete.
+	 */
+	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
+					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
+}
+
+static int ad5758_slew_rate_set(struct ad5758_state *st,
+				unsigned int sr_clk_idx,
+				unsigned int sr_step_idx)
+{
+	unsigned int mode;
+	unsigned long int mask;
+	int ret;
+
+	mask = AD5758_DAC_CONFIG_SR_EN_MSK |
+	       AD5758_DAC_CONFIG_SR_CLOCK_MSK |
+	       AD5758_DAC_CONFIG_SR_STEP_MSK;
+	mode = AD5758_DAC_CONFIG_SR_EN_MODE(1) |
+		AD5758_DAC_CONFIG_SR_STEP_MODE(sr_step_idx) |
+		AD5758_DAC_CONFIG_SR_CLOCK_MODE(sr_clk_idx);
+
+	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG, mask, mode);
+	if (ret < 0)
+		return ret;
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
+					     AD5758_CAL_MEM_UNREFRESHED_MSK);
+}
+
+static int ad5758_slew_rate_config(struct ad5758_state *st)
+{
+	unsigned int sr_clk_idx, sr_step_idx;
+	int i, res;
+	s64 diff_new, diff_old;
+	u64 sr_step, calc_slew_time;
+
+	sr_clk_idx = 0;
+	sr_step_idx = 0;
+	diff_old = S64_MAX;
+	/*
+	 * The slew time can be determined by using the formula:
+	 * Slew Time = (Full Scale Out / (Step Size x Update Clk Freq))
+	 * where Slew time is expressed in microseconds
+	 * Given the desired slew time, the following algorithm determines the
+	 * best match for the step size and the update clock frequency.
+	 */
+	for (i = 0; i < ARRAY_SIZE(ad5758_sr_clk); i++) {
+		/*
+		 * Go through each valid update clock freq and determine a raw
+		 * value for the step size by using the formula:
+		 * Step Size = Full Scale Out / (Update Clk Freq * Slew Time)
+		 */
+		sr_step = AD5758_FULL_SCALE_MICRO;
+		do_div(sr_step, ad5758_sr_clk[i]);
+		do_div(sr_step, st->slew_time);
+		/*
+		 * After a raw value for step size was determined, find the
+		 * closest valid match
+		 */
+		res = ad5758_find_closest_match(ad5758_sr_step,
+						ARRAY_SIZE(ad5758_sr_step),
+						sr_step);
+		/* Calculate the slew time */
+		calc_slew_time = AD5758_FULL_SCALE_MICRO;
+		do_div(calc_slew_time, ad5758_sr_step[res]);
+		do_div(calc_slew_time, ad5758_sr_clk[i]);
+		/*
+		 * Determine with how many microseconds the calculated slew time
+		 * is different from the desired slew time and store the diff
+		 * for the next iteration
+		 */
+		diff_new = abs(st->slew_time - calc_slew_time);
+		if (diff_new < diff_old) {
+			diff_old = diff_new;
+			sr_clk_idx = i;
+			sr_step_idx = res;
+		}
+	}
+
+	return ad5758_slew_rate_set(st, sr_clk_idx, sr_step_idx);
+}
+
+static int ad5758_set_out_range(struct ad5758_state *st, int range)
+{
+	int ret;
+
+	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
+				    AD5758_DAC_CONFIG_RANGE_MSK,
+				    AD5758_DAC_CONFIG_RANGE_MODE(range));
+	if (ret < 0)
+		return ret;
+
+	/* Wait to allow time for the internal calibrations to complete */
+	ret =  ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
+					     AD5758_CAL_MEM_UNREFRESHED_MSK);
+	if (ret < 0)
+		return ret;
+
+	return ret;
+}
+
+static int ad5758_fault_prot_switch_en(struct ad5758_state *st, bool enable)
+{
+	int ret;
+
+	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
+			AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK,
+			AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(enable));
+	if (ret < 0)
+		return ret;
+	/*
+	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
+	 * This allows the 3-wire interface communication to complete.
+	 */
+	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
+					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
+}
+
+static int ad5758_internal_buffers_en(struct ad5758_state *st, bool enable)
+{
+	int ret;
+
+	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
+				    AD5758_DAC_CONFIG_INT_EN_MSK,
+				    AD5758_DAC_CONFIG_INT_EN_MODE(enable));
+	if (ret < 0)
+		return ret;
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
+					     AD5758_CAL_MEM_UNREFRESHED_MSK);
+}
+
+static int ad5758_reg_access(struct iio_dev *indio_dev,
+			     unsigned int reg,
+			     unsigned int writeval,
+			     unsigned int *readval)
+{
+	struct ad5758_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&st->lock);
+	if (readval) {
+		ret = ad5758_spi_reg_read(st, reg);
+		if (ret < 0) {
+			mutex_unlock(&st->lock);
+			return ret;
+		}
+
+		*readval = ret;
+		ret = 0;
+	} else {
+		ret = ad5758_spi_reg_write(st, reg, writeval);
+	}
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int ad5758_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad5758_state *st = iio_priv(indio_dev);
+	int max, min, ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		ret = ad5758_spi_reg_read(st, AD5758_DAC_INPUT);
+		mutex_unlock(&st->lock);
+		if (ret < 0)
+			return ret;
+
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		min = st->out_range.min;
+		max = st->out_range.max;
+		*val = (max - min) / 1000;
+		*val2 = 16;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		min = st->out_range.min;
+		max = st->out_range.max;
+		*val = ((min * (1 << 16)) / (max - min)) / 1000;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5758_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long info)
+{
+	struct ad5758_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		ret = ad5758_spi_reg_write(st, AD5758_DAC_INPUT, val);
+		mutex_unlock(&st->lock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t ad5758_read_powerdown(struct iio_dev *indio_dev,
+				     uintptr_t priv,
+				     const struct iio_chan_spec *chan,
+				     char *buf)
+{
+	struct ad5758_state *st = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n", st->pwr_down);
+}
+
+static ssize_t ad5758_write_powerdown(struct iio_dev *indio_dev,
+				      uintptr_t priv,
+				      struct iio_chan_spec const *chan,
+				      const char *buf, size_t len)
+{
+	struct ad5758_state *st = iio_priv(indio_dev);
+	bool pwr_down;
+	unsigned int dcdc_config1_mode, dc_dc_mode, dac_config_mode, val;
+	unsigned long int dcdc_config1_msk, dac_config_msk;
+	int ret;
+
+	ret = kstrtobool(buf, &pwr_down);
+	if (ret)
+		return ret;
+
+	mutex_lock(&st->lock);
+	if (pwr_down) {
+		dc_dc_mode = AD5758_DCDC_MODE_POWER_OFF;
+		val = 0;
+	} else {
+		dc_dc_mode = st->dc_dc_mode;
+		val = 1;
+	}
+
+	dcdc_config1_mode = (AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(dc_dc_mode) |
+			     AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(val));
+	dcdc_config1_msk = (AD5758_DCDC_CONFIG1_DCDC_MODE_MSK |
+			    AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK);
+
+	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
+				    dcdc_config1_msk,
+				    dcdc_config1_mode);
+	if (ret < 0)
+		goto err_unlock;
+
+	dac_config_mode = (AD5758_DAC_CONFIG_OUT_EN_MODE(val) |
+			   AD5758_DAC_CONFIG_INT_EN_MODE(val));
+	dac_config_msk =  (AD5758_DAC_CONFIG_OUT_EN_MSK |
+			   AD5758_DAC_CONFIG_INT_EN_MSK);
+
+	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
+				    dac_config_msk,
+				    dac_config_mode);
+	if (ret < 0)
+		goto err_unlock;
+
+	st->pwr_down = pwr_down;
+
+err_unlock:
+	mutex_unlock(&st->lock);
+
+	return ret ? ret : len;
+}
+
+static const struct iio_info ad5758_info = {
+	.read_raw = ad5758_read_raw,
+	.write_raw = ad5758_write_raw,
+	.debugfs_reg_access = &ad5758_reg_access,
+};
+
+static const struct iio_chan_spec_ext_info ad5758_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ad5758_read_powerdown,
+		.write = ad5758_write_powerdown,
+		.shared = IIO_SHARED_BY_TYPE,
+	},
+	{ }
+};
+
+static struct iio_chan_spec ad5758_channels[] = {
+	{
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_OFFSET),
+		.indexed = 1,
+		.output = 1,
+		.ext_info = ad5758_ext_info,
+	},
+};
+
+static bool ad5758_is_valid_mode(enum ad5758_dc_dc_mode mode)
+{
+	switch (mode) {
+	case AD5758_DCDC_MODE_DPC_CURRENT:
+	case AD5758_DCDC_MODE_DPC_VOLTAGE:
+	case AD5758_DCDC_MODE_PPC_CURRENT:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int ad5758_crc_disable(struct ad5758_state *st)
+{
+	unsigned int mask;
+
+	mask = (AD5758_WR_FLAG_MSK(AD5758_DIGITAL_DIAG_CONFIG) << 24) | 0x5C3A;
+	st->d32[0] = cpu_to_be32(mask);
+
+	return spi_write(st->spi, &st->d32[0], 4);
+}
+
+static int ad5758_find_out_range(struct ad5758_state *st,
+				 const struct ad5758_range *range,
+				 unsigned int size,
+				 int min, int max)
+{
+	int i;
+
+	for (i = 0; i < size; i++) {
+		if ((min == range[i].min) && (max == range[i].max)) {
+			st->out_range.reg = range[i].reg;
+			st->out_range.min = range[i].min;
+			st->out_range.max = range[i].max;
+
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int ad5758_parse_dt(struct ad5758_state *st)
+{
+	unsigned int tmp, tmparray[2], size;
+	const struct ad5758_range *range;
+	int *index, ret;
+
+	st->dc_dc_ilim = 0;
+	ret = device_property_read_u32(&st->spi->dev,
+				       "adi,dc-dc-ilim-microamp", &tmp);
+	if (ret) {
+		dev_dbg(&st->spi->dev,
+			"Missing \"dc-dc-ilim-microamp\" property\n");
+	} else {
+		index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
+					ARRAY_SIZE(ad5758_dc_dc_ilim),
+					sizeof(int), cmpfunc);
+		if (!index)
+			dev_dbg(&st->spi->dev, "dc-dc-ilim out of range\n");
+		else
+			st->dc_dc_ilim = index - ad5758_dc_dc_ilim;
+	}
+
+	ret = device_property_read_u32(&st->spi->dev, "adi,dc-dc-mode",
+				       &st->dc_dc_mode);
+	if (ret) {
+		dev_err(&st->spi->dev, "Missing \"dc-dc-mode\" property\n");
+		return ret;
+	}
+
+	if (!ad5758_is_valid_mode(st->dc_dc_mode))
+		return -EINVAL;
+
+	if (st->dc_dc_mode == AD5758_DCDC_MODE_DPC_VOLTAGE) {
+		ret = device_property_read_u32_array(&st->spi->dev,
+						     "adi,range-microvolt",
+						     tmparray, 2);
+		if (ret) {
+			dev_err(&st->spi->dev,
+				"Missing \"range-microvolt\" property\n");
+			return ret;
+		}
+		range = ad5758_voltage_range;
+		size = ARRAY_SIZE(ad5758_voltage_range);
+	} else {
+		ret = device_property_read_u32_array(&st->spi->dev,
+						     "adi,range-microamp",
+						     tmparray, 2);
+		if (ret) {
+			dev_err(&st->spi->dev,
+				"Missing \"range-microamp\" property\n");
+			return ret;
+		}
+		range = ad5758_current_range;
+		size = ARRAY_SIZE(ad5758_current_range);
+	}
+
+	ret = ad5758_find_out_range(st, range, size, tmparray[0], tmparray[1]);
+	if (ret) {
+		dev_err(&st->spi->dev, "range invalid\n");
+		return ret;
+	}
+
+	ret = device_property_read_u32(&st->spi->dev, "adi,slew-time-us", &tmp);
+	if (ret) {
+		dev_dbg(&st->spi->dev, "Missing \"slew-time-us\" property\n");
+		st->slew_time = 0;
+	} else {
+		st->slew_time = tmp;
+	}
+
+	return 0;
+}
+
+static int ad5758_init(struct ad5758_state *st)
+{
+	int regval, ret;
+
+	/* Disable CRC checks */
+	ret = ad5758_crc_disable(st);
+	if (ret < 0)
+		return ret;
+
+	/* Perform a software reset */
+	ret = ad5758_soft_reset(st);
+	if (ret < 0)
+		return ret;
+
+	/* Disable CRC checks */
+	ret = ad5758_crc_disable(st);
+	if (ret < 0)
+		return ret;
+
+	/* Perform a calibration memory refresh */
+	ret = ad5758_calib_mem_refresh(st);
+	if (ret < 0)
+		return ret;
+
+	regval = ad5758_spi_reg_read(st, AD5758_DIGITAL_DIAG_RESULTS);
+	if (regval < 0)
+		return regval;
+
+	/* Clear all the error flags */
+	ret = ad5758_spi_reg_write(st, AD5758_DIGITAL_DIAG_RESULTS, regval);
+	if (ret < 0)
+		return ret;
+
+	/* Set the dc-to-dc current limit */
+	ret = ad5758_set_dc_dc_ilim(st, st->dc_dc_ilim);
+	if (ret < 0)
+		return ret;
+
+	/* Configure the dc-to-dc controller mode */
+	ret = ad5758_set_dc_dc_conv_mode(st, st->dc_dc_mode);
+	if (ret < 0)
+		return ret;
+
+	/* Configure the output range */
+	ret = ad5758_set_out_range(st, st->out_range.reg);
+	if (ret < 0)
+		return ret;
+
+	/* Enable Slew Rate Control, set the slew rate clock and step */
+	if (st->slew_time) {
+		ret = ad5758_slew_rate_config(st);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Enable the VIOUT fault protection switch (FPS is closed) */
+	ret = ad5758_fault_prot_switch_en(st, 1);
+	if (ret < 0)
+		return ret;
+
+	/* Power up the DAC and internal (INT) amplifiers */
+	ret = ad5758_internal_buffers_en(st, 1);
+	if (ret < 0)
+		return ret;
+
+	/* Enable VIOUT */
+	return ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
+				     AD5758_DAC_CONFIG_OUT_EN_MSK,
+				     AD5758_DAC_CONFIG_OUT_EN_MODE(1));
+}
+
+static int ad5758_probe(struct spi_device *spi)
+{
+	struct ad5758_state *st;
+	struct iio_dev *indio_dev;
+	struct iio_chan_spec *channels = ad5758_channels;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	spi_set_drvdata(spi, indio_dev);
+
+	st->spi = spi;
+
+	mutex_init(&st->lock);
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->info = &ad5758_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->num_channels = ARRAY_SIZE(ad5758_channels);
+
+	ret = ad5758_parse_dt(st);
+	if (ret < 0)
+		return ret;
+
+	if (st->dc_dc_mode == AD5758_DCDC_MODE_DPC_VOLTAGE)
+		channels->type = IIO_VOLTAGE;
+	else
+		channels->type = IIO_CURRENT;
+
+	indio_dev->channels = channels;
+
+	ret = ad5758_init(st);
+	if (ret < 0) {
+		dev_err(&spi->dev, "AD5758 init failed\n");
+		return ret;
+	}
+
+	return devm_iio_device_register(&st->spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad5758_id[] = {
+	{ "ad5758", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5758_id);
+
+static struct spi_driver ad5758_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+	},
+	.probe = ad5758_probe,
+	.id_table = ad5758_id,
+};
+
+module_spi_driver(ad5758_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5758 DAC");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
  2018-06-29  8:38 [PATCH v4 1/2] iio: dac: Add AD5758 support Stefan Popa
@ 2018-06-29  8:55 ` Sean Nyekjaer
  2018-06-30 15:49   ` Jonathan Cameron
  2018-06-30 21:26 ` Andy Shevchenko
  2 siblings, 0 replies; 7+ messages in thread
From: Sean Nyekjaer @ 2018-06-29  8:55 UTC (permalink / raw)
  To: Stefan Popa, jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, mchehab, davem, gregkh, akpm, linus.walleij,
	rdunlap, lukas, Ismail.Kose, vilhelm.gray, pombredanne,
	linux-iio, linux-kernel



On 29-06-2018 10:38, Stefan Popa wrote:
> The AD5758 is a single channel DAC with 16-bit precision which uses the
> SPI interface that operates at clock rates up to 50MHz.
> 
> The output can be configured as voltage or current and is available on a
> single terminal.
> 
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758.pdf
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Reviewed-by:  Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
> Changes in v4:
> 	- fixed kbuild test robot warnings.
> Changes in v3:
> 	- AD5758 can be both a current and voltage output DAC. The
> 	  decision is made based on the DT and the channel type is set
> 	  during probe.
> 	- dc-dc-mode, range-microvolt and range-microamp are required
> 	  properties.
> 	- Introduced a slew-time-us property from which slew rate clock
> 	  and slew rate step are calculated using a best match algorithm.
> 	- Dropped the union from ad5758_state struct.
> 	- Introduced a IIO_CHAN_INFO_OFFSET case part of ad5758_read_raw().
> 	- Added a TODO comment which specifies that CRC is not supported.
> 	- Kept the includes in order and removed the unused ones.
> 	- Removed unused macros and shortened the lengthy ones.
> 	- Renamed AD5758_REG_WRITE to AD5758_WR_FLAG_MSK.
> 	- Added an explanation for enum ad5758_output_range.
> 	- Used bsearch() instead of ad5758_get_array_index().
> 	- Reduced the delays.
> 	- strtobool() -> kstrtobool().
> 
> Changes in v2:
> 	- removed unnecessary parenthesis in AD5758_REG_WRITE macro.
> 	- added missing documentation fields of ad5758_state struct.
> 	- changed the type of pwr_down attribute to bool.
> 	- changed ad5758_dc_dc_ilimt[] to ad5758_dc_dc_ilim[].
> 	- ad5758_spi_reg_write() now returns spi_write(st->spi,
> 	  &st->data[0].d32, sizeof(st->data[0].d32));
> 	- removed unnecessary new line in ad5758_calib_mem_refresh().
> 	- changed the type of the mode parameter in
> 	  ad5758_set_dc_dc_conv_mode() from unsigned int to enum
> 	  ad5758_dc_dc_mode.
> 	- removed unnecessary parenthesis in ad5758_slew_rate_config().
> 	- changed the type of the enable parameter in
> 	  ad5758_fault_prot_switch_en() from unsigned char to bool.
> 	- the same as above, but for ad5758_internal_buffers_en().
> 	- added a missing mutex_unlock() in ad5758_reg_access().
> 	- moved the mutex_unlock() in ad5758_read_raw() and removed the
> 	  unreachable return.
> 	- returned directly where it was possible in ad5758_write_raw().
> 	- removed the channel, scan_type and scan_index fields.
> 	- in ad5758_parse_dt(), added missing "\n", and specified what the
> 	  default mode actually is.
> 	- returned directly at the end of ad5758_init().
> 	- in ad5758_probe() used device managed for registering the device
> 	  and returned directly without the error message.
> 
>   MAINTAINERS              |   7 +
>   drivers/iio/dac/Kconfig  |  10 +
>   drivers/iio/dac/Makefile |   1 +
>   drivers/iio/dac/ad5758.c | 899 +++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 917 insertions(+)
>   create mode 100644 drivers/iio/dac/ad5758.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00e9670..12d102d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -796,6 +796,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
>   S:	Supported
>   F:	drivers/macintosh/ams/
>   
> +ANALOG DEVICES INC AD5758 DRIVER
> +M:	Stefan Popa <stefan.popa@analog.com>
> +L:	linux-iio@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +S:	Supported
> +F:	drivers/iio/dac/ad5758.c
> +
>   ANALOG DEVICES INC AD5686 DRIVER
>   M:	Stefan Popa <stefan.popa@analog.com>
>   L:	linux-pm@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 06e90de..80beb64 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -167,6 +167,16 @@ config AD5755
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called ad5755.
>   
> +config AD5758
> +	tristate "Analog Devices AD5758 DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD5758 single channel
> +	  Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5758.
> +
>   config AD5761
>   	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
>   	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 57aa230..a1b37cf 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
>   obj-$(CONFIG_AD5592R) += ad5592r.o
>   obj-$(CONFIG_AD5593R) += ad5593r.o
>   obj-$(CONFIG_AD5755) += ad5755.o
> +obj-$(CONFIG_AD5755) += ad5758.o
>   obj-$(CONFIG_AD5761) += ad5761.o
>   obj-$(CONFIG_AD5764) += ad5764.o
>   obj-$(CONFIG_AD5791) += ad5791.o
> diff --git a/drivers/iio/dac/ad5758.c b/drivers/iio/dac/ad5758.c
> new file mode 100644
> index 0000000..bd1bed7
> --- /dev/null
> +++ b/drivers/iio/dac/ad5758.c
> @@ -0,0 +1,899 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD5758 Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + *
> + * TODO: Currently CRC is not supported in this driver
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/property.h>
> +#include <linux/delay.h>
> +#include <linux/bsearch.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <asm/div64.h>
> +
> +/* AD5758 registers definition */
> +#define AD5758_NOP				0x00
> +#define AD5758_DAC_INPUT			0x01
> +#define AD5758_DAC_OUTPUT			0x02
> +#define AD5758_CLEAR_CODE			0x03
> +#define AD5758_USER_GAIN			0x04
> +#define AD5758_USER_OFFSET			0x05
> +#define AD5758_DAC_CONFIG			0x06
> +#define AD5758_SW_LDAC				0x07
> +#define AD5758_KEY				0x08
> +#define AD5758_GP_CONFIG1			0x09
> +#define AD5758_GP_CONFIG2			0x0A
> +#define AD5758_DCDC_CONFIG1			0x0B
> +#define AD5758_DCDC_CONFIG2			0x0C
> +#define AD5758_WDT_CONFIG			0x0F
> +#define AD5758_DIGITAL_DIAG_CONFIG		0x10
> +#define AD5758_ADC_CONFIG			0x11
> +#define AD5758_FAULT_PIN_CONFIG			0x12
> +#define AD5758_TWO_STAGE_READBACK_SELECT	0x13
> +#define AD5758_DIGITAL_DIAG_RESULTS		0x14
> +#define AD5758_ANALOG_DIAG_RESULTS		0x15
> +#define AD5758_STATUS				0x16
> +#define AD5758_CHIP_ID				0x17
> +#define AD5758_FREQ_MONITOR			0x18
> +#define AD5758_DEVICE_ID_0			0x19
> +#define AD5758_DEVICE_ID_1			0x1A
> +#define AD5758_DEVICE_ID_2			0x1B
> +#define AD5758_DEVICE_ID_3			0x1C
> +
> +/* AD5758_DAC_CONFIG */
> +#define AD5758_DAC_CONFIG_RANGE_MSK		GENMASK(3, 0)
> +#define AD5758_DAC_CONFIG_RANGE_MODE(x)		(((x) & 0xF) << 0)
> +#define AD5758_DAC_CONFIG_INT_EN_MSK		BIT(5)
> +#define AD5758_DAC_CONFIG_INT_EN_MODE(x)	(((x) & 0x1) << 5)
> +#define AD5758_DAC_CONFIG_OUT_EN_MSK		BIT(6)
> +#define AD5758_DAC_CONFIG_OUT_EN_MODE(x)	(((x) & 0x1) << 6)
> +#define AD5758_DAC_CONFIG_SR_EN_MSK		BIT(8)
> +#define AD5758_DAC_CONFIG_SR_EN_MODE(x)		(((x) & 0x1) << 8)
> +#define AD5758_DAC_CONFIG_SR_CLOCK_MSK		GENMASK(12, 9)
> +#define AD5758_DAC_CONFIG_SR_CLOCK_MODE(x)	(((x) & 0xF) << 9)
> +#define AD5758_DAC_CONFIG_SR_STEP_MSK		GENMASK(15, 13)
> +#define AD5758_DAC_CONFIG_SR_STEP_MODE(x)	(((x) & 0x7) << 13)
> +
> +/* AD5758_KEY */
> +#define AD5758_KEY_CODE_RESET_1			0x15FA
> +#define AD5758_KEY_CODE_RESET_2			0xAF51
> +#define AD5758_KEY_CODE_SINGLE_ADC_CONV		0x1ADC
> +#define AD5758_KEY_CODE_RESET_WDT		0x0D06
> +#define AD5758_KEY_CODE_CALIB_MEM_REFRESH	0xFCBA
> +
> +/* AD5758_DCDC_CONFIG1 */
> +#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MSK	GENMASK(4, 0)
> +#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MODE(x)	(((x) & 0x1F) << 0)
> +#define AD5758_DCDC_CONFIG1_DCDC_MODE_MSK	GENMASK(6, 5)
> +#define AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(x)	(((x) & 0x3) << 5)
> +#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK	BIT(7)
> +#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(x)	(((x) & 0x1) << 7)
> +
> +/* AD5758_DCDC_CONFIG2 */
> +#define AD5758_DCDC_CONFIG2_ILIMIT_MSK		GENMASK(3, 1)
> +#define AD5758_DCDC_CONFIG2_ILIMIT_MODE(x)	(((x) & 0x7) << 1)
> +#define AD5758_DCDC_CONFIG2_INTR_SAT_3WI_MSK	BIT(11)
> +#define AD5758_DCDC_CONFIG2_BUSY_3WI_MSK	BIT(12)
> +
> +/* AD5758_DIGITAL_DIAG_RESULTS */
> +#define AD5758_CAL_MEM_UNREFRESHED_MSK		BIT(15)
> +
> +#define AD5758_WR_FLAG_MSK(x)		(0x80 | ((x) & 0x1F))
> +
> +#define AD5758_FULL_SCALE_MICRO	65535000000ULL
> +
> +/**
> + * struct ad5758_state - driver instance specific data
> + * @spi:	spi_device
> + * @lock:	mutex lock
> + * @out_range:	struct which stores the output range
> + * @dc_dc_mode:	variable which stores the mode of operation
> + * @dc_dc_ilim:	variable which stores the dc-to-dc converter current limit
> + * @slew_time:	variable which stores the target slew time
> + * @pwr_down:	variable which contains whether a channel is powered down or not
> + * @data:	spi transfer buffers
> + */
> +
> +struct ad5758_range {
> +	int reg;
> +	int min;
> +	int max;
> +};
> +
> +struct ad5758_state {
> +	struct spi_device *spi;
> +	struct mutex lock;
> +	struct ad5758_range out_range;
> +	unsigned int dc_dc_mode;
> +	unsigned int dc_dc_ilim;
> +	unsigned int slew_time;
> +	bool pwr_down;
> +	__be32 d32[3];
> +};
> +
> +/**
> + * Output ranges corresponding to bits [3:0] from DAC_CONFIG register
> + * 0000: 0 V to 5 V voltage range
> + * 0001: 0 V to 10 V voltage range
> + * 0010: ±5 V voltage range
> + * 0011: ±10 V voltage range
> + * 1000: 0 mA to 20 mA current range
> + * 1001: 0 mA to 24 mA current range
> + * 1010: 4 mA to 20 mA current range
> + * 1011: ±20 mA current range
> + * 1100: ±24 mA current range
> + * 1101: -1 mA to +22 mA current range
> + */
> +enum ad5758_output_range {
> +	AD5758_RANGE_0V_5V,
> +	AD5758_RANGE_0V_10V,
> +	AD5758_RANGE_PLUSMINUS_5V,
> +	AD5758_RANGE_PLUSMINUS_10V,
> +	AD5758_RANGE_0mA_20mA = 8,
> +	AD5758_RANGE_0mA_24mA,
> +	AD5758_RANGE_4mA_24mA,
> +	AD5758_RANGE_PLUSMINUS_20mA,
> +	AD5758_RANGE_PLUSMINUS_24mA,
> +	AD5758_RANGE_MINUS_1mA_PLUS_22mA,
> +};
> +
> +enum ad5758_dc_dc_mode {
> +	AD5758_DCDC_MODE_POWER_OFF,
> +	AD5758_DCDC_MODE_DPC_CURRENT,
> +	AD5758_DCDC_MODE_DPC_VOLTAGE,
> +	AD5758_DCDC_MODE_PPC_CURRENT,
> +};
> +
> +static const struct ad5758_range ad5758_voltage_range[] = {
> +	{ AD5758_RANGE_0V_5V, 0, 5000000 },
> +	{ AD5758_RANGE_0V_10V, 0, 10000000 },
> +	{ AD5758_RANGE_PLUSMINUS_5V, -5000000, 5000000 },
> +	{ AD5758_RANGE_PLUSMINUS_10V, -10000000, 10000000 }
> +};
> +
> +static const struct ad5758_range ad5758_current_range[] = {
> +	{ AD5758_RANGE_0mA_20mA, 0, 20000},
> +	{ AD5758_RANGE_0mA_24mA, 0, 24000 },
> +	{ AD5758_RANGE_4mA_24mA, 4, 24000 },
> +	{ AD5758_RANGE_PLUSMINUS_20mA, -20000, 20000 },
> +	{ AD5758_RANGE_PLUSMINUS_24mA, -24000, 24000 },
> +	{ AD5758_RANGE_MINUS_1mA_PLUS_22mA, -1000, 22000 },
> +};
> +
> +static const int ad5758_sr_clk[16] = {
> +	240000, 200000, 150000, 128000, 64000, 32000, 16000, 8000, 4000, 2000,
> +	1000, 512, 256, 128, 64, 16
> +};
> +
> +static const int ad5758_sr_step[8] = {
> +	4, 12, 64, 120, 256, 500, 1820, 2048
> +};
> +
> +static const int ad5758_dc_dc_ilim[6] = {
> +	150000, 200000, 250000, 300000, 350000, 400000
> +};
> +
> +static int ad5758_spi_reg_read(struct ad5758_state *st, unsigned int addr)
> +{
> +	struct spi_transfer t[] = {
> +		{
> +			.tx_buf = &st->d32[0],
> +			.len = 4,
> +			.cs_change = 1,
> +		}, {
> +			.tx_buf = &st->d32[1],
> +			.rx_buf = &st->d32[2],
> +			.len = 4,
> +		},
> +	};
> +	int ret;
> +
> +	st->d32[0] = cpu_to_be32(
> +		(AD5758_WR_FLAG_MSK(AD5758_TWO_STAGE_READBACK_SELECT) << 24) |
> +		(addr << 8));
> +	st->d32[1] = cpu_to_be32(AD5758_WR_FLAG_MSK(AD5758_NOP) << 24);
> +
> +	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +	if (ret < 0)
> +		return ret;
> +
> +	return (be32_to_cpu(st->d32[2]) >> 8) & 0xFFFF;
> +}
> +
> +static int ad5758_spi_reg_write(struct ad5758_state *st,
> +				unsigned int addr,
> +				unsigned int val)
> +{
> +	st->d32[0] = cpu_to_be32((AD5758_WR_FLAG_MSK(addr) << 24) |
> +				 ((val & 0xFFFF) << 8));
> +
> +	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
> +}
> +
> +static int ad5758_spi_write_mask(struct ad5758_state *st,
> +				 unsigned int addr,
> +				 unsigned long int mask,
> +				 unsigned int val)
> +{
> +	int regval;
> +
> +	regval = ad5758_spi_reg_read(st, addr);
> +	if (regval < 0)
> +		return regval;
> +
> +	regval &= ~mask;
> +	regval |= val;
> +
> +	return ad5758_spi_reg_write(st, addr, regval);
> +}
> +
> +static int cmpfunc(const void *a, const void *b)
> +{
> +	return (*(int *)a - *(int *)b);
> +}
> +
> +static int ad5758_find_closest_match(const int *array,
> +				     unsigned int size, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +		if (val <= array[i])
> +			return i;
> +	}
> +
> +	return size - 1;
> +}
> +
> +static int ad5758_wait_for_task_complete(struct ad5758_state *st,
> +					 unsigned int reg,
> +					 unsigned int mask)
> +{
> +	unsigned int timeout;
> +	int ret;
> +
> +	timeout = 10;
> +	do {
> +		ret = ad5758_spi_reg_read(st, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (!(ret & mask))
> +			return 0;
> +
> +		udelay(100);
> +	} while (--timeout);
> +
> +	dev_err(&st->spi->dev,
> +		"Error reading bit 0x%x in 0x%x register\n", mask, reg);
> +
> +	return -EIO;
> +}
> +
> +static int ad5758_calib_mem_refresh(struct ad5758_state *st)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_reg_write(st, AD5758_KEY,
> +				   AD5758_KEY_CODE_CALIB_MEM_REFRESH);
> +	if (ret < 0) {
> +		dev_err(&st->spi->dev,
> +			"Failed to initiate a calibration memory refresh\n");
> +		return ret;
> +	}
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_soft_reset(struct ad5758_state *st)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
> +
> +	/* Perform a software reset and wait 100us */
> +	udelay(100);
> +
> +	return ret;
> +}
> +
> +static int ad5758_set_dc_dc_conv_mode(struct ad5758_state *st,
> +				      enum ad5758_dc_dc_mode mode)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +				    AD5758_DCDC_CONFIG1_DCDC_MODE_MSK,
> +				    AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(mode));
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	ret = ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					    AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +	if (ret < 0)
> +		return ret;
> +
> +	st->dc_dc_mode = mode;
> +
> +	return ret;
> +}
> +
> +static int ad5758_set_dc_dc_ilim(struct ad5758_state *st, unsigned int ilim)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG2,
> +				    AD5758_DCDC_CONFIG2_ILIMIT_MSK,
> +				    AD5758_DCDC_CONFIG2_ILIMIT_MODE(ilim));
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +}
> +
> +static int ad5758_slew_rate_set(struct ad5758_state *st,
> +				unsigned int sr_clk_idx,
> +				unsigned int sr_step_idx)
> +{
> +	unsigned int mode;
> +	unsigned long int mask;
> +	int ret;
> +
> +	mask = AD5758_DAC_CONFIG_SR_EN_MSK |
> +	       AD5758_DAC_CONFIG_SR_CLOCK_MSK |
> +	       AD5758_DAC_CONFIG_SR_STEP_MSK;
> +	mode = AD5758_DAC_CONFIG_SR_EN_MODE(1) |
> +		AD5758_DAC_CONFIG_SR_STEP_MODE(sr_step_idx) |
> +		AD5758_DAC_CONFIG_SR_CLOCK_MODE(sr_clk_idx);
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG, mask, mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_slew_rate_config(struct ad5758_state *st)
> +{
> +	unsigned int sr_clk_idx, sr_step_idx;
> +	int i, res;
> +	s64 diff_new, diff_old;
> +	u64 sr_step, calc_slew_time;
> +
> +	sr_clk_idx = 0;
> +	sr_step_idx = 0;
> +	diff_old = S64_MAX;
> +	/*
> +	 * The slew time can be determined by using the formula:
> +	 * Slew Time = (Full Scale Out / (Step Size x Update Clk Freq))
> +	 * where Slew time is expressed in microseconds
> +	 * Given the desired slew time, the following algorithm determines the
> +	 * best match for the step size and the update clock frequency.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(ad5758_sr_clk); i++) {
> +		/*
> +		 * Go through each valid update clock freq and determine a raw
> +		 * value for the step size by using the formula:
> +		 * Step Size = Full Scale Out / (Update Clk Freq * Slew Time)
> +		 */
> +		sr_step = AD5758_FULL_SCALE_MICRO;
> +		do_div(sr_step, ad5758_sr_clk[i]);
> +		do_div(sr_step, st->slew_time);
> +		/*
> +		 * After a raw value for step size was determined, find the
> +		 * closest valid match
> +		 */
> +		res = ad5758_find_closest_match(ad5758_sr_step,
> +						ARRAY_SIZE(ad5758_sr_step),
> +						sr_step);
> +		/* Calculate the slew time */
> +		calc_slew_time = AD5758_FULL_SCALE_MICRO;
> +		do_div(calc_slew_time, ad5758_sr_step[res]);
> +		do_div(calc_slew_time, ad5758_sr_clk[i]);
> +		/*
> +		 * Determine with how many microseconds the calculated slew time
> +		 * is different from the desired slew time and store the diff
> +		 * for the next iteration
> +		 */
> +		diff_new = abs(st->slew_time - calc_slew_time);
> +		if (diff_new < diff_old) {
> +			diff_old = diff_new;
> +			sr_clk_idx = i;
> +			sr_step_idx = res;
> +		}
> +	}
> +
> +	return ad5758_slew_rate_set(st, sr_clk_idx, sr_step_idx);
> +}
> +
> +static int ad5758_set_out_range(struct ad5758_state *st, int range)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    AD5758_DAC_CONFIG_RANGE_MSK,
> +				    AD5758_DAC_CONFIG_RANGE_MODE(range));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	ret =  ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ret;
> +}
> +
> +static int ad5758_fault_prot_switch_en(struct ad5758_state *st, bool enable)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +			AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK,
> +			AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(enable));
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +}
> +
> +static int ad5758_internal_buffers_en(struct ad5758_state *st, bool enable)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    AD5758_DAC_CONFIG_INT_EN_MSK,
> +				    AD5758_DAC_CONFIG_INT_EN_MODE(enable));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_reg_access(struct iio_dev *indio_dev,
> +			     unsigned int reg,
> +			     unsigned int writeval,
> +			     unsigned int *readval)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	if (readval) {
> +		ret = ad5758_spi_reg_read(st, reg);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
> +
> +		*readval = ret;
> +		ret = 0;
> +	} else {
> +		ret = ad5758_spi_reg_write(st, reg, writeval);
> +	}
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +}
> +
> +static int ad5758_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long info)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	int max, min, ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret = ad5758_spi_reg_read(st, AD5758_DAC_INPUT);
> +		mutex_unlock(&st->lock);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		min = st->out_range.min;
> +		max = st->out_range.max;
> +		*val = (max - min) / 1000;
> +		*val2 = 16;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		min = st->out_range.min;
> +		max = st->out_range.max;
> +		*val = ((min * (1 << 16)) / (max - min)) / 1000;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5758_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long info)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret = ad5758_spi_reg_write(st, AD5758_DAC_INPUT, val);
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static ssize_t ad5758_read_powerdown(struct iio_dev *indio_dev,
> +				     uintptr_t priv,
> +				     const struct iio_chan_spec *chan,
> +				     char *buf)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", st->pwr_down);
> +}
> +
> +static ssize_t ad5758_write_powerdown(struct iio_dev *indio_dev,
> +				      uintptr_t priv,
> +				      struct iio_chan_spec const *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	bool pwr_down;
> +	unsigned int dcdc_config1_mode, dc_dc_mode, dac_config_mode, val;
> +	unsigned long int dcdc_config1_msk, dac_config_msk;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&st->lock);
> +	if (pwr_down) {
> +		dc_dc_mode = AD5758_DCDC_MODE_POWER_OFF;
> +		val = 0;
> +	} else {
> +		dc_dc_mode = st->dc_dc_mode;
> +		val = 1;
> +	}
> +
> +	dcdc_config1_mode = (AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(dc_dc_mode) |
> +			     AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(val));
> +	dcdc_config1_msk = (AD5758_DCDC_CONFIG1_DCDC_MODE_MSK |
> +			    AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK);
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +				    dcdc_config1_msk,
> +				    dcdc_config1_mode);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	dac_config_mode = (AD5758_DAC_CONFIG_OUT_EN_MODE(val) |
> +			   AD5758_DAC_CONFIG_INT_EN_MODE(val));
> +	dac_config_msk =  (AD5758_DAC_CONFIG_OUT_EN_MSK |
> +			   AD5758_DAC_CONFIG_INT_EN_MSK);
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    dac_config_msk,
> +				    dac_config_mode);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	st->pwr_down = pwr_down;
> +
> +err_unlock:
> +	mutex_unlock(&st->lock);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_info ad5758_info = {
> +	.read_raw = ad5758_read_raw,
> +	.write_raw = ad5758_write_raw,
> +	.debugfs_reg_access = &ad5758_reg_access,
> +};
> +
> +static const struct iio_chan_spec_ext_info ad5758_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5758_read_powerdown,
> +		.write = ad5758_write_powerdown,
> +		.shared = IIO_SHARED_BY_TYPE,
> +	},
> +	{ }
> +};
> +
> +static struct iio_chan_spec ad5758_channels[] = {
> +	{
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OFFSET),
> +		.indexed = 1,
> +		.output = 1,
> +		.ext_info = ad5758_ext_info,
> +	},
> +};
> +
> +static bool ad5758_is_valid_mode(enum ad5758_dc_dc_mode mode)
> +{
> +	switch (mode) {
> +	case AD5758_DCDC_MODE_DPC_CURRENT:
> +	case AD5758_DCDC_MODE_DPC_VOLTAGE:
> +	case AD5758_DCDC_MODE_PPC_CURRENT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int ad5758_crc_disable(struct ad5758_state *st)
> +{
> +	unsigned int mask;
> +
> +	mask = (AD5758_WR_FLAG_MSK(AD5758_DIGITAL_DIAG_CONFIG) << 24) | 0x5C3A;
> +	st->d32[0] = cpu_to_be32(mask);
> +
> +	return spi_write(st->spi, &st->d32[0], 4);
> +}
> +
> +static int ad5758_find_out_range(struct ad5758_state *st,
> +				 const struct ad5758_range *range,
> +				 unsigned int size,
> +				 int min, int max)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +		if ((min == range[i].min) && (max == range[i].max)) {
> +			st->out_range.reg = range[i].reg;
> +			st->out_range.min = range[i].min;
> +			st->out_range.max = range[i].max;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad5758_parse_dt(struct ad5758_state *st)
> +{
> +	unsigned int tmp, tmparray[2], size;
> +	const struct ad5758_range *range;
> +	int *index, ret;
> +
> +	st->dc_dc_ilim = 0;
> +	ret = device_property_read_u32(&st->spi->dev,
> +				       "adi,dc-dc-ilim-microamp", &tmp);
> +	if (ret) {
> +		dev_dbg(&st->spi->dev,
> +			"Missing \"dc-dc-ilim-microamp\" property\n");
> +	} else {
> +		index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> +					ARRAY_SIZE(ad5758_dc_dc_ilim),
> +					sizeof(int), cmpfunc);
> +		if (!index)
> +			dev_dbg(&st->spi->dev, "dc-dc-ilim out of range\n");
> +		else
> +			st->dc_dc_ilim = index - ad5758_dc_dc_ilim;
> +	}
> +
> +	ret = device_property_read_u32(&st->spi->dev, "adi,dc-dc-mode",
> +				       &st->dc_dc_mode);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Missing \"dc-dc-mode\" property\n");
> +		return ret;
> +	}
> +
> +	if (!ad5758_is_valid_mode(st->dc_dc_mode))
> +		return -EINVAL;
> +
> +	if (st->dc_dc_mode == AD5758_DCDC_MODE_DPC_VOLTAGE) {
> +		ret = device_property_read_u32_array(&st->spi->dev,
> +						     "adi,range-microvolt",
> +						     tmparray, 2);
> +		if (ret) {
> +			dev_err(&st->spi->dev,
> +				"Missing \"range-microvolt\" property\n");
> +			return ret;
> +		}
> +		range = ad5758_voltage_range;
> +		size = ARRAY_SIZE(ad5758_voltage_range);
> +	} else {
> +		ret = device_property_read_u32_array(&st->spi->dev,
> +						     "adi,range-microamp",
> +						     tmparray, 2);
> +		if (ret) {
> +			dev_err(&st->spi->dev,
> +				"Missing \"range-microamp\" property\n");
> +			return ret;
> +		}
> +		range = ad5758_current_range;
> +		size = ARRAY_SIZE(ad5758_current_range);
> +	}
> +
> +	ret = ad5758_find_out_range(st, range, size, tmparray[0], tmparray[1]);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "range invalid\n");
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(&st->spi->dev, "adi,slew-time-us", &tmp);
> +	if (ret) {
> +		dev_dbg(&st->spi->dev, "Missing \"slew-time-us\" property\n");
> +		st->slew_time = 0;
> +	} else {
> +		st->slew_time = tmp;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad5758_init(struct ad5758_state *st)
> +{
> +	int regval, ret;
> +
> +	/* Disable CRC checks */
> +	ret = ad5758_crc_disable(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Perform a software reset */
> +	ret = ad5758_soft_reset(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Disable CRC checks */
> +	ret = ad5758_crc_disable(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Perform a calibration memory refresh */
> +	ret = ad5758_calib_mem_refresh(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	regval = ad5758_spi_reg_read(st, AD5758_DIGITAL_DIAG_RESULTS);
> +	if (regval < 0)
> +		return regval;
> +
> +	/* Clear all the error flags */
> +	ret = ad5758_spi_reg_write(st, AD5758_DIGITAL_DIAG_RESULTS, regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set the dc-to-dc current limit */
> +	ret = ad5758_set_dc_dc_ilim(st, st->dc_dc_ilim);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Configure the dc-to-dc controller mode */
> +	ret = ad5758_set_dc_dc_conv_mode(st, st->dc_dc_mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Configure the output range */
> +	ret = ad5758_set_out_range(st, st->out_range.reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable Slew Rate Control, set the slew rate clock and step */
> +	if (st->slew_time) {
> +		ret = ad5758_slew_rate_config(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* Enable the VIOUT fault protection switch (FPS is closed) */
> +	ret = ad5758_fault_prot_switch_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Power up the DAC and internal (INT) amplifiers */
> +	ret = ad5758_internal_buffers_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable VIOUT */
> +	return ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				     AD5758_DAC_CONFIG_OUT_EN_MSK,
> +				     AD5758_DAC_CONFIG_OUT_EN_MODE(1));
> +}
> +
> +static int ad5758_probe(struct spi_device *spi)
> +{
> +	struct ad5758_state *st;
> +	struct iio_dev *indio_dev;
> +	struct iio_chan_spec *channels = ad5758_channels;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st->spi = spi;
> +
> +	mutex_init(&st->lock);
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->info = &ad5758_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->num_channels = ARRAY_SIZE(ad5758_channels);
> +
> +	ret = ad5758_parse_dt(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (st->dc_dc_mode == AD5758_DCDC_MODE_DPC_VOLTAGE)
> +		channels->type = IIO_VOLTAGE;
> +	else
> +		channels->type = IIO_CURRENT;
> +
> +	indio_dev->channels = channels;
> +
> +	ret = ad5758_init(st);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "AD5758 init failed\n");
> +		return ret;
> +	}
> +
> +	return devm_iio_device_register(&st->spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id ad5758_id[] = {
> +	{ "ad5758", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5758_id);
> +
> +static struct spi_driver ad5758_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +	},
> +	.probe = ad5758_probe,
> +	.id_table = ad5758_id,
> +};
> +
> +module_spi_driver(ad5758_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5758 DAC");
> +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
  2018-06-29  8:38 [PATCH v4 1/2] iio: dac: Add AD5758 support Stefan Popa
@ 2018-06-30 15:49   ` Jonathan Cameron
  2018-06-30 15:49   ` Jonathan Cameron
  2018-06-30 21:26 ` Andy Shevchenko
  2 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-06-30 15:49 UTC (permalink / raw)
  To: Stefan Popa
  Cc: Michael.Hennerich, lars, knaack.h, pmeerw, mchehab, davem,
	gregkh, akpm, linus.walleij, rdunlap, lukas, Ismail.Kose,
	vilhelm.gray, sean.nyekjaer, pombredanne, linux-iio,
	linux-kernel

On Fri, 29 Jun 2018 11:38:36 +0300
Stefan Popa <stefan.popa@analog.com> wrote:

> The AD5758 is a single channel DAC with 16-bit precision which uses the
> SPI interface that operates at clock rates up to 50MHz.
> 
> The output can be configured as voltage or current and is available on a
> single terminal.
> 
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758.pdf
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>

Hi Stefan,

A few minors + the way you currently handle the choice of voltage vs
current prevents you having multiple instances of this device with
different settings.   I would pick between between two constant
options of the whole structure.  Alternative would be to make a fresh
copy of the structure for each instance of the device, but that is
fiddlier for such a simple case.

Jonathan

> ---
> Changes in v4:
> 	- fixed kbuild test robot warnings.
> Changes in v3:
> 	- AD5758 can be both a current and voltage output DAC. The
> 	  decision is made based on the DT and the channel type is set
> 	  during probe.
> 	- dc-dc-mode, range-microvolt and range-microamp are required
> 	  properties.
> 	- Introduced a slew-time-us property from which slew rate clock
> 	  and slew rate step are calculated using a best match algorithm.
> 	- Dropped the union from ad5758_state struct.
> 	- Introduced a IIO_CHAN_INFO_OFFSET case part of ad5758_read_raw().
> 	- Added a TODO comment which specifies that CRC is not supported.
> 	- Kept the includes in order and removed the unused ones.
> 	- Removed unused macros and shortened the lengthy ones.
> 	- Renamed AD5758_REG_WRITE to AD5758_WR_FLAG_MSK.
> 	- Added an explanation for enum ad5758_output_range.
> 	- Used bsearch() instead of ad5758_get_array_index().
> 	- Reduced the delays.
> 	- strtobool() -> kstrtobool().
> 
> Changes in v2:
> 	- removed unnecessary parenthesis in AD5758_REG_WRITE macro.
> 	- added missing documentation fields of ad5758_state struct.
> 	- changed the type of pwr_down attribute to bool.
> 	- changed ad5758_dc_dc_ilimt[] to ad5758_dc_dc_ilim[].
> 	- ad5758_spi_reg_write() now returns spi_write(st->spi,
> 	  &st->data[0].d32, sizeof(st->data[0].d32));
> 	- removed unnecessary new line in ad5758_calib_mem_refresh().
> 	- changed the type of the mode parameter in
> 	  ad5758_set_dc_dc_conv_mode() from unsigned int to enum
> 	  ad5758_dc_dc_mode.
> 	- removed unnecessary parenthesis in ad5758_slew_rate_config().
> 	- changed the type of the enable parameter in
> 	  ad5758_fault_prot_switch_en() from unsigned char to bool.
> 	- the same as above, but for ad5758_internal_buffers_en().
> 	- added a missing mutex_unlock() in ad5758_reg_access().
> 	- moved the mutex_unlock() in ad5758_read_raw() and removed the
> 	  unreachable return.
> 	- returned directly where it was possible in ad5758_write_raw().
> 	- removed the channel, scan_type and scan_index fields.
> 	- in ad5758_parse_dt(), added missing "\n", and specified what the
> 	  default mode actually is.
> 	- returned directly at the end of ad5758_init().
> 	- in ad5758_probe() used device managed for registering the device
> 	  and returned directly without the error message.
> 
>  MAINTAINERS              |   7 +
>  drivers/iio/dac/Kconfig  |  10 +
>  drivers/iio/dac/Makefile |   1 +
>  drivers/iio/dac/ad5758.c | 899 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 917 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5758.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00e9670..12d102d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -796,6 +796,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
>  S:	Supported
>  F:	drivers/macintosh/ams/
>  
> +ANALOG DEVICES INC AD5758 DRIVER
> +M:	Stefan Popa <stefan.popa@analog.com>
> +L:	linux-iio@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +S:	Supported
> +F:	drivers/iio/dac/ad5758.c
> +
>  ANALOG DEVICES INC AD5686 DRIVER
>  M:	Stefan Popa <stefan.popa@analog.com>
>  L:	linux-pm@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 06e90de..80beb64 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -167,6 +167,16 @@ config AD5755
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5755.
>  
> +config AD5758
> +	tristate "Analog Devices AD5758 DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD5758 single channel
> +	  Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5758.
> +
>  config AD5761
>  	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
>  	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 57aa230..a1b37cf 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
>  obj-$(CONFIG_AD5592R) += ad5592r.o
>  obj-$(CONFIG_AD5593R) += ad5593r.o
>  obj-$(CONFIG_AD5755) += ad5755.o
> +obj-$(CONFIG_AD5755) += ad5758.o
>  obj-$(CONFIG_AD5761) += ad5761.o
>  obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
> diff --git a/drivers/iio/dac/ad5758.c b/drivers/iio/dac/ad5758.c
> new file mode 100644
> index 0000000..bd1bed7
> --- /dev/null
> +++ b/drivers/iio/dac/ad5758.c
> @@ -0,0 +1,899 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD5758 Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + *
> + * TODO: Currently CRC is not supported in this driver
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/property.h>
> +#include <linux/delay.h>
> +#include <linux/bsearch.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <asm/div64.h>
> +
> +/* AD5758 registers definition */
> +#define AD5758_NOP				0x00
> +#define AD5758_DAC_INPUT			0x01
> +#define AD5758_DAC_OUTPUT			0x02
> +#define AD5758_CLEAR_CODE			0x03
> +#define AD5758_USER_GAIN			0x04
> +#define AD5758_USER_OFFSET			0x05
> +#define AD5758_DAC_CONFIG			0x06
> +#define AD5758_SW_LDAC				0x07
> +#define AD5758_KEY				0x08
> +#define AD5758_GP_CONFIG1			0x09
> +#define AD5758_GP_CONFIG2			0x0A
> +#define AD5758_DCDC_CONFIG1			0x0B
> +#define AD5758_DCDC_CONFIG2			0x0C
> +#define AD5758_WDT_CONFIG			0x0F
> +#define AD5758_DIGITAL_DIAG_CONFIG		0x10
> +#define AD5758_ADC_CONFIG			0x11
> +#define AD5758_FAULT_PIN_CONFIG			0x12
> +#define AD5758_TWO_STAGE_READBACK_SELECT	0x13
> +#define AD5758_DIGITAL_DIAG_RESULTS		0x14
> +#define AD5758_ANALOG_DIAG_RESULTS		0x15
> +#define AD5758_STATUS				0x16
> +#define AD5758_CHIP_ID				0x17
> +#define AD5758_FREQ_MONITOR			0x18
> +#define AD5758_DEVICE_ID_0			0x19
> +#define AD5758_DEVICE_ID_1			0x1A
> +#define AD5758_DEVICE_ID_2			0x1B
> +#define AD5758_DEVICE_ID_3			0x1C
> +
> +/* AD5758_DAC_CONFIG */
> +#define AD5758_DAC_CONFIG_RANGE_MSK		GENMASK(3, 0)
> +#define AD5758_DAC_CONFIG_RANGE_MODE(x)		(((x) & 0xF) << 0)
> +#define AD5758_DAC_CONFIG_INT_EN_MSK		BIT(5)
> +#define AD5758_DAC_CONFIG_INT_EN_MODE(x)	(((x) & 0x1) << 5)
> +#define AD5758_DAC_CONFIG_OUT_EN_MSK		BIT(6)
> +#define AD5758_DAC_CONFIG_OUT_EN_MODE(x)	(((x) & 0x1) << 6)
> +#define AD5758_DAC_CONFIG_SR_EN_MSK		BIT(8)
> +#define AD5758_DAC_CONFIG_SR_EN_MODE(x)		(((x) & 0x1) << 8)
> +#define AD5758_DAC_CONFIG_SR_CLOCK_MSK		GENMASK(12, 9)
> +#define AD5758_DAC_CONFIG_SR_CLOCK_MODE(x)	(((x) & 0xF) << 9)
> +#define AD5758_DAC_CONFIG_SR_STEP_MSK		GENMASK(15, 13)
> +#define AD5758_DAC_CONFIG_SR_STEP_MODE(x)	(((x) & 0x7) << 13)
> +
> +/* AD5758_KEY */
> +#define AD5758_KEY_CODE_RESET_1			0x15FA
> +#define AD5758_KEY_CODE_RESET_2			0xAF51
> +#define AD5758_KEY_CODE_SINGLE_ADC_CONV		0x1ADC
> +#define AD5758_KEY_CODE_RESET_WDT		0x0D06
> +#define AD5758_KEY_CODE_CALIB_MEM_REFRESH	0xFCBA
> +
> +/* AD5758_DCDC_CONFIG1 */
> +#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MSK	GENMASK(4, 0)
> +#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MODE(x)	(((x) & 0x1F) << 0)
> +#define AD5758_DCDC_CONFIG1_DCDC_MODE_MSK	GENMASK(6, 5)
> +#define AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(x)	(((x) & 0x3) << 5)
> +#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK	BIT(7)
> +#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(x)	(((x) & 0x1) << 7)
> +
> +/* AD5758_DCDC_CONFIG2 */
> +#define AD5758_DCDC_CONFIG2_ILIMIT_MSK		GENMASK(3, 1)
> +#define AD5758_DCDC_CONFIG2_ILIMIT_MODE(x)	(((x) & 0x7) << 1)
> +#define AD5758_DCDC_CONFIG2_INTR_SAT_3WI_MSK	BIT(11)
> +#define AD5758_DCDC_CONFIG2_BUSY_3WI_MSK	BIT(12)
> +
> +/* AD5758_DIGITAL_DIAG_RESULTS */
> +#define AD5758_CAL_MEM_UNREFRESHED_MSK		BIT(15)
> +
> +#define AD5758_WR_FLAG_MSK(x)		(0x80 | ((x) & 0x1F))
> +
> +#define AD5758_FULL_SCALE_MICRO	65535000000ULL
> +
> +/**
> + * struct ad5758_state - driver instance specific data
> + * @spi:	spi_device
> + * @lock:	mutex lock
> + * @out_range:	struct which stores the output range
> + * @dc_dc_mode:	variable which stores the mode of operation
> + * @dc_dc_ilim:	variable which stores the dc-to-dc converter current limit
> + * @slew_time:	variable which stores the target slew time
> + * @pwr_down:	variable which contains whether a channel is powered down or not
> + * @data:	spi transfer buffers
> + */
> +
> +struct ad5758_range {
> +	int reg;
> +	int min;
> +	int max;
> +};
> +
> +struct ad5758_state {
> +	struct spi_device *spi;
> +	struct mutex lock;
> +	struct ad5758_range out_range;
> +	unsigned int dc_dc_mode;
> +	unsigned int dc_dc_ilim;
> +	unsigned int slew_time;
> +	bool pwr_down;
> +	__be32 d32[3];
> +};
> +
> +/**
> + * Output ranges corresponding to bits [3:0] from DAC_CONFIG register
> + * 0000: 0 V to 5 V voltage range
> + * 0001: 0 V to 10 V voltage range
> + * 0010: ±5 V voltage range
> + * 0011: ±10 V voltage range
> + * 1000: 0 mA to 20 mA current range
> + * 1001: 0 mA to 24 mA current range
> + * 1010: 4 mA to 20 mA current range
> + * 1011: ±20 mA current range
> + * 1100: ±24 mA current range
> + * 1101: -1 mA to +22 mA current range
> + */
> +enum ad5758_output_range {
> +	AD5758_RANGE_0V_5V,
> +	AD5758_RANGE_0V_10V,
> +	AD5758_RANGE_PLUSMINUS_5V,
> +	AD5758_RANGE_PLUSMINUS_10V,
> +	AD5758_RANGE_0mA_20mA = 8,
> +	AD5758_RANGE_0mA_24mA,
> +	AD5758_RANGE_4mA_24mA,
> +	AD5758_RANGE_PLUSMINUS_20mA,
> +	AD5758_RANGE_PLUSMINUS_24mA,
> +	AD5758_RANGE_MINUS_1mA_PLUS_22mA,
> +};
> +
> +enum ad5758_dc_dc_mode {
> +	AD5758_DCDC_MODE_POWER_OFF,
> +	AD5758_DCDC_MODE_DPC_CURRENT,
> +	AD5758_DCDC_MODE_DPC_VOLTAGE,
> +	AD5758_DCDC_MODE_PPC_CURRENT,
> +};
> +
> +static const struct ad5758_range ad5758_voltage_range[] = {
> +	{ AD5758_RANGE_0V_5V, 0, 5000000 },
> +	{ AD5758_RANGE_0V_10V, 0, 10000000 },
> +	{ AD5758_RANGE_PLUSMINUS_5V, -5000000, 5000000 },
> +	{ AD5758_RANGE_PLUSMINUS_10V, -10000000, 10000000 }
> +};
> +
> +static const struct ad5758_range ad5758_current_range[] = {
> +	{ AD5758_RANGE_0mA_20mA, 0, 20000},
> +	{ AD5758_RANGE_0mA_24mA, 0, 24000 },
> +	{ AD5758_RANGE_4mA_24mA, 4, 24000 },
> +	{ AD5758_RANGE_PLUSMINUS_20mA, -20000, 20000 },
> +	{ AD5758_RANGE_PLUSMINUS_24mA, -24000, 24000 },
> +	{ AD5758_RANGE_MINUS_1mA_PLUS_22mA, -1000, 22000 },
> +};
> +
> +static const int ad5758_sr_clk[16] = {
> +	240000, 200000, 150000, 128000, 64000, 32000, 16000, 8000, 4000, 2000,
> +	1000, 512, 256, 128, 64, 16
> +};
> +
> +static const int ad5758_sr_step[8] = {
> +	4, 12, 64, 120, 256, 500, 1820, 2048
> +};
> +
> +static const int ad5758_dc_dc_ilim[6] = {
> +	150000, 200000, 250000, 300000, 350000, 400000
> +};
> +
> +static int ad5758_spi_reg_read(struct ad5758_state *st, unsigned int addr)
> +{
> +	struct spi_transfer t[] = {
> +		{
> +			.tx_buf = &st->d32[0],
> +			.len = 4,
> +			.cs_change = 1,
> +		}, {
> +			.tx_buf = &st->d32[1],
> +			.rx_buf = &st->d32[2],
> +			.len = 4,
> +		},
> +	};
> +	int ret;
> +
> +	st->d32[0] = cpu_to_be32(
> +		(AD5758_WR_FLAG_MSK(AD5758_TWO_STAGE_READBACK_SELECT) << 24) |
> +		(addr << 8));
> +	st->d32[1] = cpu_to_be32(AD5758_WR_FLAG_MSK(AD5758_NOP) << 24);
> +
> +	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +	if (ret < 0)
> +		return ret;
> +
> +	return (be32_to_cpu(st->d32[2]) >> 8) & 0xFFFF;
> +}
> +
> +static int ad5758_spi_reg_write(struct ad5758_state *st,
> +				unsigned int addr,
> +				unsigned int val)
> +{
> +	st->d32[0] = cpu_to_be32((AD5758_WR_FLAG_MSK(addr) << 24) |
> +				 ((val & 0xFFFF) << 8));
> +
> +	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
> +}
> +
> +static int ad5758_spi_write_mask(struct ad5758_state *st,
> +				 unsigned int addr,
> +				 unsigned long int mask,
> +				 unsigned int val)
> +{
> +	int regval;
> +
> +	regval = ad5758_spi_reg_read(st, addr);
> +	if (regval < 0)
> +		return regval;
> +
> +	regval &= ~mask;
> +	regval |= val;
> +
> +	return ad5758_spi_reg_write(st, addr, regval);
> +}
> +
> +static int cmpfunc(const void *a, const void *b)
> +{
> +	return (*(int *)a - *(int *)b);
> +}
> +
> +static int ad5758_find_closest_match(const int *array,
> +				     unsigned int size, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +		if (val <= array[i])
> +			return i;
> +	}
> +
> +	return size - 1;
> +}
> +
> +static int ad5758_wait_for_task_complete(struct ad5758_state *st,
> +					 unsigned int reg,
> +					 unsigned int mask)
> +{
> +	unsigned int timeout;
> +	int ret;
> +
> +	timeout = 10;
> +	do {
> +		ret = ad5758_spi_reg_read(st, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (!(ret & mask))
> +			return 0;
> +
> +		udelay(100);
> +	} while (--timeout);
> +
> +	dev_err(&st->spi->dev,
> +		"Error reading bit 0x%x in 0x%x register\n", mask, reg);
> +
> +	return -EIO;
> +}
> +
> +static int ad5758_calib_mem_refresh(struct ad5758_state *st)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_reg_write(st, AD5758_KEY,
> +				   AD5758_KEY_CODE_CALIB_MEM_REFRESH);
> +	if (ret < 0) {
> +		dev_err(&st->spi->dev,
> +			"Failed to initiate a calibration memory refresh\n");
> +		return ret;
> +	}
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_soft_reset(struct ad5758_state *st)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
> +
> +	/* Perform a software reset and wait 100us */
> +	udelay(100);
> +
> +	return ret;
> +}
> +
> +static int ad5758_set_dc_dc_conv_mode(struct ad5758_state *st,
> +				      enum ad5758_dc_dc_mode mode)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +				    AD5758_DCDC_CONFIG1_DCDC_MODE_MSK,
> +				    AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(mode));
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	ret = ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					    AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +	if (ret < 0)
> +		return ret;
> +
> +	st->dc_dc_mode = mode;
> +
> +	return ret;
> +}
> +
> +static int ad5758_set_dc_dc_ilim(struct ad5758_state *st, unsigned int ilim)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG2,
> +				    AD5758_DCDC_CONFIG2_ILIMIT_MSK,
> +				    AD5758_DCDC_CONFIG2_ILIMIT_MODE(ilim));
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +}
> +
> +static int ad5758_slew_rate_set(struct ad5758_state *st,
> +				unsigned int sr_clk_idx,
> +				unsigned int sr_step_idx)
> +{
> +	unsigned int mode;
> +	unsigned long int mask;
> +	int ret;
> +
> +	mask = AD5758_DAC_CONFIG_SR_EN_MSK |
> +	       AD5758_DAC_CONFIG_SR_CLOCK_MSK |
> +	       AD5758_DAC_CONFIG_SR_STEP_MSK;
> +	mode = AD5758_DAC_CONFIG_SR_EN_MODE(1) |
> +		AD5758_DAC_CONFIG_SR_STEP_MODE(sr_step_idx) |
> +		AD5758_DAC_CONFIG_SR_CLOCK_MODE(sr_clk_idx);
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG, mask, mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_slew_rate_config(struct ad5758_state *st)
> +{
> +	unsigned int sr_clk_idx, sr_step_idx;
> +	int i, res;
> +	s64 diff_new, diff_old;
> +	u64 sr_step, calc_slew_time;
> +
> +	sr_clk_idx = 0;
> +	sr_step_idx = 0;
> +	diff_old = S64_MAX;
> +	/*
> +	 * The slew time can be determined by using the formula:
> +	 * Slew Time = (Full Scale Out / (Step Size x Update Clk Freq))
> +	 * where Slew time is expressed in microseconds
> +	 * Given the desired slew time, the following algorithm determines the
> +	 * best match for the step size and the update clock frequency.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(ad5758_sr_clk); i++) {
> +		/*
> +		 * Go through each valid update clock freq and determine a raw
> +		 * value for the step size by using the formula:
> +		 * Step Size = Full Scale Out / (Update Clk Freq * Slew Time)
> +		 */
> +		sr_step = AD5758_FULL_SCALE_MICRO;
> +		do_div(sr_step, ad5758_sr_clk[i]);
> +		do_div(sr_step, st->slew_time);
> +		/*
> +		 * After a raw value for step size was determined, find the
> +		 * closest valid match
> +		 */
> +		res = ad5758_find_closest_match(ad5758_sr_step,
> +						ARRAY_SIZE(ad5758_sr_step),
> +						sr_step);
> +		/* Calculate the slew time */
> +		calc_slew_time = AD5758_FULL_SCALE_MICRO;
> +		do_div(calc_slew_time, ad5758_sr_step[res]);
> +		do_div(calc_slew_time, ad5758_sr_clk[i]);
> +		/*
> +		 * Determine with how many microseconds the calculated slew time
> +		 * is different from the desired slew time and store the diff
> +		 * for the next iteration
> +		 */
> +		diff_new = abs(st->slew_time - calc_slew_time);
> +		if (diff_new < diff_old) {
> +			diff_old = diff_new;
> +			sr_clk_idx = i;
> +			sr_step_idx = res;
> +		}
> +	}
> +
> +	return ad5758_slew_rate_set(st, sr_clk_idx, sr_step_idx);
> +}
> +
> +static int ad5758_set_out_range(struct ad5758_state *st, int range)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    AD5758_DAC_CONFIG_RANGE_MSK,
> +				    AD5758_DAC_CONFIG_RANGE_MODE(range));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	ret =  ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +	if (ret < 0)
> +		return ret;

same as
ret = ad5758_wait_for_task_complete?  You've done that everywhere else so I
assume it is safe (I haven't checked).  Also odd spacing after the = above
so please check that throughout.

> +
> +	return ret;
> +}
> +
> +static int ad5758_fault_prot_switch_en(struct ad5758_state *st, bool enable)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +			AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK,
> +			AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(enable));
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +}
> +
> +static int ad5758_internal_buffers_en(struct ad5758_state *st, bool enable)
> +{
> +	int ret;
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    AD5758_DAC_CONFIG_INT_EN_MSK,
> +				    AD5758_DAC_CONFIG_INT_EN_MODE(enable));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_reg_access(struct iio_dev *indio_dev,
> +			     unsigned int reg,
> +			     unsigned int writeval,
> +			     unsigned int *readval)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	if (readval) {
> +		ret = ad5758_spi_reg_read(st, reg);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
> +
> +		*readval = ret;
> +		ret = 0;
> +	} else {
> +		ret = ad5758_spi_reg_write(st, reg, writeval);
> +	}
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +}
> +
> +static int ad5758_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long info)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	int max, min, ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret = ad5758_spi_reg_read(st, AD5758_DAC_INPUT);
> +		mutex_unlock(&st->lock);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		min = st->out_range.min;
> +		max = st->out_range.max;
> +		*val = (max - min) / 1000;
> +		*val2 = 16;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		min = st->out_range.min;
> +		max = st->out_range.max;
> +		*val = ((min * (1 << 16)) / (max - min)) / 1000;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5758_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long info)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret = ad5758_spi_reg_write(st, AD5758_DAC_INPUT, val);
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static ssize_t ad5758_read_powerdown(struct iio_dev *indio_dev,
> +				     uintptr_t priv,
> +				     const struct iio_chan_spec *chan,
> +				     char *buf)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", st->pwr_down);
> +}
> +
> +static ssize_t ad5758_write_powerdown(struct iio_dev *indio_dev,
> +				      uintptr_t priv,
> +				      struct iio_chan_spec const *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad5758_state *st = iio_priv(indio_dev);
> +	bool pwr_down;
> +	unsigned int dcdc_config1_mode, dc_dc_mode, dac_config_mode, val;
> +	unsigned long int dcdc_config1_msk, dac_config_msk;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&st->lock);
> +	if (pwr_down) {
> +		dc_dc_mode = AD5758_DCDC_MODE_POWER_OFF;
> +		val = 0;
> +	} else {
> +		dc_dc_mode = st->dc_dc_mode;
> +		val = 1;
> +	}
> +
> +	dcdc_config1_mode = (AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(dc_dc_mode) |
> +			     AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(val));
> +	dcdc_config1_msk = (AD5758_DCDC_CONFIG1_DCDC_MODE_MSK |
> +			    AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK);
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +				    dcdc_config1_msk,
> +				    dcdc_config1_mode);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	dac_config_mode = (AD5758_DAC_CONFIG_OUT_EN_MODE(val) |
> +			   AD5758_DAC_CONFIG_INT_EN_MODE(val));
> +	dac_config_msk =  (AD5758_DAC_CONFIG_OUT_EN_MSK |

I wouldn't have the double space after the = 
That sort things is fragile to later changes in the driver and doesn't really
make much different to readability.
There are some excess brackets in here which will get 'fixed'
by someone following the static checkers if we don't do it now.

> +			   AD5758_DAC_CONFIG_INT_EN_MSK);
> +
> +	ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    dac_config_msk,
> +				    dac_config_mode);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	st->pwr_down = pwr_down;
> +
> +err_unlock:
> +	mutex_unlock(&st->lock);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_info ad5758_info = {
> +	.read_raw = ad5758_read_raw,
> +	.write_raw = ad5758_write_raw,
> +	.debugfs_reg_access = &ad5758_reg_access,
> +};
> +
> +static const struct iio_chan_spec_ext_info ad5758_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5758_read_powerdown,
> +		.write = ad5758_write_powerdown,
> +		.shared = IIO_SHARED_BY_TYPE,
> +	},
> +	{ }
> +};
> +
> +static struct iio_chan_spec ad5758_channels[] = {

Hmm. So this is a template that is then updated.

So what happens if there are two of these devices with different
modes?  I.e. one voltage, one current?  You need be using a copy
of this, not the actual template here, which should be constant.

> +	{
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OFFSET),
> +		.indexed = 1,
> +		.output = 1,
> +		.ext_info = ad5758_ext_info,
> +	},
> +};
> +
> +static bool ad5758_is_valid_mode(enum ad5758_dc_dc_mode mode)
> +{
> +	switch (mode) {
> +	case AD5758_DCDC_MODE_DPC_CURRENT:
> +	case AD5758_DCDC_MODE_DPC_VOLTAGE:
> +	case AD5758_DCDC_MODE_PPC_CURRENT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int ad5758_crc_disable(struct ad5758_state *st)
> +{
> +	unsigned int mask;
> +
> +	mask = (AD5758_WR_FLAG_MSK(AD5758_DIGITAL_DIAG_CONFIG) << 24) | 0x5C3A;
> +	st->d32[0] = cpu_to_be32(mask);
> +
> +	return spi_write(st->spi, &st->d32[0], 4);
> +}
> +
> +static int ad5758_find_out_range(struct ad5758_state *st,
> +				 const struct ad5758_range *range,
> +				 unsigned int size,
> +				 int min, int max)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +		if ((min == range[i].min) && (max == range[i].max)) {
> +			st->out_range.reg = range[i].reg;
> +			st->out_range.min = range[i].min;
> +			st->out_range.max = range[i].max;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad5758_parse_dt(struct ad5758_state *st)
> +{
> +	unsigned int tmp, tmparray[2], size;
> +	const struct ad5758_range *range;
> +	int *index, ret;
> +
> +	st->dc_dc_ilim = 0;
> +	ret = device_property_read_u32(&st->spi->dev,
> +				       "adi,dc-dc-ilim-microamp", &tmp);
> +	if (ret) {
> +		dev_dbg(&st->spi->dev,
> +			"Missing \"dc-dc-ilim-microamp\" property\n");
> +	} else {
> +		index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> +					ARRAY_SIZE(ad5758_dc_dc_ilim),
> +					sizeof(int), cmpfunc);
> +		if (!index)
> +			dev_dbg(&st->spi->dev, "dc-dc-ilim out of range\n");
> +		else
> +			st->dc_dc_ilim = index - ad5758_dc_dc_ilim;
> +	}
> +
> +	ret = device_property_read_u32(&st->spi->dev, "adi,dc-dc-mode",
> +				       &st->dc_dc_mode);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Missing \"dc-dc-mode\" property\n");
> +		return ret;
> +	}
> +
> +	if (!ad5758_is_valid_mode(st->dc_dc_mode))
> +		return -EINVAL;
> +
> +	if (st->dc_dc_mode == AD5758_DCDC_MODE_DPC_VOLTAGE) {
> +		ret = device_property_read_u32_array(&st->spi->dev,
> +						     "adi,range-microvolt",
> +						     tmparray, 2);
> +		if (ret) {
> +			dev_err(&st->spi->dev,
> +				"Missing \"range-microvolt\" property\n");
> +			return ret;
> +		}
> +		range = ad5758_voltage_range;
> +		size = ARRAY_SIZE(ad5758_voltage_range);
> +	} else {
> +		ret = device_property_read_u32_array(&st->spi->dev,
> +						     "adi,range-microamp",
> +						     tmparray, 2);
> +		if (ret) {
> +			dev_err(&st->spi->dev,
> +				"Missing \"range-microamp\" property\n");
> +			return ret;
> +		}
> +		range = ad5758_current_range;
> +		size = ARRAY_SIZE(ad5758_current_range);
> +	}
> +
> +	ret = ad5758_find_out_range(st, range, size, tmparray[0], tmparray[1]);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "range invalid\n");
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(&st->spi->dev, "adi,slew-time-us", &tmp);
> +	if (ret) {
> +		dev_dbg(&st->spi->dev, "Missing \"slew-time-us\" property\n");
> +		st->slew_time = 0;
> +	} else {
> +		st->slew_time = tmp;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad5758_init(struct ad5758_state *st)
> +{
> +	int regval, ret;
> +
> +	/* Disable CRC checks */
> +	ret = ad5758_crc_disable(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Perform a software reset */
> +	ret = ad5758_soft_reset(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Disable CRC checks */
> +	ret = ad5758_crc_disable(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Perform a calibration memory refresh */
> +	ret = ad5758_calib_mem_refresh(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	regval = ad5758_spi_reg_read(st, AD5758_DIGITAL_DIAG_RESULTS);
> +	if (regval < 0)
> +		return regval;
> +
> +	/* Clear all the error flags */
> +	ret = ad5758_spi_reg_write(st, AD5758_DIGITAL_DIAG_RESULTS, regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set the dc-to-dc current limit */
> +	ret = ad5758_set_dc_dc_ilim(st, st->dc_dc_ilim);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Configure the dc-to-dc controller mode */
> +	ret = ad5758_set_dc_dc_conv_mode(st, st->dc_dc_mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Configure the output range */
> +	ret = ad5758_set_out_range(st, st->out_range.reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable Slew Rate Control, set the slew rate clock and step */
> +	if (st->slew_time) {
> +		ret = ad5758_slew_rate_config(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* Enable the VIOUT fault protection switch (FPS is closed) */
> +	ret = ad5758_fault_prot_switch_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Power up the DAC and internal (INT) amplifiers */
> +	ret = ad5758_internal_buffers_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable VIOUT */
> +	return ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				     AD5758_DAC_CONFIG_OUT_EN_MSK,
> +				     AD5758_DAC_CONFIG_OUT_EN_MODE(1));
> +}
> +
> +static int ad5758_probe(struct spi_device *spi)
> +{
> +	struct ad5758_state *st;
> +	struct iio_dev *indio_dev;
> +	struct iio_chan_spec *channels = ad5758_channels;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st->spi = spi;
> +
> +	mutex_init(&st->lock);
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->info = &ad5758_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->num_channels = ARRAY_SIZE(ad5758_channels);
> +
> +	ret = ad5758_parse_dt(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (st->dc_dc_mode == AD5758_DCDC_MODE_DPC_VOLTAGE)
> +		channels->type = IIO_VOLTAGE;
> +	else
> +		channels->type = IIO_CURRENT;
> +
> +	indio_dev->channels = channels;
> +
> +	ret = ad5758_init(st);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "AD5758 init failed\n");
> +		return ret;
> +	}
> +
> +	return devm_iio_device_register(&st->spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id ad5758_id[] = {
> +	{ "ad5758", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5758_id);
> +
> +static struct spi_driver ad5758_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +	},
> +	.probe = ad5758_probe,
> +	.id_table = ad5758_id,
> +};
> +
> +module_spi_driver(ad5758_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5758 DAC");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
@ 2018-06-30 15:49   ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-06-30 15:49 UTC (permalink / raw)
  To: Stefan Popa
  Cc: Michael.Hennerich, lars, knaack.h, pmeerw, mchehab, davem,
	gregkh, akpm, linus.walleij, rdunlap, lukas, Ismail.Kose,
	vilhelm.gray, sean.nyekjaer, pombredanne, linux-iio,
	linux-kernel

On Fri, 29 Jun 2018 11:38:36 +0300
Stefan Popa <stefan.popa@analog.com> wrote:

> The AD5758 is a single channel DAC with 16-bit precision which uses the
> SPI interface that operates at clock rates up to 50MHz.
>=20
> The output can be configured as voltage or current and is available on a
> single terminal.
>=20
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758=
.pdf
>=20
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>

Hi Stefan,

A few minors + the way you currently handle the choice of voltage vs
current prevents you having multiple instances of this device with
different settings.   I would pick between between two constant
options of the whole structure.  Alternative would be to make a fresh
copy of the structure for each instance of the device, but that is
fiddlier for such a simple case.

Jonathan

> ---
> Changes in v4:
> 	- fixed kbuild test robot warnings.
> Changes in v3:
> 	- AD5758 can be both a current and voltage output DAC. The
> 	  decision is made based on the DT and the channel type is set
> 	  during probe.
> 	- dc-dc-mode, range-microvolt and range-microamp are required
> 	  properties.
> 	- Introduced a slew-time-us property from which slew rate clock
> 	  and slew rate step are calculated using a best match algorithm.
> 	- Dropped the union from ad5758_state struct.
> 	- Introduced a IIO_CHAN_INFO_OFFSET case part of ad5758_read_raw().
> 	- Added a TODO comment which specifies that CRC is not supported.
> 	- Kept the includes in order and removed the unused ones.
> 	- Removed unused macros and shortened the lengthy ones.
> 	- Renamed AD5758_REG_WRITE to AD5758_WR_FLAG_MSK.
> 	- Added an explanation for enum ad5758_output_range.
> 	- Used bsearch() instead of ad5758_get_array_index().
> 	- Reduced the delays.
> 	- strtobool() -> kstrtobool().
>=20
> Changes in v2:
> 	- removed unnecessary parenthesis in AD5758_REG_WRITE macro.
> 	- added missing documentation fields of ad5758_state struct.
> 	- changed the type of pwr_down attribute to bool.
> 	- changed ad5758_dc_dc_ilimt[] to ad5758_dc_dc_ilim[].
> 	- ad5758_spi_reg_write() now returns spi_write(st->spi,
> 	  &st->data[0].d32, sizeof(st->data[0].d32));
> 	- removed unnecessary new line in ad5758_calib_mem_refresh().
> 	- changed the type of the mode parameter in
> 	  ad5758_set_dc_dc_conv_mode() from unsigned int to enum
> 	  ad5758_dc_dc_mode.
> 	- removed unnecessary parenthesis in ad5758_slew_rate_config().
> 	- changed the type of the enable parameter in
> 	  ad5758_fault_prot_switch_en() from unsigned char to bool.
> 	- the same as above, but for ad5758_internal_buffers_en().
> 	- added a missing mutex_unlock() in ad5758_reg_access().
> 	- moved the mutex_unlock() in ad5758_read_raw() and removed the
> 	  unreachable return.
> 	- returned directly where it was possible in ad5758_write_raw().
> 	- removed the channel, scan_type and scan_index fields.
> 	- in ad5758_parse_dt(), added missing "\n", and specified what the
> 	  default mode actually is.
> 	- returned directly at the end of ad5758_init().
> 	- in ad5758_probe() used device managed for registering the device
> 	  and returned directly without the error message.
>=20
>  MAINTAINERS              |   7 +
>  drivers/iio/dac/Kconfig  |  10 +
>  drivers/iio/dac/Makefile |   1 +
>  drivers/iio/dac/ad5758.c | 899 +++++++++++++++++++++++++++++++++++++++++=
++++++
>  4 files changed, 917 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5758.c
>=20
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00e9670..12d102d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -796,6 +796,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
>  S:	Supported
>  F:	drivers/macintosh/ams/
> =20
> +ANALOG DEVICES INC AD5758 DRIVER
> +M:	Stefan Popa <stefan.popa@analog.com>
> +L:	linux-iio@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +S:	Supported
> +F:	drivers/iio/dac/ad5758.c
> +
>  ANALOG DEVICES INC AD5686 DRIVER
>  M:	Stefan Popa <stefan.popa@analog.com>
>  L:	linux-pm@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 06e90de..80beb64 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -167,6 +167,16 @@ config AD5755
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5755.
> =20
> +config AD5758
> +	tristate "Analog Devices AD5758 DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD5758 single channel
> +	  Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5758.
> +
>  config AD5761
>  	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
>  	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 57aa230..a1b37cf 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_AD5592R_BASE) +=3D ad5592r-base.o
>  obj-$(CONFIG_AD5592R) +=3D ad5592r.o
>  obj-$(CONFIG_AD5593R) +=3D ad5593r.o
>  obj-$(CONFIG_AD5755) +=3D ad5755.o
> +obj-$(CONFIG_AD5755) +=3D ad5758.o
>  obj-$(CONFIG_AD5761) +=3D ad5761.o
>  obj-$(CONFIG_AD5764) +=3D ad5764.o
>  obj-$(CONFIG_AD5791) +=3D ad5791.o
> diff --git a/drivers/iio/dac/ad5758.c b/drivers/iio/dac/ad5758.c
> new file mode 100644
> index 0000000..bd1bed7
> --- /dev/null
> +++ b/drivers/iio/dac/ad5758.c
> @@ -0,0 +1,899 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD5758 Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + *
> + * TODO: Currently CRC is not supported in this driver
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/property.h>
> +#include <linux/delay.h>
> +#include <linux/bsearch.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <asm/div64.h>
> +
> +/* AD5758 registers definition */
> +#define AD5758_NOP				0x00
> +#define AD5758_DAC_INPUT			0x01
> +#define AD5758_DAC_OUTPUT			0x02
> +#define AD5758_CLEAR_CODE			0x03
> +#define AD5758_USER_GAIN			0x04
> +#define AD5758_USER_OFFSET			0x05
> +#define AD5758_DAC_CONFIG			0x06
> +#define AD5758_SW_LDAC				0x07
> +#define AD5758_KEY				0x08
> +#define AD5758_GP_CONFIG1			0x09
> +#define AD5758_GP_CONFIG2			0x0A
> +#define AD5758_DCDC_CONFIG1			0x0B
> +#define AD5758_DCDC_CONFIG2			0x0C
> +#define AD5758_WDT_CONFIG			0x0F
> +#define AD5758_DIGITAL_DIAG_CONFIG		0x10
> +#define AD5758_ADC_CONFIG			0x11
> +#define AD5758_FAULT_PIN_CONFIG			0x12
> +#define AD5758_TWO_STAGE_READBACK_SELECT	0x13
> +#define AD5758_DIGITAL_DIAG_RESULTS		0x14
> +#define AD5758_ANALOG_DIAG_RESULTS		0x15
> +#define AD5758_STATUS				0x16
> +#define AD5758_CHIP_ID				0x17
> +#define AD5758_FREQ_MONITOR			0x18
> +#define AD5758_DEVICE_ID_0			0x19
> +#define AD5758_DEVICE_ID_1			0x1A
> +#define AD5758_DEVICE_ID_2			0x1B
> +#define AD5758_DEVICE_ID_3			0x1C
> +
> +/* AD5758_DAC_CONFIG */
> +#define AD5758_DAC_CONFIG_RANGE_MSK		GENMASK(3, 0)
> +#define AD5758_DAC_CONFIG_RANGE_MODE(x)		(((x) & 0xF) << 0)
> +#define AD5758_DAC_CONFIG_INT_EN_MSK		BIT(5)
> +#define AD5758_DAC_CONFIG_INT_EN_MODE(x)	(((x) & 0x1) << 5)
> +#define AD5758_DAC_CONFIG_OUT_EN_MSK		BIT(6)
> +#define AD5758_DAC_CONFIG_OUT_EN_MODE(x)	(((x) & 0x1) << 6)
> +#define AD5758_DAC_CONFIG_SR_EN_MSK		BIT(8)
> +#define AD5758_DAC_CONFIG_SR_EN_MODE(x)		(((x) & 0x1) << 8)
> +#define AD5758_DAC_CONFIG_SR_CLOCK_MSK		GENMASK(12, 9)
> +#define AD5758_DAC_CONFIG_SR_CLOCK_MODE(x)	(((x) & 0xF) << 9)
> +#define AD5758_DAC_CONFIG_SR_STEP_MSK		GENMASK(15, 13)
> +#define AD5758_DAC_CONFIG_SR_STEP_MODE(x)	(((x) & 0x7) << 13)
> +
> +/* AD5758_KEY */
> +#define AD5758_KEY_CODE_RESET_1			0x15FA
> +#define AD5758_KEY_CODE_RESET_2			0xAF51
> +#define AD5758_KEY_CODE_SINGLE_ADC_CONV		0x1ADC
> +#define AD5758_KEY_CODE_RESET_WDT		0x0D06
> +#define AD5758_KEY_CODE_CALIB_MEM_REFRESH	0xFCBA
> +
> +/* AD5758_DCDC_CONFIG1 */
> +#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MSK	GENMASK(4, 0)
> +#define AD5758_DCDC_CONFIG1_DCDC_VPROG_MODE(x)	(((x) & 0x1F) << 0)
> +#define AD5758_DCDC_CONFIG1_DCDC_MODE_MSK	GENMASK(6, 5)
> +#define AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(x)	(((x) & 0x3) << 5)
> +#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK	BIT(7)
> +#define AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(x)	(((x) & 0x1) << 7)
> +
> +/* AD5758_DCDC_CONFIG2 */
> +#define AD5758_DCDC_CONFIG2_ILIMIT_MSK		GENMASK(3, 1)
> +#define AD5758_DCDC_CONFIG2_ILIMIT_MODE(x)	(((x) & 0x7) << 1)
> +#define AD5758_DCDC_CONFIG2_INTR_SAT_3WI_MSK	BIT(11)
> +#define AD5758_DCDC_CONFIG2_BUSY_3WI_MSK	BIT(12)
> +
> +/* AD5758_DIGITAL_DIAG_RESULTS */
> +#define AD5758_CAL_MEM_UNREFRESHED_MSK		BIT(15)
> +
> +#define AD5758_WR_FLAG_MSK(x)		(0x80 | ((x) & 0x1F))
> +
> +#define AD5758_FULL_SCALE_MICRO	65535000000ULL
> +
> +/**
> + * struct ad5758_state - driver instance specific data
> + * @spi:	spi_device
> + * @lock:	mutex lock
> + * @out_range:	struct which stores the output range
> + * @dc_dc_mode:	variable which stores the mode of operation
> + * @dc_dc_ilim:	variable which stores the dc-to-dc converter current lim=
it
> + * @slew_time:	variable which stores the target slew time
> + * @pwr_down:	variable which contains whether a channel is powered down =
or not
> + * @data:	spi transfer buffers
> + */
> +
> +struct ad5758_range {
> +	int reg;
> +	int min;
> +	int max;
> +};
> +
> +struct ad5758_state {
> +	struct spi_device *spi;
> +	struct mutex lock;
> +	struct ad5758_range out_range;
> +	unsigned int dc_dc_mode;
> +	unsigned int dc_dc_ilim;
> +	unsigned int slew_time;
> +	bool pwr_down;
> +	__be32 d32[3];
> +};
> +
> +/**
> + * Output ranges corresponding to bits [3:0] from DAC_CONFIG register
> + * 0000: 0 V to 5 V voltage range
> + * 0001: 0 V to 10 V voltage range
> + * 0010: =C2=B15 V voltage range
> + * 0011: =C2=B110 V voltage range
> + * 1000: 0 mA to 20 mA current range
> + * 1001: 0 mA to 24 mA current range
> + * 1010: 4 mA to 20 mA current range
> + * 1011: =C2=B120 mA current range
> + * 1100: =C2=B124 mA current range
> + * 1101: -1 mA to +22 mA current range
> + */
> +enum ad5758_output_range {
> +	AD5758_RANGE_0V_5V,
> +	AD5758_RANGE_0V_10V,
> +	AD5758_RANGE_PLUSMINUS_5V,
> +	AD5758_RANGE_PLUSMINUS_10V,
> +	AD5758_RANGE_0mA_20mA =3D 8,
> +	AD5758_RANGE_0mA_24mA,
> +	AD5758_RANGE_4mA_24mA,
> +	AD5758_RANGE_PLUSMINUS_20mA,
> +	AD5758_RANGE_PLUSMINUS_24mA,
> +	AD5758_RANGE_MINUS_1mA_PLUS_22mA,
> +};
> +
> +enum ad5758_dc_dc_mode {
> +	AD5758_DCDC_MODE_POWER_OFF,
> +	AD5758_DCDC_MODE_DPC_CURRENT,
> +	AD5758_DCDC_MODE_DPC_VOLTAGE,
> +	AD5758_DCDC_MODE_PPC_CURRENT,
> +};
> +
> +static const struct ad5758_range ad5758_voltage_range[] =3D {
> +	{ AD5758_RANGE_0V_5V, 0, 5000000 },
> +	{ AD5758_RANGE_0V_10V, 0, 10000000 },
> +	{ AD5758_RANGE_PLUSMINUS_5V, -5000000, 5000000 },
> +	{ AD5758_RANGE_PLUSMINUS_10V, -10000000, 10000000 }
> +};
> +
> +static const struct ad5758_range ad5758_current_range[] =3D {
> +	{ AD5758_RANGE_0mA_20mA, 0, 20000},
> +	{ AD5758_RANGE_0mA_24mA, 0, 24000 },
> +	{ AD5758_RANGE_4mA_24mA, 4, 24000 },
> +	{ AD5758_RANGE_PLUSMINUS_20mA, -20000, 20000 },
> +	{ AD5758_RANGE_PLUSMINUS_24mA, -24000, 24000 },
> +	{ AD5758_RANGE_MINUS_1mA_PLUS_22mA, -1000, 22000 },
> +};
> +
> +static const int ad5758_sr_clk[16] =3D {
> +	240000, 200000, 150000, 128000, 64000, 32000, 16000, 8000, 4000, 2000,
> +	1000, 512, 256, 128, 64, 16
> +};
> +
> +static const int ad5758_sr_step[8] =3D {
> +	4, 12, 64, 120, 256, 500, 1820, 2048
> +};
> +
> +static const int ad5758_dc_dc_ilim[6] =3D {
> +	150000, 200000, 250000, 300000, 350000, 400000
> +};
> +
> +static int ad5758_spi_reg_read(struct ad5758_state *st, unsigned int add=
r)
> +{
> +	struct spi_transfer t[] =3D {
> +		{
> +			.tx_buf =3D &st->d32[0],
> +			.len =3D 4,
> +			.cs_change =3D 1,
> +		}, {
> +			.tx_buf =3D &st->d32[1],
> +			.rx_buf =3D &st->d32[2],
> +			.len =3D 4,
> +		},
> +	};
> +	int ret;
> +
> +	st->d32[0] =3D cpu_to_be32(
> +		(AD5758_WR_FLAG_MSK(AD5758_TWO_STAGE_READBACK_SELECT) << 24) |
> +		(addr << 8));
> +	st->d32[1] =3D cpu_to_be32(AD5758_WR_FLAG_MSK(AD5758_NOP) << 24);
> +
> +	ret =3D spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +	if (ret < 0)
> +		return ret;
> +
> +	return (be32_to_cpu(st->d32[2]) >> 8) & 0xFFFF;
> +}
> +
> +static int ad5758_spi_reg_write(struct ad5758_state *st,
> +				unsigned int addr,
> +				unsigned int val)
> +{
> +	st->d32[0] =3D cpu_to_be32((AD5758_WR_FLAG_MSK(addr) << 24) |
> +				 ((val & 0xFFFF) << 8));
> +
> +	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
> +}
> +
> +static int ad5758_spi_write_mask(struct ad5758_state *st,
> +				 unsigned int addr,
> +				 unsigned long int mask,
> +				 unsigned int val)
> +{
> +	int regval;
> +
> +	regval =3D ad5758_spi_reg_read(st, addr);
> +	if (regval < 0)
> +		return regval;
> +
> +	regval &=3D ~mask;
> +	regval |=3D val;
> +
> +	return ad5758_spi_reg_write(st, addr, regval);
> +}
> +
> +static int cmpfunc(const void *a, const void *b)
> +{
> +	return (*(int *)a - *(int *)b);
> +}
> +
> +static int ad5758_find_closest_match(const int *array,
> +				     unsigned int size, int val)
> +{
> +	int i;
> +
> +	for (i =3D 0; i < size; i++) {
> +		if (val <=3D array[i])
> +			return i;
> +	}
> +
> +	return size - 1;
> +}
> +
> +static int ad5758_wait_for_task_complete(struct ad5758_state *st,
> +					 unsigned int reg,
> +					 unsigned int mask)
> +{
> +	unsigned int timeout;
> +	int ret;
> +
> +	timeout =3D 10;
> +	do {
> +		ret =3D ad5758_spi_reg_read(st, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (!(ret & mask))
> +			return 0;
> +
> +		udelay(100);
> +	} while (--timeout);
> +
> +	dev_err(&st->spi->dev,
> +		"Error reading bit 0x%x in 0x%x register\n", mask, reg);
> +
> +	return -EIO;
> +}
> +
> +static int ad5758_calib_mem_refresh(struct ad5758_state *st)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_reg_write(st, AD5758_KEY,
> +				   AD5758_KEY_CODE_CALIB_MEM_REFRESH);
> +	if (ret < 0) {
> +		dev_err(&st->spi->dev,
> +			"Failed to initiate a calibration memory refresh\n");
> +		return ret;
> +	}
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_soft_reset(struct ad5758_state *st)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret =3D ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
> +
> +	/* Perform a software reset and wait 100us */
> +	udelay(100);
> +
> +	return ret;
> +}
> +
> +static int ad5758_set_dc_dc_conv_mode(struct ad5758_state *st,
> +				      enum ad5758_dc_dc_mode mode)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +				    AD5758_DCDC_CONFIG1_DCDC_MODE_MSK,
> +				    AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(mode));
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	ret =3D ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					    AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +	if (ret < 0)
> +		return ret;
> +
> +	st->dc_dc_mode =3D mode;
> +
> +	return ret;
> +}
> +
> +static int ad5758_set_dc_dc_ilim(struct ad5758_state *st, unsigned int i=
lim)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG2,
> +				    AD5758_DCDC_CONFIG2_ILIMIT_MSK,
> +				    AD5758_DCDC_CONFIG2_ILIMIT_MODE(ilim));
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +}
> +
> +static int ad5758_slew_rate_set(struct ad5758_state *st,
> +				unsigned int sr_clk_idx,
> +				unsigned int sr_step_idx)
> +{
> +	unsigned int mode;
> +	unsigned long int mask;
> +	int ret;
> +
> +	mask =3D AD5758_DAC_CONFIG_SR_EN_MSK |
> +	       AD5758_DAC_CONFIG_SR_CLOCK_MSK |
> +	       AD5758_DAC_CONFIG_SR_STEP_MSK;
> +	mode =3D AD5758_DAC_CONFIG_SR_EN_MODE(1) |
> +		AD5758_DAC_CONFIG_SR_STEP_MODE(sr_step_idx) |
> +		AD5758_DAC_CONFIG_SR_CLOCK_MODE(sr_clk_idx);
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DAC_CONFIG, mask, mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_slew_rate_config(struct ad5758_state *st)
> +{
> +	unsigned int sr_clk_idx, sr_step_idx;
> +	int i, res;
> +	s64 diff_new, diff_old;
> +	u64 sr_step, calc_slew_time;
> +
> +	sr_clk_idx =3D 0;
> +	sr_step_idx =3D 0;
> +	diff_old =3D S64_MAX;
> +	/*
> +	 * The slew time can be determined by using the formula:
> +	 * Slew Time =3D (Full Scale Out / (Step Size x Update Clk Freq))
> +	 * where Slew time is expressed in microseconds
> +	 * Given the desired slew time, the following algorithm determines the
> +	 * best match for the step size and the update clock frequency.
> +	 */
> +	for (i =3D 0; i < ARRAY_SIZE(ad5758_sr_clk); i++) {
> +		/*
> +		 * Go through each valid update clock freq and determine a raw
> +		 * value for the step size by using the formula:
> +		 * Step Size =3D Full Scale Out / (Update Clk Freq * Slew Time)
> +		 */
> +		sr_step =3D AD5758_FULL_SCALE_MICRO;
> +		do_div(sr_step, ad5758_sr_clk[i]);
> +		do_div(sr_step, st->slew_time);
> +		/*
> +		 * After a raw value for step size was determined, find the
> +		 * closest valid match
> +		 */
> +		res =3D ad5758_find_closest_match(ad5758_sr_step,
> +						ARRAY_SIZE(ad5758_sr_step),
> +						sr_step);
> +		/* Calculate the slew time */
> +		calc_slew_time =3D AD5758_FULL_SCALE_MICRO;
> +		do_div(calc_slew_time, ad5758_sr_step[res]);
> +		do_div(calc_slew_time, ad5758_sr_clk[i]);
> +		/*
> +		 * Determine with how many microseconds the calculated slew time
> +		 * is different from the desired slew time and store the diff
> +		 * for the next iteration
> +		 */
> +		diff_new =3D abs(st->slew_time - calc_slew_time);
> +		if (diff_new < diff_old) {
> +			diff_old =3D diff_new;
> +			sr_clk_idx =3D i;
> +			sr_step_idx =3D res;
> +		}
> +	}
> +
> +	return ad5758_slew_rate_set(st, sr_clk_idx, sr_step_idx);
> +}
> +
> +static int ad5758_set_out_range(struct ad5758_state *st, int range)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    AD5758_DAC_CONFIG_RANGE_MSK,
> +				    AD5758_DAC_CONFIG_RANGE_MODE(range));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	ret =3D  ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +	if (ret < 0)
> +		return ret;

same as
ret =3D ad5758_wait_for_task_complete?  You've done that everywhere else so=
 I
assume it is safe (I haven't checked).  Also odd spacing after the =3D above
so please check that throughout.

> +
> +	return ret;
> +}
> +
> +static int ad5758_fault_prot_switch_en(struct ad5758_state *st, bool ena=
ble)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +			AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK,
> +			AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(enable));
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
> +	 * This allows the 3-wire interface communication to complete.
> +	 */
> +	return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
> +					     AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
> +}
> +
> +static int ad5758_internal_buffers_en(struct ad5758_state *st, bool enab=
le)
> +{
> +	int ret;
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    AD5758_DAC_CONFIG_INT_EN_MSK,
> +				    AD5758_DAC_CONFIG_INT_EN_MODE(enable));
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait to allow time for the internal calibrations to complete */
> +	return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
> +					     AD5758_CAL_MEM_UNREFRESHED_MSK);
> +}
> +
> +static int ad5758_reg_access(struct iio_dev *indio_dev,
> +			     unsigned int reg,
> +			     unsigned int writeval,
> +			     unsigned int *readval)
> +{
> +	struct ad5758_state *st =3D iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	if (readval) {
> +		ret =3D ad5758_spi_reg_read(st, reg);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
> +
> +		*readval =3D ret;
> +		ret =3D 0;
> +	} else {
> +		ret =3D ad5758_spi_reg_write(st, reg, writeval);
> +	}
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +}
> +
> +static int ad5758_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long info)
> +{
> +	struct ad5758_state *st =3D iio_priv(indio_dev);
> +	int max, min, ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret =3D ad5758_spi_reg_read(st, AD5758_DAC_INPUT);
> +		mutex_unlock(&st->lock);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val =3D ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		min =3D st->out_range.min;
> +		max =3D st->out_range.max;
> +		*val =3D (max - min) / 1000;
> +		*val2 =3D 16;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		min =3D st->out_range.min;
> +		max =3D st->out_range.max;
> +		*val =3D ((min * (1 << 16)) / (max - min)) / 1000;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5758_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long info)
> +{
> +	struct ad5758_state *st =3D iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret =3D ad5758_spi_reg_write(st, AD5758_DAC_INPUT, val);
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static ssize_t ad5758_read_powerdown(struct iio_dev *indio_dev,
> +				     uintptr_t priv,
> +				     const struct iio_chan_spec *chan,
> +				     char *buf)
> +{
> +	struct ad5758_state *st =3D iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", st->pwr_down);
> +}
> +
> +static ssize_t ad5758_write_powerdown(struct iio_dev *indio_dev,
> +				      uintptr_t priv,
> +				      struct iio_chan_spec const *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad5758_state *st =3D iio_priv(indio_dev);
> +	bool pwr_down;
> +	unsigned int dcdc_config1_mode, dc_dc_mode, dac_config_mode, val;
> +	unsigned long int dcdc_config1_msk, dac_config_msk;
> +	int ret;
> +
> +	ret =3D kstrtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&st->lock);
> +	if (pwr_down) {
> +		dc_dc_mode =3D AD5758_DCDC_MODE_POWER_OFF;
> +		val =3D 0;
> +	} else {
> +		dc_dc_mode =3D st->dc_dc_mode;
> +		val =3D 1;
> +	}
> +
> +	dcdc_config1_mode =3D (AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(dc_dc_mode) |
> +			     AD5758_DCDC_CONFIG1_PROT_SW_EN_MODE(val));
> +	dcdc_config1_msk =3D (AD5758_DCDC_CONFIG1_DCDC_MODE_MSK |
> +			    AD5758_DCDC_CONFIG1_PROT_SW_EN_MSK);
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
> +				    dcdc_config1_msk,
> +				    dcdc_config1_mode);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	dac_config_mode =3D (AD5758_DAC_CONFIG_OUT_EN_MODE(val) |
> +			   AD5758_DAC_CONFIG_INT_EN_MODE(val));
> +	dac_config_msk =3D  (AD5758_DAC_CONFIG_OUT_EN_MSK |

I wouldn't have the double space after the =3D=20
That sort things is fragile to later changes in the driver and doesn't real=
ly
make much different to readability.
There are some excess brackets in here which will get 'fixed'
by someone following the static checkers if we don't do it now.

> +			   AD5758_DAC_CONFIG_INT_EN_MSK);
> +
> +	ret =3D ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				    dac_config_msk,
> +				    dac_config_mode);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	st->pwr_down =3D pwr_down;
> +
> +err_unlock:
> +	mutex_unlock(&st->lock);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_info ad5758_info =3D {
> +	.read_raw =3D ad5758_read_raw,
> +	.write_raw =3D ad5758_write_raw,
> +	.debugfs_reg_access =3D &ad5758_reg_access,
> +};
> +
> +static const struct iio_chan_spec_ext_info ad5758_ext_info[] =3D {
> +	{
> +		.name =3D "powerdown",
> +		.read =3D ad5758_read_powerdown,
> +		.write =3D ad5758_write_powerdown,
> +		.shared =3D IIO_SHARED_BY_TYPE,
> +	},
> +	{ }
> +};
> +
> +static struct iio_chan_spec ad5758_channels[] =3D {

Hmm. So this is a template that is then updated.

So what happens if there are two of these devices with different
modes?  I.e. one voltage, one current?  You need be using a copy
of this, not the actual template here, which should be constant.

> +	{
> +		.info_mask_shared_by_type =3D BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OFFSET),
> +		.indexed =3D 1,
> +		.output =3D 1,
> +		.ext_info =3D ad5758_ext_info,
> +	},
> +};
> +
> +static bool ad5758_is_valid_mode(enum ad5758_dc_dc_mode mode)
> +{
> +	switch (mode) {
> +	case AD5758_DCDC_MODE_DPC_CURRENT:
> +	case AD5758_DCDC_MODE_DPC_VOLTAGE:
> +	case AD5758_DCDC_MODE_PPC_CURRENT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int ad5758_crc_disable(struct ad5758_state *st)
> +{
> +	unsigned int mask;
> +
> +	mask =3D (AD5758_WR_FLAG_MSK(AD5758_DIGITAL_DIAG_CONFIG) << 24) | 0x5C3=
A;
> +	st->d32[0] =3D cpu_to_be32(mask);
> +
> +	return spi_write(st->spi, &st->d32[0], 4);
> +}
> +
> +static int ad5758_find_out_range(struct ad5758_state *st,
> +				 const struct ad5758_range *range,
> +				 unsigned int size,
> +				 int min, int max)
> +{
> +	int i;
> +
> +	for (i =3D 0; i < size; i++) {
> +		if ((min =3D=3D range[i].min) && (max =3D=3D range[i].max)) {
> +			st->out_range.reg =3D range[i].reg;
> +			st->out_range.min =3D range[i].min;
> +			st->out_range.max =3D range[i].max;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad5758_parse_dt(struct ad5758_state *st)
> +{
> +	unsigned int tmp, tmparray[2], size;
> +	const struct ad5758_range *range;
> +	int *index, ret;
> +
> +	st->dc_dc_ilim =3D 0;
> +	ret =3D device_property_read_u32(&st->spi->dev,
> +				       "adi,dc-dc-ilim-microamp", &tmp);
> +	if (ret) {
> +		dev_dbg(&st->spi->dev,
> +			"Missing \"dc-dc-ilim-microamp\" property\n");
> +	} else {
> +		index =3D (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> +					ARRAY_SIZE(ad5758_dc_dc_ilim),
> +					sizeof(int), cmpfunc);
> +		if (!index)
> +			dev_dbg(&st->spi->dev, "dc-dc-ilim out of range\n");
> +		else
> +			st->dc_dc_ilim =3D index - ad5758_dc_dc_ilim;
> +	}
> +
> +	ret =3D device_property_read_u32(&st->spi->dev, "adi,dc-dc-mode",
> +				       &st->dc_dc_mode);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Missing \"dc-dc-mode\" property\n");
> +		return ret;
> +	}
> +
> +	if (!ad5758_is_valid_mode(st->dc_dc_mode))
> +		return -EINVAL;
> +
> +	if (st->dc_dc_mode =3D=3D AD5758_DCDC_MODE_DPC_VOLTAGE) {
> +		ret =3D device_property_read_u32_array(&st->spi->dev,
> +						     "adi,range-microvolt",
> +						     tmparray, 2);
> +		if (ret) {
> +			dev_err(&st->spi->dev,
> +				"Missing \"range-microvolt\" property\n");
> +			return ret;
> +		}
> +		range =3D ad5758_voltage_range;
> +		size =3D ARRAY_SIZE(ad5758_voltage_range);
> +	} else {
> +		ret =3D device_property_read_u32_array(&st->spi->dev,
> +						     "adi,range-microamp",
> +						     tmparray, 2);
> +		if (ret) {
> +			dev_err(&st->spi->dev,
> +				"Missing \"range-microamp\" property\n");
> +			return ret;
> +		}
> +		range =3D ad5758_current_range;
> +		size =3D ARRAY_SIZE(ad5758_current_range);
> +	}
> +
> +	ret =3D ad5758_find_out_range(st, range, size, tmparray[0], tmparray[1]=
);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "range invalid\n");
> +		return ret;
> +	}
> +
> +	ret =3D device_property_read_u32(&st->spi->dev, "adi,slew-time-us", &tm=
p);
> +	if (ret) {
> +		dev_dbg(&st->spi->dev, "Missing \"slew-time-us\" property\n");
> +		st->slew_time =3D 0;
> +	} else {
> +		st->slew_time =3D tmp;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad5758_init(struct ad5758_state *st)
> +{
> +	int regval, ret;
> +
> +	/* Disable CRC checks */
> +	ret =3D ad5758_crc_disable(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Perform a software reset */
> +	ret =3D ad5758_soft_reset(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Disable CRC checks */
> +	ret =3D ad5758_crc_disable(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Perform a calibration memory refresh */
> +	ret =3D ad5758_calib_mem_refresh(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	regval =3D ad5758_spi_reg_read(st, AD5758_DIGITAL_DIAG_RESULTS);
> +	if (regval < 0)
> +		return regval;
> +
> +	/* Clear all the error flags */
> +	ret =3D ad5758_spi_reg_write(st, AD5758_DIGITAL_DIAG_RESULTS, regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set the dc-to-dc current limit */
> +	ret =3D ad5758_set_dc_dc_ilim(st, st->dc_dc_ilim);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Configure the dc-to-dc controller mode */
> +	ret =3D ad5758_set_dc_dc_conv_mode(st, st->dc_dc_mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Configure the output range */
> +	ret =3D ad5758_set_out_range(st, st->out_range.reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable Slew Rate Control, set the slew rate clock and step */
> +	if (st->slew_time) {
> +		ret =3D ad5758_slew_rate_config(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* Enable the VIOUT fault protection switch (FPS is closed) */
> +	ret =3D ad5758_fault_prot_switch_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Power up the DAC and internal (INT) amplifiers */
> +	ret =3D ad5758_internal_buffers_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable VIOUT */
> +	return ad5758_spi_write_mask(st, AD5758_DAC_CONFIG,
> +				     AD5758_DAC_CONFIG_OUT_EN_MSK,
> +				     AD5758_DAC_CONFIG_OUT_EN_MODE(1));
> +}
> +
> +static int ad5758_probe(struct spi_device *spi)
> +{
> +	struct ad5758_state *st;
> +	struct iio_dev *indio_dev;
> +	struct iio_chan_spec *channels =3D ad5758_channels;
> +	int ret;
> +
> +	indio_dev =3D devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st =3D iio_priv(indio_dev);
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st->spi =3D spi;
> +
> +	mutex_init(&st->lock);
> +
> +	indio_dev->dev.parent =3D &spi->dev;
> +	indio_dev->name =3D spi_get_device_id(spi)->name;
> +	indio_dev->info =3D &ad5758_info;
> +	indio_dev->modes =3D INDIO_DIRECT_MODE;
> +	indio_dev->num_channels =3D ARRAY_SIZE(ad5758_channels);
> +
> +	ret =3D ad5758_parse_dt(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (st->dc_dc_mode =3D=3D AD5758_DCDC_MODE_DPC_VOLTAGE)
> +		channels->type =3D IIO_VOLTAGE;
> +	else
> +		channels->type =3D IIO_CURRENT;
> +
> +	indio_dev->channels =3D channels;
> +
> +	ret =3D ad5758_init(st);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "AD5758 init failed\n");
> +		return ret;
> +	}
> +
> +	return devm_iio_device_register(&st->spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id ad5758_id[] =3D {
> +	{ "ad5758", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5758_id);
> +
> +static struct spi_driver ad5758_driver =3D {
> +	.driver =3D {
> +		.name =3D KBUILD_MODNAME,
> +	},
> +	.probe =3D ad5758_probe,
> +	.id_table =3D ad5758_id,
> +};
> +
> +module_spi_driver(ad5758_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5758 DAC");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
  2018-06-29  8:38 [PATCH v4 1/2] iio: dac: Add AD5758 support Stefan Popa
  2018-06-29  8:55 ` Sean Nyekjaer
  2018-06-30 15:49   ` Jonathan Cameron
@ 2018-06-30 21:26 ` Andy Shevchenko
  2018-07-04 13:26     ` Popa, Stefan Serban
  2 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2018-06-30 21:26 UTC (permalink / raw)
  To: Stefan Popa
  Cc: Jonathan Cameron, Michael Hennerich, Lars-Peter Clausen,
	Hartmut Knaack, Peter Meerwald, Mauro Carvalho Chehab,
	David S. Miller, Greg Kroah-Hartman, Andrew Morton,
	Linus Walleij, Randy Dunlap, Lukas Wunner, Ismail.Kose,
	William Breathitt Gray, sean.nyekjaer, Philippe Ombredanne,
	linux-iio, Linux Kernel Mailing List

On Fri, Jun 29, 2018 at 11:38 AM, Stefan Popa <stefan.popa@analog.com> wrote:
> The AD5758 is a single channel DAC with 16-bit precision which uses the
> SPI interface that operates at clock rates up to 50MHz.
>
> The output can be configured as voltage or current and is available on a
> single terminal.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758.pdf

Thanks for an update.
Few comments below.

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/property.h>
> +#include <linux/delay.h>
> +#include <linux/bsearch.h>

Perhaps keep them ordered?

> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +

> +#include <asm/div64.h>

ASM? Hmm...

> +static int cmpfunc(const void *a, const void *b)
> +{
> +       return (*(int *)a - *(int *)b);

Surrounding parens are not needed.

> +}
> +

> +static int ad5758_find_closest_match(const int *array,
> +                                    unsigned int size, int val)
> +{
> +       int i;
> +
> +       for (i = 0; i < size; i++) {
> +               if (val <= array[i])
> +                       return i;
> +       }
> +
> +       return size - 1;
> +}

Isn't it what bsearch() covers as well?

> +       do {
> +               ret = ad5758_spi_reg_read(st, reg);
> +               if (ret < 0)
> +                       return ret;
> +
> +               if (!(ret & mask))
> +                       return 0;
> +

> +               udelay(100);

If it's not called from atomic context, perhaps switch to usleep_range() ?

> +       } while (--timeout);


> +static int ad5758_soft_reset(struct ad5758_state *st)
> +{
> +       int ret;
> +
> +       ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
> +

> +       /* Perform a software reset and wait 100us */
> +       udelay(100);

Ditto.

> +
> +       return ret;
> +}

> +static int ad5758_find_out_range(struct ad5758_state *st,
> +                                const struct ad5758_range *range,
> +                                unsigned int size,
> +                                int min, int max)
> +{
> +       int i;
> +
> +       for (i = 0; i < size; i++) {
> +               if ((min == range[i].min) && (max == range[i].max)) {
> +                       st->out_range.reg = range[i].reg;
> +                       st->out_range.min = range[i].min;
> +                       st->out_range.max = range[i].max;
> +
> +                       return 0;
> +               }
> +       }

One more candidate to use bsearch().

> +
> +       return -EINVAL;
> +}

> +               index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> +                                       ARRAY_SIZE(ad5758_dc_dc_ilim),
> +                                       sizeof(int), cmpfunc);

I'm not sure you need that casting.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
  2018-06-30 21:26 ` Andy Shevchenko
@ 2018-07-04 13:26     ` Popa, Stefan Serban
  0 siblings, 0 replies; 7+ messages in thread
From: Popa, Stefan Serban @ 2018-07-04 13:26 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: vilhelm.gray, lars, linux-kernel, knaack.h, rdunlap, jic23,
	Hennerich, Michael, linus.walleij, linux-iio, mchehab,
	Ismail.Kose, akpm, lukas, pmeerw, sean.nyekjaer, gregkh,
	pombredanne, davem

On Du, 2018-07-01 at 00:26 +0300, Andy Shevchenko wrote:
> On Fri, Jun 29, 2018 at 11:38 AM, Stefan Popa <stefan.popa@analog.com> wrote:
> > 
> > The AD5758 is a single channel DAC with 16-bit precision which uses the
> > SPI interface that operates at clock rates up to 50MHz.
> > 
> > The output can be configured as voltage or current and is available on a
> > single terminal.
> > 
> > Datasheet:
> > http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758.pdf
> Thanks for an update.
> Few comments below.
> 
> > 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> Perhaps keep them ordered?
> 
> > 
> > +
> > +#include 
> > +#include 
> > +
> > 
> > +#include 
> ASM? Hmm...
> 
> > 
> > +static int cmpfunc(const void *a, const void *b)
> > +{
> > +       return (*(int *)a - *(int *)b);
> Surrounding parens are not needed.
> 
> > 
> > +}
> > +
> > 
> > +static int ad5758_find_closest_match(const int *array,
> > +                                    unsigned int size, int val)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < size; i++) {
> > +               if (val <= array[i])
> > +                       return i;
> > +       }
> > +
> > +       return size - 1;
> > +}
> Isn't it what bsearch() covers as well?
> 

bsearch() looks for an item in an array and returns NULL if it is not found.
This function returns the index of the closest value to val even if there is 
no exact match.

> > 
> > +       do {
> > +               ret = ad5758_spi_reg_read(st, reg);
> > +               if (ret < 0)
> > +                       return ret;
> > +
> > +               if (!(ret & mask))
> > +                       return 0;
> > +
> > 
> > +               udelay(100);
> If it's not called from atomic context, perhaps switch to usleep_range() ?
> 
> > 
> > +       } while (--timeout);
> 
> > 
> > +static int ad5758_soft_reset(struct ad5758_state *st)
> > +{
> > +       int ret;
> > +
> > +       ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
> > +
> > 
> > +       /* Perform a software reset and wait 100us */
> > +       udelay(100);
> Ditto.
> 
> > 
> > +
> > +       return ret;
> > +}
> > 
> > +static int ad5758_find_out_range(struct ad5758_state *st,
> > +                                const struct ad5758_range *range,
> > +                                unsigned int size,
> > +                                int min, int max)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < size; i++) {
> > +               if ((min == range[i].min) && (max == range[i].max)) {
> > +                       st->out_range.reg = range[i].reg;
> > +                       st->out_range.min = range[i].min;
> > +                       st->out_range.max = range[i].max;
> > +
> > +                       return 0;
> > +               }
> > +       }
> One more candidate to use bsearch().
> 

I am not sure if bsearch() will work in this case since it requires that the given 
array is sorted. Moreover, both ad5758_voltage_range[] and ad5758_current_range[] 
contain duplicate elements.

> > 
> > +
> > +       return -EINVAL;
> > +}
> > 
> > +               index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> > +                                       ARRAY_SIZE(ad5758_dc_dc_ilim),
> > +                                       sizeof(int), cmpfunc);
> I'm not sure you need that casting.
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
@ 2018-07-04 13:26     ` Popa, Stefan Serban
  0 siblings, 0 replies; 7+ messages in thread
From: Popa, Stefan Serban @ 2018-07-04 13:26 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: vilhelm.gray, lars, linux-kernel, knaack.h, rdunlap, jic23,
	Hennerich, Michael, linus.walleij, linux-iio, mchehab,
	Ismail.Kose, akpm, lukas, pmeerw, sean.nyekjaer, gregkh,
	pombredanne, davem

T24gRHUsIDIwMTgtMDctMDEgYXQgMDA6MjYgKzAzMDAsIEFuZHkgU2hldmNoZW5rbyB3cm90ZToN
Cj4gT24gRnJpLCBKdW4gMjksIDIwMTggYXQgMTE6MzggQU0sIFN0ZWZhbiBQb3BhIDxzdGVmYW4u
cG9wYUBhbmFsb2cuY29tPiB3cm90ZToNCj4gPiANCj4gPiBUaGUgQUQ1NzU4IGlzIGEgc2luZ2xl
IGNoYW5uZWwgREFDIHdpdGggMTYtYml0IHByZWNpc2lvbiB3aGljaCB1c2VzIHRoZQ0KPiA+IFNQ
SSBpbnRlcmZhY2UgdGhhdCBvcGVyYXRlcyBhdCBjbG9jayByYXRlcyB1cCB0byA1ME1Iei4NCj4g
PiANCj4gPiBUaGUgb3V0cHV0IGNhbiBiZSBjb25maWd1cmVkIGFzIHZvbHRhZ2Ugb3IgY3VycmVu
dCBhbmQgaXMgYXZhaWxhYmxlIG9uIGENCj4gPiBzaW5nbGUgdGVybWluYWwuDQo+ID4gDQo+ID4g
RGF0YXNoZWV0Og0KPiA+IGh0dHA6Ly93d3cuYW5hbG9nLmNvbS9tZWRpYS9lbi90ZWNobmljYWwt
ZG9jdW1lbnRhdGlvbi9kYXRhLXNoZWV0cy9hZDU3NTgucGRmDQo+IFRoYW5rcyBmb3IgYW4gdXBk
YXRlLg0KPiBGZXcgY29tbWVudHMgYmVsb3cuDQo+IA0KPiA+IA0KPiA+ICsjaW5jbHVkZSANCj4g
PiArI2luY2x1ZGUgDQo+ID4gKyNpbmNsdWRlIA0KPiA+ICsjaW5jbHVkZSANCj4gPiArI2luY2x1
ZGUgDQo+ID4gKyNpbmNsdWRlIA0KPiBQZXJoYXBzIGtlZXAgdGhlbSBvcmRlcmVkPw0KPiANCj4g
PiANCj4gPiArDQo+ID4gKyNpbmNsdWRlIA0KPiA+ICsjaW5jbHVkZSANCj4gPiArDQo+ID4gDQo+
ID4gKyNpbmNsdWRlIA0KPiBBU00/IEhtbS4uLg0KPiANCj4gPiANCj4gPiArc3RhdGljIGludCBj
bXBmdW5jKGNvbnN0IHZvaWQgKmEsIGNvbnN0IHZvaWQgKmIpDQo+ID4gK3sNCj4gPiArwqDCoMKg
wqDCoMKgwqByZXR1cm4gKCooaW50ICopYSAtICooaW50ICopYik7DQo+IFN1cnJvdW5kaW5nIHBh
cmVucyBhcmUgbm90IG5lZWRlZC4NCj4gDQo+ID4gDQo+ID4gK30NCj4gPiArDQo+ID4gDQo+ID4g
K3N0YXRpYyBpbnQgYWQ1NzU4X2ZpbmRfY2xvc2VzdF9tYXRjaChjb25zdCBpbnQgKmFycmF5LA0K
PiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqB1bnNpZ25lZCBpbnQgc2l6ZSwgaW50IHZhbCkNCj4gPiArew0K
PiA+ICvCoMKgwqDCoMKgwqDCoGludCBpOw0KPiA+ICsNCj4gPiArwqDCoMKgwqDCoMKgwqBmb3Ig
KGkgPSAwOyBpIDwgc2l6ZTsgaSsrKSB7DQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoGlmICh2YWwgPD0gYXJyYXlbaV0pDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqByZXR1cm4gaTsNCj4gPiArwqDCoMKgwqDCoMKgwqB9DQo+ID4g
Kw0KPiA+ICvCoMKgwqDCoMKgwqDCoHJldHVybiBzaXplIC0gMTsNCj4gPiArfQ0KPiBJc24ndCBp
dCB3aGF0IGJzZWFyY2goKSBjb3ZlcnMgYXMgd2VsbD8NCj4gDQoNCmJzZWFyY2goKSBsb29rcyBm
b3IgYW4gaXRlbSBpbiBhbiBhcnJheSBhbmQgcmV0dXJucyBOVUxMIGlmIGl0IGlzIG5vdCBmb3Vu
ZC4NClRoaXMgZnVuY3Rpb24gcmV0dXJucyB0aGUgaW5kZXggb2YgdGhlIGNsb3Nlc3QgdmFsdWUg
dG8gdmFsIGV2ZW4gaWYgdGhlcmUgaXPCoA0Kbm8gZXhhY3QgbWF0Y2guDQoNCj4gPiANCj4gPiAr
wqDCoMKgwqDCoMKgwqBkbyB7DQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJl
dCA9IGFkNTc1OF9zcGlfcmVnX3JlYWQoc3QsIHJlZyk7DQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoGlmIChyZXQgPCAwKQ0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgcmV0dXJuIHJldDsNCj4gPiArDQo+ID4gK8KgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoGlmICghKHJldCAmIG1hc2spKQ0KPiA+ICvCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcmV0dXJuIDA7DQo+ID4gKw0KPiA+IA0K
PiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB1ZGVsYXkoMTAwKTsNCj4gSWYgaXQn
cyBub3QgY2FsbGVkIGZyb20gYXRvbWljIGNvbnRleHQsIHBlcmhhcHMgc3dpdGNoIHRvIHVzbGVl
cF9yYW5nZSgpID8NCj4gDQo+ID4gDQo+ID4gK8KgwqDCoMKgwqDCoMKgfSB3aGlsZSAoLS10aW1l
b3V0KTsNCj4gDQo+ID4gDQo+ID4gK3N0YXRpYyBpbnQgYWQ1NzU4X3NvZnRfcmVzZXQoc3RydWN0
IGFkNTc1OF9zdGF0ZSAqc3QpDQo+ID4gK3sNCj4gPiArwqDCoMKgwqDCoMKgwqBpbnQgcmV0Ow0K
PiA+ICsNCj4gPiArwqDCoMKgwqDCoMKgwqByZXQgPSBhZDU3NThfc3BpX3JlZ193cml0ZShzdCwg
QUQ1NzU4X0tFWSwgQUQ1NzU4X0tFWV9DT0RFX1JFU0VUXzEpOw0KPiA+ICvCoMKgwqDCoMKgwqDC
oGlmIChyZXQgPCAwKQ0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqByZXR1cm4g
cmV0Ow0KPiA+ICsNCj4gPiArwqDCoMKgwqDCoMKgwqByZXQgPSBhZDU3NThfc3BpX3JlZ193cml0
ZShzdCwgQUQ1NzU4X0tFWSwgQUQ1NzU4X0tFWV9DT0RFX1JFU0VUXzIpOw0KPiA+ICsNCj4gPiAN
Cj4gPiArwqDCoMKgwqDCoMKgwqAvKiBQZXJmb3JtIGEgc29mdHdhcmUgcmVzZXQgYW5kIHdhaXQg
MTAwdXMgKi8NCj4gPiArwqDCoMKgwqDCoMKgwqB1ZGVsYXkoMTAwKTsNCj4gRGl0dG8uDQo+IA0K
PiA+IA0KPiA+ICsNCj4gPiArwqDCoMKgwqDCoMKgwqByZXR1cm4gcmV0Ow0KPiA+ICt9DQo+ID4g
DQo+ID4gK3N0YXRpYyBpbnQgYWQ1NzU4X2ZpbmRfb3V0X3JhbmdlKHN0cnVjdCBhZDU3NThfc3Rh
dGUgKnN0LA0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgY29uc3Qgc3RydWN0IGFkNTc1OF9yYW5nZSAqcmFuZ2UsDQo+
ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqB1bnNpZ25lZCBpbnQgc2l6ZSwNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGludCBtaW4sIGludCBt
YXgpDQo+ID4gK3sNCj4gPiArwqDCoMKgwqDCoMKgwqBpbnQgaTsNCj4gPiArDQo+ID4gK8KgwqDC
oMKgwqDCoMKgZm9yIChpID0gMDsgaSA8IHNpemU7IGkrKykgew0KPiA+ICvCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqBpZiAoKG1pbiA9PSByYW5nZVtpXS5taW4pICYmIChtYXggPT0gcmFu
Z2VbaV0ubWF4KSkgew0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgc3QtPm91dF9yYW5nZS5yZWcgPSByYW5nZVtpXS5yZWc7DQo+ID4gK8KgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBzdC0+b3V0X3JhbmdlLm1pbiA9
IHJhbmdlW2ldLm1pbjsNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoHN0LT5vdXRfcmFuZ2UubWF4ID0gcmFuZ2VbaV0ubWF4Ow0KPiA+ICsNCj4gPiAr
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJldHVybiAwOw0K
PiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB9DQo+ID4gK8KgwqDCoMKgwqDCoMKg
fQ0KPiBPbmUgbW9yZSBjYW5kaWRhdGUgdG8gdXNlIGJzZWFyY2goKS4NCj4gDQoNCkkgYW0gbm90
IHN1cmUgaWYgYnNlYXJjaCgpIHdpbGwgd29yayBpbiB0aGlzIGNhc2Ugc2luY2UgaXQgcmVxdWly
ZXMgdGhhdCB0aGUgZ2l2ZW7CoA0KYXJyYXkgaXMgc29ydGVkLiBNb3Jlb3ZlciwgYm90aMKgYWQ1
NzU4X3ZvbHRhZ2VfcmFuZ2VbXSBhbmTCoGFkNTc1OF9jdXJyZW50X3JhbmdlW13CoA0KY29udGFp
biBkdXBsaWNhdGUgZWxlbWVudHMuDQoNCj4gPiANCj4gPiArDQo+ID4gK8KgwqDCoMKgwqDCoMKg
cmV0dXJuIC1FSU5WQUw7DQo+ID4gK30NCj4gPiANCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgaW5kZXggPSAoaW50ICopIGJzZWFyY2goJnRtcCwgYWQ1NzU4X2RjX2RjX2lsaW0s
DQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoEFSUkFZX1NJWkUoYWQ1NzU4X2RjX2RjX2lsaW0p
LA0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBzaXplb2YoaW50KSwgY21wZnVuYyk7DQo+IEkn
bSBub3Qgc3VyZSB5b3UgbmVlZCB0aGF0IGNhc3RpbmcuDQo+IA==

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-07-04 13:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-29  8:38 [PATCH v4 1/2] iio: dac: Add AD5758 support Stefan Popa
2018-06-29  8:55 ` Sean Nyekjaer
2018-06-30 15:49 ` Jonathan Cameron
2018-06-30 15:49   ` Jonathan Cameron
2018-06-30 21:26 ` Andy Shevchenko
2018-07-04 13:26   ` Popa, Stefan Serban
2018-07-04 13:26     ` Popa, Stefan Serban

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.