diff mbox series

[v4,6/6] drivers:iio:dac: Add AD3552R driver support

Message ID 20210820165927.4524-7-mihail.chindris@analog.com (mailing list archive)
State Changes Requested
Headers show
Series iio: Add output buffer support and DAC example | expand

Commit Message

Chindris, Mihail Aug. 20, 2021, 4:59 p.m. UTC
The AD3552R-16 is a low drift ultrafast, 16-bit accuracy,
current output digital-to-analog converter (DAC) designed
to generate multiple output voltage span ranges.
The AD3552R-16 operates with a fixed 2.5V reference.

analog.com/media/en/technical-documentation/data-sheets/ad3552r.pdf

Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
---
 drivers/iio/dac/Kconfig   |   10 +
 drivers/iio/dac/Makefile  |    1 +
 drivers/iio/dac/ad3552r.c | 1419 +++++++++++++++++++++++++++++++++++++
 3 files changed, 1430 insertions(+)
 create mode 100644 drivers/iio/dac/ad3552r.c

Comments

kernel test robot Aug. 20, 2021, 7:55 p.m. UTC | #1
Hi Mihail,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 94a853eca720ac9e385e59f27e859b4a01123f58]

url:    https://github.com/0day-ci/linux/commits/Mihail-Chindris/iio-Add-output-buffer-support-and-DAC-example/20210821-010349
base:   94a853eca720ac9e385e59f27e859b4a01123f58
config: s390-buildonly-randconfig-r006-20210821 (attached as .config)
compiler: s390-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/56f19f129ca383448b35a3c77ac5fcc1eb4df3b4
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mihail-Chindris/iio-Add-output-buffer-support-and-DAC-example/20210821-010349
        git checkout 56f19f129ca383448b35a3c77ac5fcc1eb4df3b4
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash drivers/iio/dac/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/dac/ad3552r.c:17:10: fatal error: linux/unaligned/be_byteshift.h: No such file or directory
      17 | #include <linux/unaligned/be_byteshift.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.


vim +17 drivers/iio/dac/ad3552r.c

     2	
     3	/*
     4	 * Analog Devices AD3552R
     5	 * Digital to Analog converter driver
     6	 *
     7	 * Copyright 2021 Analog Devices Inc.
     8	 */
     9	#include <linux/iopoll.h>
    10	#include <linux/device.h>
    11	#include <linux/iio/trigger.h>
    12	#include <linux/iio/triggered_buffer.h>
    13	#include <linux/iio/trigger_consumer.h>
    14	#include <linux/spi/spi.h>
    15	#include <linux/types.h>
    16	#include <linux/time64.h>
  > 17	#include <linux/unaligned/be_byteshift.h>
    18	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Aug. 20, 2021, 10:15 p.m. UTC | #2
Hi Mihail,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 94a853eca720ac9e385e59f27e859b4a01123f58]

url:    https://github.com/0day-ci/linux/commits/Mihail-Chindris/iio-Add-output-buffer-support-and-DAC-example/20210821-010349
base:   94a853eca720ac9e385e59f27e859b4a01123f58
config: hexagon-randconfig-r026-20210821 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d9c5613e856cf2addfbf892fc4c1ce9ef9feceaa)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/56f19f129ca383448b35a3c77ac5fcc1eb4df3b4
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mihail-Chindris/iio-Add-output-buffer-support-and-DAC-example/20210821-010349
        git checkout 56f19f129ca383448b35a3c77ac5fcc1eb4df3b4
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/iio/dac/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/dac/ad3552r.c:17:10: fatal error: 'linux/unaligned/be_byteshift.h' file not found
   #include <linux/unaligned/be_byteshift.h>
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +17 drivers/iio/dac/ad3552r.c

     2	
     3	/*
     4	 * Analog Devices AD3552R
     5	 * Digital to Analog converter driver
     6	 *
     7	 * Copyright 2021 Analog Devices Inc.
     8	 */
     9	#include <linux/iopoll.h>
    10	#include <linux/device.h>
    11	#include <linux/iio/trigger.h>
    12	#include <linux/iio/triggered_buffer.h>
    13	#include <linux/iio/trigger_consumer.h>
    14	#include <linux/spi/spi.h>
    15	#include <linux/types.h>
    16	#include <linux/time64.h>
  > 17	#include <linux/unaligned/be_byteshift.h>
    18	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Nuno Sá Aug. 23, 2021, 2:01 p.m. UTC | #3
On Fri, 2021-08-20 at 16:59 +0000, Mihail Chindris wrote:
> The AD3552R-16 is a low drift ultrafast, 16-bit accuracy,
> current output digital-to-analog converter (DAC) designed
> to generate multiple output voltage span ranges.
> The AD3552R-16 operates with a fixed 2.5V reference.
> 
> analog.com/media/en/technical-documentation/data-sheets/ad3552r.pdf
> 
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> ---
>  drivers/iio/dac/Kconfig   |   10 +
>  drivers/iio/dac/Makefile  |    1 +
>  drivers/iio/dac/ad3552r.c | 1419
> +++++++++++++++++++++++++++++++++++++
>  3 files changed, 1430 insertions(+)
>  create mode 100644 drivers/iio/dac/ad3552r.c
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 75e1f2b48638..ced6428f2c92 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -6,6 +6,16 @@
>  
>  menu "Digital to analog converters"
>  
> +config AD3552R
> +	tristate "Analog Devices AD3552R DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD3552R
> +	  Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad3552r.
> +
>  config AD5064
>  	tristate "Analog Devices AD5064 and similar multi-channel DAC
> driver"
>  	depends on (SPI_MASTER && I2C!=m) || I2C
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 33e16f14902a..dffe36efd8ff 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_AD3552R) += ad3552r.o
>  obj-$(CONFIG_AD5360) += ad5360.o
>  obj-$(CONFIG_AD5380) += ad5380.o
>  obj-$(CONFIG_AD5421) += ad5421.o
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> new file mode 100644
> index 000000000000..89993dd87522
> --- /dev/null
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -0,0 +1,1419 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Analog Devices AD3552R
> + * Digital to Analog converter driver
> + *
> + * Copyright 2021 Analog Devices Inc.
> + */
> +#include <linux/iopoll.h>
> +#include <linux/device.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/time64.h>
> +#include <linux/unaligned/be_byteshift.h>
> +
> +/* Register addresses */
> +/* Primary address space */
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_A		0x00
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_B		0x01
> +#define AD3552R_REG_ADDR_DEVICE_CONFIG			0x02
> +#define AD3552R_REG_ADDR_CHIP_TYPE			0x03
> +#define AD3552R_REG_ADDR_PRODUCT_ID_L			0x04
> +#define AD3552R_REG_ADDR_PRODUCT_ID_H			0x05
> +#define AD3552R_REG_ADDR_CHIP_GRADE			0x06
> +#define AD3552R_REG_ADDR_SCRATCH_PAD			0x0A
> +#define AD3552R_REG_ADDR_SPI_REVISION			0x0B
> +#define AD3552R_REG_ADDR_VENDOR_L			0x0C
> +#define AD3552R_REG_ADDR_VENDOR_H			0x0D
> +#define AD3552R_REG_ADDR_STREAM_MODE			0x0E
> +#define AD3552R_REG_ADDR_TRANSFER_REGISTER		0x0F
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C		0x10
> +#define AD3552R_REG_ADDR_INTERFACE_STATUS_A		0x11
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_D		0x14
> +#define AD3552R_REG_ADDR_SH_REFERENCE_CONFIG		0x15
> +#define AD3552R_REG_ADDR_ERR_ALARM_MASK			0x16
> +#define AD3552R_REG_ADDR_ERR_STATUS			0x17
> +#define AD3552R_REG_ADDR_POWERDOWN_CONFIG		0x18
> +#define AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE		0x19
> +#define AD3552R_REG_ADDR_CH_OFFSET(ch)			(0x1B +
> (ch) * 2)
> +#define AD3552R_REG_ADDR_CH_GAIN(ch)			(0x1C + (ch) *
> 2)
> +/*
> + * Secondary region
> + * For multibyte registers specify the highest address because the
> access is
> + * done in descending order
> + */
> +#define AD3552R_SECONDARY_REGION_START			0x28
> +#define AD3552R_REG_ADDR_HW_LDAC_16B			0x28
> +#define AD3552R_REG_ADDR_CH_DAC_16B(ch)			(0x2C -
> (1 - ch) * 2)
> +#define AD3552R_REG_ADDR_DAC_PAGE_MASK_16B		0x2E
> +#define AD3552R_REG_ADDR_CH_SELECT_16B			0x2F
> +#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B		0x31
> +#define AD3552R_REG_ADDR_SW_LDAC_16B			0x32
> +#define AD3552R_REG_ADDR_CH_INPUT_16B(ch)		(0x36 - (1 -
> ch) * 2)
> +/* 3 bytes registers */
> +#define AD3552R_REG_START_24B				0x37
> +#define AD3552R_REG_ADDR_HW_LDAC_24B			0x37
> +#define AD3552R_REG_ADDR_CH_DAC_24B(ch)			(0x3D -
> (1 - ch) * 3)
> +#define AD3552R_REG_ADDR_DAC_PAGE_MASK_24B		0x40
> +#define AD3552R_REG_ADDR_CH_SELECT_24B			0x41
> +#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B		0x44
> +#define AD3552R_REG_ADDR_SW_LDAC_24B			0x45
> +#define AD3552R_REG_ADDR_CH_INPUT_24B(ch)		(0x4B - (1 -
> ch) * 3)
> +
> +#define AD3552R_REG_ADDR_MAX				0x4B
> +
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_A */
> +#define AD3552R_MASK_SOFTWARE_RESET			(BIT(7) |
> BIT(0))
> +#define AD3552R_MASK_ADDR_ASCENSION			BIT(5)
> +#define AD3552R_MASK_SDO_ACTIVE				BIT(4)
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_B */
> +#define AD3552R_MASK_SINGLE_INST			BIT(7)
> +#define AD3552R_MASK_SHORT_INSTRUCTION			BIT(3)
> +/* AD3552R_REG_ADDR_DEVICE_CONFIG */
> +#define AD3552R_MASK_DEVICE_STATUS(n)			BIT(4 +
> (n))
> +#define AD3552R_MASK_CUSTOM_MODES			(BIT(3) |
> BIT(2))
> +#define AD3552R_MASK_OPERATING_MODES			(BIT(1) |
> BIT(0))
> +/* AD3552R_REG_ADDR_CHIP_TYPE */
> +#define AD3552R_MASK_CLASS				0x0F
> +/* AD3552R_REG_ADDR_CHIP_GRADE */
> +#define AD3552R_MASK_GRADE				0xF0
> +#define AD3552R_MASK_DEVICE_REVISION			0x0F
> +/* AD3552R_REG_ADDR_STREAM_MODE */
> +#define AD3552R_MASK_LENGTH				0x0F
> +/* AD3552R_REG_ADDR_TRANSFER_REGISTER */
> +#define AD3552R_MASK_MULTI_IO_MODE			(BIT(7) |
> BIT(6))
> +#define AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE		BIT(2)
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_C */
> +#define AD3552R_MASK_CRC_ENABLE				(BIT(7)
> | BIT(6) |\
> +							 BIT(1) |
> BIT(0))
> +#define AD3552R_MASK_STRICT_REGISTER_ACCESS		BIT(5)
> +/* AD3552R_REG_ADDR_INTERFACE_STATUS_A */
> +#define AD3552R_MASK_INTERFACE_NOT_READY		BIT(7)
> +#define AD3552R_MASK_CLOCK_COUNTING_ERROR		BIT(5)
> +#define AD3552R_MASK_INVALID_OR_NO_CRC			BIT(3)
> +#define AD3552R_MASK_WRITE_TO_READ_ONLY_REGISTER	BIT(2)
> +#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS		BIT(1)
> +#define AD3552R_MASK_REGISTER_ADDRESS_INVALID		BIT(0)
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_D */
> +#define AD3552R_MASK_ALERT_ENABLE_PULLUP		BIT(6)
> +#define AD3552R_MASK_MEM_CRC_EN				BIT(4)
> +#define AD3552R_MASK_SDO_DRIVE_STRENGTH			(BIT(3)
> | BIT(2))
> +#define AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN		BIT(1)
> +#define AD3552R_MASK_SPI_CONFIG_DDR			BIT(0)
> +/* AD3552R_REG_ADDR_SH_REFERENCE_CONFIG */
> +#define AD3552R_MASK_IDUMP_FAST_MODE			BIT(6)
> +#define AD3552R_MASK_SAMPLE_HOLD_DIFFERENTIAL_USER_EN	BIT(5)
> +#define AD3552R_MASK_SAMPLE_HOLD_USER_TRIM		(BIT(4) |
> BIT(3))
> +#define AD3552R_MASK_SAMPLE_HOLD_USER_ENABLE		BIT(2)
> +#define AD3552R_MASK_REFERENCE_VOLTAGE_SEL		(BIT(1) |
> BIT(0))
> +/* AD3552R_REG_ADDR_ERR_ALARM_MASK */
> +#define AD3552R_MASK_REF_RANGE_ALARM			BIT(6)
> +#define AD3552R_MASK_CLOCK_COUNT_ERR_ALARM		BIT(5)
> +#define AD3552R_MASK_MEM_CRC_ERR_ALARM			BIT(4)
> +#define AD3552R_MASK_SPI_CRC_ERR_ALARM			BIT(3)
> +#define AD3552R_MASK_WRITE_TO_READ_ONLY_ALARM		BIT(2)
> +#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS_ALARM	BIT(1)
> +#define AD3552R_MASK_REGISTER_ADDRESS_INVALID_ALARM	BIT(0)
> +/* AD3552R_REG_ADDR_ERR_STATUS */
> +#define AD3552R_MASK_REF_RANGE_ERR_STATUS			BIT(6)
> +#define AD3552R_MASK_DUAL_SPI_STREAM_EXCEEDS_DAC_ERR_STATUS	BIT(5)
> +#define AD3552R_MASK_MEM_CRC_ERR_STATUS				
> BIT(4)
> +#define AD3552R_MASK_RESET_STATUS				BIT(0)
> +/* AD3552R_REG_ADDR_POWERDOWN_CONFIG */
> +#define AD3552R_MASK_CH_DAC_POWERDOWN(ch)		BIT(4 + (ch))
> +#define AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(ch)		BIT(ch)
> +/* AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE */
> +#define AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch)		((ch) ? 0xF0 :
> 0x0F)
> +/* AD3552R_REG_ADDR_CH_GAIN */
> +#define AD3552R_MASK_CH_RANGE_OVERRIDE			BIT(7)
> +#define AD3552R_MASK_CH_GAIN_SCALING_N			(BIT(6)
> | BIT(5))
> +#define AD3552R_MASK_CH_GAIN_SCALING_P			(BIT(4)
> | BIT(3))
> +#define AD3552R_MASK_CH_OFFSET_POLARITY			BIT(2)
> +#define AD3552R_MASK_CH_OFFSET_BIT_8			BIT(0)
> +/* AD3552R_REG_ADDR_CH_OFFSET */
> +#define AD3552R_MASK_CH_OFFSET_BITS_0_7			0xFF
> +
> +/* Useful defines */
> +#define AD3552R_NUM_CH					2
> +#define AD3552R_MASK_CH(ch)				BIT(ch)
> +#define AD3552R_PAGE_CH					2
> +#define AD3552R_MAX_REG_SIZE				3
> +#define AD3552R_READ_BIT				(1 << 7)
> +#define AD3552R_ADDR_MASK				(~AD3552R_READ_
> BIT)
> +#define AD3552R_CRC_ENABLE_VALUE			(BIT(6) |
> BIT(1))
> +#define AD3552R_CRC_DISABLE_VALUE			(BIT(1) |
> BIT(0))
> +#define AD3552R_CRC_POLY				0x07
> +#define AD3552R_CRC_SEED				0xA5
> +#define AD3552R_MASK_DAC_12B				0xFFF0
> +#define AD3552R_DEFAULT_CONFIG_B_VALUE			0x8
> +#define SCRATCH_PAD_TEST_VAL1				0x34
> +#define SCRATCH_PAD_TEST_VAL2				0xB2
> +#define TO_MICROS					1000000
> +#define GAIN_SCALE					1000
> +#define AD3552R_READ					true
> +#define AD3552R_WRITE					false
> +#define LDAC_PULSE_US					10
> +
> +enum ad3552r_ch_output_range {
> +	/* Range from 0 V to 2.5 V. Requires Rfb1x connection */
> +	AD3552R_CH_OUTPUT_RANGE_0__2_5V,
> +	/* Range from 0 V to 5 V. Requires Rfb1x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_0__5V,
> +	/* Range from 0 V to 10 V. Requires Rfb2x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_0__10V,
> +	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_NEG_5__5V,
> +	/* Range from -6.5 V to 3.5 V. Requires Rfb4x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_NEG_10__10V,
> +};
> +
> +static const s32 ch_ranges[][2] = {
> +	[AD3552R_CH_OUTPUT_RANGE_0__2_5V]	= {0, 2500},
> +	[AD3552R_CH_OUTPUT_RANGE_0__5V]		= {0, 5000},
> +	[AD3552R_CH_OUTPUT_RANGE_0__10V]	= {0, 10000},
> +	[AD3552R_CH_OUTPUT_RANGE_NEG_5__5V]	= {-5000, 5000},
> +	[AD3552R_CH_OUTPUT_RANGE_NEG_10__10V]	= {-10000, 10000}
> +};
> +
> +enum ad3552r_ch_gain_scaling {
> +	/* Gain scaling of 1 */
> +	AD3552R_CH_GAIN_SCALING_1,
> +	/* Gain scaling of 0.5 */
> +	AD3552R_CH_GAIN_SCALING_0_5,
> +	/* Gain scaling of 0.25 */
> +	AD3552R_CH_GAIN_SCALING_0_25,
> +	/* Gain scaling of 0.125 */
> +	AD3552R_CH_GAIN_SCALING_0_125,
> +};
> +
> +/* Gain * GAIN_SCALE */
> +static const s32 gains_scaling_table[] = {
> +	[AD3552R_CH_GAIN_SCALING_1]		= 1000,
> +	[AD3552R_CH_GAIN_SCALING_0_5]		= 500,
> +	[AD3552R_CH_GAIN_SCALING_0_25]		= 250,
> +	[AD3552R_CH_GAIN_SCALING_0_125]		= 125
> +};
> +
> +enum ad3552r_dev_attributes {
> +	/* - Direct register values */
> +	/* From 0-3 */
> +	AD3552R_SDO_DRIVE_STRENGTH,
> +	/*
> +	 * 0 -> Internal Vref, vref_io pin floating (default)
> +	 * 1 -> Internal Vref, vref_io driven by internal vref
> +	 * 2 or 3 -> External Vref
> +	 */
> +	AD3552R_VREF_SELECT,
> +	/* Enable / Disable CRC */
> +	AD3552R_CRC_ENABLE,
> +	/* Spi mode: Strandard, Dual or Quad */
> +	AD3552R_SPI_MULTI_IO_MODE,
> +	/* Spi data rate: Single or dual */
> +	AD3552R_SPI_DATA_RATE,
> +	/* Dual spi synchronous mode */
> +	AD3552R_SPI_SYNCHRONOUS_ENABLE,
> +
> +	/* - Direct register values (Private) */
> +	/* Read registers in ascending order if set. Else descending */
> +	AD3552R_ADDR_ASCENSION,
> +	/* Single instruction mode if set. Else, stream mode */
> +	AD3552R_SINGLE_INST,
> +	/* Number of addresses to loop on when stream writing. */
> +	AD3552R_STREAM_MODE,
> +	/* Keep stream value if set. */
> +	AD3552R_STREAM_LENGTH_KEEP_VALUE,
> +};
> +
> +enum ad3552r_ch_attributes {
> +	/* DAC powerdown */
> +	AD3552R_CH_DAC_POWERDOWN,
> +	/* DAC amplifier powerdown */
> +	AD3552R_CH_AMPLIFIER_POWERDOWN,
> +	/* Select the output range. Select from enum
> ad3552r_ch_output_range */
> +	AD3552R_CH_OUTPUT_RANGE_SEL,
> +	/*
> +	 * Over-rider the range selector in order to manually set the
> output
> +	 * voltage range
> +	 */
> +	AD3552R_CH_RANGE_OVERRIDE,
> +	/* Manually set the offset voltage */
> +	AD3552R_CH_GAIN_OFFSET,
> +	/* Sets the polarity of the offset. */
> +	AD3552R_CH_GAIN_OFFSET_POLARITY,
> +	/* PDAC gain scaling */
> +	AD3552R_CH_GAIN_SCALING_P,
> +	/* NDAC gain scaling */
> +	AD3552R_CH_GAIN_SCALING_N,
> +	/* Trigger a software LDAC */
> +	AD3552R_CH_TRIGGER_SOFTWARE_LDAC,
> +	/* Hardware LDAC Mask */
> +	AD3552R_CH_HW_LDAC_MASK,
> +	/* Rfb value */
> +	AD3552R_CH_RFB,
> +	/* Channel select. When set allow Input -> DAC and Mask -> DAC
> */
> +	AD3552R_CH_SELECT,
> +	/* Raw value to be set to dac */
> +	AD3552R_CH_CODE
> +};
> +
> +struct ad3552r_ch_data {
> +	u16	gain_offset : 9;
> +	u16	range_override : 1;
> +	u16	n : 2;
> +	u16	p : 2;
> +	u16	offset_polarity : 1;
> +	u16	rfb;
> +	u8	range;
> +	s32	scale_int;
> +	s32	scale_dec;
> +	s32	offset_int;
> +	s32	offset_dec;
> +	bool	prec_en;
> +};
> +
> +struct ad3552r_desc {
> +	struct iio_dev		*indio_dev;
> +	struct mutex		lock;
> +	struct gpio_desc	*gpio_reset;
> +	struct gpio_desc	*gpio_ldac;
> +	struct spi_device	*spi;
> +	struct ad3552r_ch_data	ch_data[AD3552R_NUM_CH];
> +	struct iio_chan_spec	channels[AD3552R_NUM_CH + 1];
> +	unsigned long		enabled_ch;
> +	unsigned int		num_ch;
> +	bool			use_input_regs;
> +	u8 buf_data[2 * (AD3552R_MAX_REG_SIZE + 2)]
> ____cacheline_aligned;
> +};
> +
> +static const u16 addr_mask_map[][2] = {
> +	[AD3552R_ADDR_ASCENSION] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> +			AD3552R_MASK_ADDR_ASCENSION
> +	},
> +	[AD3552R_SINGLE_INST] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +			AD3552R_MASK_SINGLE_INST
> +	},
> +	[AD3552R_STREAM_MODE] = {
> +			AD3552R_REG_ADDR_STREAM_MODE,
> +			AD3552R_MASK_LENGTH
> +	},
> +	[AD3552R_STREAM_LENGTH_KEEP_VALUE] = {
> +			AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +			AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE
> +	},
> +	[AD3552R_SDO_DRIVE_STRENGTH] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_SDO_DRIVE_STRENGTH
> +	},
> +	[AD3552R_VREF_SELECT] = {
> +			AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> +			AD3552R_MASK_REFERENCE_VOLTAGE_SEL
> +	},
> +	[AD3552R_CRC_ENABLE] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_C,
> +			AD3552R_MASK_CRC_ENABLE
> +	},
> +	[AD3552R_SPI_MULTI_IO_MODE] = {
> +			AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +			AD3552R_MASK_MULTI_IO_MODE
> +	},
> +	[AD3552R_SPI_DATA_RATE] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_SPI_CONFIG_DDR
> +	},
> +	[AD3552R_SPI_SYNCHRONOUS_ENABLE] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN
> +	},
> +};
> +
> +/* 0 -> reg addr, 1->ch0 mask, 2->ch1 mask */
> +static const u16 addr_mask_map_ch[][3] = {
> +	[AD3552R_CH_DAC_POWERDOWN] = {
> +			AD3552R_REG_ADDR_POWERDOWN_CONFIG,
> +			AD3552R_MASK_CH_DAC_POWERDOWN(0),
> +			AD3552R_MASK_CH_DAC_POWERDOWN(1)
> +	},
> +	[AD3552R_CH_AMPLIFIER_POWERDOWN] = {
> +			AD3552R_REG_ADDR_POWERDOWN_CONFIG,
> +			AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(0),
> +			AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(1)
> +	},
> +	[AD3552R_CH_OUTPUT_RANGE_SEL] = {
> +			AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
> +			AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0),
> +			AD3552R_MASK_CH_OUTPUT_RANGE_SEL(1)
> +	},
> +	/*
> +	 * This attributes are update by the chip on 16B and 24B no
> matter to
> +	 * what register the write is done
> +	 */
> +	[AD3552R_CH_TRIGGER_SOFTWARE_LDAC] = {
> +			AD3552R_REG_ADDR_SW_LDAC_16B,
> +			AD3552R_MASK_CH(0),
> +			AD3552R_MASK_CH(1)
> +	},
> +	[AD3552R_CH_HW_LDAC_MASK] = {
> +			AD3552R_REG_ADDR_HW_LDAC_16B,
> +			AD3552R_MASK_CH(0),
> +			AD3552R_MASK_CH(1)
> +	},
> +	[AD3552R_CH_SELECT] = {
> +			AD3552R_REG_ADDR_CH_SELECT_16B,
> +			AD3552R_MASK_CH(0),
> +			AD3552R_MASK_CH(1)
> +	}
> +};
> +
> +static u8 _ad3552r_reg_len(u8 addr)
> +{
> +	if (addr > AD3552R_REG_ADDR_MAX)
> +		return 0;
> +
> +	switch (addr) {
> +	case AD3552R_REG_ADDR_HW_LDAC_16B:
> +	case AD3552R_REG_ADDR_CH_SELECT_16B:
> +	case AD3552R_REG_ADDR_SW_LDAC_16B:
> +	case AD3552R_REG_ADDR_HW_LDAC_24B:
> +	case AD3552R_REG_ADDR_CH_SELECT_24B:
> +	case AD3552R_REG_ADDR_SW_LDAC_24B:
> +		return 1;
> +	default:
> +		break;
> +	}
> +
> +	if (addr > AD3552R_REG_ADDR_HW_LDAC_24B)
> +		return 3;
> +	if (addr > AD3552R_REG_ADDR_HW_LDAC_16B)
> +		return 2;
> +
> +	return 1;
> +}
> +
> +/* SPI transfer to device */
> +static int ad3552r_transfer(struct ad3552r_desc *dac, u8 addr, u32
> len,
> +			    u8 *data, bool is_read)
> +{
> +	int err;
> +	u8 instr;
> +
> +	instr = addr & AD3552R_ADDR_MASK;
> +	instr |= is_read ? AD3552R_READ_BIT : 0;
> +	dac->buf_data[0] = instr;
> +	if (is_read) {
> +		err = spi_write_then_read(dac->spi, dac->buf_data, 1,
> +					  dac->buf_data + 1, len);
> +		if (err)
> +			return err;
> +
> +		memcpy(data, dac->buf_data + 1, len);
> +
> +		return 0;
> +	}
> +
> +	memcpy(dac->buf_data + 1, data, len);
> +	return spi_write(dac->spi, dac->buf_data, len + 1);
> +}
> +
> +static int ad3552r_write_reg(struct ad3552r_desc *dac, u8 addr, u16
> val)
> +{
> +	u8 reg_len, buf[AD3552R_MAX_REG_SIZE] = { 0 };
> +
> +	reg_len = _ad3552r_reg_len(addr);
> +	if (!reg_len)
> +		return -EINVAL;
> +
> +	if (reg_len == 2)
> +		/* Only DAC register are 2 bytes wide */
> +		val &= AD3552R_MASK_DAC_12B;
> +	if (reg_len == 1)
> +		buf[0] = val & 0xFF;
> +	else
> +		/* reg_len can be 2 or 3, but 3rd bytes needs to be set
> to 0 */
> +		*((u16 *)buf) = cpu_to_be16(val);
> 

As you already know, this is not really ok :).

> +	return ad3552r_transfer(dac, addr, reg_len, buf,
> +				AD3552R_WRITE);
> +}
> +
> +static int ad3552r_read_reg(struct ad3552r_desc *dac, u8 addr, u16
> *val)
> +{
> +	int err;
> +	u8  reg_len, buf[AD3552R_MAX_REG_SIZE] = { 0 };
> +
> +	reg_len = _ad3552r_reg_len(addr);
> +	if (!reg_len)
> +		return -EINVAL;
> +
> +	err = ad3552r_transfer(dac, addr, reg_len, buf, AD3552R_READ);
> +	if (err)
> +		return err;
> +
> +	if (reg_len == 1)
> +		*val = buf[0];
> +	else
> +		/* reg_len can be 2 or 3, but only first 2 bytes are
> relevant */
> +		*val = be16_to_cpu(*((u16 *)buf));
> +

ditto

> +	return 0;
> +}
> +
> +/* Update field of a register, shift val if needed */
> +static int ad3552r_update_reg_field(struct ad3552r_desc *dac, u8
> addr, u16 mask,
> +				    u16 val)
> +{
> +	int ret;
> +	u16 reg;
> +
> +	ret = ad3552r_read_reg(dac, addr, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	reg = (reg & ~mask) | (val << __ffs(mask));
> +
> +	return ad3552r_write_reg(dac, addr, reg);
> +}
> +
> +static int ad3552r_set_dev_value(struct ad3552r_desc *dac,
> +				 enum ad3552r_dev_attributes attr,
> +				 u16 val)
> +{
> +	switch (attr) {
> +	case AD3552R_SPI_MULTI_IO_MODE:
> +	case AD3552R_SPI_DATA_RATE:
> +	case AD3552R_SPI_SYNCHRONOUS_ENABLE:
> +	case AD3552R_CRC_ENABLE:
> +		/* Not implemented */
> +		return -EINVAL;
> +	default:
> +		return ad3552r_update_reg_field(dac,
> addr_mask_map[attr][0],
> +						addr_mask_map[attr][1],
> val);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad3552r_set_offset_value(struct ad3552r_desc *dac, u8 ch,
> int val)
> +{
> +	int err;
> +
> +	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_CH_OFFSET(ch),
> +				val & AD3552R_MASK_CH_OFFSET_BITS_0_7);
> +	if (err)
> +		return err;
> +
> +	err = ad3552r_update_reg_field(dac,
> +				       AD3552R_REG_ADDR_CH_GAIN(ch),
> +				       AD3552R_MASK_CH_OFFSET_BIT_8,
> +				       (val >> 8) &
> AD3552R_MASK_CH_OFFSET_BIT_8);
> +	if (err)
> +		return err;
> +
> +	dac->ch_data[ch].gain_offset = val;
> +
> +	return 0;
> +}
> +
> +static int ad3552r_set_gain_value(struct ad3552r_desc *dac,
> +				  enum ad3552r_ch_attributes attr,
> +				  u8 ch,
> +				  int val)
> +{
> +	int reg_mask, err;
> +
> +	if (attr == AD3552R_CH_GAIN_OFFSET)
> +		return ad3552r_set_offset_value(dac, ch, val);
> +
> +	switch (attr) {
> +	case AD3552R_CH_RANGE_OVERRIDE:
> +		val = !!val;
> +		reg_mask = AD3552R_MASK_CH_RANGE_OVERRIDE;
> +		break;
> +	case AD3552R_CH_GAIN_OFFSET_POLARITY:
> +		val = !!val;
> +		reg_mask = AD3552R_MASK_CH_OFFSET_POLARITY;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_P:
> +		reg_mask = AD3552R_MASK_CH_GAIN_SCALING_P;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_N:
> +		reg_mask = AD3552R_MASK_CH_GAIN_SCALING_N;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	err = ad3552r_update_reg_field(dac,
> AD3552R_REG_ADDR_CH_GAIN(ch),
> +				       reg_mask, val);
> +	if (err)
> +		return err;
> +
> +	switch (attr) {
> +	case AD3552R_CH_RANGE_OVERRIDE:
> +		dac->ch_data[ch].range_override = val;
> +		break;
> +	case AD3552R_CH_GAIN_OFFSET_POLARITY:
> +		dac->ch_data[ch].offset_polarity = val;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_P:
> +		dac->ch_data[ch].p = val;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_N:
> +		dac->ch_data[ch].n = val;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Iterate over mask and write required bytes */
> +static int ad3552r_write_codes(struct ad3552r_desc *dac, u32 mask,
> u8 *vals)
> +{
> +	int err, i, reg_len, k = 0;
> +	unsigned long lmask = mask;
> +	u8 addr, buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE];
> +	u16 val;
> +
> +	/* If writing to consecutive registers do just one transfer */
> +
> +	if (mask == (AD3552R_MASK_CH(0) | AD3552R_MASK_CH(1)) &&
> +	    dac->ch_data[0].prec_en == dac->ch_data[1].prec_en) {
> +		if (dac->use_input_regs) {
> +			if (dac->ch_data[0].prec_en)
> +				addr =
> AD3552R_REG_ADDR_CH_INPUT_24B(1);
> +			else
> +				addr =
> AD3552R_REG_ADDR_CH_INPUT_16B(1);
> +		} else {
> +			if (dac->ch_data[0].prec_en)
> +				addr = AD3552R_REG_ADDR_CH_DAC_24B(1);
> +			else
> +				addr = AD3552R_REG_ADDR_CH_DAC_16B(1);
> +		}
> +
> +		reg_len = _ad3552r_reg_len(addr);
> +		buff[0] = vals[0];
> +		buff[reg_len] = vals[2];
> +		if (dac->ch_data[0].prec_en) {
> +			/* Reg_len is 3 here */
> +			buff[1] = vals[1];
> +			buff[2] = 0;
> +			buff[4] = vals[3];
> +			buff[5] = 0;
> +		} else {
> +			buff[1] = vals[1] & 0xf0;
> +			buff[3] = vals[3] & 0xf0;
> +		}
> +
> +		err = ad3552r_transfer(dac, addr, reg_len * 2, buff,
> +				       AD3552R_WRITE);
> +		if (err)
> +			return err;
> +	} else {
> +
> +		k = 0;
> +		for_each_set_bit(i, &lmask, AD3552R_NUM_CH + 1) {
> +			/* Writing to mask CH */
> +			if (i == AD3552R_PAGE_CH)
> +				addr = dac->ch_data[0].prec_en ?
> +					AD3552R_REG_ADDR_INPUT_PAGE_MAS
> K_24B :
> +					AD3552R_REG_ADDR_INPUT_PAGE_MAS
> K_16B;
> +			else
> +				addr = dac->ch_data[i].prec_en ?
> +					AD3552R_REG_ADDR_CH_INPUT_24B(i
> ) :
> +					AD3552R_REG_ADDR_CH_INPUT_16B(i
> );
> +
> +			reg_len = _ad3552r_reg_len(addr);
> +			val = be16_to_cpu(*((u16 *)(vals + k)));
> +
> +			k += 2;
> +			err = ad3552r_write_reg(dac, addr, val);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	if (dac->gpio_ldac) {
> +		gpiod_set_value_cansleep(dac->gpio_ldac, 0);
> +		usleep_range(LDAC_PULSE_US, LDAC_PULSE_US + 10);
> +		gpiod_set_value_cansleep(dac->gpio_ldac, 1);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad3552r_get_ch_value(struct ad3552r_desc *dac,
> +				enum ad3552r_ch_attributes attr,
> +				u8 ch,
> +				u16 *val)
> +{
> +	int ret;
> +	u16 reg;
> +	u8  addr;
> +	u16 mask;
> +
> +	/* Attributes not defined in addr_mask_map_ch */
> +	switch (attr) {
> +	case AD3552R_CH_CODE:
> +		return ad3552r_read_reg(dac,
> AD3552R_REG_ADDR_CH_DAC_24B(ch),
> +					val);
> +	case AD3552R_CH_RFB:
> +		*val = dac->ch_data[ch].rfb;
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	if (attr >= AD3552R_CH_RANGE_OVERRIDE &&
> +	    attr <= AD3552R_CH_GAIN_SCALING_N)
> +		return -EINVAL;
> +
> +	addr = addr_mask_map_ch[attr][0];
> +	if (addr == AD3552R_REG_ADDR_SW_LDAC_24B ||
> +	    addr == AD3552R_REG_ADDR_SW_LDAC_16B) {
> +		dev_err(&dac->indio_dev->dev, "Write only
> registers\n");
> +		/* LDAC are write only registers */
> +		return -EINVAL;
> +	}
> +
> +	ret = ad3552r_read_reg(dac, addr, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	mask = addr_mask_map_ch[attr][ch + 1];
> +	*val = (reg & mask) >> __ffs(mask);
> +
> +	return 0;
> +}
> +
> +static int ad3552r_set_ch_value(struct ad3552r_desc *dac,
> +				enum ad3552r_ch_attributes attr,
> +				u8 ch,
> +				u16 val)
> +{
> +	int ret;
> +
> +	/* Attributes not defined in addr_mask_map_ch */
> +	switch (attr) {
> +	case AD3552R_CH_CODE:
> +		return ad3552r_write_reg(dac,
> AD3552R_REG_ADDR_CH_DAC_24B(ch),
> +					 val);
> +	case AD3552R_CH_RFB:
> +		dac->ch_data[ch].rfb = val;
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	if (attr >= AD3552R_CH_RANGE_OVERRIDE &&
> +	    attr <= AD3552R_CH_GAIN_SCALING_N)
> +		return ad3552r_set_gain_value(dac, attr, ch, val);
> +
> +	/* Update register related to attributes in chip */
> +	ret = ad3552r_update_reg_field(dac, addr_mask_map_ch[attr][0],
> +				       addr_mask_map_ch[attr][ch + 1],
> val);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Update software structures */
> +	if (attr == AD3552R_CH_OUTPUT_RANGE_SEL) {
> +		val %= AD3552R_CH_OUTPUT_RANGE_NEG_10__10V + 1;
> +		dac->ch_data[ch].range = val;
> +	}
> +
> +	return ret;
> +}
> +
> +static ssize_t ad3552r_write_ext(struct iio_dev *indio_dev,
> +				 uintptr_t private,
> +				 const struct iio_chan_spec *chan,
> +				 const char *buf, size_t len)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	int val, frac, err;
> +
> +	err = iio_str_to_fixpoint(buf, 0, &val, &frac);
> +	if (err < 0)
> +		return err;
> +
> +	dac->ch_data[chan->channel].prec_en = !!val;
> +
> +	return len;
> +}
> +
> +static ssize_t ad3552r_read_ext(struct iio_dev *indio_dev,
> +				uintptr_t private,
> +				const struct iio_chan_spec *chan,
> +				char *buf)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	int val;
> +
> +	if (private != 0)
> +		return -EINVAL;
> +
> +	val = dac->ch_data[chan->channel].prec_en;
> +
> +	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +}
> +
> +#define AD3552R_CH_ATTR(_name, _what) { \
> +	.name = _name, \
> +	.read = ad3552r_read_ext, \
> +	.write = ad3552r_write_ext, \
> +	.private = _what, \
> +	.shared = IIO_SEPARATE, \
> +}
> +
> +static const struct iio_chan_spec_ext_info ad3552r_ext_info[] = {
> +	AD3552R_CH_ATTR("precision_mode_en", 0),
> +	{},
> +};
> +
> +#define AD3552R_CH_DAC(_idx) ((struct iio_chan_spec) {		
> \
> +	.type = IIO_VOLTAGE,					\
> +	.output = true,						\
> +	.indexed = true,					\
> +	.channel = _idx,					\
> +	.scan_index = _idx,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_BE,				\
> +	},							\
> +	.ext_info = ad3552r_ext_info,				\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +				BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_ENABLE) |	\
> +				BIT(IIO_CHAN_INFO_OFFSET),	\
> +})
> +
> +#define AD3552R_CH_DAC_PAGE(_idx) ((struct iio_chan_spec) {	\
> +	.type = IIO_VOLTAGE,					\
> +	.output = true,						\
> +	.indexed = true,					\
> +	.channel = _idx,					\
> +	.scan_index = _idx,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_BE,				\
> +	},							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +	.modified = 1,						\
> +	.channel2 = IIO_MOD_X_AND_Z,				\
> +})
> +
> +static int ad3552r_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long mask)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	u16 tmp_val;
> +	int err;
> +	u8 ch = chan->channel;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&dac->lock);
> +		if (chan->channel == AD3552R_PAGE_CH)
> +			err = ad3552r_read_reg(dac,
> +					       AD3552R_REG_ADDR_DAC_PAG
> E_MASK_24B,
> +					       &tmp_val);
> +		else
> +			err = ad3552r_get_ch_value(dac,
> AD3552R_CH_CODE, ch,
> +						   &tmp_val);
> +		if (err < 0) {
> +			mutex_unlock(&dac->lock);
> +			return err;
> +		}
> +
> +		*val = tmp_val;
> +		mutex_unlock(&dac->lock);
> +		break;
> +	case IIO_CHAN_INFO_ENABLE:
> +		mutex_lock(&dac->lock);
> +		err = ad3552r_get_ch_value(dac,
> AD3552R_CH_DAC_POWERDOWN,
> +					   ch, &tmp_val);
> +		if (err < 0) {
> +			mutex_unlock(&dac->lock);
> +			return err;
> +		}
> +		*val = !tmp_val;
> +		mutex_unlock(&dac->lock);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = dac->ch_data[ch].scale_int;
> +		*val2 = dac->ch_data[ch].scale_dec;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = dac->ch_data[ch].offset_int;
> +		*val2 = dac->ch_data[ch].offset_dec;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int ad3552r_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val,
> +			     int val2,
> +			     long mask)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	enum ad3552r_ch_attributes attr;
> +	int err = 0;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->channel == AD3552R_PAGE_CH) {
> +			mutex_lock(&dac->lock);
> +			err = ad3552r_write_reg(dac,
> +						AD3552R_REG_ADDR_DAC_PA
> GE_MASK_24B,
> +						val);
> +			mutex_unlock(&dac->lock);
> +
> +			return err;
> +		}
> +
> +		attr = AD3552R_CH_CODE;
> +		break;
> +	case IIO_CHAN_INFO_ENABLE:
> +		attr = AD3552R_CH_DAC_POWERDOWN;
> +		val = !val;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&dac->lock);
> +	err = ad3552r_set_ch_value(dac, attr, chan->channel, val);
> +	mutex_unlock(&dac->lock);
> +
> +	return err;
> +}
> +
> +static int ad3552r_update_scan_mode(struct iio_dev *indio_dev,
> +				    const unsigned long *scan_mask)
> +{
> +	u32 mask;
> +
> +	mask = *scan_mask;
> +	/* If writing to mask, can't write to other channels */
> +	if ((mask & AD3552R_MASK_CH(AD3552R_PAGE_CH)) &&
> +	    (mask & (~AD3552R_MASK_CH(AD3552R_PAGE_CH))))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/*
> + * Device type specific information.
> + */
> +static const struct iio_info ad3552r_iio_info = {
> +	.read_raw = ad3552r_read_raw,
> +	.write_raw = ad3552r_write_raw,
> +	.update_scan_mode = ad3552r_update_scan_mode
> +};
> +
> +static irqreturn_t ad3552r_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func	*pf = p;
> +	struct iio_dev		*indio_dev = pf->indio_dev;
> +	struct iio_buffer	*buf = indio_dev->buffer;
> +	struct ad3552r_desc	*dac = iio_priv(indio_dev);
> +	char			buff[AD3552R_NUM_CH *
> AD3552R_MAX_REG_SIZE];
> +	int			err;
> +
> +	memset(buff, 0, sizeof(buff));
> +	mutex_lock(&dac->lock);
> +	err = iio_buffer_remove_sample(buf, buff);
> +	if (err)
> +		goto end;
> +
> +	err = ad3552r_write_codes(dac, *indio_dev->active_scan_mask,
> buff);
> +	if (err)
> +		goto end;
> +
> +end:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	mutex_unlock(&dac->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int ad3552r_setup_trigger_buffer(struct device *dev,
> +					struct iio_dev *indio_dev, int
> irq)
> +{
> +	struct ad3552r_desc	*dac = iio_priv(indio_dev);
> +	struct iio_trigger	*hwtrig;
> +	int			err;
> +
> +	/* Configure trigger buffer */
> +	err = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
> +						  &ad3552r_trigger_hand
> ler,
> +						  IIO_BUFFER_DIRECTION_
> OUT,
> +						  NULL,
> +						  NULL);
> +	if (err)
> +		return err;
> +
> +	if (!irq)
> +		return 0;
> +
> +	hwtrig = devm_iio_trigger_alloc(dev, "%s-ldac-dev%d",
> +					indio_dev->name,
> +					iio_device_id(indio_dev));
> +	if (!hwtrig)
> +		return -ENOMEM;
> +

Well, As I already expressed this internally, I do not really agree
with this. The device does not really have a trigger (at least one in
the way we are used too) and the only IRQ it has is an ALERT pin which
is not used for alerting us that the device can consume more data.
Hence, I do not think we should be allocating a trigger here. IMO, we
should just use 'devm_iio_triggered_buffer_setup_ext()' and then use a
hrtimer trigger (for example) to poll for new data. I think for these
kind of devices, introducing a new pwm-trigger would also be a nice
thing to consider.

The ldac pin is a trigger for the device, not for us (controller).

- Nuno Sá
Jonathan Cameron Aug. 30, 2021, 4:54 p.m. UTC | #4
On Fri, 20 Aug 2021 16:59:27 +0000
Mihail Chindris <mihail.chindris@analog.com> wrote:

> The AD3552R-16 is a low drift ultrafast, 16-bit accuracy,
> current output digital-to-analog converter (DAC) designed
> to generate multiple output voltage span ranges.
> The AD3552R-16 operates with a fixed 2.5V reference.

The following is a fairly quick review.  Lots here so I'll take a closer
look at a later version.  Only so much stamina, though I'm almost caught
up with what came in whilst I was on vacation!

> 
> analog.com/media/en/technical-documentation/data-sheets/ad3552r.pdf
We have a datasheet tag these days.

> 
> Signed-off-by: Mihail Chindris <mihail.chindris@analog.com>
> ---
>  drivers/iio/dac/Kconfig   |   10 +
>  drivers/iio/dac/Makefile  |    1 +
>  drivers/iio/dac/ad3552r.c | 1419 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 1430 insertions(+)
>  create mode 100644 drivers/iio/dac/ad3552r.c
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 75e1f2b48638..ced6428f2c92 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -6,6 +6,16 @@
>  
>  menu "Digital to analog converters"
>  
> +config AD3552R
> +	tristate "Analog Devices AD3552R DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD3552R
> +	  Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad3552r.
> +
>  config AD5064
>  	tristate "Analog Devices AD5064 and similar multi-channel DAC driver"
>  	depends on (SPI_MASTER && I2C!=m) || I2C
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 33e16f14902a..dffe36efd8ff 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_AD3552R) += ad3552r.o
>  obj-$(CONFIG_AD5360) += ad5360.o
>  obj-$(CONFIG_AD5380) += ad5380.o
>  obj-$(CONFIG_AD5421) += ad5421.o
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> new file mode 100644
> index 000000000000..89993dd87522
> --- /dev/null
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -0,0 +1,1419 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
No blank line here.

> +/*
> + * Analog Devices AD3552R
> + * Digital to Analog converter driver
> + *
> + * Copyright 2021 Analog Devices Inc.
> + */
> +#include <linux/iopoll.h>

Alphabetical order.   If you prefer you can pull the subsystem headers
out afterwards in their own block.

> +#include <linux/device.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/time64.h>
> +#include <linux/unaligned/be_byteshift.h>
> +
> +/* Register addresses */
> +/* Primary address space */
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_A		0x00
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_B		0x01
> +#define AD3552R_REG_ADDR_DEVICE_CONFIG			0x02
> +#define AD3552R_REG_ADDR_CHIP_TYPE			0x03
> +#define AD3552R_REG_ADDR_PRODUCT_ID_L			0x04
> +#define AD3552R_REG_ADDR_PRODUCT_ID_H			0x05
> +#define AD3552R_REG_ADDR_CHIP_GRADE			0x06
> +#define AD3552R_REG_ADDR_SCRATCH_PAD			0x0A
> +#define AD3552R_REG_ADDR_SPI_REVISION			0x0B
> +#define AD3552R_REG_ADDR_VENDOR_L			0x0C
> +#define AD3552R_REG_ADDR_VENDOR_H			0x0D
> +#define AD3552R_REG_ADDR_STREAM_MODE			0x0E
> +#define AD3552R_REG_ADDR_TRANSFER_REGISTER		0x0F
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C		0x10
> +#define AD3552R_REG_ADDR_INTERFACE_STATUS_A		0x11
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_D		0x14
> +#define AD3552R_REG_ADDR_SH_REFERENCE_CONFIG		0x15
> +#define AD3552R_REG_ADDR_ERR_ALARM_MASK			0x16
> +#define AD3552R_REG_ADDR_ERR_STATUS			0x17
> +#define AD3552R_REG_ADDR_POWERDOWN_CONFIG		0x18
> +#define AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE		0x19
> +#define AD3552R_REG_ADDR_CH_OFFSET(ch)			(0x1B + (ch) * 2)
> +#define AD3552R_REG_ADDR_CH_GAIN(ch)			(0x1C + (ch) * 2)
> +/*
> + * Secondary region
> + * For multibyte registers specify the highest address because the access is
> + * done in descending order
> + */
> +#define AD3552R_SECONDARY_REGION_START			0x28
> +#define AD3552R_REG_ADDR_HW_LDAC_16B			0x28
> +#define AD3552R_REG_ADDR_CH_DAC_16B(ch)			(0x2C - (1 - ch) * 2)
> +#define AD3552R_REG_ADDR_DAC_PAGE_MASK_16B		0x2E
> +#define AD3552R_REG_ADDR_CH_SELECT_16B			0x2F
> +#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B		0x31
> +#define AD3552R_REG_ADDR_SW_LDAC_16B			0x32
> +#define AD3552R_REG_ADDR_CH_INPUT_16B(ch)		(0x36 - (1 - ch) * 2)
> +/* 3 bytes registers */
> +#define AD3552R_REG_START_24B				0x37
> +#define AD3552R_REG_ADDR_HW_LDAC_24B			0x37
> +#define AD3552R_REG_ADDR_CH_DAC_24B(ch)			(0x3D - (1 - ch) * 3)
> +#define AD3552R_REG_ADDR_DAC_PAGE_MASK_24B		0x40
> +#define AD3552R_REG_ADDR_CH_SELECT_24B			0x41
> +#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B		0x44
> +#define AD3552R_REG_ADDR_SW_LDAC_24B			0x45
> +#define AD3552R_REG_ADDR_CH_INPUT_24B(ch)		(0x4B - (1 - ch) * 3)
> +
> +#define AD3552R_REG_ADDR_MAX				0x4B
> +
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_A */
> +#define AD3552R_MASK_SOFTWARE_RESET			(BIT(7) | BIT(0))
> +#define AD3552R_MASK_ADDR_ASCENSION			BIT(5)
> +#define AD3552R_MASK_SDO_ACTIVE				BIT(4)
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_B */
> +#define AD3552R_MASK_SINGLE_INST			BIT(7)
> +#define AD3552R_MASK_SHORT_INSTRUCTION			BIT(3)
> +/* AD3552R_REG_ADDR_DEVICE_CONFIG */
> +#define AD3552R_MASK_DEVICE_STATUS(n)			BIT(4 + (n))
> +#define AD3552R_MASK_CUSTOM_MODES			(BIT(3) | BIT(2))
> +#define AD3552R_MASK_OPERATING_MODES			(BIT(1) | BIT(0))
> +/* AD3552R_REG_ADDR_CHIP_TYPE */
> +#define AD3552R_MASK_CLASS				0x0F
> +/* AD3552R_REG_ADDR_CHIP_GRADE */
> +#define AD3552R_MASK_GRADE				0xF0
> +#define AD3552R_MASK_DEVICE_REVISION			0x0F
> +/* AD3552R_REG_ADDR_STREAM_MODE */
> +#define AD3552R_MASK_LENGTH				0x0F
> +/* AD3552R_REG_ADDR_TRANSFER_REGISTER */
> +#define AD3552R_MASK_MULTI_IO_MODE			(BIT(7) | BIT(6))
> +#define AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE		BIT(2)
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_C */
> +#define AD3552R_MASK_CRC_ENABLE				(BIT(7) | BIT(6) |\
> +							 BIT(1) | BIT(0))
> +#define AD3552R_MASK_STRICT_REGISTER_ACCESS		BIT(5)
> +/* AD3552R_REG_ADDR_INTERFACE_STATUS_A */
> +#define AD3552R_MASK_INTERFACE_NOT_READY		BIT(7)
> +#define AD3552R_MASK_CLOCK_COUNTING_ERROR		BIT(5)
> +#define AD3552R_MASK_INVALID_OR_NO_CRC			BIT(3)
> +#define AD3552R_MASK_WRITE_TO_READ_ONLY_REGISTER	BIT(2)
> +#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS		BIT(1)
> +#define AD3552R_MASK_REGISTER_ADDRESS_INVALID		BIT(0)
> +/* AD3552R_REG_ADDR_INTERFACE_CONFIG_D */
> +#define AD3552R_MASK_ALERT_ENABLE_PULLUP		BIT(6)
> +#define AD3552R_MASK_MEM_CRC_EN				BIT(4)
> +#define AD3552R_MASK_SDO_DRIVE_STRENGTH			(BIT(3) | BIT(2))
> +#define AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN		BIT(1)
> +#define AD3552R_MASK_SPI_CONFIG_DDR			BIT(0)
> +/* AD3552R_REG_ADDR_SH_REFERENCE_CONFIG */
> +#define AD3552R_MASK_IDUMP_FAST_MODE			BIT(6)
> +#define AD3552R_MASK_SAMPLE_HOLD_DIFFERENTIAL_USER_EN	BIT(5)
> +#define AD3552R_MASK_SAMPLE_HOLD_USER_TRIM		(BIT(4) | BIT(3))
> +#define AD3552R_MASK_SAMPLE_HOLD_USER_ENABLE		BIT(2)
> +#define AD3552R_MASK_REFERENCE_VOLTAGE_SEL		(BIT(1) | BIT(0))
> +/* AD3552R_REG_ADDR_ERR_ALARM_MASK */
> +#define AD3552R_MASK_REF_RANGE_ALARM			BIT(6)
> +#define AD3552R_MASK_CLOCK_COUNT_ERR_ALARM		BIT(5)
> +#define AD3552R_MASK_MEM_CRC_ERR_ALARM			BIT(4)
> +#define AD3552R_MASK_SPI_CRC_ERR_ALARM			BIT(3)
> +#define AD3552R_MASK_WRITE_TO_READ_ONLY_ALARM		BIT(2)
> +#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS_ALARM	BIT(1)
> +#define AD3552R_MASK_REGISTER_ADDRESS_INVALID_ALARM	BIT(0)
> +/* AD3552R_REG_ADDR_ERR_STATUS */
> +#define AD3552R_MASK_REF_RANGE_ERR_STATUS			BIT(6)
> +#define AD3552R_MASK_DUAL_SPI_STREAM_EXCEEDS_DAC_ERR_STATUS	BIT(5)
> +#define AD3552R_MASK_MEM_CRC_ERR_STATUS				BIT(4)
> +#define AD3552R_MASK_RESET_STATUS				BIT(0)
> +/* AD3552R_REG_ADDR_POWERDOWN_CONFIG */
> +#define AD3552R_MASK_CH_DAC_POWERDOWN(ch)		BIT(4 + (ch))
> +#define AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(ch)		BIT(ch)
> +/* AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE */
> +#define AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch)		((ch) ? 0xF0 : 0x0F)
> +/* AD3552R_REG_ADDR_CH_GAIN */
With appropriate naming the defines can make this clear without the comment.
Also good to have them next to the reg addre define.

#define AD3552R_CH_GAIN_REG		WHATEVER
#define   AD3552R_CH_GAIN_RANGE_OVERRIDE		BIT(7)
etc works well.  Subtle indenting is enough to make it easy to read.

> +#define AD3552R_MASK_CH_RANGE_OVERRIDE			BIT(7)
> +#define AD3552R_MASK_CH_GAIN_SCALING_N			(BIT(6) | BIT(5))
> +#define AD3552R_MASK_CH_GAIN_SCALING_P			(BIT(4) | BIT(3))

GENMASK or BIT() for all masks (GENMASK for all multibit ones that are contiguous)

> +#define AD3552R_MASK_CH_OFFSET_POLARITY			BIT(2)
> +#define AD3552R_MASK_CH_OFFSET_BIT_8			BIT(0)
> +/* AD3552R_REG_ADDR_CH_OFFSET */
> +#define AD3552R_MASK_CH_OFFSET_BITS_0_7			0xFF
GENMASK.

> +
> +/* Useful defines */
> +#define AD3552R_NUM_CH					2
> +#define AD3552R_MASK_CH(ch)				BIT(ch)
> +#define AD3552R_PAGE_CH					2
> +#define AD3552R_MAX_REG_SIZE				3
> +#define AD3552R_READ_BIT				(1 << 7)
> +#define AD3552R_ADDR_MASK				(~AD3552R_READ_BIT)
GENMASK, rather than this unusual approach.

> +#define AD3552R_CRC_ENABLE_VALUE			(BIT(6) | BIT(1))
> +#define AD3552R_CRC_DISABLE_VALUE			(BIT(1) | BIT(0))
> +#define AD3552R_CRC_POLY				0x07
> +#define AD3552R_CRC_SEED				0xA5
> +#define AD3552R_MASK_DAC_12B				0xFFF0
GENMASK.

> +#define AD3552R_DEFAULT_CONFIG_B_VALUE			0x8
> +#define SCRATCH_PAD_TEST_VAL1				0x34
> +#define SCRATCH_PAD_TEST_VAL2				0xB2
> +#define TO_MICROS					1000000

Should be a standard definition you can use for this. Check units.h

> +#define GAIN_SCALE					1000
> +#define AD3552R_READ					true
> +#define AD3552R_WRITE					false

If these provide value, then renaming your parameter to convey the same thing
without adding a macro that just evaluates to a boolean.

> +#define LDAC_PULSE_US					10
prefix to avoid potential naming clashes.

> +
> +enum ad3552r_ch_output_range {
> +	/* Range from 0 V to 2.5 V. Requires Rfb1x connection */
> +	AD3552R_CH_OUTPUT_RANGE_0__2_5V,
> +	/* Range from 0 V to 5 V. Requires Rfb1x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_0__5V,
> +	/* Range from 0 V to 10 V. Requires Rfb2x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_0__10V,
> +	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_NEG_5__5V,
> +	/* Range from -6.5 V to 3.5 V. Requires Rfb4x connection  */
> +	AD3552R_CH_OUTPUT_RANGE_NEG_10__10V,
> +};
> +
> +static const s32 ch_ranges[][2] = {
> +	[AD3552R_CH_OUTPUT_RANGE_0__2_5V]	= {0, 2500},
> +	[AD3552R_CH_OUTPUT_RANGE_0__5V]		= {0, 5000},
> +	[AD3552R_CH_OUTPUT_RANGE_0__10V]	= {0, 10000},
> +	[AD3552R_CH_OUTPUT_RANGE_NEG_5__5V]	= {-5000, 5000},
> +	[AD3552R_CH_OUTPUT_RANGE_NEG_10__10V]	= {-10000, 10000}
> +};
> +
> +enum ad3552r_ch_gain_scaling {
> +	/* Gain scaling of 1 */
> +	AD3552R_CH_GAIN_SCALING_1,
> +	/* Gain scaling of 0.5 */
> +	AD3552R_CH_GAIN_SCALING_0_5,
> +	/* Gain scaling of 0.25 */
> +	AD3552R_CH_GAIN_SCALING_0_25,
> +	/* Gain scaling of 0.125 */
> +	AD3552R_CH_GAIN_SCALING_0_125,
> +};
> +
> +/* Gain * GAIN_SCALE */
> +static const s32 gains_scaling_table[] = {
> +	[AD3552R_CH_GAIN_SCALING_1]		= 1000,
> +	[AD3552R_CH_GAIN_SCALING_0_5]		= 500,
> +	[AD3552R_CH_GAIN_SCALING_0_25]		= 250,
> +	[AD3552R_CH_GAIN_SCALING_0_125]		= 125
> +};
> +
> +enum ad3552r_dev_attributes {
> +	/* - Direct register values */
> +	/* From 0-3 */
> +	AD3552R_SDO_DRIVE_STRENGTH,
> +	/*
> +	 * 0 -> Internal Vref, vref_io pin floating (default)
> +	 * 1 -> Internal Vref, vref_io driven by internal vref
> +	 * 2 or 3 -> External Vref
> +	 */
> +	AD3552R_VREF_SELECT,
> +	/* Enable / Disable CRC */
> +	AD3552R_CRC_ENABLE,
> +	/* Spi mode: Strandard, Dual or Quad */
> +	AD3552R_SPI_MULTI_IO_MODE,
> +	/* Spi data rate: Single or dual */
> +	AD3552R_SPI_DATA_RATE,
> +	/* Dual spi synchronous mode */
> +	AD3552R_SPI_SYNCHRONOUS_ENABLE,
> +
> +	/* - Direct register values (Private) */
> +	/* Read registers in ascending order if set. Else descending */
> +	AD3552R_ADDR_ASCENSION,
> +	/* Single instruction mode if set. Else, stream mode */
> +	AD3552R_SINGLE_INST,
> +	/* Number of addresses to loop on when stream writing. */
> +	AD3552R_STREAM_MODE,
> +	/* Keep stream value if set. */
> +	AD3552R_STREAM_LENGTH_KEEP_VALUE,
> +};
> +
> +enum ad3552r_ch_attributes {
> +	/* DAC powerdown */
> +	AD3552R_CH_DAC_POWERDOWN,
> +	/* DAC amplifier powerdown */
> +	AD3552R_CH_AMPLIFIER_POWERDOWN,
> +	/* Select the output range. Select from enum ad3552r_ch_output_range */
> +	AD3552R_CH_OUTPUT_RANGE_SEL,
> +	/*
> +	 * Over-rider the range selector in order to manually set the output
> +	 * voltage range
> +	 */
> +	AD3552R_CH_RANGE_OVERRIDE,
> +	/* Manually set the offset voltage */
> +	AD3552R_CH_GAIN_OFFSET,
> +	/* Sets the polarity of the offset. */
> +	AD3552R_CH_GAIN_OFFSET_POLARITY,
> +	/* PDAC gain scaling */
> +	AD3552R_CH_GAIN_SCALING_P,
> +	/* NDAC gain scaling */
> +	AD3552R_CH_GAIN_SCALING_N,
> +	/* Trigger a software LDAC */
> +	AD3552R_CH_TRIGGER_SOFTWARE_LDAC,
> +	/* Hardware LDAC Mask */
> +	AD3552R_CH_HW_LDAC_MASK,
> +	/* Rfb value */
> +	AD3552R_CH_RFB,
> +	/* Channel select. When set allow Input -> DAC and Mask -> DAC */
> +	AD3552R_CH_SELECT,
> +	/* Raw value to be set to dac */
> +	AD3552R_CH_CODE
> +};
> +
> +struct ad3552r_ch_data {
> +	u16	gain_offset : 9;
> +	u16	range_override : 1;
> +	u16	n : 2;
> +	u16	p : 2;
> +	u16	offset_polarity : 1;
I'd be tempted to not use bitfields here and instead just used fairly small types and m ake
the various booleans actual booleans. 

Structure will end up a bit bigger but that shouldn't matter too much if at all given how large
it is anyway.

> +	u16	rfb;
> +	u8	range;
> +	s32	scale_int;
> +	s32	scale_dec;
> +	s32	offset_int;
> +	s32	offset_dec;
> +	bool	prec_en;
> +};
> +
> +struct ad3552r_desc {
> +	struct iio_dev		*indio_dev;
As below, get rid of this.

> +	struct mutex		lock;
What is the scope of the lock.  All locks need comments stating the scope.

> +	struct gpio_desc	*gpio_reset;
> +	struct gpio_desc	*gpio_ldac;
> +	struct spi_device	*spi;
> +	struct ad3552r_ch_data	ch_data[AD3552R_NUM_CH];
> +	struct iio_chan_spec	channels[AD3552R_NUM_CH + 1];
> +	unsigned long		enabled_ch;
> +	unsigned int		num_ch;
> +	bool			use_input_regs;
> +	u8 buf_data[2 * (AD3552R_MAX_REG_SIZE + 2)] ____cacheline_aligned;

Comment needed to explain the size of this.

> +};
> +
> +static const u16 addr_mask_map[][2] = {
> +	[AD3552R_ADDR_ASCENSION] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> +			AD3552R_MASK_ADDR_ASCENSION
> +	},
> +	[AD3552R_SINGLE_INST] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +			AD3552R_MASK_SINGLE_INST
> +	},
> +	[AD3552R_STREAM_MODE] = {
> +			AD3552R_REG_ADDR_STREAM_MODE,
> +			AD3552R_MASK_LENGTH
> +	},
> +	[AD3552R_STREAM_LENGTH_KEEP_VALUE] = {
> +			AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +			AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE
> +	},
> +	[AD3552R_SDO_DRIVE_STRENGTH] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_SDO_DRIVE_STRENGTH
> +	},
> +	[AD3552R_VREF_SELECT] = {
> +			AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
> +			AD3552R_MASK_REFERENCE_VOLTAGE_SEL
> +	},
> +	[AD3552R_CRC_ENABLE] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_C,
> +			AD3552R_MASK_CRC_ENABLE
> +	},
> +	[AD3552R_SPI_MULTI_IO_MODE] = {
> +			AD3552R_REG_ADDR_TRANSFER_REGISTER,
> +			AD3552R_MASK_MULTI_IO_MODE
> +	},
> +	[AD3552R_SPI_DATA_RATE] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_SPI_CONFIG_DDR
> +	},
> +	[AD3552R_SPI_SYNCHRONOUS_ENABLE] = {
> +			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> +			AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN
> +	},
> +};
> +
> +/* 0 -> reg addr, 1->ch0 mask, 2->ch1 mask */
> +static const u16 addr_mask_map_ch[][3] = {
> +	[AD3552R_CH_DAC_POWERDOWN] = {
> +			AD3552R_REG_ADDR_POWERDOWN_CONFIG,
> +			AD3552R_MASK_CH_DAC_POWERDOWN(0),
> +			AD3552R_MASK_CH_DAC_POWERDOWN(1)
> +	},
> +	[AD3552R_CH_AMPLIFIER_POWERDOWN] = {
> +			AD3552R_REG_ADDR_POWERDOWN_CONFIG,
> +			AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(0),
> +			AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(1)
> +	},
> +	[AD3552R_CH_OUTPUT_RANGE_SEL] = {
> +			AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
> +			AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0),
> +			AD3552R_MASK_CH_OUTPUT_RANGE_SEL(1)
> +	},
> +	/*
> +	 * This attributes are update by the chip on 16B and 24B no matter to
> +	 * what register the write is done
> +	 */
> +	[AD3552R_CH_TRIGGER_SOFTWARE_LDAC] = {
> +			AD3552R_REG_ADDR_SW_LDAC_16B,
> +			AD3552R_MASK_CH(0),
> +			AD3552R_MASK_CH(1)
> +	},
> +	[AD3552R_CH_HW_LDAC_MASK] = {
> +			AD3552R_REG_ADDR_HW_LDAC_16B,
> +			AD3552R_MASK_CH(0),
> +			AD3552R_MASK_CH(1)
> +	},
> +	[AD3552R_CH_SELECT] = {
> +			AD3552R_REG_ADDR_CH_SELECT_16B,
> +			AD3552R_MASK_CH(0),
> +			AD3552R_MASK_CH(1)
> +	}
> +};
> +
> +static u8 _ad3552r_reg_len(u8 addr)
> +{
> +	if (addr > AD3552R_REG_ADDR_MAX)
> +		return 0;
> +
> +	switch (addr) {
> +	case AD3552R_REG_ADDR_HW_LDAC_16B:
> +	case AD3552R_REG_ADDR_CH_SELECT_16B:
> +	case AD3552R_REG_ADDR_SW_LDAC_16B:
> +	case AD3552R_REG_ADDR_HW_LDAC_24B:
> +	case AD3552R_REG_ADDR_CH_SELECT_24B:
> +	case AD3552R_REG_ADDR_SW_LDAC_24B:
> +		return 1;
> +	default:
> +		break;
> +	}
> +
> +	if (addr > AD3552R_REG_ADDR_HW_LDAC_24B)
> +		return 3;
> +	if (addr > AD3552R_REG_ADDR_HW_LDAC_16B)
> +		return 2;
> +
> +	return 1;
> +}
> +
> +/* SPI transfer to device */
> +static int ad3552r_transfer(struct ad3552r_desc *dac, u8 addr, u32 len,
> +			    u8 *data, bool is_read)
> +{
> +	int err;
> +	u8 instr;
> +
> +	instr = addr & AD3552R_ADDR_MASK;
> +	instr |= is_read ? AD3552R_READ_BIT : 0;
> +	dac->buf_data[0] = instr;
> +	if (is_read) {
> +		err = spi_write_then_read(dac->spi, dac->buf_data, 1,
> +					  dac->buf_data + 1, len);
> +		if (err)
> +			return err;
> +
> +		memcpy(data, dac->buf_data + 1, len);
> +
> +		return 0;
> +	}
> +
> +	memcpy(dac->buf_data + 1, data, len);
> +	return spi_write(dac->spi, dac->buf_data, len + 1);
> +}
> +
> +static int ad3552r_write_reg(struct ad3552r_desc *dac, u8 addr, u16 val)
> +{
> +	u8 reg_len, buf[AD3552R_MAX_REG_SIZE] = { 0 };
> +
> +	reg_len = _ad3552r_reg_len(addr);
> +	if (!reg_len)
> +		return -EINVAL;
> +
> +	if (reg_len == 2)
> +		/* Only DAC register are 2 bytes wide */
> +		val &= AD3552R_MASK_DAC_12B;
> +	if (reg_len == 1)
> +		buf[0] = val & 0xFF;
> +	else
> +		/* reg_len can be 2 or 3, but 3rd bytes needs to be set to 0 */
> +		*((u16 *)buf) = cpu_to_be16(val);

put_unaligned_be16()  no one said buf is 2 byte aligned...

> +
> +	return ad3552r_transfer(dac, addr, reg_len, buf,
> +				AD3552R_WRITE);

Single line.

> +}
> +
> +static int ad3552r_read_reg(struct ad3552r_desc *dac, u8 addr, u16 *val)
> +{
> +	int err;
> +	u8  reg_len, buf[AD3552R_MAX_REG_SIZE] = { 0 };
> +
> +	reg_len = _ad3552r_reg_len(addr);
> +	if (!reg_len)
> +		return -EINVAL;
> +
> +	err = ad3552r_transfer(dac, addr, reg_len, buf, AD3552R_READ);
> +	if (err)
> +		return err;
> +
> +	if (reg_len == 1)
> +		*val = buf[0];
> +	else
> +		/* reg_len can be 2 or 3, but only first 2 bytes are relevant */
> +		*val = be16_to_cpu(*((u16 *)buf));

get_unaligned_be16()

> +
> +	return 0;
> +}
> +
> +/* Update field of a register, shift val if needed */
> +static int ad3552r_update_reg_field(struct ad3552r_desc *dac, u8 addr, u16 mask,
> +				    u16 val)
> +{
> +	int ret;
> +	u16 reg;
> +
> +	ret = ad3552r_read_reg(dac, addr, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	reg = (reg & ~mask) | (val << __ffs(mask));
> +
> +	return ad3552r_write_reg(dac, addr, reg);
> +}
> +
> +static int ad3552r_set_dev_value(struct ad3552r_desc *dac,
> +				 enum ad3552r_dev_attributes attr,
> +				 u16 val)
> +{
> +	switch (attr) {
> +	case AD3552R_SPI_MULTI_IO_MODE:
> +	case AD3552R_SPI_DATA_RATE:
> +	case AD3552R_SPI_SYNCHRONOUS_ENABLE:
> +	case AD3552R_CRC_ENABLE:
> +		/* Not implemented */
> +		return -EINVAL;
> +	default:
> +		return ad3552r_update_reg_field(dac, addr_mask_map[attr][0],
> +						addr_mask_map[attr][1], val);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad3552r_set_offset_value(struct ad3552r_desc *dac, u8 ch, int val)
> +{
> +	int err;
> +
> +	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_CH_OFFSET(ch),
> +				val & AD3552R_MASK_CH_OFFSET_BITS_0_7);
> +	if (err)
> +		return err;
> +
> +	err = ad3552r_update_reg_field(dac,
> +				       AD3552R_REG_ADDR_CH_GAIN(ch),
> +				       AD3552R_MASK_CH_OFFSET_BIT_8,
> +				       (val >> 8) & AD3552R_MASK_CH_OFFSET_BIT_8);
> +	if (err)
> +		return err;
> +
> +	dac->ch_data[ch].gain_offset = val;
> +
> +	return 0;
> +}
> +
> +static int ad3552r_set_gain_value(struct ad3552r_desc *dac,
> +				  enum ad3552r_ch_attributes attr,
> +				  u8 ch,
> +				  int val)
> +{
> +	int reg_mask, err;
> +
> +	if (attr == AD3552R_CH_GAIN_OFFSET)
> +		return ad3552r_set_offset_value(dac, ch, val);
> +
> +	switch (attr) {
> +	case AD3552R_CH_RANGE_OVERRIDE:
> +		val = !!val;
> +		reg_mask = AD3552R_MASK_CH_RANGE_OVERRIDE;
> +		break;
> +	case AD3552R_CH_GAIN_OFFSET_POLARITY:
> +		val = !!val;
> +		reg_mask = AD3552R_MASK_CH_OFFSET_POLARITY;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_P:
> +		reg_mask = AD3552R_MASK_CH_GAIN_SCALING_P;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_N:
> +		reg_mask = AD3552R_MASK_CH_GAIN_SCALING_N;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	err = ad3552r_update_reg_field(dac, AD3552R_REG_ADDR_CH_GAIN(ch),
> +				       reg_mask, val);
> +	if (err)
> +		return err;
> +
> +	switch (attr) {
> +	case AD3552R_CH_RANGE_OVERRIDE:
> +		dac->ch_data[ch].range_override = val;
> +		break;
> +	case AD3552R_CH_GAIN_OFFSET_POLARITY:
> +		dac->ch_data[ch].offset_polarity = val;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_P:
> +		dac->ch_data[ch].p = val;
> +		break;
> +	case AD3552R_CH_GAIN_SCALING_N:
> +		dac->ch_data[ch].n = val;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Iterate over mask and write required bytes */
> +static int ad3552r_write_codes(struct ad3552r_desc *dac, u32 mask, u8 *vals)
> +{
> +	int err, i, reg_len, k = 0;
> +	unsigned long lmask = mask;
> +	u8 addr, buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE];
> +	u16 val;
> +
> +	/* If writing to consecutive registers do just one transfer */
> +
> +	if (mask == (AD3552R_MASK_CH(0) | AD3552R_MASK_CH(1)) &&
> +	    dac->ch_data[0].prec_en == dac->ch_data[1].prec_en) {
> +		if (dac->use_input_regs) {
> +			if (dac->ch_data[0].prec_en)
> +				addr = AD3552R_REG_ADDR_CH_INPUT_24B(1);
> +			else
> +				addr = AD3552R_REG_ADDR_CH_INPUT_16B(1);
> +		} else {
> +			if (dac->ch_data[0].prec_en)
> +				addr = AD3552R_REG_ADDR_CH_DAC_24B(1);
> +			else
> +				addr = AD3552R_REG_ADDR_CH_DAC_16B(1);
> +		}
> +
> +		reg_len = _ad3552r_reg_len(addr);
> +		buff[0] = vals[0];
> +		buff[reg_len] = vals[2];
> +		if (dac->ch_data[0].prec_en) {
> +			/* Reg_len is 3 here */
> +			buff[1] = vals[1];
> +			buff[2] = 0;
> +			buff[4] = vals[3];
> +			buff[5] = 0;
> +		} else {
> +			buff[1] = vals[1] & 0xf0;
> +			buff[3] = vals[3] & 0xf0;
> +		}
> +
> +		err = ad3552r_transfer(dac, addr, reg_len * 2, buff,
> +				       AD3552R_WRITE);
> +		if (err)
> +			return err;
> +	} else {
> +
> +		k = 0;
> +		for_each_set_bit(i, &lmask, AD3552R_NUM_CH + 1) {
> +			/* Writing to mask CH */
> +			if (i == AD3552R_PAGE_CH)
> +				addr = dac->ch_data[0].prec_en ?
> +					AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B :
> +					AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B;
> +			else
> +				addr = dac->ch_data[i].prec_en ?
> +					AD3552R_REG_ADDR_CH_INPUT_24B(i) :
> +					AD3552R_REG_ADDR_CH_INPUT_16B(i);
> +
> +			reg_len = _ad3552r_reg_len(addr);
> +			val = be16_to_cpu(*((u16 *)(vals + k)));
> +
> +			k += 2;
> +			err = ad3552r_write_reg(dac, addr, val);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	if (dac->gpio_ldac) {
> +		gpiod_set_value_cansleep(dac->gpio_ldac, 0);
> +		usleep_range(LDAC_PULSE_US, LDAC_PULSE_US + 10);
> +		gpiod_set_value_cansleep(dac->gpio_ldac, 1);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad3552r_get_ch_value(struct ad3552r_desc *dac,
> +				enum ad3552r_ch_attributes attr,
> +				u8 ch,
> +				u16 *val)
> +{
> +	int ret;
> +	u16 reg;
> +	u8  addr;
> +	u16 mask;
> +
> +	/* Attributes not defined in addr_mask_map_ch */
> +	switch (attr) {
> +	case AD3552R_CH_CODE:
> +		return ad3552r_read_reg(dac, AD3552R_REG_ADDR_CH_DAC_24B(ch),
> +					val);
> +	case AD3552R_CH_RFB:
> +		*val = dac->ch_data[ch].rfb;
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	if (attr >= AD3552R_CH_RANGE_OVERRIDE &&
> +	    attr <= AD3552R_CH_GAIN_SCALING_N)

I'd assume this couldn't happen or am I missing a path by which it can?
Don't bother checking for things we know never happen.

> +		return -EINVAL;
> +
> +	addr = addr_mask_map_ch[attr][0];
> +	if (addr == AD3552R_REG_ADDR_SW_LDAC_24B ||
> +	    addr == AD3552R_REG_ADDR_SW_LDAC_16B) {
Any plausible way you can actually hit this?  If so I'd expect it to be closed
down earlier than this point.

> +		dev_err(&dac->indio_dev->dev, "Write only registers\n");
> +		/* LDAC are write only registers */
> +		return -EINVAL;
> +	}
> +
> +	ret = ad3552r_read_reg(dac, addr, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	mask = addr_mask_map_ch[attr][ch + 1];
> +	*val = (reg & mask) >> __ffs(mask);

FIELD_GET might do the job here for you though with everything dynamic
perhaps not.

> +
> +	return 0;
> +}
> +
> +static int ad3552r_set_ch_value(struct ad3552r_desc *dac,
> +				enum ad3552r_ch_attributes attr,
> +				u8 ch,
> +				u16 val)
> +{
> +	int ret;
> +
> +	/* Attributes not defined in addr_mask_map_ch */
> +	switch (attr) {
> +	case AD3552R_CH_CODE:
> +		return ad3552r_write_reg(dac, AD3552R_REG_ADDR_CH_DAC_24B(ch),
> +					 val);
> +	case AD3552R_CH_RFB:
> +		dac->ch_data[ch].rfb = val;
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	if (attr >= AD3552R_CH_RANGE_OVERRIDE &&
> +	    attr <= AD3552R_CH_GAIN_SCALING_N)
> +		return ad3552r_set_gain_value(dac, attr, ch, val);
> +
> +	/* Update register related to attributes in chip */
> +	ret = ad3552r_update_reg_field(dac, addr_mask_map_ch[attr][0],
> +				       addr_mask_map_ch[attr][ch + 1], val);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Update software structures */
> +	if (attr == AD3552R_CH_OUTPUT_RANGE_SEL) {
> +		val %= AD3552R_CH_OUTPUT_RANGE_NEG_10__10V + 1;
> +		dac->ch_data[ch].range = val;
> +	}
> +
> +	return ret;
> +}
> +
> +static ssize_t ad3552r_write_ext(struct iio_dev *indio_dev,
> +				 uintptr_t private,
> +				 const struct iio_chan_spec *chan,
> +				 const char *buf, size_t len)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	int val, frac, err;
> +
> +	err = iio_str_to_fixpoint(buf, 0, &val, &frac);
> +	if (err < 0)
> +		return err;
> +
> +	dac->ch_data[chan->channel].prec_en = !!val;
> +
> +	return len;
> +}
> +
> +static ssize_t ad3552r_read_ext(struct iio_dev *indio_dev,
> +				uintptr_t private,
> +				const struct iio_chan_spec *chan,
> +				char *buf)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	int val;
> +
> +	if (private != 0)
> +		return -EINVAL;
> +
> +	val = dac->ch_data[chan->channel].prec_en;
> +
> +	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +}
> +
> +#define AD3552R_CH_ATTR(_name, _what) { \
> +	.name = _name, \
> +	.read = ad3552r_read_ext, \
> +	.write = ad3552r_write_ext, \
> +	.private = _what, \
> +	.shared = IIO_SEPARATE, \
> +}
> +
> +static const struct iio_chan_spec_ext_info ad3552r_ext_info[] = {
> +	AD3552R_CH_ATTR("precision_mode_en", 0),
> +	{},
> +};
> +
> +#define AD3552R_CH_DAC(_idx) ((struct iio_chan_spec) {		\
> +	.type = IIO_VOLTAGE,					\
> +	.output = true,						\
> +	.indexed = true,					\
> +	.channel = _idx,					\
> +	.scan_index = _idx,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_BE,				\
> +	},							\
> +	.ext_info = ad3552r_ext_info,				\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +				BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_ENABLE) |	\
> +				BIT(IIO_CHAN_INFO_OFFSET),	\
> +})
> +
> +#define AD3552R_CH_DAC_PAGE(_idx) ((struct iio_chan_spec) {	\
> +	.type = IIO_VOLTAGE,					\
> +	.output = true,						\
> +	.indexed = true,					\
> +	.channel = _idx,					\
> +	.scan_index = _idx,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_BE,				\
> +	},							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +	.modified = 1,						\
> +	.channel2 = IIO_MOD_X_AND_Z,				\

This needs a comment.  What is this channel?  MOD_X_AND_Z is a pretty
non obvious for a DAC!


> +})
> +
> +static int ad3552r_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long mask)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	u16 tmp_val;
> +	int err;
> +	u8 ch = chan->channel;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&dac->lock);
> +		if (chan->channel == AD3552R_PAGE_CH)
> +			err = ad3552r_read_reg(dac,
> +					       AD3552R_REG_ADDR_DAC_PAGE_MASK_24B,
> +					       &tmp_val);
> +		else
> +			err = ad3552r_get_ch_value(dac, AD3552R_CH_CODE, ch,
> +						   &tmp_val);
> +		if (err < 0) {
> +			mutex_unlock(&dac->lock);
> +			return err;
> +		}
> +
> +		*val = tmp_val;
> +		mutex_unlock(&dac->lock);
> +		break;
> +	case IIO_CHAN_INFO_ENABLE:
> +		mutex_lock(&dac->lock);
> +		err = ad3552r_get_ch_value(dac, AD3552R_CH_DAC_POWERDOWN,
> +					   ch, &tmp_val);
> +		if (err < 0) {
> +			mutex_unlock(&dac->lock);
> +			return err;
> +		}
> +		*val = !tmp_val;
> +		mutex_unlock(&dac->lock);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = dac->ch_data[ch].scale_int;
> +		*val2 = dac->ch_data[ch].scale_dec;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = dac->ch_data[ch].offset_int;
> +		*val2 = dac->ch_data[ch].offset_dec;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int ad3552r_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val,
> +			     int val2,
> +			     long mask)
> +{
> +	struct ad3552r_desc *dac = iio_priv(indio_dev);
> +	enum ad3552r_ch_attributes attr;
> +	int err = 0;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->channel == AD3552R_PAGE_CH) {
> +			mutex_lock(&dac->lock);
> +			err = ad3552r_write_reg(dac,
> +						AD3552R_REG_ADDR_DAC_PAGE_MASK_24B,
> +						val);
> +			mutex_unlock(&dac->lock);
> +
> +			return err;
> +		}
> +
> +		attr = AD3552R_CH_CODE;
> +		break;
> +	case IIO_CHAN_INFO_ENABLE:
> +		attr = AD3552R_CH_DAC_POWERDOWN;
> +		val = !val;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&dac->lock);
> +	err = ad3552r_set_ch_value(dac, attr, chan->channel, val);
> +	mutex_unlock(&dac->lock);
> +
> +	return err;
> +}
> +
> +static int ad3552r_update_scan_mode(struct iio_dev *indio_dev,
> +				    const unsigned long *scan_mask)
> +{
> +	u32 mask;
> +
> +	mask = *scan_mask;
> +	/* If writing to mask, can't write to other channels */
> +	if ((mask & AD3552R_MASK_CH(AD3552R_PAGE_CH)) &&
> +	    (mask & (~AD3552R_MASK_CH(AD3552R_PAGE_CH))))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/*
> + * Device type specific information.
> + */
> +static const struct iio_info ad3552r_iio_info = {
> +	.read_raw = ad3552r_read_raw,
> +	.write_raw = ad3552r_write_raw,
> +	.update_scan_mode = ad3552r_update_scan_mode
> +};
> +
> +static irqreturn_t ad3552r_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func	*pf = p;
> +	struct iio_dev		*indio_dev = pf->indio_dev;
> +	struct iio_buffer	*buf = indio_dev->buffer;
> +	struct ad3552r_desc	*dac = iio_priv(indio_dev);
> +	char			buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE];
> +	int			err;
> +
> +	memset(buff, 0, sizeof(buff));
> +	mutex_lock(&dac->lock);
> +	err = iio_buffer_remove_sample(buf, buff);
> +	if (err)
> +		goto end;
> +
> +	err = ad3552r_write_codes(dac, *indio_dev->active_scan_mask, buff);
> +	if (err)
> +		goto end;
> +
> +end:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	mutex_unlock(&dac->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int ad3552r_setup_trigger_buffer(struct device *dev,
> +					struct iio_dev *indio_dev, int irq)
> +{
> +	struct ad3552r_desc	*dac = iio_priv(indio_dev);

This sort of careful indenting tends to cause noise in the long run because future
changes require the indenting to be updated to be consistent.
I would encourage you to drop it and just have a single space for all definitions.


> +	struct iio_trigger	*hwtrig;
> +	int			err;
> +
> +	/* Configure trigger buffer */

Function is fairly self explanatory. I would drop the comment.  Comments rot
so best not have them if they don't provide substantial information not
available from a quick glance at the code.   Often the "why" not the "what".

> +	err = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
> +						  &ad3552r_trigger_handler,
> +						  IIO_BUFFER_DIRECTION_OUT,
> +						  NULL,
> +						  NULL);
> +	if (err)
> +		return err;
> +
> +	if (!irq)
> +		return 0;
> +
> +	hwtrig = devm_iio_trigger_alloc(dev, "%s-ldac-dev%d",
> +					indio_dev->name,
> +					iio_device_id(indio_dev));
> +	if (!hwtrig)
> +		return -ENOMEM;
> +
> +	hwtrig->dev.parent = dev;
> +	iio_trigger_set_drvdata(hwtrig, dac);
> +	err = devm_iio_trigger_register(dev, hwtrig);
> +	if (err < 0)
> +		return err;
> +
> +	return devm_request_threaded_irq(dev, irq,
> +					 iio_trigger_generic_data_rdy_poll,

What is this trigger? Already been raised in other review...

> +					 NULL,
> +					 IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					 indio_dev->name,
> +					 hwtrig);
> +}
> +
> +static int ad3552r_check_scratch_pad(struct ad3552r_desc *dac)
> +{
> +	const u16 val1 = SCRATCH_PAD_TEST_VAL1;
> +	const u16 val2 = SCRATCH_PAD_TEST_VAL2;
> +	u16 val;
> +	int err;
> +
> +	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, val1);
> +	if (err < 0)
> +		return err;
> +
> +	err = ad3552r_read_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, &val);
> +	if (err < 0)
> +		return err;
> +
> +	if (val1 != val)
> +		return -ENODEV;
> +
> +	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, val2);
> +	if (err < 0)
> +		return err;
> +
> +	err = ad3552r_read_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, &val);
> +	if (err < 0)
> +		return err;
> +
> +	if (val2 != val)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +struct reg_addr_pool {
> +	struct ad3552r_desc *dac;
> +	u8		    addr;
> +};
> +
> +static u16 ad3552r_read_reg_pool(struct reg_addr_pool *addr)
pool?

> +{
> +	u16 val = 0;
> +
> +	ad3552r_read_reg(addr->dac, addr->addr, &val);

Why not have ad3552r_read_reg return an int that is val?  Seems that would
make life easier for you.

> +
> +	return val;
> +}
> +
> +static int ad3552r_reset(struct ad3552r_desc *dac)
> +{
> +	struct reg_addr_pool addr;
> +	int ret;
> +	u16 val;
> +
> +	dac->gpio_reset = devm_gpiod_get_optional(&dac->spi->dev, "reset",
> +						  GPIOD_OUT_LOW);
> +	if (IS_ERR(dac->gpio_reset))
> +		return PTR_ERR(dac->gpio_reset);
> +
> +	if (dac->gpio_reset) {
> +		/* Perform hardware reset */
> +		usleep_range(10, 20);
> +		gpiod_set_value_cansleep(dac->gpio_reset, 1);
> +	} else {
> +		/* Perform software reset if no GPIO provided */
> +		ret = ad3552r_update_reg_field(dac, AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
> +					       AD3552R_MASK_SOFTWARE_RESET,
> +					       AD3552R_MASK_SOFTWARE_RESET);
> +		if (ret < 0)
> +			return ret;
> +
> +	}
> +
> +	addr.dac = dac;
> +	addr.addr = AD3552R_REG_ADDR_INTERFACE_CONFIG_B;
> +	ret = readx_poll_timeout(ad3552r_read_reg_pool,
> +				 &addr,
> +				 val,

Where it doesn't greatly hurt readability, try to keep multiple parameters on one line
as long as you are below 80 chars.

> +				 (val == AD3552R_DEFAULT_CONFIG_B_VALUE),

why the brackets?  cond is appropriately bracketed in the macro definitions so you
shouldn't need them here.

> +				 5000,
> +				 50000);
> +	if (ret) {
> +		dev_err(&dac->spi->dev, "Err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = readx_poll_timeout(ad3552r_read_reg_pool,
> +				 &addr,
> +				 val,
> +				 (!(val & AD3552R_MASK_INTERFACE_NOT_READY)),
Again, too many brackets and lines.
> +				 5000,
> +				 50000);
> +	if (ret) {
> +		dev_err(&dac->spi->dev, "Err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = ad3552r_set_dev_value(dac, AD3552R_ADDR_ASCENSION, 0);
> +	if (ret < 0)
I think this can only return <= 0 to so you should be fine with

return ad3552r_set_dev_value()

If I'm wrong you might want to push any forcing of no positive returns
into a lower level function.

> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void ad3552r_get_custom_range(struct ad3552r_desc *dac, s32 i, s32 *v_min,
> +				     s32 *v_max)
> +{
> +	s64 vref, tmp, common, offset, gn, gp;
> +	/*
> +	 * From datasheet formula (In Volts):
> +	 *	Vmin = 2.5 + [(GainN + Offset / 1024) * 2.5 * Rfb * 1.03]
> +	 *	Vmax = 2.5 - [(GainP + Offset / 1024) * 2.5 * Rfb * 1.03]
> +	 * Calculus are converted to milivolts
> +	 */
> +	vref = 2500;
> +	/* 2.5 * 1.03 * 1000 (To mV) */
> +	common = 2575 * dac->ch_data[i].rfb;
> +	offset = dac->ch_data[i].gain_offset;
> +	if (dac->ch_data[i].offset_polarity)
> +		offset *= -1;
> +
> +	gn = gains_scaling_table[dac->ch_data[i].n];
> +	tmp = (1024 * gn + GAIN_SCALE * offset) * common;
> +	tmp = div_s64(tmp, 1024  * GAIN_SCALE);
> +	*v_max = vref + tmp;
> +
> +	gp = gains_scaling_table[dac->ch_data[i].p];
> +	tmp = (1024 * gp - GAIN_SCALE * offset) * common;
> +	tmp = div_s64(tmp, 1024 * GAIN_SCALE);
> +	*v_min = vref - tmp;
> +}
> +
> +static void ad3552r_set_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
Would expect something called 'set' to be writing to hardware.  Perhaps
calc_gain_and_offset?

> +{
> +	s32 idx, v_max, v_min, span, rem;
> +	s64 tmp;
> +
> +	if (dac->ch_data[ch].range_override) {
> +		ad3552r_get_custom_range(dac, ch, &v_min, &v_max);
> +	} else {
> +		/* Normal range */
> +		idx = dac->ch_data[ch].range;
> +		v_max = ch_ranges[idx][1];
> +		v_min = ch_ranges[idx][0];
> +	}
> +
> +	/*
> +	 * From datasheet formula:
> +	 *	Vout = Span * (D / 65536) + Vmin
> +	 * Converted to scale and offset:
> +	 *	Scale = Span / 65536
> +	 *	Offset = 65536 * Vmin / Span
> +	 *
> +	 * Reminders are in micros in order to be printed as
> +	 * IIO_VAL_INT_PLUS_MICRO
> +	 */
> +	span = v_max - v_min;
> +	dac->ch_data[ch].scale_int = div_s64_rem(span, 65536, &rem);
> +	dac->ch_data[ch].scale_dec = DIV_ROUND_CLOSEST((s64)rem * TO_MICROS,
> +							65536);
> +
> +	dac->ch_data[ch].offset_int = div_s64_rem(v_min * 65536, span,
> +							&rem);
> +	tmp = (s64)rem * TO_MICROS;
> +	dac->ch_data[ch].offset_dec = div_s64(tmp, span);
> +}
> +
> +static const char * const gain_dts_names[] = {
> +	"adi,gain-scaling-p",
> +	"adi,gain-scaling-n",
> +	"adi,rfb"
> +};
> +
> +static int ad3552r_configure_device(struct ad3552r_desc *dac)
> +{
> +	static const enum ad3552r_ch_attributes gain_attrs[] = {
> +		AD3552R_CH_GAIN_SCALING_P,
> +		AD3552R_CH_GAIN_SCALING_N,
> +		AD3552R_CH_RFB
> +	};
> +	struct fwnode_handle	*child, *custom_gain_child = NULL;

Odd spacing. Also easier to read as two separate lines.  As a general
rule don't share lines if setting the variables to anything.


> +	int i, err, cnt = 0;
> +	u32 val, ch;
> +	bool is_custom;
> +
> +	dac->gpio_ldac = devm_gpiod_get_optional(&dac->spi->dev, "ldac",
As you have a bunch of accesses to &dac->spi->dev in here, I'd have
a local struct device *dev; 

> +						 GPIOD_OUT_HIGH);
> +	if (IS_ERR(dac->gpio_ldac))
> +		return PTR_ERR(dac->gpio_ldac);
> +
> +	dac->use_input_regs = device_property_read_bool(&dac->spi->dev,
> +							"adi,synch_channels");
> +
> +	err = device_property_read_u32(&dac->spi->dev, "adi,vref-select", &val);
> +	if (!err) {
> +		if (val > 2) {
> +			dev_err(&dac->spi->dev, "%s must be less than 3\n",
> +				"adi,vref-select");
> +			return -EINVAL;
> +		}
> +		err = ad3552r_set_dev_value(dac, AD3552R_VREF_SELECT, val);
> +		if (err)
> +			return err;

I gave comments on this stuff in the review of the binding patch.  Will follow
through to changes in here.

> +	}
> +
> +	err = device_property_read_u32(&dac->spi->dev, "adi,sdo-drive-strength",
> +				       &val);
> +	if (!err) {
> +		if (val > 3) {
> +			dev_err(&dac->spi->dev, "%s must be less than 4\n",
> +				"adi,sdo-drive-strength");
> +			return -EINVAL;
> +		}
> +		err = ad3552r_set_dev_value(dac, AD3552R_SDO_DRIVE_STRENGTH,
> +					    val);
> +		if (err)
> +			return err;
> +	}
> +
> +	dac->num_ch = device_get_child_node_count(&dac->spi->dev);
> +	if (!dac->num_ch) {
> +		dev_err(&dac->spi->dev, "No channels defined\n");
> +		return -ENODEV;
> +	}
> +
> +	device_for_each_child_node(&dac->spi->dev, child) {
> +		err = fwnode_property_read_u32(child, "reg", &ch);
> +		if (err) {
> +			dev_err(&dac->spi->dev, "Mandory reg property missing\n");
> +			goto put_child;
> +		}
> +		if (ch >= AD3552R_NUM_CH) {
> +			dev_err(&dac->spi->dev, "reg must be less than %d\n",
> +				AD3552R_NUM_CH);
> +			err = -EINVAL;
> +			goto put_child;
> +		}
> +
> +		if (fwnode_property_present(child, "adi,output-range")) {
> +			is_custom = false;
> +			err = fwnode_property_read_u32(child,
> +						       "adi,output-range",
> +						       &val);
> +			if (err) {
> +				dev_err(&dac->spi->dev,
> +					"Mandory adi,output-range property missing\n");
> +				goto put_child;
> +			}
> +
> +			if (val > AD3552R_CH_OUTPUT_RANGE_NEG_10__10V) {
> +				dev_err(&dac->spi->dev,
> +					"adi,output-range must be less or equal than %d\n",
> +					AD3552R_CH_OUTPUT_RANGE_NEG_10__10V + 1);
> +				err = -EINVAL;
> +				goto put_child;
> +			}
> +
> +			err = ad3552r_set_ch_value(dac,
> +						   AD3552R_CH_OUTPUT_RANGE_SEL,
> +						   ch, val);
> +			if (err)
> +				goto put_child;
> +		} else {
> +			is_custom = true;
> +			custom_gain_child =
> +				fwnode_get_named_child_node(child,
> +							    "custom-output-range-config");
> +			if (IS_ERR(custom_gain_child)) {
> +				err = PTR_ERR(custom_gain_child);
> +				dev_err(&dac->spi->dev,
> +					"Mandory custom-output-range-config property missing\n");
> +				goto put_child;
> +			}
> +
> +			err = fwnode_property_read_u32(custom_gain_child,
> +						       "adi,gain-offset", &val);
> +			if (err) {
> +				dev_err(&dac->spi->dev,
> +					"Mandory adi,gain-offset property missing\n");
> +				goto put_child;
> +			}
> +
> +			err = ad3552r_set_ch_value(dac,
> +						   AD3552R_CH_GAIN_OFFSET,
> +						   ch, abs((s32)val));
> +			if (err)
> +				goto put_child;
> +
> +			err = ad3552r_set_ch_value(dac, AD3552R_CH_GAIN_OFFSET_POLARITY,
> +						   ch, (s32)val < 0);
> +			if (err)
> +				goto put_child;
> +
> +			for (i = 0; i < ARRAY_SIZE(gain_attrs); ++i) {
> +				err = fwnode_property_read_u32(custom_gain_child,
> +							       gain_dts_names[i],
> +							       &val);
> +				if (err) {
> +					dev_err(&dac->spi->dev,
> +						"Mandory %s property missing\n",
> +						gain_dts_names[i]);
> +					goto put_child;
> +				}
> +
> +				err = ad3552r_set_ch_value(dac, gain_attrs[i],
> +							   ch, val);
> +				if (err)
> +					goto put_child;
> +			}
> +		}
> +
> +		ad3552r_set_gain_and_offset(dac, ch);
> +		err = ad3552r_set_ch_value(dac, AD3552R_CH_RANGE_OVERRIDE, ch,
> +					   is_custom);
> +		if (err)
> +			goto put_child;
> +
> +		dac->enabled_ch |= BIT(ch);
> +
> +		err = ad3552r_set_ch_value(dac, AD3552R_CH_SELECT, ch, 1);
> +		if (err < 0)
> +			return err;
> +
> +		dac->channels[cnt] = AD3552R_CH_DAC(ch);
> +		++cnt;
> +
> +	}
> +
> +	if (cnt == AD3552R_NUM_CH) {
> +		dac->channels[cnt] = AD3552R_CH_DAC_PAGE(AD3552R_PAGE_CH);
> +		++cnt;
> +	} else {
> +		/* Disable unused channels */
> +		for_each_clear_bit(ch, &dac->enabled_ch, AD3552R_PAGE_CH) {
> +			err = ad3552r_set_ch_value(dac,
> +						   AD3552R_CH_AMPLIFIER_POWERDOWN,
> +						   ch,
> +						   0);
> +			if (err)
> +				goto put_child;
> +		}
> +	}
> +
> +	dac->num_ch = cnt;
> +
> +put_child:
> +	if (!IS_ERR_OR_NULL(custom_gain_child))
> +		fwnode_handle_put(custom_gain_child);

I would think about factoring out the custom_gain bit as a separate function and then you can
have this error handling in there.

> +	fwnode_handle_put(child);
> +
> +	return err;
> +}
> +
> +static int ad3552r_init(struct ad3552r_desc *dac)
> +{
> +	int err;
> +
> +	err = ad3552r_reset(dac);
> +	if (err) {
> +		dev_err(&dac->spi->dev, "Reset failed\n");
> +		return err;
> +	}
> +
> +	err = ad3552r_check_scratch_pad(dac);
> +	if (err) {
> +		dev_err(&dac->spi->dev, "Scratch pad test failed\n");
> +		return err;
> +	}
> +
> +	return ad3552r_configure_device(dac);
> +}
> +
> +static int ad3552r_probe(struct spi_device *spi)
> +{
> +	struct ad3552r_desc	*dac;
> +	struct iio_dev		*indio_dev;
> +	int			err;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*dac));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	dac = iio_priv(indio_dev);
> +	dac->indio_dev = indio_dev;

This is usually a bad sign.  It's normally possible and sensible to
architect a driver to only navigate a heriarchy in one direction.  This
is adding a pointer the other way.  Not much used anyway so please clear it
out entirely.

> +	dac->spi = spi;
> +
> +	mutex_init(&dac->lock);
> +
> +	err = ad3552r_init(dac);
> +	if (err)
> +		return err;
> +
> +	/* Config triggered buffer device */
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = "ad3552r";
> +	indio_dev->info = &ad3552r_iio_info;
> +	indio_dev->num_channels = dac->num_ch;
> +	indio_dev->channels = dac->channels;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	err = ad3552r_setup_trigger_buffer(&spi->dev, indio_dev, spi->irq);
> +	if (err)
> +		return err;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct of_device_id ad3552r_of_match[] = {
> +	{ .compatible = "adi,ad3552r" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad3552r_of_match);
> +
> +static struct spi_driver ad3552r_driver = {
> +	.driver = {
> +		.name = "ad3552r",
> +		.of_match_table = ad3552r_of_match,
> +	},
> +	.probe = ad3552r_probe
> +};
> +module_spi_driver(ad3552r_driver);
> +
> +MODULE_AUTHOR("Mihail Chindris <mihail.chindris@analog.com>");
> +MODULE_DESCRIPTION("Analog Device AD3552R DAC");
> +MODULE_LICENSE("GPL v2");
diff mbox series

Patch

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 75e1f2b48638..ced6428f2c92 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -6,6 +6,16 @@ 
 
 menu "Digital to analog converters"
 
+config AD3552R
+	tristate "Analog Devices AD3552R DAC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Analog Devices AD3552R
+	  Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad3552r.
+
 config AD5064
 	tristate "Analog Devices AD5064 and similar multi-channel DAC driver"
 	depends on (SPI_MASTER && I2C!=m) || I2C
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 33e16f14902a..dffe36efd8ff 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -4,6 +4,7 @@ 
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AD3552R) += ad3552r.o
 obj-$(CONFIG_AD5360) += ad5360.o
 obj-$(CONFIG_AD5380) += ad5380.o
 obj-$(CONFIG_AD5421) += ad5421.o
diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
new file mode 100644
index 000000000000..89993dd87522
--- /dev/null
+++ b/drivers/iio/dac/ad3552r.c
@@ -0,0 +1,1419 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Analog Devices AD3552R
+ * Digital to Analog converter driver
+ *
+ * Copyright 2021 Analog Devices Inc.
+ */
+#include <linux/iopoll.h>
+#include <linux/device.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/time64.h>
+#include <linux/unaligned/be_byteshift.h>
+
+/* Register addresses */
+/* Primary address space */
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_A		0x00
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_B		0x01
+#define AD3552R_REG_ADDR_DEVICE_CONFIG			0x02
+#define AD3552R_REG_ADDR_CHIP_TYPE			0x03
+#define AD3552R_REG_ADDR_PRODUCT_ID_L			0x04
+#define AD3552R_REG_ADDR_PRODUCT_ID_H			0x05
+#define AD3552R_REG_ADDR_CHIP_GRADE			0x06
+#define AD3552R_REG_ADDR_SCRATCH_PAD			0x0A
+#define AD3552R_REG_ADDR_SPI_REVISION			0x0B
+#define AD3552R_REG_ADDR_VENDOR_L			0x0C
+#define AD3552R_REG_ADDR_VENDOR_H			0x0D
+#define AD3552R_REG_ADDR_STREAM_MODE			0x0E
+#define AD3552R_REG_ADDR_TRANSFER_REGISTER		0x0F
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C		0x10
+#define AD3552R_REG_ADDR_INTERFACE_STATUS_A		0x11
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_D		0x14
+#define AD3552R_REG_ADDR_SH_REFERENCE_CONFIG		0x15
+#define AD3552R_REG_ADDR_ERR_ALARM_MASK			0x16
+#define AD3552R_REG_ADDR_ERR_STATUS			0x17
+#define AD3552R_REG_ADDR_POWERDOWN_CONFIG		0x18
+#define AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE		0x19
+#define AD3552R_REG_ADDR_CH_OFFSET(ch)			(0x1B + (ch) * 2)
+#define AD3552R_REG_ADDR_CH_GAIN(ch)			(0x1C + (ch) * 2)
+/*
+ * Secondary region
+ * For multibyte registers specify the highest address because the access is
+ * done in descending order
+ */
+#define AD3552R_SECONDARY_REGION_START			0x28
+#define AD3552R_REG_ADDR_HW_LDAC_16B			0x28
+#define AD3552R_REG_ADDR_CH_DAC_16B(ch)			(0x2C - (1 - ch) * 2)
+#define AD3552R_REG_ADDR_DAC_PAGE_MASK_16B		0x2E
+#define AD3552R_REG_ADDR_CH_SELECT_16B			0x2F
+#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B		0x31
+#define AD3552R_REG_ADDR_SW_LDAC_16B			0x32
+#define AD3552R_REG_ADDR_CH_INPUT_16B(ch)		(0x36 - (1 - ch) * 2)
+/* 3 bytes registers */
+#define AD3552R_REG_START_24B				0x37
+#define AD3552R_REG_ADDR_HW_LDAC_24B			0x37
+#define AD3552R_REG_ADDR_CH_DAC_24B(ch)			(0x3D - (1 - ch) * 3)
+#define AD3552R_REG_ADDR_DAC_PAGE_MASK_24B		0x40
+#define AD3552R_REG_ADDR_CH_SELECT_24B			0x41
+#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B		0x44
+#define AD3552R_REG_ADDR_SW_LDAC_24B			0x45
+#define AD3552R_REG_ADDR_CH_INPUT_24B(ch)		(0x4B - (1 - ch) * 3)
+
+#define AD3552R_REG_ADDR_MAX				0x4B
+
+/* AD3552R_REG_ADDR_INTERFACE_CONFIG_A */
+#define AD3552R_MASK_SOFTWARE_RESET			(BIT(7) | BIT(0))
+#define AD3552R_MASK_ADDR_ASCENSION			BIT(5)
+#define AD3552R_MASK_SDO_ACTIVE				BIT(4)
+/* AD3552R_REG_ADDR_INTERFACE_CONFIG_B */
+#define AD3552R_MASK_SINGLE_INST			BIT(7)
+#define AD3552R_MASK_SHORT_INSTRUCTION			BIT(3)
+/* AD3552R_REG_ADDR_DEVICE_CONFIG */
+#define AD3552R_MASK_DEVICE_STATUS(n)			BIT(4 + (n))
+#define AD3552R_MASK_CUSTOM_MODES			(BIT(3) | BIT(2))
+#define AD3552R_MASK_OPERATING_MODES			(BIT(1) | BIT(0))
+/* AD3552R_REG_ADDR_CHIP_TYPE */
+#define AD3552R_MASK_CLASS				0x0F
+/* AD3552R_REG_ADDR_CHIP_GRADE */
+#define AD3552R_MASK_GRADE				0xF0
+#define AD3552R_MASK_DEVICE_REVISION			0x0F
+/* AD3552R_REG_ADDR_STREAM_MODE */
+#define AD3552R_MASK_LENGTH				0x0F
+/* AD3552R_REG_ADDR_TRANSFER_REGISTER */
+#define AD3552R_MASK_MULTI_IO_MODE			(BIT(7) | BIT(6))
+#define AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE		BIT(2)
+/* AD3552R_REG_ADDR_INTERFACE_CONFIG_C */
+#define AD3552R_MASK_CRC_ENABLE				(BIT(7) | BIT(6) |\
+							 BIT(1) | BIT(0))
+#define AD3552R_MASK_STRICT_REGISTER_ACCESS		BIT(5)
+/* AD3552R_REG_ADDR_INTERFACE_STATUS_A */
+#define AD3552R_MASK_INTERFACE_NOT_READY		BIT(7)
+#define AD3552R_MASK_CLOCK_COUNTING_ERROR		BIT(5)
+#define AD3552R_MASK_INVALID_OR_NO_CRC			BIT(3)
+#define AD3552R_MASK_WRITE_TO_READ_ONLY_REGISTER	BIT(2)
+#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS		BIT(1)
+#define AD3552R_MASK_REGISTER_ADDRESS_INVALID		BIT(0)
+/* AD3552R_REG_ADDR_INTERFACE_CONFIG_D */
+#define AD3552R_MASK_ALERT_ENABLE_PULLUP		BIT(6)
+#define AD3552R_MASK_MEM_CRC_EN				BIT(4)
+#define AD3552R_MASK_SDO_DRIVE_STRENGTH			(BIT(3) | BIT(2))
+#define AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN		BIT(1)
+#define AD3552R_MASK_SPI_CONFIG_DDR			BIT(0)
+/* AD3552R_REG_ADDR_SH_REFERENCE_CONFIG */
+#define AD3552R_MASK_IDUMP_FAST_MODE			BIT(6)
+#define AD3552R_MASK_SAMPLE_HOLD_DIFFERENTIAL_USER_EN	BIT(5)
+#define AD3552R_MASK_SAMPLE_HOLD_USER_TRIM		(BIT(4) | BIT(3))
+#define AD3552R_MASK_SAMPLE_HOLD_USER_ENABLE		BIT(2)
+#define AD3552R_MASK_REFERENCE_VOLTAGE_SEL		(BIT(1) | BIT(0))
+/* AD3552R_REG_ADDR_ERR_ALARM_MASK */
+#define AD3552R_MASK_REF_RANGE_ALARM			BIT(6)
+#define AD3552R_MASK_CLOCK_COUNT_ERR_ALARM		BIT(5)
+#define AD3552R_MASK_MEM_CRC_ERR_ALARM			BIT(4)
+#define AD3552R_MASK_SPI_CRC_ERR_ALARM			BIT(3)
+#define AD3552R_MASK_WRITE_TO_READ_ONLY_ALARM		BIT(2)
+#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS_ALARM	BIT(1)
+#define AD3552R_MASK_REGISTER_ADDRESS_INVALID_ALARM	BIT(0)
+/* AD3552R_REG_ADDR_ERR_STATUS */
+#define AD3552R_MASK_REF_RANGE_ERR_STATUS			BIT(6)
+#define AD3552R_MASK_DUAL_SPI_STREAM_EXCEEDS_DAC_ERR_STATUS	BIT(5)
+#define AD3552R_MASK_MEM_CRC_ERR_STATUS				BIT(4)
+#define AD3552R_MASK_RESET_STATUS				BIT(0)
+/* AD3552R_REG_ADDR_POWERDOWN_CONFIG */
+#define AD3552R_MASK_CH_DAC_POWERDOWN(ch)		BIT(4 + (ch))
+#define AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(ch)		BIT(ch)
+/* AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE */
+#define AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch)		((ch) ? 0xF0 : 0x0F)
+/* AD3552R_REG_ADDR_CH_GAIN */
+#define AD3552R_MASK_CH_RANGE_OVERRIDE			BIT(7)
+#define AD3552R_MASK_CH_GAIN_SCALING_N			(BIT(6) | BIT(5))
+#define AD3552R_MASK_CH_GAIN_SCALING_P			(BIT(4) | BIT(3))
+#define AD3552R_MASK_CH_OFFSET_POLARITY			BIT(2)
+#define AD3552R_MASK_CH_OFFSET_BIT_8			BIT(0)
+/* AD3552R_REG_ADDR_CH_OFFSET */
+#define AD3552R_MASK_CH_OFFSET_BITS_0_7			0xFF
+
+/* Useful defines */
+#define AD3552R_NUM_CH					2
+#define AD3552R_MASK_CH(ch)				BIT(ch)
+#define AD3552R_PAGE_CH					2
+#define AD3552R_MAX_REG_SIZE				3
+#define AD3552R_READ_BIT				(1 << 7)
+#define AD3552R_ADDR_MASK				(~AD3552R_READ_BIT)
+#define AD3552R_CRC_ENABLE_VALUE			(BIT(6) | BIT(1))
+#define AD3552R_CRC_DISABLE_VALUE			(BIT(1) | BIT(0))
+#define AD3552R_CRC_POLY				0x07
+#define AD3552R_CRC_SEED				0xA5
+#define AD3552R_MASK_DAC_12B				0xFFF0
+#define AD3552R_DEFAULT_CONFIG_B_VALUE			0x8
+#define SCRATCH_PAD_TEST_VAL1				0x34
+#define SCRATCH_PAD_TEST_VAL2				0xB2
+#define TO_MICROS					1000000
+#define GAIN_SCALE					1000
+#define AD3552R_READ					true
+#define AD3552R_WRITE					false
+#define LDAC_PULSE_US					10
+
+enum ad3552r_ch_output_range {
+	/* Range from 0 V to 2.5 V. Requires Rfb1x connection */
+	AD3552R_CH_OUTPUT_RANGE_0__2_5V,
+	/* Range from 0 V to 5 V. Requires Rfb1x connection  */
+	AD3552R_CH_OUTPUT_RANGE_0__5V,
+	/* Range from 0 V to 10 V. Requires Rfb2x connection  */
+	AD3552R_CH_OUTPUT_RANGE_0__10V,
+	/* Range from -2.5 V to 7.5 V. Requires Rfb2x connection  */
+	AD3552R_CH_OUTPUT_RANGE_NEG_5__5V,
+	/* Range from -6.5 V to 3.5 V. Requires Rfb4x connection  */
+	AD3552R_CH_OUTPUT_RANGE_NEG_10__10V,
+};
+
+static const s32 ch_ranges[][2] = {
+	[AD3552R_CH_OUTPUT_RANGE_0__2_5V]	= {0, 2500},
+	[AD3552R_CH_OUTPUT_RANGE_0__5V]		= {0, 5000},
+	[AD3552R_CH_OUTPUT_RANGE_0__10V]	= {0, 10000},
+	[AD3552R_CH_OUTPUT_RANGE_NEG_5__5V]	= {-5000, 5000},
+	[AD3552R_CH_OUTPUT_RANGE_NEG_10__10V]	= {-10000, 10000}
+};
+
+enum ad3552r_ch_gain_scaling {
+	/* Gain scaling of 1 */
+	AD3552R_CH_GAIN_SCALING_1,
+	/* Gain scaling of 0.5 */
+	AD3552R_CH_GAIN_SCALING_0_5,
+	/* Gain scaling of 0.25 */
+	AD3552R_CH_GAIN_SCALING_0_25,
+	/* Gain scaling of 0.125 */
+	AD3552R_CH_GAIN_SCALING_0_125,
+};
+
+/* Gain * GAIN_SCALE */
+static const s32 gains_scaling_table[] = {
+	[AD3552R_CH_GAIN_SCALING_1]		= 1000,
+	[AD3552R_CH_GAIN_SCALING_0_5]		= 500,
+	[AD3552R_CH_GAIN_SCALING_0_25]		= 250,
+	[AD3552R_CH_GAIN_SCALING_0_125]		= 125
+};
+
+enum ad3552r_dev_attributes {
+	/* - Direct register values */
+	/* From 0-3 */
+	AD3552R_SDO_DRIVE_STRENGTH,
+	/*
+	 * 0 -> Internal Vref, vref_io pin floating (default)
+	 * 1 -> Internal Vref, vref_io driven by internal vref
+	 * 2 or 3 -> External Vref
+	 */
+	AD3552R_VREF_SELECT,
+	/* Enable / Disable CRC */
+	AD3552R_CRC_ENABLE,
+	/* Spi mode: Strandard, Dual or Quad */
+	AD3552R_SPI_MULTI_IO_MODE,
+	/* Spi data rate: Single or dual */
+	AD3552R_SPI_DATA_RATE,
+	/* Dual spi synchronous mode */
+	AD3552R_SPI_SYNCHRONOUS_ENABLE,
+
+	/* - Direct register values (Private) */
+	/* Read registers in ascending order if set. Else descending */
+	AD3552R_ADDR_ASCENSION,
+	/* Single instruction mode if set. Else, stream mode */
+	AD3552R_SINGLE_INST,
+	/* Number of addresses to loop on when stream writing. */
+	AD3552R_STREAM_MODE,
+	/* Keep stream value if set. */
+	AD3552R_STREAM_LENGTH_KEEP_VALUE,
+};
+
+enum ad3552r_ch_attributes {
+	/* DAC powerdown */
+	AD3552R_CH_DAC_POWERDOWN,
+	/* DAC amplifier powerdown */
+	AD3552R_CH_AMPLIFIER_POWERDOWN,
+	/* Select the output range. Select from enum ad3552r_ch_output_range */
+	AD3552R_CH_OUTPUT_RANGE_SEL,
+	/*
+	 * Over-rider the range selector in order to manually set the output
+	 * voltage range
+	 */
+	AD3552R_CH_RANGE_OVERRIDE,
+	/* Manually set the offset voltage */
+	AD3552R_CH_GAIN_OFFSET,
+	/* Sets the polarity of the offset. */
+	AD3552R_CH_GAIN_OFFSET_POLARITY,
+	/* PDAC gain scaling */
+	AD3552R_CH_GAIN_SCALING_P,
+	/* NDAC gain scaling */
+	AD3552R_CH_GAIN_SCALING_N,
+	/* Trigger a software LDAC */
+	AD3552R_CH_TRIGGER_SOFTWARE_LDAC,
+	/* Hardware LDAC Mask */
+	AD3552R_CH_HW_LDAC_MASK,
+	/* Rfb value */
+	AD3552R_CH_RFB,
+	/* Channel select. When set allow Input -> DAC and Mask -> DAC */
+	AD3552R_CH_SELECT,
+	/* Raw value to be set to dac */
+	AD3552R_CH_CODE
+};
+
+struct ad3552r_ch_data {
+	u16	gain_offset : 9;
+	u16	range_override : 1;
+	u16	n : 2;
+	u16	p : 2;
+	u16	offset_polarity : 1;
+	u16	rfb;
+	u8	range;
+	s32	scale_int;
+	s32	scale_dec;
+	s32	offset_int;
+	s32	offset_dec;
+	bool	prec_en;
+};
+
+struct ad3552r_desc {
+	struct iio_dev		*indio_dev;
+	struct mutex		lock;
+	struct gpio_desc	*gpio_reset;
+	struct gpio_desc	*gpio_ldac;
+	struct spi_device	*spi;
+	struct ad3552r_ch_data	ch_data[AD3552R_NUM_CH];
+	struct iio_chan_spec	channels[AD3552R_NUM_CH + 1];
+	unsigned long		enabled_ch;
+	unsigned int		num_ch;
+	bool			use_input_regs;
+	u8 buf_data[2 * (AD3552R_MAX_REG_SIZE + 2)] ____cacheline_aligned;
+};
+
+static const u16 addr_mask_map[][2] = {
+	[AD3552R_ADDR_ASCENSION] = {
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
+			AD3552R_MASK_ADDR_ASCENSION
+	},
+	[AD3552R_SINGLE_INST] = {
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
+			AD3552R_MASK_SINGLE_INST
+	},
+	[AD3552R_STREAM_MODE] = {
+			AD3552R_REG_ADDR_STREAM_MODE,
+			AD3552R_MASK_LENGTH
+	},
+	[AD3552R_STREAM_LENGTH_KEEP_VALUE] = {
+			AD3552R_REG_ADDR_TRANSFER_REGISTER,
+			AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE
+	},
+	[AD3552R_SDO_DRIVE_STRENGTH] = {
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+			AD3552R_MASK_SDO_DRIVE_STRENGTH
+	},
+	[AD3552R_VREF_SELECT] = {
+			AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
+			AD3552R_MASK_REFERENCE_VOLTAGE_SEL
+	},
+	[AD3552R_CRC_ENABLE] = {
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_C,
+			AD3552R_MASK_CRC_ENABLE
+	},
+	[AD3552R_SPI_MULTI_IO_MODE] = {
+			AD3552R_REG_ADDR_TRANSFER_REGISTER,
+			AD3552R_MASK_MULTI_IO_MODE
+	},
+	[AD3552R_SPI_DATA_RATE] = {
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+			AD3552R_MASK_SPI_CONFIG_DDR
+	},
+	[AD3552R_SPI_SYNCHRONOUS_ENABLE] = {
+			AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+			AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN
+	},
+};
+
+/* 0 -> reg addr, 1->ch0 mask, 2->ch1 mask */
+static const u16 addr_mask_map_ch[][3] = {
+	[AD3552R_CH_DAC_POWERDOWN] = {
+			AD3552R_REG_ADDR_POWERDOWN_CONFIG,
+			AD3552R_MASK_CH_DAC_POWERDOWN(0),
+			AD3552R_MASK_CH_DAC_POWERDOWN(1)
+	},
+	[AD3552R_CH_AMPLIFIER_POWERDOWN] = {
+			AD3552R_REG_ADDR_POWERDOWN_CONFIG,
+			AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(0),
+			AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(1)
+	},
+	[AD3552R_CH_OUTPUT_RANGE_SEL] = {
+			AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
+			AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0),
+			AD3552R_MASK_CH_OUTPUT_RANGE_SEL(1)
+	},
+	/*
+	 * This attributes are update by the chip on 16B and 24B no matter to
+	 * what register the write is done
+	 */
+	[AD3552R_CH_TRIGGER_SOFTWARE_LDAC] = {
+			AD3552R_REG_ADDR_SW_LDAC_16B,
+			AD3552R_MASK_CH(0),
+			AD3552R_MASK_CH(1)
+	},
+	[AD3552R_CH_HW_LDAC_MASK] = {
+			AD3552R_REG_ADDR_HW_LDAC_16B,
+			AD3552R_MASK_CH(0),
+			AD3552R_MASK_CH(1)
+	},
+	[AD3552R_CH_SELECT] = {
+			AD3552R_REG_ADDR_CH_SELECT_16B,
+			AD3552R_MASK_CH(0),
+			AD3552R_MASK_CH(1)
+	}
+};
+
+static u8 _ad3552r_reg_len(u8 addr)
+{
+	if (addr > AD3552R_REG_ADDR_MAX)
+		return 0;
+
+	switch (addr) {
+	case AD3552R_REG_ADDR_HW_LDAC_16B:
+	case AD3552R_REG_ADDR_CH_SELECT_16B:
+	case AD3552R_REG_ADDR_SW_LDAC_16B:
+	case AD3552R_REG_ADDR_HW_LDAC_24B:
+	case AD3552R_REG_ADDR_CH_SELECT_24B:
+	case AD3552R_REG_ADDR_SW_LDAC_24B:
+		return 1;
+	default:
+		break;
+	}
+
+	if (addr > AD3552R_REG_ADDR_HW_LDAC_24B)
+		return 3;
+	if (addr > AD3552R_REG_ADDR_HW_LDAC_16B)
+		return 2;
+
+	return 1;
+}
+
+/* SPI transfer to device */
+static int ad3552r_transfer(struct ad3552r_desc *dac, u8 addr, u32 len,
+			    u8 *data, bool is_read)
+{
+	int err;
+	u8 instr;
+
+	instr = addr & AD3552R_ADDR_MASK;
+	instr |= is_read ? AD3552R_READ_BIT : 0;
+	dac->buf_data[0] = instr;
+	if (is_read) {
+		err = spi_write_then_read(dac->spi, dac->buf_data, 1,
+					  dac->buf_data + 1, len);
+		if (err)
+			return err;
+
+		memcpy(data, dac->buf_data + 1, len);
+
+		return 0;
+	}
+
+	memcpy(dac->buf_data + 1, data, len);
+	return spi_write(dac->spi, dac->buf_data, len + 1);
+}
+
+static int ad3552r_write_reg(struct ad3552r_desc *dac, u8 addr, u16 val)
+{
+	u8 reg_len, buf[AD3552R_MAX_REG_SIZE] = { 0 };
+
+	reg_len = _ad3552r_reg_len(addr);
+	if (!reg_len)
+		return -EINVAL;
+
+	if (reg_len == 2)
+		/* Only DAC register are 2 bytes wide */
+		val &= AD3552R_MASK_DAC_12B;
+	if (reg_len == 1)
+		buf[0] = val & 0xFF;
+	else
+		/* reg_len can be 2 or 3, but 3rd bytes needs to be set to 0 */
+		*((u16 *)buf) = cpu_to_be16(val);
+
+	return ad3552r_transfer(dac, addr, reg_len, buf,
+				AD3552R_WRITE);
+}
+
+static int ad3552r_read_reg(struct ad3552r_desc *dac, u8 addr, u16 *val)
+{
+	int err;
+	u8  reg_len, buf[AD3552R_MAX_REG_SIZE] = { 0 };
+
+	reg_len = _ad3552r_reg_len(addr);
+	if (!reg_len)
+		return -EINVAL;
+
+	err = ad3552r_transfer(dac, addr, reg_len, buf, AD3552R_READ);
+	if (err)
+		return err;
+
+	if (reg_len == 1)
+		*val = buf[0];
+	else
+		/* reg_len can be 2 or 3, but only first 2 bytes are relevant */
+		*val = be16_to_cpu(*((u16 *)buf));
+
+	return 0;
+}
+
+/* Update field of a register, shift val if needed */
+static int ad3552r_update_reg_field(struct ad3552r_desc *dac, u8 addr, u16 mask,
+				    u16 val)
+{
+	int ret;
+	u16 reg;
+
+	ret = ad3552r_read_reg(dac, addr, &reg);
+	if (ret < 0)
+		return ret;
+
+	reg = (reg & ~mask) | (val << __ffs(mask));
+
+	return ad3552r_write_reg(dac, addr, reg);
+}
+
+static int ad3552r_set_dev_value(struct ad3552r_desc *dac,
+				 enum ad3552r_dev_attributes attr,
+				 u16 val)
+{
+	switch (attr) {
+	case AD3552R_SPI_MULTI_IO_MODE:
+	case AD3552R_SPI_DATA_RATE:
+	case AD3552R_SPI_SYNCHRONOUS_ENABLE:
+	case AD3552R_CRC_ENABLE:
+		/* Not implemented */
+		return -EINVAL;
+	default:
+		return ad3552r_update_reg_field(dac, addr_mask_map[attr][0],
+						addr_mask_map[attr][1], val);
+	}
+
+	return 0;
+}
+
+static int ad3552r_set_offset_value(struct ad3552r_desc *dac, u8 ch, int val)
+{
+	int err;
+
+	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_CH_OFFSET(ch),
+				val & AD3552R_MASK_CH_OFFSET_BITS_0_7);
+	if (err)
+		return err;
+
+	err = ad3552r_update_reg_field(dac,
+				       AD3552R_REG_ADDR_CH_GAIN(ch),
+				       AD3552R_MASK_CH_OFFSET_BIT_8,
+				       (val >> 8) & AD3552R_MASK_CH_OFFSET_BIT_8);
+	if (err)
+		return err;
+
+	dac->ch_data[ch].gain_offset = val;
+
+	return 0;
+}
+
+static int ad3552r_set_gain_value(struct ad3552r_desc *dac,
+				  enum ad3552r_ch_attributes attr,
+				  u8 ch,
+				  int val)
+{
+	int reg_mask, err;
+
+	if (attr == AD3552R_CH_GAIN_OFFSET)
+		return ad3552r_set_offset_value(dac, ch, val);
+
+	switch (attr) {
+	case AD3552R_CH_RANGE_OVERRIDE:
+		val = !!val;
+		reg_mask = AD3552R_MASK_CH_RANGE_OVERRIDE;
+		break;
+	case AD3552R_CH_GAIN_OFFSET_POLARITY:
+		val = !!val;
+		reg_mask = AD3552R_MASK_CH_OFFSET_POLARITY;
+		break;
+	case AD3552R_CH_GAIN_SCALING_P:
+		reg_mask = AD3552R_MASK_CH_GAIN_SCALING_P;
+		break;
+	case AD3552R_CH_GAIN_SCALING_N:
+		reg_mask = AD3552R_MASK_CH_GAIN_SCALING_N;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	err = ad3552r_update_reg_field(dac, AD3552R_REG_ADDR_CH_GAIN(ch),
+				       reg_mask, val);
+	if (err)
+		return err;
+
+	switch (attr) {
+	case AD3552R_CH_RANGE_OVERRIDE:
+		dac->ch_data[ch].range_override = val;
+		break;
+	case AD3552R_CH_GAIN_OFFSET_POLARITY:
+		dac->ch_data[ch].offset_polarity = val;
+		break;
+	case AD3552R_CH_GAIN_SCALING_P:
+		dac->ch_data[ch].p = val;
+		break;
+	case AD3552R_CH_GAIN_SCALING_N:
+		dac->ch_data[ch].n = val;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Iterate over mask and write required bytes */
+static int ad3552r_write_codes(struct ad3552r_desc *dac, u32 mask, u8 *vals)
+{
+	int err, i, reg_len, k = 0;
+	unsigned long lmask = mask;
+	u8 addr, buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE];
+	u16 val;
+
+	/* If writing to consecutive registers do just one transfer */
+
+	if (mask == (AD3552R_MASK_CH(0) | AD3552R_MASK_CH(1)) &&
+	    dac->ch_data[0].prec_en == dac->ch_data[1].prec_en) {
+		if (dac->use_input_regs) {
+			if (dac->ch_data[0].prec_en)
+				addr = AD3552R_REG_ADDR_CH_INPUT_24B(1);
+			else
+				addr = AD3552R_REG_ADDR_CH_INPUT_16B(1);
+		} else {
+			if (dac->ch_data[0].prec_en)
+				addr = AD3552R_REG_ADDR_CH_DAC_24B(1);
+			else
+				addr = AD3552R_REG_ADDR_CH_DAC_16B(1);
+		}
+
+		reg_len = _ad3552r_reg_len(addr);
+		buff[0] = vals[0];
+		buff[reg_len] = vals[2];
+		if (dac->ch_data[0].prec_en) {
+			/* Reg_len is 3 here */
+			buff[1] = vals[1];
+			buff[2] = 0;
+			buff[4] = vals[3];
+			buff[5] = 0;
+		} else {
+			buff[1] = vals[1] & 0xf0;
+			buff[3] = vals[3] & 0xf0;
+		}
+
+		err = ad3552r_transfer(dac, addr, reg_len * 2, buff,
+				       AD3552R_WRITE);
+		if (err)
+			return err;
+	} else {
+
+		k = 0;
+		for_each_set_bit(i, &lmask, AD3552R_NUM_CH + 1) {
+			/* Writing to mask CH */
+			if (i == AD3552R_PAGE_CH)
+				addr = dac->ch_data[0].prec_en ?
+					AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B :
+					AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B;
+			else
+				addr = dac->ch_data[i].prec_en ?
+					AD3552R_REG_ADDR_CH_INPUT_24B(i) :
+					AD3552R_REG_ADDR_CH_INPUT_16B(i);
+
+			reg_len = _ad3552r_reg_len(addr);
+			val = be16_to_cpu(*((u16 *)(vals + k)));
+
+			k += 2;
+			err = ad3552r_write_reg(dac, addr, val);
+			if (err)
+				return err;
+		}
+	}
+
+	if (dac->gpio_ldac) {
+		gpiod_set_value_cansleep(dac->gpio_ldac, 0);
+		usleep_range(LDAC_PULSE_US, LDAC_PULSE_US + 10);
+		gpiod_set_value_cansleep(dac->gpio_ldac, 1);
+	}
+
+	return 0;
+}
+
+static int ad3552r_get_ch_value(struct ad3552r_desc *dac,
+				enum ad3552r_ch_attributes attr,
+				u8 ch,
+				u16 *val)
+{
+	int ret;
+	u16 reg;
+	u8  addr;
+	u16 mask;
+
+	/* Attributes not defined in addr_mask_map_ch */
+	switch (attr) {
+	case AD3552R_CH_CODE:
+		return ad3552r_read_reg(dac, AD3552R_REG_ADDR_CH_DAC_24B(ch),
+					val);
+	case AD3552R_CH_RFB:
+		*val = dac->ch_data[ch].rfb;
+		return 0;
+	default:
+		break;
+	}
+
+	if (attr >= AD3552R_CH_RANGE_OVERRIDE &&
+	    attr <= AD3552R_CH_GAIN_SCALING_N)
+		return -EINVAL;
+
+	addr = addr_mask_map_ch[attr][0];
+	if (addr == AD3552R_REG_ADDR_SW_LDAC_24B ||
+	    addr == AD3552R_REG_ADDR_SW_LDAC_16B) {
+		dev_err(&dac->indio_dev->dev, "Write only registers\n");
+		/* LDAC are write only registers */
+		return -EINVAL;
+	}
+
+	ret = ad3552r_read_reg(dac, addr, &reg);
+	if (ret < 0)
+		return ret;
+
+	mask = addr_mask_map_ch[attr][ch + 1];
+	*val = (reg & mask) >> __ffs(mask);
+
+	return 0;
+}
+
+static int ad3552r_set_ch_value(struct ad3552r_desc *dac,
+				enum ad3552r_ch_attributes attr,
+				u8 ch,
+				u16 val)
+{
+	int ret;
+
+	/* Attributes not defined in addr_mask_map_ch */
+	switch (attr) {
+	case AD3552R_CH_CODE:
+		return ad3552r_write_reg(dac, AD3552R_REG_ADDR_CH_DAC_24B(ch),
+					 val);
+	case AD3552R_CH_RFB:
+		dac->ch_data[ch].rfb = val;
+		return 0;
+	default:
+		break;
+	}
+
+	if (attr >= AD3552R_CH_RANGE_OVERRIDE &&
+	    attr <= AD3552R_CH_GAIN_SCALING_N)
+		return ad3552r_set_gain_value(dac, attr, ch, val);
+
+	/* Update register related to attributes in chip */
+	ret = ad3552r_update_reg_field(dac, addr_mask_map_ch[attr][0],
+				       addr_mask_map_ch[attr][ch + 1], val);
+	if (ret < 0)
+		return ret;
+
+	/* Update software structures */
+	if (attr == AD3552R_CH_OUTPUT_RANGE_SEL) {
+		val %= AD3552R_CH_OUTPUT_RANGE_NEG_10__10V + 1;
+		dac->ch_data[ch].range = val;
+	}
+
+	return ret;
+}
+
+static ssize_t ad3552r_write_ext(struct iio_dev *indio_dev,
+				 uintptr_t private,
+				 const struct iio_chan_spec *chan,
+				 const char *buf, size_t len)
+{
+	struct ad3552r_desc *dac = iio_priv(indio_dev);
+	int val, frac, err;
+
+	err = iio_str_to_fixpoint(buf, 0, &val, &frac);
+	if (err < 0)
+		return err;
+
+	dac->ch_data[chan->channel].prec_en = !!val;
+
+	return len;
+}
+
+static ssize_t ad3552r_read_ext(struct iio_dev *indio_dev,
+				uintptr_t private,
+				const struct iio_chan_spec *chan,
+				char *buf)
+{
+	struct ad3552r_desc *dac = iio_priv(indio_dev);
+	int val;
+
+	if (private != 0)
+		return -EINVAL;
+
+	val = dac->ch_data[chan->channel].prec_en;
+
+	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
+}
+
+#define AD3552R_CH_ATTR(_name, _what) { \
+	.name = _name, \
+	.read = ad3552r_read_ext, \
+	.write = ad3552r_write_ext, \
+	.private = _what, \
+	.shared = IIO_SEPARATE, \
+}
+
+static const struct iio_chan_spec_ext_info ad3552r_ext_info[] = {
+	AD3552R_CH_ATTR("precision_mode_en", 0),
+	{},
+};
+
+#define AD3552R_CH_DAC(_idx) ((struct iio_chan_spec) {		\
+	.type = IIO_VOLTAGE,					\
+	.output = true,						\
+	.indexed = true,					\
+	.channel = _idx,					\
+	.scan_index = _idx,					\
+	.scan_type = {						\
+		.sign = 'u',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+		.endianness = IIO_BE,				\
+	},							\
+	.ext_info = ad3552r_ext_info,				\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+				BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_ENABLE) |	\
+				BIT(IIO_CHAN_INFO_OFFSET),	\
+})
+
+#define AD3552R_CH_DAC_PAGE(_idx) ((struct iio_chan_spec) {	\
+	.type = IIO_VOLTAGE,					\
+	.output = true,						\
+	.indexed = true,					\
+	.channel = _idx,					\
+	.scan_index = _idx,					\
+	.scan_type = {						\
+		.sign = 'u',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+		.endianness = IIO_BE,				\
+	},							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
+	.modified = 1,						\
+	.channel2 = IIO_MOD_X_AND_Z,				\
+})
+
+static int ad3552r_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+			    int *val2,
+			    long mask)
+{
+	struct ad3552r_desc *dac = iio_priv(indio_dev);
+	u16 tmp_val;
+	int err;
+	u8 ch = chan->channel;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&dac->lock);
+		if (chan->channel == AD3552R_PAGE_CH)
+			err = ad3552r_read_reg(dac,
+					       AD3552R_REG_ADDR_DAC_PAGE_MASK_24B,
+					       &tmp_val);
+		else
+			err = ad3552r_get_ch_value(dac, AD3552R_CH_CODE, ch,
+						   &tmp_val);
+		if (err < 0) {
+			mutex_unlock(&dac->lock);
+			return err;
+		}
+
+		*val = tmp_val;
+		mutex_unlock(&dac->lock);
+		break;
+	case IIO_CHAN_INFO_ENABLE:
+		mutex_lock(&dac->lock);
+		err = ad3552r_get_ch_value(dac, AD3552R_CH_DAC_POWERDOWN,
+					   ch, &tmp_val);
+		if (err < 0) {
+			mutex_unlock(&dac->lock);
+			return err;
+		}
+		*val = !tmp_val;
+		mutex_unlock(&dac->lock);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		*val = dac->ch_data[ch].scale_int;
+		*val2 = dac->ch_data[ch].scale_dec;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = dac->ch_data[ch].offset_int;
+		*val2 = dac->ch_data[ch].offset_dec;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+
+	return IIO_VAL_INT;
+}
+
+static int ad3552r_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val,
+			     int val2,
+			     long mask)
+{
+	struct ad3552r_desc *dac = iio_priv(indio_dev);
+	enum ad3552r_ch_attributes attr;
+	int err = 0;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (chan->channel == AD3552R_PAGE_CH) {
+			mutex_lock(&dac->lock);
+			err = ad3552r_write_reg(dac,
+						AD3552R_REG_ADDR_DAC_PAGE_MASK_24B,
+						val);
+			mutex_unlock(&dac->lock);
+
+			return err;
+		}
+
+		attr = AD3552R_CH_CODE;
+		break;
+	case IIO_CHAN_INFO_ENABLE:
+		attr = AD3552R_CH_DAC_POWERDOWN;
+		val = !val;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	mutex_lock(&dac->lock);
+	err = ad3552r_set_ch_value(dac, attr, chan->channel, val);
+	mutex_unlock(&dac->lock);
+
+	return err;
+}
+
+static int ad3552r_update_scan_mode(struct iio_dev *indio_dev,
+				    const unsigned long *scan_mask)
+{
+	u32 mask;
+
+	mask = *scan_mask;
+	/* If writing to mask, can't write to other channels */
+	if ((mask & AD3552R_MASK_CH(AD3552R_PAGE_CH)) &&
+	    (mask & (~AD3552R_MASK_CH(AD3552R_PAGE_CH))))
+		return -EINVAL;
+
+	return 0;
+}
+
+/*
+ * Device type specific information.
+ */
+static const struct iio_info ad3552r_iio_info = {
+	.read_raw = ad3552r_read_raw,
+	.write_raw = ad3552r_write_raw,
+	.update_scan_mode = ad3552r_update_scan_mode
+};
+
+static irqreturn_t ad3552r_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func	*pf = p;
+	struct iio_dev		*indio_dev = pf->indio_dev;
+	struct iio_buffer	*buf = indio_dev->buffer;
+	struct ad3552r_desc	*dac = iio_priv(indio_dev);
+	char			buff[AD3552R_NUM_CH * AD3552R_MAX_REG_SIZE];
+	int			err;
+
+	memset(buff, 0, sizeof(buff));
+	mutex_lock(&dac->lock);
+	err = iio_buffer_remove_sample(buf, buff);
+	if (err)
+		goto end;
+
+	err = ad3552r_write_codes(dac, *indio_dev->active_scan_mask, buff);
+	if (err)
+		goto end;
+
+end:
+	iio_trigger_notify_done(indio_dev->trig);
+	mutex_unlock(&dac->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int ad3552r_setup_trigger_buffer(struct device *dev,
+					struct iio_dev *indio_dev, int irq)
+{
+	struct ad3552r_desc	*dac = iio_priv(indio_dev);
+	struct iio_trigger	*hwtrig;
+	int			err;
+
+	/* Configure trigger buffer */
+	err = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
+						  &ad3552r_trigger_handler,
+						  IIO_BUFFER_DIRECTION_OUT,
+						  NULL,
+						  NULL);
+	if (err)
+		return err;
+
+	if (!irq)
+		return 0;
+
+	hwtrig = devm_iio_trigger_alloc(dev, "%s-ldac-dev%d",
+					indio_dev->name,
+					iio_device_id(indio_dev));
+	if (!hwtrig)
+		return -ENOMEM;
+
+	hwtrig->dev.parent = dev;
+	iio_trigger_set_drvdata(hwtrig, dac);
+	err = devm_iio_trigger_register(dev, hwtrig);
+	if (err < 0)
+		return err;
+
+	return devm_request_threaded_irq(dev, irq,
+					 iio_trigger_generic_data_rdy_poll,
+					 NULL,
+					 IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					 indio_dev->name,
+					 hwtrig);
+}
+
+static int ad3552r_check_scratch_pad(struct ad3552r_desc *dac)
+{
+	const u16 val1 = SCRATCH_PAD_TEST_VAL1;
+	const u16 val2 = SCRATCH_PAD_TEST_VAL2;
+	u16 val;
+	int err;
+
+	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, val1);
+	if (err < 0)
+		return err;
+
+	err = ad3552r_read_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, &val);
+	if (err < 0)
+		return err;
+
+	if (val1 != val)
+		return -ENODEV;
+
+	err = ad3552r_write_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, val2);
+	if (err < 0)
+		return err;
+
+	err = ad3552r_read_reg(dac, AD3552R_REG_ADDR_SCRATCH_PAD, &val);
+	if (err < 0)
+		return err;
+
+	if (val2 != val)
+		return -ENODEV;
+
+	return 0;
+}
+
+struct reg_addr_pool {
+	struct ad3552r_desc *dac;
+	u8		    addr;
+};
+
+static u16 ad3552r_read_reg_pool(struct reg_addr_pool *addr)
+{
+	u16 val = 0;
+
+	ad3552r_read_reg(addr->dac, addr->addr, &val);
+
+	return val;
+}
+
+static int ad3552r_reset(struct ad3552r_desc *dac)
+{
+	struct reg_addr_pool addr;
+	int ret;
+	u16 val;
+
+	dac->gpio_reset = devm_gpiod_get_optional(&dac->spi->dev, "reset",
+						  GPIOD_OUT_LOW);
+	if (IS_ERR(dac->gpio_reset))
+		return PTR_ERR(dac->gpio_reset);
+
+	if (dac->gpio_reset) {
+		/* Perform hardware reset */
+		usleep_range(10, 20);
+		gpiod_set_value_cansleep(dac->gpio_reset, 1);
+	} else {
+		/* Perform software reset if no GPIO provided */
+		ret = ad3552r_update_reg_field(dac, AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
+					       AD3552R_MASK_SOFTWARE_RESET,
+					       AD3552R_MASK_SOFTWARE_RESET);
+		if (ret < 0)
+			return ret;
+
+	}
+
+	addr.dac = dac;
+	addr.addr = AD3552R_REG_ADDR_INTERFACE_CONFIG_B;
+	ret = readx_poll_timeout(ad3552r_read_reg_pool,
+				 &addr,
+				 val,
+				 (val == AD3552R_DEFAULT_CONFIG_B_VALUE),
+				 5000,
+				 50000);
+	if (ret) {
+		dev_err(&dac->spi->dev, "Err: %d\n", ret);
+		return ret;
+	}
+
+	ret = readx_poll_timeout(ad3552r_read_reg_pool,
+				 &addr,
+				 val,
+				 (!(val & AD3552R_MASK_INTERFACE_NOT_READY)),
+				 5000,
+				 50000);
+	if (ret) {
+		dev_err(&dac->spi->dev, "Err: %d\n", ret);
+		return ret;
+	}
+
+	ret = ad3552r_set_dev_value(dac, AD3552R_ADDR_ASCENSION, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void ad3552r_get_custom_range(struct ad3552r_desc *dac, s32 i, s32 *v_min,
+				     s32 *v_max)
+{
+	s64 vref, tmp, common, offset, gn, gp;
+	/*
+	 * From datasheet formula (In Volts):
+	 *	Vmin = 2.5 + [(GainN + Offset / 1024) * 2.5 * Rfb * 1.03]
+	 *	Vmax = 2.5 - [(GainP + Offset / 1024) * 2.5 * Rfb * 1.03]
+	 * Calculus are converted to milivolts
+	 */
+	vref = 2500;
+	/* 2.5 * 1.03 * 1000 (To mV) */
+	common = 2575 * dac->ch_data[i].rfb;
+	offset = dac->ch_data[i].gain_offset;
+	if (dac->ch_data[i].offset_polarity)
+		offset *= -1;
+
+	gn = gains_scaling_table[dac->ch_data[i].n];
+	tmp = (1024 * gn + GAIN_SCALE * offset) * common;
+	tmp = div_s64(tmp, 1024  * GAIN_SCALE);
+	*v_max = vref + tmp;
+
+	gp = gains_scaling_table[dac->ch_data[i].p];
+	tmp = (1024 * gp - GAIN_SCALE * offset) * common;
+	tmp = div_s64(tmp, 1024 * GAIN_SCALE);
+	*v_min = vref - tmp;
+}
+
+static void ad3552r_set_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
+{
+	s32 idx, v_max, v_min, span, rem;
+	s64 tmp;
+
+	if (dac->ch_data[ch].range_override) {
+		ad3552r_get_custom_range(dac, ch, &v_min, &v_max);
+	} else {
+		/* Normal range */
+		idx = dac->ch_data[ch].range;
+		v_max = ch_ranges[idx][1];
+		v_min = ch_ranges[idx][0];
+	}
+
+	/*
+	 * From datasheet formula:
+	 *	Vout = Span * (D / 65536) + Vmin
+	 * Converted to scale and offset:
+	 *	Scale = Span / 65536
+	 *	Offset = 65536 * Vmin / Span
+	 *
+	 * Reminders are in micros in order to be printed as
+	 * IIO_VAL_INT_PLUS_MICRO
+	 */
+	span = v_max - v_min;
+	dac->ch_data[ch].scale_int = div_s64_rem(span, 65536, &rem);
+	dac->ch_data[ch].scale_dec = DIV_ROUND_CLOSEST((s64)rem * TO_MICROS,
+							65536);
+
+	dac->ch_data[ch].offset_int = div_s64_rem(v_min * 65536, span,
+							&rem);
+	tmp = (s64)rem * TO_MICROS;
+	dac->ch_data[ch].offset_dec = div_s64(tmp, span);
+}
+
+static const char * const gain_dts_names[] = {
+	"adi,gain-scaling-p",
+	"adi,gain-scaling-n",
+	"adi,rfb"
+};
+
+static int ad3552r_configure_device(struct ad3552r_desc *dac)
+{
+	static const enum ad3552r_ch_attributes gain_attrs[] = {
+		AD3552R_CH_GAIN_SCALING_P,
+		AD3552R_CH_GAIN_SCALING_N,
+		AD3552R_CH_RFB
+	};
+	struct fwnode_handle	*child, *custom_gain_child = NULL;
+	int i, err, cnt = 0;
+	u32 val, ch;
+	bool is_custom;
+
+	dac->gpio_ldac = devm_gpiod_get_optional(&dac->spi->dev, "ldac",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(dac->gpio_ldac))
+		return PTR_ERR(dac->gpio_ldac);
+
+	dac->use_input_regs = device_property_read_bool(&dac->spi->dev,
+							"adi,synch_channels");
+
+	err = device_property_read_u32(&dac->spi->dev, "adi,vref-select", &val);
+	if (!err) {
+		if (val > 2) {
+			dev_err(&dac->spi->dev, "%s must be less than 3\n",
+				"adi,vref-select");
+			return -EINVAL;
+		}
+		err = ad3552r_set_dev_value(dac, AD3552R_VREF_SELECT, val);
+		if (err)
+			return err;
+	}
+
+	err = device_property_read_u32(&dac->spi->dev, "adi,sdo-drive-strength",
+				       &val);
+	if (!err) {
+		if (val > 3) {
+			dev_err(&dac->spi->dev, "%s must be less than 4\n",
+				"adi,sdo-drive-strength");
+			return -EINVAL;
+		}
+		err = ad3552r_set_dev_value(dac, AD3552R_SDO_DRIVE_STRENGTH,
+					    val);
+		if (err)
+			return err;
+	}
+
+	dac->num_ch = device_get_child_node_count(&dac->spi->dev);
+	if (!dac->num_ch) {
+		dev_err(&dac->spi->dev, "No channels defined\n");
+		return -ENODEV;
+	}
+
+	device_for_each_child_node(&dac->spi->dev, child) {
+		err = fwnode_property_read_u32(child, "reg", &ch);
+		if (err) {
+			dev_err(&dac->spi->dev, "Mandory reg property missing\n");
+			goto put_child;
+		}
+		if (ch >= AD3552R_NUM_CH) {
+			dev_err(&dac->spi->dev, "reg must be less than %d\n",
+				AD3552R_NUM_CH);
+			err = -EINVAL;
+			goto put_child;
+		}
+
+		if (fwnode_property_present(child, "adi,output-range")) {
+			is_custom = false;
+			err = fwnode_property_read_u32(child,
+						       "adi,output-range",
+						       &val);
+			if (err) {
+				dev_err(&dac->spi->dev,
+					"Mandory adi,output-range property missing\n");
+				goto put_child;
+			}
+
+			if (val > AD3552R_CH_OUTPUT_RANGE_NEG_10__10V) {
+				dev_err(&dac->spi->dev,
+					"adi,output-range must be less or equal than %d\n",
+					AD3552R_CH_OUTPUT_RANGE_NEG_10__10V + 1);
+				err = -EINVAL;
+				goto put_child;
+			}
+
+			err = ad3552r_set_ch_value(dac,
+						   AD3552R_CH_OUTPUT_RANGE_SEL,
+						   ch, val);
+			if (err)
+				goto put_child;
+		} else {
+			is_custom = true;
+			custom_gain_child =
+				fwnode_get_named_child_node(child,
+							    "custom-output-range-config");
+			if (IS_ERR(custom_gain_child)) {
+				err = PTR_ERR(custom_gain_child);
+				dev_err(&dac->spi->dev,
+					"Mandory custom-output-range-config property missing\n");
+				goto put_child;
+			}
+
+			err = fwnode_property_read_u32(custom_gain_child,
+						       "adi,gain-offset", &val);
+			if (err) {
+				dev_err(&dac->spi->dev,
+					"Mandory adi,gain-offset property missing\n");
+				goto put_child;
+			}
+
+			err = ad3552r_set_ch_value(dac,
+						   AD3552R_CH_GAIN_OFFSET,
+						   ch, abs((s32)val));
+			if (err)
+				goto put_child;
+
+			err = ad3552r_set_ch_value(dac, AD3552R_CH_GAIN_OFFSET_POLARITY,
+						   ch, (s32)val < 0);
+			if (err)
+				goto put_child;
+
+			for (i = 0; i < ARRAY_SIZE(gain_attrs); ++i) {
+				err = fwnode_property_read_u32(custom_gain_child,
+							       gain_dts_names[i],
+							       &val);
+				if (err) {
+					dev_err(&dac->spi->dev,
+						"Mandory %s property missing\n",
+						gain_dts_names[i]);
+					goto put_child;
+				}
+
+				err = ad3552r_set_ch_value(dac, gain_attrs[i],
+							   ch, val);
+				if (err)
+					goto put_child;
+			}
+		}
+
+		ad3552r_set_gain_and_offset(dac, ch);
+		err = ad3552r_set_ch_value(dac, AD3552R_CH_RANGE_OVERRIDE, ch,
+					   is_custom);
+		if (err)
+			goto put_child;
+
+		dac->enabled_ch |= BIT(ch);
+
+		err = ad3552r_set_ch_value(dac, AD3552R_CH_SELECT, ch, 1);
+		if (err < 0)
+			return err;
+
+		dac->channels[cnt] = AD3552R_CH_DAC(ch);
+		++cnt;
+
+	}
+
+	if (cnt == AD3552R_NUM_CH) {
+		dac->channels[cnt] = AD3552R_CH_DAC_PAGE(AD3552R_PAGE_CH);
+		++cnt;
+	} else {
+		/* Disable unused channels */
+		for_each_clear_bit(ch, &dac->enabled_ch, AD3552R_PAGE_CH) {
+			err = ad3552r_set_ch_value(dac,
+						   AD3552R_CH_AMPLIFIER_POWERDOWN,
+						   ch,
+						   0);
+			if (err)
+				goto put_child;
+		}
+	}
+
+	dac->num_ch = cnt;
+
+put_child:
+	if (!IS_ERR_OR_NULL(custom_gain_child))
+		fwnode_handle_put(custom_gain_child);
+	fwnode_handle_put(child);
+
+	return err;
+}
+
+static int ad3552r_init(struct ad3552r_desc *dac)
+{
+	int err;
+
+	err = ad3552r_reset(dac);
+	if (err) {
+		dev_err(&dac->spi->dev, "Reset failed\n");
+		return err;
+	}
+
+	err = ad3552r_check_scratch_pad(dac);
+	if (err) {
+		dev_err(&dac->spi->dev, "Scratch pad test failed\n");
+		return err;
+	}
+
+	return ad3552r_configure_device(dac);
+}
+
+static int ad3552r_probe(struct spi_device *spi)
+{
+	struct ad3552r_desc	*dac;
+	struct iio_dev		*indio_dev;
+	int			err;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*dac));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	dac = iio_priv(indio_dev);
+	dac->indio_dev = indio_dev;
+	dac->spi = spi;
+
+	mutex_init(&dac->lock);
+
+	err = ad3552r_init(dac);
+	if (err)
+		return err;
+
+	/* Config triggered buffer device */
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = "ad3552r";
+	indio_dev->info = &ad3552r_iio_info;
+	indio_dev->num_channels = dac->num_ch;
+	indio_dev->channels = dac->channels;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	err = ad3552r_setup_trigger_buffer(&spi->dev, indio_dev, spi->irq);
+	if (err)
+		return err;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad3552r_of_match[] = {
+	{ .compatible = "adi,ad3552r" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad3552r_of_match);
+
+static struct spi_driver ad3552r_driver = {
+	.driver = {
+		.name = "ad3552r",
+		.of_match_table = ad3552r_of_match,
+	},
+	.probe = ad3552r_probe
+};
+module_spi_driver(ad3552r_driver);
+
+MODULE_AUTHOR("Mihail Chindris <mihail.chindris@analog.com>");
+MODULE_DESCRIPTION("Analog Device AD3552R DAC");
+MODULE_LICENSE("GPL v2");