2018-06-28 15:48:18

by Stefan Popa

[permalink] [raw]
Subject: [PATCH v3 1/2] iio: dac: Add AD5758 support

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 <[email protected]>
---
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 <[email protected]>
S: Supported
F: drivers/macintosh/ams/

+ANALOG DEVICES INC AD5758 DRIVER
+M: Stefan Popa <[email protected]>
+L: [email protected]
+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 <[email protected]>
L: [email protected]
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..2aaddf9
--- /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 65535000000
+
+/**
+ * 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);
+}
+
+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 <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD5758 DAC");
+MODULE_LICENSE("GPL v2");
--
2.7.4



2018-06-28 19:06:56

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] iio: dac: Add AD5758 support

Hi Stefan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on v4.18-rc2 next-20180628]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Stefan-Popa/iio-dac-Add-AD5758-support/20180628-205028
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/iio/dac/ad5758.c:402:27: sparse: constant 65535000000 is so big it is long
drivers/iio/dac/ad5758.c:413:34: sparse: constant 65535000000 is so big it is long
>> drivers/iio/dac/ad5758.c:237:5: sparse: symbol 'cmpfunc' was not declared. Should it be static?

Please review and possibly fold the followup patch.

vim +402 drivers/iio/dac/ad5758.c

236
> 237 int cmpfunc(const void *a, const void *b)
238 {
239 return (*(int *)a - *(int *)b);
240 }
241
242 static int ad5758_find_closest_match(const int *array,
243 unsigned int size, int val)
244 {
245 int i;
246
247 for (i = 0; i < size; i++) {
248 if (val <= array[i])
249 return i;
250 }
251
252 return size - 1;
253 }
254
255 static int ad5758_wait_for_task_complete(struct ad5758_state *st,
256 unsigned int reg,
257 unsigned int mask)
258 {
259 unsigned int timeout;
260 int ret;
261
262 timeout = 10;
263 do {
264 ret = ad5758_spi_reg_read(st, reg);
265 if (ret < 0)
266 return ret;
267
268 if (!(ret & mask))
269 return 0;
270
271 udelay(100);
272 } while (--timeout);
273
274 dev_err(&st->spi->dev,
275 "Error reading bit 0x%x in 0x%x register\n", mask, reg);
276
277 return -EIO;
278 }
279
280 static int ad5758_calib_mem_refresh(struct ad5758_state *st)
281 {
282 int ret;
283
284 ret = ad5758_spi_reg_write(st, AD5758_KEY,
285 AD5758_KEY_CODE_CALIB_MEM_REFRESH);
286 if (ret < 0) {
287 dev_err(&st->spi->dev,
288 "Failed to initiate a calibration memory refresh\n");
289 return ret;
290 }
291
292 /* Wait to allow time for the internal calibrations to complete */
293 return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
294 AD5758_CAL_MEM_UNREFRESHED_MSK);
295 }
296
297 static int ad5758_soft_reset(struct ad5758_state *st)
298 {
299 int ret;
300
301 ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
302 if (ret < 0)
303 return ret;
304
305 ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
306
307 /* Perform a software reset and wait 100us */
308 udelay(100);
309
310 return ret;
311 }
312
313 static int ad5758_set_dc_dc_conv_mode(struct ad5758_state *st,
314 enum ad5758_dc_dc_mode mode)
315 {
316 int ret;
317
318 ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG1,
319 AD5758_DCDC_CONFIG1_DCDC_MODE_MSK,
320 AD5758_DCDC_CONFIG1_DCDC_MODE_MODE(mode));
321 if (ret < 0)
322 return ret;
323
324 /*
325 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
326 * This allows the 3-wire interface communication to complete.
327 */
328 ret = ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
329 AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
330 if (ret < 0)
331 return ret;
332
333 st->dc_dc_mode = mode;
334
335 return ret;
336 }
337
338 static int ad5758_set_dc_dc_ilim(struct ad5758_state *st, unsigned int ilim)
339 {
340 int ret;
341
342 ret = ad5758_spi_write_mask(st, AD5758_DCDC_CONFIG2,
343 AD5758_DCDC_CONFIG2_ILIMIT_MSK,
344 AD5758_DCDC_CONFIG2_ILIMIT_MODE(ilim));
345 if (ret < 0)
346 return ret;
347 /*
348 * Poll the BUSY_3WI bit in the DCDC_CONFIG2 register until it is 0.
349 * This allows the 3-wire interface communication to complete.
350 */
351 return ad5758_wait_for_task_complete(st, AD5758_DCDC_CONFIG2,
352 AD5758_DCDC_CONFIG2_BUSY_3WI_MSK);
353 }
354
355 static int ad5758_slew_rate_set(struct ad5758_state *st,
356 unsigned int sr_clk_idx,
357 unsigned int sr_step_idx)
358 {
359 unsigned int mode;
360 unsigned long int mask;
361 int ret;
362
363 mask = AD5758_DAC_CONFIG_SR_EN_MSK |
364 AD5758_DAC_CONFIG_SR_CLOCK_MSK |
365 AD5758_DAC_CONFIG_SR_STEP_MSK;
366 mode = AD5758_DAC_CONFIG_SR_EN_MODE(1) |
367 AD5758_DAC_CONFIG_SR_STEP_MODE(sr_step_idx) |
368 AD5758_DAC_CONFIG_SR_CLOCK_MODE(sr_clk_idx);
369
370 ret = ad5758_spi_write_mask(st, AD5758_DAC_CONFIG, mask, mode);
371 if (ret < 0)
372 return ret;
373
374 /* Wait to allow time for the internal calibrations to complete */
375 return ad5758_wait_for_task_complete(st, AD5758_DIGITAL_DIAG_RESULTS,
376 AD5758_CAL_MEM_UNREFRESHED_MSK);
377 }
378
379 static int ad5758_slew_rate_config(struct ad5758_state *st)
380 {
381 unsigned int sr_clk_idx, sr_step_idx;
382 int i, res;
383 s64 diff_new, diff_old;
384 u64 sr_step, calc_slew_time;
385
386 sr_clk_idx = 0;
387 sr_step_idx = 0;
388 diff_old = S64_MAX;
389 /*
390 * The slew time can be determined by using the formula:
391 * Slew Time = (Full Scale Out / (Step Size x Update Clk Freq))
392 * where Slew time is expressed in microseconds
393 * Given the desired slew time, the following algorithm determines the
394 * best match for the step size and the update clock frequency.
395 */
396 for (i = 0; i < ARRAY_SIZE(ad5758_sr_clk); i++) {
397 /*
398 * Go through each valid update clock freq and determine a raw
399 * value for the step size by using the formula:
400 * Step Size = Full Scale Out / (Update Clk Freq * Slew Time)
401 */
> 402 sr_step = AD5758_FULL_SCALE_MICRO;
403 do_div(sr_step, ad5758_sr_clk[i]);
404 do_div(sr_step, st->slew_time);
405 /*
406 * After a raw value for step size was determined, find the
407 * closest valid match
408 */
409 res = ad5758_find_closest_match(ad5758_sr_step,
410 ARRAY_SIZE(ad5758_sr_step),
411 sr_step);
412 /* Calculate the slew time */
413 calc_slew_time = AD5758_FULL_SCALE_MICRO;
414 do_div(calc_slew_time, ad5758_sr_step[res]);
415 do_div(calc_slew_time, ad5758_sr_clk[i]);
416 /*
417 * Determine with how many microseconds the calculated slew time
418 * is different from the desired slew time and store the diff
419 * for the next iteration
420 */
421 diff_new = abs(st->slew_time - calc_slew_time);
422 if (diff_new < diff_old) {
423 diff_old = diff_new;
424 sr_clk_idx = i;
425 sr_step_idx = res;
426 }
427 }
428
429 return ad5758_slew_rate_set(st, sr_clk_idx, sr_step_idx);
430 }
431

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

2018-06-28 19:07:47

by Fengguang Wu

[permalink] [raw]
Subject: [RFC PATCH] iio: dac: cmpfunc() can be static


Fixes: 2c1562568d29 ("iio: dac: Add AD5758 support")
Signed-off-by: kbuild test robot <[email protected]>
---
ad5758.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5758.c b/drivers/iio/dac/ad5758.c
index 2aaddf9..55c0a61 100644
--- a/drivers/iio/dac/ad5758.c
+++ b/drivers/iio/dac/ad5758.c
@@ -234,7 +234,7 @@ static int ad5758_spi_write_mask(struct ad5758_state *st,
return ad5758_spi_reg_write(st, addr, regval);
}

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

2018-06-29 06:22:34

by Himanshu Jha

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] iio: dac: Add AD5758 support

On Thu, Jun 28, 2018 at 03:13:32PM +0300, 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 <[email protected]>
> ---
[..]
> +static const int ad5758_dc_dc_ilim[6] = {
> + 150000, 200000, 250000, 300000, 350000, 400000
> +};
[..]
> +int cmpfunc(const void *a, const void *b)
> +{
> + return (*(int *)a - *(int *)b);
> +}

Since Kbuild hit a Sparse error in this function, I would also like to
add that the above implementation of cmpfunc() is not safe.

For eg: https://wandbox.org/permlink/MqGnA3nVg7eAN4I7

The following will be safer:

static int cmpfunc(const void *a, const void *b)
{
int arg1 = *(const int*)a;
int arg2 = *(const int*)b;
return (arg1 > arg2) - (arg1 < arg2);
}

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

Thanks.
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

2018-06-29 08:58:00

by Himanshu Jha

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] iio: dac: Add AD5758 support

On Fri, Jun 29, 2018 at 03:18:18AM +0530, Himanshu Jha wrote:
> On Thu, Jun 28, 2018 at 03:13:32PM +0300, 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 <[email protected]>
> > ---
> [..]
> > +static const int ad5758_dc_dc_ilim[6] = {
> > + 150000, 200000, 250000, 300000, 350000, 400000
> > +};
> [..]
> > +int cmpfunc(const void *a, const void *b)
> > +{
> > + return (*(int *)a - *(int *)b);
> > +}
>
> Since Kbuild hit a Sparse error in this function, I would also like to
> add that the above implementation of cmpfunc() is not safe.
>
> For eg: https://wandbox.org/permlink/MqGnA3nVg7eAN4I7
>
> The following will be safer:
>
> static int cmpfunc(const void *a, const void *b)
> {
> int arg1 = *(const int*)a;
> int arg2 = *(const int*)b;
> return (arg1 > arg2) - (arg1 < arg2);
> }
>
> [..]
> > + index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> > + ARRAY_SIZE(ad5758_dc_dc_ilim),
> > + sizeof(int), cmpfunc);

I somehow realized that my argument is useless since we would not have
large integer subtraction, which could lead to signed integer
overflow(UB) and hence cause problems(as evident in the eg link I posted).

Sorry Stefan :(

--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology