* [PATCH 0/2] Add support for the AD5754 DAC @ 2022-10-04 7:18 Ciprian Regus 2022-10-04 7:18 ` [PATCH 1/2] dt-bindings: iio: dac: add adi,ad5754.yaml Ciprian Regus 2022-10-04 7:18 ` [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver Ciprian Regus 0 siblings, 2 replies; 7+ messages in thread From: Ciprian Regus @ 2022-10-04 7:18 UTC (permalink / raw) To: jic23, krzysztof.kozlowski+dt, robh+dt, andriy.shevchenko, linux-kernel, linux-iio, devicetree Cc: Ciprian Regus These patches implement support for the AD5754 DAC (both dual and quad channel variants). The AD5724/AD5734/AD5754 are quad, 12-/14-/16-bit, serial input, voltage output DACs. The devices operate from single- supply voltages from +4.5 V up to +16.5 V or dual-supply voltages from ±4.5 V up to ±16.5 V. The input coding is user-selectable twos complement or offset binary for a bipolar output (depending on the state of Pin BIN/2sComp), and straight binary for a unipolar output. Ciprian Regus (2): dt-bindings: iio: dac: add adi,ad5754.yaml drivers: iio: dac: Add AD5754 DAC driver .../bindings/iio/dac/adi,ad5754.yaml | 173 +++++ MAINTAINERS | 8 + drivers/iio/dac/Kconfig | 12 + drivers/iio/dac/Makefile | 1 + drivers/iio/dac/ad5754.c | 668 ++++++++++++++++++ 5 files changed, 862 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml create mode 100644 drivers/iio/dac/ad5754.c -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dt-bindings: iio: dac: add adi,ad5754.yaml 2022-10-04 7:18 [PATCH 0/2] Add support for the AD5754 DAC Ciprian Regus @ 2022-10-04 7:18 ` Ciprian Regus 2022-10-04 11:26 ` Krzysztof Kozlowski 2022-10-04 7:18 ` [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver Ciprian Regus 1 sibling, 1 reply; 7+ messages in thread From: Ciprian Regus @ 2022-10-04 7:18 UTC (permalink / raw) To: jic23, krzysztof.kozlowski+dt, robh+dt, andriy.shevchenko, linux-kernel, linux-iio, devicetree Cc: Ciprian Regus Add devicetree bindings documentation for the AD5754 DAC driver. Signed-off-by: Ciprian Regus <ciprian.regus@analog.com> --- .../bindings/iio/dac/adi,ad5754.yaml | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml new file mode 100644 index 000000000000..e96112bc15d0 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml @@ -0,0 +1,173 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/adi,ad5754.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD5754 DAC device driver + +maintainers: + - Ciprian Regus <ciprian.regus@analog.com> + +description: | + Bindings for the AD5754 and other chip variants digital-to-analog + converters. + + https://www.analog.com/media/en/technical-documentation/data-sheets/AD5724_5734_5754.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad5722_5732_5752.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad5724r_5734r_5754r.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722R_5732R_5752R.pdf + +properties: + compatible: + enum: + - adi,ad5722 + - adi,ad5732 + - adi,ad5752 + - adi,ad5724 + - adi,ad5734 + - adi,ad5754 + - adi,ad5722r + - adi,ad5732r + - adi,ad5752r + - adi,ad5724r + - adi,ad5734r + - adi,ad5754r + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 30000000 + + spi-cpol: true + + vref-supply: + description: + The regulator to use as an external reference. If this is not provided, + the internal reference will be used for chips that have this feature. + The external reference must be 2.5V. + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + +patternProperties: + "^channel@([0-3])$": + type: object + description: Configurations for the DAC channels + + properties: + reg: + description: Channel number + maxItems: 1 + + output-range-microvolt: + description: | + Voltage range of a channel as <minimum, maximum>. + oneOf: + - items: + - const: 0 + - enum: [5000000, 10000000, 10800000] + - items: + - const: -5000000 + - const: 5000000 + - items: + - const: -10000000 + - const: 10000000 + - items: + - const: -10800000 + - const: 10800000 + + required: + - reg + - output-range-microvolt + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + + - if: + properties: + compatible: + contains: + enum: + - adi,ad5722 + - adi,ad5732 + - adi,ad5752 + - adi,ad5722r + - adi,ad5732r + - adi,ad5752r + then: + patternProperties: + "^channel@([0-3])$": + type: object + properties: + reg: + description: Channel number + enum: [0, 1] + + - if: + properties: + compatible: + contains: + enum: + - adi,ad5724 + - adi,ad5734 + - adi,ad5754 + - adi,ad5724r + - adi,ad5734r + - adi,ad5754r + then: + patternProperties: + "^channel@([0-3])$": + type: object + properties: + reg: + description: Channel number + enum: [0, 1, 2, 3] + +required: + - compatible + - reg + - spi-max-frequency + - spi-cpol + +unevaluatedProperties: false + +examples: + - | + spi { + status = "okay"; + + #address-cells = <1>; + #size-cells = <0>; + + dac@0 { + compatible = "adi,ad5754r"; + reg = <0>; + spi-max-frequency = <1000000>; + spi-cpol; + + #address-cells = <1>; + #size-cells = <0>; + + channel@0 { + reg = <0>; + output-range-microvolt = <0 5000000>; + }; + channel@1 { + reg = <1>; + output-range-microvolt = <0 5000000>; + }; + channel@2 { + reg = <2>; + output-range-microvolt = <0 5000000>; + }; + channel@3 { + reg = <3>; + output-range-microvolt = <0 5000000>; + }; + }; + }; -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: dac: add adi,ad5754.yaml 2022-10-04 7:18 ` [PATCH 1/2] dt-bindings: iio: dac: add adi,ad5754.yaml Ciprian Regus @ 2022-10-04 11:26 ` Krzysztof Kozlowski 0 siblings, 0 replies; 7+ messages in thread From: Krzysztof Kozlowski @ 2022-10-04 11:26 UTC (permalink / raw) To: Ciprian Regus, jic23, krzysztof.kozlowski+dt, robh+dt, andriy.shevchenko, linux-kernel, linux-iio, devicetree On 04/10/2022 09:18, Ciprian Regus wrote: > Add devicetree bindings documentation for the AD5754 DAC driver. > > Signed-off-by: Ciprian Regus <ciprian.regus@analog.com> > --- > .../bindings/iio/dac/adi,ad5754.yaml | 173 ++++++++++++++++++ > 1 file changed, 173 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml > new file mode 100644 > index 000000000000..e96112bc15d0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml > @@ -0,0 +1,173 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/dac/adi,ad5754.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices AD5754 DAC device driver Drop "device driver" unless "driver" and "device" are description of hardware of some sort. Then please tell me what is this hardware? A motor driver? > + > +maintainers: > + - Ciprian Regus <ciprian.regus@analog.com> > + > +description: | > + Bindings for the AD5754 and other chip variants digital-to-analog > + converters. > + > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD5724_5734_5754.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad5722_5732_5752.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad5724r_5734r_5754r.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722R_5732R_5752R.pdf > + > +properties: > + compatible: > + enum: > + - adi,ad5722 > + - adi,ad5732 > + - adi,ad5752 > + - adi,ad5724 > + - adi,ad5734 > + - adi,ad5754 > + - adi,ad5722r > + - adi,ad5732r > + - adi,ad5752r > + - adi,ad5724r > + - adi,ad5734r > + - adi,ad5754r > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 30000000 > + > + spi-cpol: true > + > + vref-supply: > + description: > + The regulator to use as an external reference. If this is not provided, > + the internal reference will be used for chips that have this feature. > + The external reference must be 2.5V. > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > +patternProperties: > + "^channel@([0-3])$": > + type: object > + description: Configurations for the DAC channels > + > + properties: > + reg: > + description: Channel number > + maxItems: 1 > + > + output-range-microvolt: Other such properties have vendor prefix. Did you receive feedback to make it generic (without vendor prefix)? > + description: | > + Voltage range of a channel as <minimum, maximum>. > + oneOf: > + - items: > + - const: 0 > + - enum: [5000000, 10000000, 10800000] > + - items: > + - const: -5000000 > + - const: 5000000 > + - items: > + - const: -10000000 > + - const: 10000000 > + - items: > + - const: -10800000 > + - const: 10800000 > + > + required: > + - reg > + - output-range-microvolt > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - adi,ad5722 > + - adi,ad5732 > + - adi,ad5752 > + - adi,ad5722r > + - adi,ad5732r > + - adi,ad5752r > + then: > + patternProperties: > + "^channel@([0-3])$": > + type: object > + properties: > + reg: > + description: Channel number > + enum: [0, 1] > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - adi,ad5724 > + - adi,ad5734 > + - adi,ad5754 > + - adi,ad5724r > + - adi,ad5734r > + - adi,ad5754r > + then: > + patternProperties: > + "^channel@([0-3])$": > + type: object > + properties: > + reg: > + description: Channel number > + enum: [0, 1, 2, 3] > + > +required: > + - compatible > + - reg > + - spi-max-frequency > + - spi-cpol > + > +unevaluatedProperties: false > + > +examples: > + - | > + spi { > + status = "okay"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + dac@0 { Messed up indentation. Use 4 spaces for entire example DTS. > + compatible = "adi,ad5754r"; > + reg = <0>; > + spi-max-frequency = <1000000>; > + spi-cpol; > + Best regards, Krzysztof ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver 2022-10-04 7:18 [PATCH 0/2] Add support for the AD5754 DAC Ciprian Regus 2022-10-04 7:18 ` [PATCH 1/2] dt-bindings: iio: dac: add adi,ad5754.yaml Ciprian Regus @ 2022-10-04 7:18 ` Ciprian Regus 2022-10-04 8:40 ` Marcus Folkesson ` (2 more replies) 1 sibling, 3 replies; 7+ messages in thread From: Ciprian Regus @ 2022-10-04 7:18 UTC (permalink / raw) To: jic23, krzysztof.kozlowski+dt, robh+dt, andriy.shevchenko, linux-kernel, linux-iio, devicetree Cc: Ciprian Regus The AD5724/AD5734/AD5754 are quad, 12-/14-/16-bit, serial input, voltage output DACs. The devices operate from single- supply voltages from +4.5 V up to +16.5 V or dual-supply voltages from ±4.5 V up to ±16.5 V. The input coding is user-selectable twos complement or offset binary for a bipolar output (depending on the state of Pin BIN/2sComp), and straight binary for a unipolar output. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5724_5734_5754.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5722_5732_5752.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5724r_5734r_5754r.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722R_5732R_5752R.pdf Signed-off-by: Ciprian Regus <ciprian.regus@analog.com> --- MAINTAINERS | 8 + drivers/iio/dac/Kconfig | 12 + drivers/iio/dac/Makefile | 1 + drivers/iio/dac/ad5754.c | 668 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 689 insertions(+) create mode 100644 drivers/iio/dac/ad5754.c diff --git a/MAINTAINERS b/MAINTAINERS index 4e5d1d737cfb..6fa48820c969 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1091,6 +1091,14 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git T: git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git F: drivers/net/amt.c +ANALOG DEVICES INC AD5754 DRIVER +M: Ciprian Regus <ciprian.regus@analog.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/dac/adi,ad5754.yaml +F: drivers/iio/dac/ad5754.c + ANALOG DEVICES INC AD7192 DRIVER M: Alexandru Tachici <alexandru.tachici@analog.com> L: linux-iio@vger.kernel.org diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index 80521bd28d0f..013278b629f9 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -170,6 +170,18 @@ config AD5696_I2C To compile this driver as a module, choose M here: the module will be called ad5696. +config AD5754 + tristate "Analog Devices AD5752/AD5732/AD5722/AD5754/AD5734/AD5724 DAC driver" + depends on SPI + select REGMAP_SPI + help + Say yes here to build support for Analog Devices AD5752, AD5732, + AD5722, AD5754, AD5734, AD5724 dual/quad channel Digital to + Analog Converter. + + To compile this driver as a module, choose M here: the + module will be called ad5754. + config AD5755 tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver" depends on SPI_MASTER diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index ec3e42713f00..d6c18deed1f7 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_AD5449) += ad5449.o obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o obj-$(CONFIG_AD5592R) += ad5592r.o obj-$(CONFIG_AD5593R) += ad5593r.o +obj-$(CONFIG_AD5754) += ad5754.o obj-$(CONFIG_AD5755) += ad5755.o obj-$(CONFIG_AD5755) += ad5758.o obj-$(CONFIG_AD5761) += ad5761.o diff --git a/drivers/iio/dac/ad5754.c b/drivers/iio/dac/ad5754.c new file mode 100644 index 000000000000..71623067363f --- /dev/null +++ b/drivers/iio/dac/ad5754.c @@ -0,0 +1,668 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2022 Analog Devices, Inc. + * Author: Ciprian Regus <ciprian.regus@analog.com> + */ + +#include <linux/bitfield.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/iio/iio.h> +#include <linux/kernel.h> +#include <linux/mod_devicetable.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#include <asm/unaligned.h> + +#define AD5754_INT_VREF 2500 +#define AD5754_FRAME_SIZE 3 +#define AD5754_MAX_CHANNELS 4 +#define AD5754_MAX_RESOLUTION 16 + +#define AD5754_DATA_MASK(_lsb) GENMASK(15, _lsb) + +#define AD5754_RANGE_MASK GENMASK(2, 0) + +#define AD5754_REG_RD BIT(7) + +#define AD5754_CLEAR_FUNC BIT(2) +#define AD5754_LOAD_FUNC (BIT(2) | BIT(0)) +#define AD5754_NOOP_FUNC GENMASK(4, 3) + +#define AD5754_PU_ADDR 0 +#define AD5754_PU_MASK GENMASK(3, 0) +#define AD5754_PU_CH(x) BIT(x) +#define AD5754_INT_REF_MASK BIT(4) +#define AD5754_INT_REF BIT(4) + +#define AD5754_DAC_REG 0 +#define AD5754_RANGE_REG BIT(0) +#define AD5754_PWR_REG BIT(1) +#define AD5754_CTRL_REG GENMASK(1, 0) + +#define AD5754_REG_ADDR(reg, addr) (((reg) << 3) | (addr)) + +#define AD5754_CHANNEL(_channel) \ + { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = _channel, \ + .output = 1, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET) \ + } + +enum ad5754_num_channels { + AD5754_2_CHANNELS, + AD5754_4_CHANNELS, +}; + +struct ad5754_span_tbl { + int min; + int max; +}; + +const struct ad5754_span_tbl ad5754_range[] = { + {0, 5000000}, + {0, 10000000}, + {0, 10800000}, + {-5000000, 5000000}, + {-10000000, 10000000}, + {-10800000, 10800000}, +}; + +enum AD5754_TYPE { + AD5722, + AD5732, + AD5752, + AD5724, + AD5734, + AD5754, + AD5722R, + AD5732R, + AD5752R, + AD5724R, + AD5734R, + AD5754R, +}; + +struct ad5754_chip_info { + const char *name; + u32 resolution; + bool internal_vref; + const u32 data_mask; + const struct iio_chan_spec *channels; + u32 num_channels; +}; + +const struct iio_chan_spec ad5754_channels[][AD5754_MAX_CHANNELS] = { + [AD5754_2_CHANNELS] = { + AD5754_CHANNEL(0), + AD5754_CHANNEL(1), + }, + [AD5754_4_CHANNELS] = { + AD5754_CHANNEL(0), + AD5754_CHANNEL(1), + AD5754_CHANNEL(2), + AD5754_CHANNEL(3), + }, +}; + +const struct ad5754_chip_info ad5754_chip_info_data[] = { + [AD5722] = { + .name = "ad5722", + .resolution = 12, + .data_mask = AD5754_DATA_MASK(4), + .internal_vref = false, + .num_channels = 2, + .channels = ad5754_channels[AD5754_2_CHANNELS], + }, + [AD5732] = { + .name = "ad5732", + .resolution = 14, + .data_mask = AD5754_DATA_MASK(2), + .internal_vref = false, + .num_channels = 2, + .channels = ad5754_channels[AD5754_2_CHANNELS], + }, + [AD5752] = { + .name = "ad5752", + .resolution = 16, + .data_mask = AD5754_DATA_MASK(0), + .internal_vref = false, + .num_channels = 2, + .channels = ad5754_channels[AD5754_2_CHANNELS], + }, + [AD5724] = { + .name = "ad5724", + .resolution = 12, + .data_mask = AD5754_DATA_MASK(4), + .internal_vref = false, + .num_channels = 4, + .channels = ad5754_channels[AD5754_4_CHANNELS], + }, + [AD5734] = { + .name = "ad5734", + .resolution = 14, + .data_mask = AD5754_DATA_MASK(2), + .internal_vref = false, + .num_channels = 4, + .channels = ad5754_channels[AD5754_4_CHANNELS], + }, + [AD5754] = { + .name = "ad5754", + .resolution = 16, + .data_mask = AD5754_DATA_MASK(0), + .internal_vref = false, + .num_channels = 4, + .channels = ad5754_channels[AD5754_4_CHANNELS], + }, + [AD5722R] = { + .name = "ad5722r", + .resolution = 12, + .data_mask = AD5754_DATA_MASK(4), + .internal_vref = true, + .num_channels = 2, + .channels = ad5754_channels[AD5754_2_CHANNELS], + }, + [AD5732R] = { + .name = "ad5732r", + .resolution = 14, + .data_mask = AD5754_DATA_MASK(2), + .internal_vref = true, + .num_channels = 2, + .channels = ad5754_channels[AD5754_2_CHANNELS], + }, + [AD5752R] = { + .name = "ad5752r", + .resolution = 16, + .data_mask = AD5754_DATA_MASK(0), + .internal_vref = true, + .num_channels = 2, + .channels = ad5754_channels[AD5754_2_CHANNELS], + }, + [AD5724R] = { + .name = "ad5724r", + .resolution = 12, + .data_mask = AD5754_DATA_MASK(4), + .internal_vref = true, + .num_channels = 4, + .channels = ad5754_channels[AD5754_4_CHANNELS], + }, + [AD5734R] = { + .name = "ad5734r", + .resolution = 14, + .data_mask = AD5754_DATA_MASK(2), + .internal_vref = true, + .num_channels = 4, + .channels = ad5754_channels[AD5754_4_CHANNELS], + }, + [AD5754R] = { + .name = "ad5754r", + .resolution = 16, + .data_mask = AD5754_DATA_MASK(0), + .internal_vref = true, + .num_channels = 4, + .channels = ad5754_channels[AD5754_4_CHANNELS], + } +}; + +struct ad5754_state { + struct regmap *regmap; + struct spi_device *spi; + struct device *dev; + + const struct ad5754_chip_info *chip_info; + + u32 range_idx[AD5754_MAX_CHANNELS]; + int offset[AD5754_MAX_CHANNELS]; + u32 dac_max_code; + u32 data_mask; + u32 sub_lsb; + u32 vref; + + /* + * DMA (thus cache coherency maintenance) may require the + * transfer buffers to live in their own cache lines. + */ + u8 buff[AD5754_FRAME_SIZE] __aligned(IIO_DMA_MINALIGN); +}; + +/* + * The channel addresses for 2 channel chip variants are not sequential: + * A2 A1 A0 Channel + * 0 0 0 DAC A + * 0 1 0 DAC B + * + * This is not the case for 4 channel chips: + * A2 A1 A0 Channel + * 0 0 0 DAC A + * 0 0 1 DAC B + * 0 1 0 DAC C + * 0 1 1 DAC D + */ +static unsigned int ad5754_real_ch(struct ad5754_state *st, + u32 channel, + u32 *real_channel) +{ + switch (st->chip_info->num_channels) { + case 2: + if (channel == 0) + *real_channel = 0; + else + *real_channel = 2; + break; + case 4: + *real_channel = channel; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int ad5754_get_output_range(struct ad5754_state *st, + struct fwnode_handle *channel_node, + u32 ch_idx) +{ + u32 range[2]; + int min, max; + int ret; + u32 i; + + ret = fwnode_property_read_u32_array(channel_node, + "output-range-microvolt", + range, 2); + if (ret) + return ret; + + min = range[0]; + max = range[1]; + + for (i = 0; i < ARRAY_SIZE(ad5754_range); i++) { + if (ad5754_range[i].min != min || ad5754_range[i].max != max) + continue; + + st->range_idx[ch_idx] = i; + if (min < 0) + st->offset[ch_idx] = -BIT(st->chip_info->resolution - 1); + + return 0; + } + + return -EINVAL; +} + +static int ad5754_reg_write(void *context, unsigned int reg, unsigned int val) +{ + struct ad5754_state *st = context; + struct spi_transfer xfer = { + .tx_buf = st->buff, + .len = 3, + }; + + st->buff[0] = reg; + put_unaligned_be16(val, &st->buff[1]); + + return spi_sync_transfer(st->spi, &xfer, 1); +}; + +static int ad5754_reg_read(void *context, unsigned int reg, unsigned int *val) +{ + struct ad5754_state *st = context; + struct spi_transfer xfer[] = { + { + .tx_buf = st->buff, + .len = AD5754_FRAME_SIZE, + }, + }; + int ret; + + st->buff[0] = AD5754_REG_RD | reg; + ret = spi_sync_transfer(st->spi, xfer, 1); + if (ret) + return ret; + + xfer->rx_buf = st->buff; + st->buff[0] = AD5754_NOOP_FUNC; + st->buff[1] = 0; + st->buff[2] = 0; + ret = spi_sync_transfer(st->spi, xfer, 1); + if (ret) + return ret; + + *val = get_unaligned_be16(&st->buff[1]); + + return 0; +}; + +static const struct regmap_config ad5754_regmap_config = { + .reg_bits = 8, + .val_bits = 16, + .reg_write = ad5754_reg_write, + .reg_read = ad5754_reg_read, +}; + +static int ad5754_set_dac_code(struct ad5754_state *st, u32 channel, u32 code) +{ + struct reg_sequence xfer_seq[2] = { + { AD5754_REG_ADDR(AD5754_DAC_REG, channel), code << st->sub_lsb }, + { AD5754_REG_ADDR(AD5754_CTRL_REG, AD5754_LOAD_FUNC), 0 }, + }; + + return regmap_multi_reg_write(st->regmap, xfer_seq, 2); +} + +static int ad5754_enable_channels(struct ad5754_state *st) +{ + struct fwnode_handle *channel_node = NULL; + u32 real_channel; + u32 power_reg; + u32 index; + int ret; + + fwnode_for_each_available_child_node(dev_fwnode(st->dev), channel_node) { + ret = fwnode_property_read_u32(channel_node, "reg", &index); + if (ret) { + dev_err(st->dev, "Failed to read channel reg: %d\n", ret); + goto free_node; + } + if (index >= st->chip_info->num_channels) { + dev_err(st->dev, "Channel index %u is too large\n", index); + goto free_node; + } + + ret = ad5754_real_ch(st, index, &real_channel); + if (ret) + goto free_node; + + ret = ad5754_get_output_range(st, channel_node, index); + if (ret) + goto free_node; + + ret = regmap_write_bits(st->regmap, + AD5754_REG_ADDR(AD5754_RANGE_REG, real_channel), + AD5754_RANGE_MASK, st->range_idx[index]); + if (ret) + goto free_node; + + ret = regmap_read(st->regmap, + AD5754_REG_ADDR(AD5754_PWR_REG, AD5754_PU_ADDR), + &power_reg); + if (ret) + goto free_node; + + ret = regmap_update_bits(st->regmap, + AD5754_REG_ADDR(AD5754_PWR_REG, AD5754_PU_ADDR), + AD5754_PU_MASK, AD5754_PU_CH(real_channel) | + (power_reg & AD5754_PU_MASK)); + if (ret) + goto free_node; + + /* Channel power up delay */ + fsleep(10); + } + + return 0; + +free_node: + fwnode_handle_put(channel_node); + + return ret; +} + +static int ad5754_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long info) +{ + struct ad5754_state *st = iio_priv(indio_dev); + u32 real_channel; + int ret; + + switch (info) { + case IIO_CHAN_INFO_RAW: + if (val < 0 || val > st->dac_max_code) { + dev_err(st->dev, "Invalid DAC code %d\n", val); + return -EINVAL; + } + ret = ad5754_real_ch(st, chan->channel, &real_channel); + if (ret) + return ret; + + return ad5754_set_dac_code(st, real_channel, val); + default: + return -EINVAL; + } +} + +static int ad5754_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long info) +{ + struct ad5754_state *st = iio_priv(indio_dev); + const struct ad5754_span_tbl *range; + u32 real_channel; + u32 gain; + int ret; + + ret = ad5754_real_ch(st, chan->channel, &real_channel); + if (ret) + return ret; + + switch (info) { + case IIO_CHAN_INFO_RAW: + ret = regmap_read(st->regmap, real_channel, val); + if (ret) + return ret; + + *val >>= st->sub_lsb; + + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + range = &ad5754_range[st->range_idx[chan->channel]]; + gain = (range->max - range->min) / 2500; + *val = st->vref * gain / 1000; + *val2 = st->chip_info->resolution; + + return IIO_VAL_FRACTIONAL_LOG2; + case IIO_CHAN_INFO_OFFSET: + *val = st->offset[chan->channel]; + + return IIO_VAL_INT; + default: + return -EINVAL; + } +} + +static int ad5754_int_vref_enable(struct ad5754_state *st) +{ + return regmap_update_bits(st->regmap, + AD5754_REG_ADDR(AD5754_PWR_REG, AD5754_PU_ADDR), + AD5754_INT_REF_MASK, + FIELD_PREP(AD5754_INT_REF_MASK, 1)); +} + +static void ad5754_disable_regulator(void *regulator) +{ + regulator_disable(regulator); +} + +static const struct iio_info ad5754_info = { + .read_raw = &ad5754_read_raw, + .write_raw = &ad5754_write_raw, +}; + +static int ad5754_probe(struct spi_device *spi) +{ + struct regulator *vref_reg; + struct iio_dev *indio_dev; + struct ad5754_state *st; + struct device *dev; + int ret; + + dev = &spi->dev; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + spi_set_drvdata(spi, indio_dev); + + st->spi = spi; + st->dev = dev; + st->chip_info = device_get_match_data(dev); + if (!st->chip_info) + st->chip_info = + (const struct ad5754_chip_info *)spi_get_device_id(spi)->driver_data; + + st->regmap = devm_regmap_init(st->dev, NULL, st, &ad5754_regmap_config); + if (IS_ERR(st->regmap)) + return dev_err_probe(st->dev, PTR_ERR(vref_reg), + "Regmap init error\n"); + + st->dac_max_code = BIT(st->chip_info->resolution) - 1; + st->sub_lsb = AD5754_MAX_RESOLUTION - st->chip_info->resolution; + + vref_reg = devm_regulator_get_optional(st->dev, "vref"); + if (IS_ERR(vref_reg)) { + if (!st->chip_info->internal_vref) + return dev_err_probe(st->dev, PTR_ERR(vref_reg), + "Failed to get the vref regulator\n"); + + st->vref = AD5754_INT_VREF; + ret = ad5754_int_vref_enable(st); + if (ret) + return ret; + } else { + ret = regulator_enable(vref_reg); + if (ret) + return dev_err_probe(st->dev, PTR_ERR(vref_reg), + "Failed to enable the vref regulator\n"); + + ret = devm_add_action_or_reset(dev, ad5754_disable_regulator, vref_reg); + if (ret) + return ret; + + ret = regulator_get_voltage(vref_reg); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to get vref\n"); + + st->vref = ret / 1000; + } + + indio_dev->name = st->chip_info->name; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &ad5754_info; + indio_dev->channels = st->chip_info->channels; + indio_dev->num_channels = st->chip_info->num_channels; + + ret = ad5754_enable_channels(st); + if (ret) + return ret; + + return devm_iio_device_register(st->dev, indio_dev); +} + +static int __init ad5754_register_driver(struct spi_driver *spi) +{ + return spi_register_driver(spi); +} + +static int ad5754_unregister_driver(struct spi_driver *spi) +{ + spi_unregister_driver(spi); + + return 0; +} + +static const struct spi_device_id ad5754_id[] = { + { "ad5722", (kernel_ulong_t)&ad5754_chip_info_data[AD5722], }, + { "ad5732", (kernel_ulong_t)&ad5754_chip_info_data[AD5732], }, + { "ad5752", (kernel_ulong_t)&ad5754_chip_info_data[AD5752], }, + { "ad5724", (kernel_ulong_t)&ad5754_chip_info_data[AD5724], }, + { "ad5734", (kernel_ulong_t)&ad5754_chip_info_data[AD5734], }, + { "ad5754", (kernel_ulong_t)&ad5754_chip_info_data[AD5754], }, + { "ad5722r", (kernel_ulong_t)&ad5754_chip_info_data[AD5722R], }, + { "ad5732r", (kernel_ulong_t)&ad5754_chip_info_data[AD5732R], }, + { "ad5752r", (kernel_ulong_t)&ad5754_chip_info_data[AD5752R], }, + { "ad5724r", (kernel_ulong_t)&ad5754_chip_info_data[AD5724R], }, + { "ad5734r", (kernel_ulong_t)&ad5754_chip_info_data[AD5734R], }, + { "ad5754r", (kernel_ulong_t)&ad5754_chip_info_data[AD5754R], }, + {}, +}; + +static const struct of_device_id ad5754_dt_id[] = { + { + .compatible = "adi,ad5722", + .data = &ad5754_chip_info_data[AD5722], + }, + { + .compatible = "adi,ad5732", + .data = &ad5754_chip_info_data[AD5732], + }, + { + .compatible = "adi,ad5752", + .data = &ad5754_chip_info_data[AD5752], + }, + { + .compatible = "adi,ad5724", + .data = &ad5754_chip_info_data[AD5724], + }, + { + .compatible = "adi,ad5734", + .data = &ad5754_chip_info_data[AD5734], + }, + { + .compatible = "adi,ad5754", + .data = &ad5754_chip_info_data[AD5754], + }, + { + .compatible = "adi,ad5722r", + .data = &ad5754_chip_info_data[AD5722R], + }, + { + .compatible = "adi,ad5732r", + .data = &ad5754_chip_info_data[AD5732R], + }, + { + .compatible = "adi,ad5752r", + .data = &ad5754_chip_info_data[AD5752R], + }, + { + .compatible = "adi,ad5724r", + .data = &ad5754_chip_info_data[AD5724R], + }, + { + .compatible = "adi,ad5734r", + .data = &ad5754_chip_info_data[AD5734R], + }, + { + .compatible = "adi,ad5754r", + .data = &ad5754_chip_info_data[AD5754R], + }, + {}, +}; +MODULE_DEVICE_TABLE(of, ad5754_dt_id); + +static struct spi_driver ad5754_driver = { + .driver = { + .name = "ad5754", + .of_match_table = ad5754_dt_id, + }, + .probe = ad5754_probe, + .id_table = ad5754_id, +}; + +module_driver(ad5754_driver, + ad5754_register_driver, + ad5754_unregister_driver); + +MODULE_AUTHOR("Ciprian Regus <ciprian.regus@analog.com>"); +MODULE_DESCRIPTION("Analog Devices AD5754 DAC"); +MODULE_LICENSE("GPL"); -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver 2022-10-04 7:18 ` [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver Ciprian Regus @ 2022-10-04 8:40 ` Marcus Folkesson 2022-10-04 8:50 ` Andy Shevchenko 2022-10-09 17:09 ` Jonathan Cameron 2 siblings, 0 replies; 7+ messages in thread From: Marcus Folkesson @ 2022-10-04 8:40 UTC (permalink / raw) To: Ciprian Regus Cc: jic23, krzysztof.kozlowski+dt, robh+dt, andriy.shevchenko, linux-kernel, linux-iio, devicetree [-- Attachment #1: Type: text/plain, Size: 4773 bytes --] Hi, See comments inline On Tue, Oct 04, 2022 at 10:18:25AM +0300, Ciprian Regus wrote: > The AD5724/AD5734/AD5754 are quad, 12-/14-/16-bit, serial > input, voltage output DACs. The devices operate from single- > supply voltages from +4.5 V up to +16.5 V or dual-supply > voltages from ±4.5 V up to ±16.5 V. The input coding is > user-selectable twos complement or offset binary for a bipolar > output (depending on the state of Pin BIN/2sComp), and straight > binary for a unipolar output. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5724_5734_5754.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5722_5732_5752.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5724r_5734r_5754r.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722R_5732R_5752R.pdf > Signed-off-by: Ciprian Regus <ciprian.regus@analog.com> > --- [...] > +struct ad5754_span_tbl { > + int min; > + int max; > +}; > + > +const struct ad5754_span_tbl ad5754_range[] = { > + {0, 5000000}, > + {0, 10000000}, > + {0, 10800000}, > + {-5000000, 5000000}, > + {-10000000, 10000000}, > + {-10800000, 10800000}, > +}; Should these be static? > + > +enum AD5754_TYPE { > + AD5722, > + AD5732, > + AD5752, > + AD5724, > + AD5734, > + AD5754, > + AD5722R, > + AD5732R, > + AD5752R, > + AD5724R, > + AD5734R, > + AD5754R, > +}; > + > +struct ad5754_chip_info { > + const char *name; > + u32 resolution; > + bool internal_vref; > + const u32 data_mask; > + const struct iio_chan_spec *channels; > + u32 num_channels; > +}; > + > +const struct iio_chan_spec ad5754_channels[][AD5754_MAX_CHANNELS] = { static? > + [AD5754_2_CHANNELS] = { > + AD5754_CHANNEL(0), > + AD5754_CHANNEL(1), > + }, > + [AD5754_4_CHANNELS] = { > + AD5754_CHANNEL(0), > + AD5754_CHANNEL(1), > + AD5754_CHANNEL(2), > + AD5754_CHANNEL(3), > + }, > +}; > + > +const struct ad5754_chip_info ad5754_chip_info_data[] = { static? [...] > + > + > +static int ad5754_probe(struct spi_device *spi) > +{ > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ad5754_state *st; > + struct device *dev; > + int ret; > + > + dev = &spi->dev; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + spi_set_drvdata(spi, indio_dev); > + > + st->spi = spi; > + st->dev = dev; > + st->chip_info = device_get_match_data(dev); > + if (!st->chip_info) > + st->chip_info = > + (const struct ad5754_chip_info *)spi_get_device_id(spi)->driver_data; > + > + st->regmap = devm_regmap_init(st->dev, NULL, st, &ad5754_regmap_config); > + if (IS_ERR(st->regmap)) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Regmap init error\n"); Are you sure you want to pass vref_reg here? :-) st->regmap > + > + st->dac_max_code = BIT(st->chip_info->resolution) - 1; > + st->sub_lsb = AD5754_MAX_RESOLUTION - st->chip_info->resolution; > + > + vref_reg = devm_regulator_get_optional(st->dev, "vref"); > + if (IS_ERR(vref_reg)) { > + if (!st->chip_info->internal_vref) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Failed to get the vref regulator\n"); > + > + st->vref = AD5754_INT_VREF; > + ret = ad5754_int_vref_enable(st); > + if (ret) > + return ret; > + } else { > + ret = regulator_enable(vref_reg); > + if (ret) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Failed to enable the vref regulator\n"); ret contains the error code here, not vref_reg. > + > + ret = devm_add_action_or_reset(dev, ad5754_disable_regulator, vref_reg); > + if (ret) > + return ret; > + > + ret = regulator_get_voltage(vref_reg); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to get vref\n"); > + > + st->vref = ret / 1000; > + } > + > + indio_dev->name = st->chip_info->name; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &ad5754_info; > + indio_dev->channels = st->chip_info->channels; > + indio_dev->num_channels = st->chip_info->num_channels; > + > + ret = ad5754_enable_channels(st); > + if (ret) > + return ret; > + > + return devm_iio_device_register(st->dev, indio_dev); > +} > + [...] > +module_driver(ad5754_driver, > + ad5754_register_driver, > + ad5754_unregister_driver); Use module_spi_driver() instead > + > +MODULE_AUTHOR("Ciprian Regus <ciprian.regus@analog.com>"); > +MODULE_DESCRIPTION("Analog Devices AD5754 DAC"); > +MODULE_LICENSE("GPL"); > -- > 2.30.2 > Best regards Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver 2022-10-04 7:18 ` [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver Ciprian Regus 2022-10-04 8:40 ` Marcus Folkesson @ 2022-10-04 8:50 ` Andy Shevchenko 2022-10-09 17:09 ` Jonathan Cameron 2 siblings, 0 replies; 7+ messages in thread From: Andy Shevchenko @ 2022-10-04 8:50 UTC (permalink / raw) To: Ciprian Regus Cc: jic23, krzysztof.kozlowski+dt, robh+dt, linux-kernel, linux-iio, devicetree On Tue, Oct 04, 2022 at 10:18:25AM +0300, Ciprian Regus wrote: > The AD5724/AD5734/AD5754 are quad, 12-/14-/16-bit, serial > input, voltage output DACs. The devices operate from single- > supply voltages from +4.5 V up to +16.5 V or dual-supply > voltages from ±4.5 V up to ±16.5 V. The input coding is > user-selectable twos complement or offset binary for a bipolar > output (depending on the state of Pin BIN/2sComp), and straight > binary for a unipolar output. ... > +#define AD5754_INT_VREF 2500 Units? (Like _mV or _uV or what? Note, small u is OK to have in such cases) ... > +#define AD5754_CLEAR_FUNC BIT(2) > +#define AD5754_LOAD_FUNC (BIT(2) | BIT(0)) > +#define AD5754_NOOP_FUNC GENMASK(4, 3) Seems like abuse of BIT and GENMASK, use plain numbers as it's probably is. Otherwise _each_ bit should have it's own descriptive meaning. ... > +#define AD5754_DAC_REG 0 > +#define AD5754_RANGE_REG BIT(0) > +#define AD5754_PWR_REG BIT(1) ... > +#define AD5754_CTRL_REG GENMASK(1, 0) Why _REG uses GENMASK()? ... > +struct ad5754_span_tbl { > + int min; > + int max; > +}; I'm wondering if linear_range.h can anyhow help with this code. ... > +struct ad5754_state { > + struct regmap *regmap; > + struct spi_device *spi; > + struct device *dev; You always can derive dev from regmap, is this one different? > + > + const struct ad5754_chip_info *chip_info; > + > + u32 range_idx[AD5754_MAX_CHANNELS]; > + int offset[AD5754_MAX_CHANNELS]; > + u32 dac_max_code; > + u32 data_mask; > + u32 sub_lsb; > + u32 vref; > + > + /* > + * DMA (thus cache coherency maintenance) may require the > + * transfer buffers to live in their own cache lines. > + */ > + u8 buff[AD5754_FRAME_SIZE] __aligned(IIO_DMA_MINALIGN); > +}; ... > +static const struct regmap_config ad5754_regmap_config = { > + .reg_bits = 8, > + .val_bits = 16, > + .reg_write = ad5754_reg_write, > + .reg_read = ad5754_reg_read, No max register address? > +}; ... > + struct fwnode_handle *channel_node = NULL; Redundant assignment. ... > + fwnode_for_each_available_child_node(dev_fwnode(st->dev), channel_node) { Why not device_for_each_child_node() ? (Yes, it uses available ones) > + } ... > + range = &ad5754_range[st->range_idx[chan->channel]]; > + gain = (range->max - range->min) / 2500; > + *val = st->vref * gain / 1000; > + *val2 = st->chip_info->resolution; Yeah, looks familiar to the linear_range APIs. ... > +static int ad5754_probe(struct spi_device *spi) > +{ > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ad5754_state *st; > + struct device *dev; > + int ret; > + dev = &spi->dev; Can be done in the definition block (inline). ... > + st->chip_info = device_get_match_data(dev); > + if (!st->chip_info) > + st->chip_info = > + (const struct ad5754_chip_info *)spi_get_device_id(spi)->driver_data; This can look better with a temporary variable. But doesn't matter since we would like to have these lines to be packed in a new SPI API helper in the future. ... > + st->vref = ret / 1000; Do we have uV_PER_mV or so? ... > +static const struct spi_device_id ad5754_id[] = { > + {}, No comma for the terminator line. > +}; ... > +static const struct of_device_id ad5754_dt_id[] = { > + {}, Ditto. > +}; ... > +module_driver(ad5754_driver, > + ad5754_register_driver, > + ad5754_unregister_driver); Why not module_spi_driver()? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver 2022-10-04 7:18 ` [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver Ciprian Regus 2022-10-04 8:40 ` Marcus Folkesson 2022-10-04 8:50 ` Andy Shevchenko @ 2022-10-09 17:09 ` Jonathan Cameron 2 siblings, 0 replies; 7+ messages in thread From: Jonathan Cameron @ 2022-10-09 17:09 UTC (permalink / raw) To: Ciprian Regus Cc: krzysztof.kozlowski+dt, robh+dt, andriy.shevchenko, linux-kernel, linux-iio, devicetree On Tue, 4 Oct 2022 10:18:25 +0300 Ciprian Regus <ciprian.regus@analog.com> wrote: > The AD5724/AD5734/AD5754 are quad, 12-/14-/16-bit, serial > input, voltage output DACs. The devices operate from single- > supply voltages from +4.5 V up to +16.5 V or dual-supply > voltages from ±4.5 V up to ±16.5 V. The input coding is > user-selectable twos complement or offset binary for a bipolar > output (depending on the state of Pin BIN/2sComp), and straight > binary for a unipolar output. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5724_5734_5754.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5722_5732_5752.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5724r_5734r_5754r.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722R_5732R_5752R.pdf > Signed-off-by: Ciprian Regus <ciprian.regus@analog.com> Hi Ciprian I'll try and avoid to much duplication of the comments of others! > --- > MAINTAINERS | 8 + > drivers/iio/dac/Kconfig | 12 + > drivers/iio/dac/Makefile | 1 + > drivers/iio/dac/ad5754.c | 668 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 689 insertions(+) > create mode 100644 drivers/iio/dac/ad5754.c > > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > index 80521bd28d0f..013278b629f9 100644 > --- a/drivers/iio/dac/Kconfig > +++ b/drivers/iio/dac/Kconfig > @@ -170,6 +170,18 @@ config AD5696_I2C > To compile this driver as a module, choose M here: the module will be > called ad5696. > > +config AD5754 > + tristate "Analog Devices AD5752/AD5732/AD5722/AD5754/AD5734/AD5724 DAC driver" This is close to the 'and similar' boundary. We can't keep extending names in the menu option for ever. If you judge there to be little chance of other parts being added to this driver thine as is. > + depends on SPI > + select REGMAP_SPI > + help > + Say yes here to build support for Analog Devices AD5752, AD5732, > + AD5722, AD5754, AD5734, AD5724 dual/quad channel Digital to > + Analog Converter. What are the R parts and should we mention those? > + > + To compile this driver as a module, choose M here: the > + module will be called ad5754. > + > config AD5755 > tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver" > depends on SPI_MASTER > diff --git a/drivers/iio/dac/ad5754.c b/drivers/iio/dac/ad5754.c > new file mode 100644 > index 000000000000..71623067363f > --- /dev/null > +++ b/drivers/iio/dac/ad5754.c > @@ -0,0 +1,668 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022 Analog Devices, Inc. > + * Author: Ciprian Regus <ciprian.regus@analog.com> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/iio/iio.h> > +#include <linux/kernel.h> > +#include <linux/mod_devicetable.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > + > +#include <asm/unaligned.h> > + > +#define AD5754_INT_VREF 2500 > +#define AD5754_FRAME_SIZE 3 > +#define AD5754_MAX_CHANNELS 4 > +#define AD5754_MAX_RESOLUTION 16 > + > +#define AD5754_DATA_MASK(_lsb) GENMASK(15, _lsb) > + > +#define AD5754_RANGE_MASK GENMASK(2, 0) > + > +#define AD5754_REG_RD BIT(7) > + > +#define AD5754_CLEAR_FUNC BIT(2) > +#define AD5754_LOAD_FUNC (BIT(2) | BIT(0)) > +#define AD5754_NOOP_FUNC GENMASK(4, 3) > + > +#define AD5754_PU_ADDR 0 > +#define AD5754_PU_MASK GENMASK(3, 0) > +#define AD5754_PU_CH(x) BIT(x) > +#define AD5754_INT_REF_MASK BIT(4) > +#define AD5754_INT_REF BIT(4) > + > +#define AD5754_DAC_REG 0 > +#define AD5754_RANGE_REG BIT(0) > +#define AD5754_PWR_REG BIT(1) > +#define AD5754_CTRL_REG GENMASK(1, 0) Huh. A really strange datasheet table for these as it breaks them down into 3 separate 1 bit fields that are next to each other. I'd ignore that and treat them as a 3 bit field. Then these are just 0, 1, 2, 3 within that field. > + > +enum ad5754_num_channels { > + AD5754_2_CHANNELS, > + AD5754_4_CHANNELS, > +}; This can go after you move away from an array of arrays of channels. > + > +const struct iio_chan_spec ad5754_channels[][AD5754_MAX_CHANNELS] = { > + [AD5754_2_CHANNELS] = { > + AD5754_CHANNEL(0), > + AD5754_CHANNEL(1), > + }, No real point in having these in an array. Have const struct iio_chan_spec ad5754_2_channels[] = { ... }; const struct iio_chan_spec ad5754_4_channels[] = { ... } Storage is smaller and easier code also shorter as you don't have to index the array. > + [AD5754_4_CHANNELS] = { > + AD5754_CHANNEL(0), > + AD5754_CHANNEL(1), > + AD5754_CHANNEL(2), > + AD5754_CHANNEL(3), > + }, > +}; > + > +const struct ad5754_chip_info ad5754_chip_info_data[] = { > + [AD5722] = { > + .name = "ad5722", > + .resolution = 12, > + .data_mask = AD5754_DATA_MASK(4), > + .internal_vref = false, > + .num_channels = 2, > + .channels = ad5754_channels[AD5754_2_CHANNELS], > + }, > + [AD5732] = { > + .name = "ad5732", > + .resolution = 14, > + .data_mask = AD5754_DATA_MASK(2), > + .internal_vref = false, > + .num_channels = 2, > + .channels = ad5754_channels[AD5754_2_CHANNELS], > + }, > + [AD5752] = { > + .name = "ad5752", > + .resolution = 16, > + .data_mask = AD5754_DATA_MASK(0), > + .internal_vref = false, > + .num_channels = 2, > + .channels = ad5754_channels[AD5754_2_CHANNELS], > + }, > + [AD5724] = { > + .name = "ad5724", > + .resolution = 12, > + .data_mask = AD5754_DATA_MASK(4), > + .internal_vref = false, > + .num_channels = 4, > + .channels = ad5754_channels[AD5754_4_CHANNELS], > + }, > + [AD5734] = { > + .name = "ad5734", > + .resolution = 14, > + .data_mask = AD5754_DATA_MASK(2), > + .internal_vref = false, > + .num_channels = 4, > + .channels = ad5754_channels[AD5754_4_CHANNELS], > + }, > + [AD5754] = { > + .name = "ad5754", > + .resolution = 16, > + .data_mask = AD5754_DATA_MASK(0), > + .internal_vref = false, > + .num_channels = 4, > + .channels = ad5754_channels[AD5754_4_CHANNELS], > + }, > + [AD5722R] = { > + .name = "ad5722r", > + .resolution = 12, > + .data_mask = AD5754_DATA_MASK(4), > + .internal_vref = true, > + .num_channels = 2, > + .channels = ad5754_channels[AD5754_2_CHANNELS], > + }, > + [AD5732R] = { > + .name = "ad5732r", > + .resolution = 14, > + .data_mask = AD5754_DATA_MASK(2), > + .internal_vref = true, > + .num_channels = 2, > + .channels = ad5754_channels[AD5754_2_CHANNELS], > + }, > + [AD5752R] = { > + .name = "ad5752r", > + .resolution = 16, > + .data_mask = AD5754_DATA_MASK(0), > + .internal_vref = true, > + .num_channels = 2, > + .channels = ad5754_channels[AD5754_2_CHANNELS], > + }, > + [AD5724R] = { > + .name = "ad5724r", > + .resolution = 12, > + .data_mask = AD5754_DATA_MASK(4), > + .internal_vref = true, > + .num_channels = 4, > + .channels = ad5754_channels[AD5754_4_CHANNELS], > + }, > + [AD5734R] = { > + .name = "ad5734r", > + .resolution = 14, > + .data_mask = AD5754_DATA_MASK(2), > + .internal_vref = true, > + .num_channels = 4, > + .channels = ad5754_channels[AD5754_4_CHANNELS], > + }, > + [AD5754R] = { > + .name = "ad5754r", > + .resolution = 16, > + .data_mask = AD5754_DATA_MASK(0), > + .internal_vref = true, > + .num_channels = 4, > + .channels = ad5754_channels[AD5754_4_CHANNELS], > + } Add a comma here as we may add more entries in future and that will reduce the noise in such a patch set > +}; > + > +static int ad5754_enable_channels(struct ad5754_state *st) > +{ > + struct fwnode_handle *channel_node = NULL; > + u32 real_channel; > + u32 power_reg; > + u32 index; > + int ret; > + > + fwnode_for_each_available_child_node(dev_fwnode(st->dev), channel_node) { > + ret = fwnode_property_read_u32(channel_node, "reg", &index); > + if (ret) { > + dev_err(st->dev, "Failed to read channel reg: %d\n", ret); > + goto free_node; > + } > + if (index >= st->chip_info->num_channels) { > + dev_err(st->dev, "Channel index %u is too large\n", index); > + goto free_node; > + } > + > + ret = ad5754_real_ch(st, index, &real_channel); > + if (ret) > + goto free_node; > + > + ret = ad5754_get_output_range(st, channel_node, index); > + if (ret) > + goto free_node; > + > + ret = regmap_write_bits(st->regmap, > + AD5754_REG_ADDR(AD5754_RANGE_REG, real_channel), > + AD5754_RANGE_MASK, st->range_idx[index]); > + if (ret) > + goto free_node; > + > + ret = regmap_read(st->regmap, > + AD5754_REG_ADDR(AD5754_PWR_REG, AD5754_PU_ADDR), > + &power_reg); I'm going to take a wild guess (without reading the datasheet) that this turned something on. Do we not want to ensure it's turned off again in the remove / devm_ unwind path? > + if (ret) > + goto free_node; > + > + ret = regmap_update_bits(st->regmap, > + AD5754_REG_ADDR(AD5754_PWR_REG, AD5754_PU_ADDR), > + AD5754_PU_MASK, AD5754_PU_CH(real_channel) | > + (power_reg & AD5754_PU_MASK)); > + if (ret) > + goto free_node; > + > + /* Channel power up delay */ > + fsleep(10); > + } > + > + return 0; > + > +free_node: > + fwnode_handle_put(channel_node); > + > + return ret; > +} > + > +static int ad5754_int_vref_enable(struct ad5754_state *st) > +{ > + return regmap_update_bits(st->regmap, > + AD5754_REG_ADDR(AD5754_PWR_REG, AD5754_PU_ADDR), > + AD5754_INT_REF_MASK, > + FIELD_PREP(AD5754_INT_REF_MASK, 1)); Whilst it is going to make for a long line / less than perfect formatting I'd prefer you don't have this wrapper, but just call the regmap_update_bits() at the call site > +} > + > +static void ad5754_disable_regulator(void *regulator) > +{ > + regulator_disable(regulator); > +} > + > +static const struct iio_info ad5754_info = { > + .read_raw = &ad5754_read_raw, > + .write_raw = &ad5754_write_raw, > +}; > + > +static int ad5754_probe(struct spi_device *spi) > +{ > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ad5754_state *st; > + struct device *dev; > + int ret; > + > + dev = &spi->dev; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + spi_set_drvdata(spi, indio_dev); Used anywhere? I might have missed it. > + > + st->spi = spi; > + st->dev = dev; Keeping copies of dev and spi seems excessive. > + st->chip_info = device_get_match_data(dev); > + if (!st->chip_info) > + st->chip_info = > + (const struct ad5754_chip_info *)spi_get_device_id(spi)->driver_data; > + > + st->regmap = devm_regmap_init(st->dev, NULL, st, &ad5754_regmap_config); > + if (IS_ERR(st->regmap)) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Regmap init error\n"); > + > + st->dac_max_code = BIT(st->chip_info->resolution) - 1; Why cache this? Just check against >= BIT(...) where needed > + st->sub_lsb = AD5754_MAX_RESOLUTION - st->chip_info->resolution; Similarly, seems unnecessary to cache this value given how cheap it is to compute. > + > + vref_reg = devm_regulator_get_optional(st->dev, "vref"); > + if (IS_ERR(vref_reg)) { > + if (!st->chip_info->internal_vref) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Failed to get the vref regulator\n"); > + > + st->vref = AD5754_INT_VREF; > + ret = ad5754_int_vref_enable(st); > + if (ret) > + return ret; > + } else { > + ret = regulator_enable(vref_reg); > + if (ret) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), ret > + "Failed to enable the vref regulator\n"); > + > + ret = devm_add_action_or_reset(dev, ad5754_disable_regulator, vref_reg); > + if (ret) > + return ret; > + > + ret = regulator_get_voltage(vref_reg); Could move this to where it's used. Tends not to be a hot path so cost of asking the supply for ti's voltage is normally fine. > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to get vref\n"); > + > + st->vref = ret / 1000; > + } > + > + indio_dev->name = st->chip_info->name; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &ad5754_info; > + indio_dev->channels = st->chip_info->channels; > + indio_dev->num_channels = st->chip_info->num_channels; > + > + ret = ad5754_enable_channels(st); > + if (ret) > + return ret; > + > + return devm_iio_device_register(st->dev, indio_dev); > +} > + > + > +static const struct spi_device_id ad5754_id[] = { > + { "ad5722", (kernel_ulong_t)&ad5754_chip_info_data[AD5722], }, > + { "ad5732", (kernel_ulong_t)&ad5754_chip_info_data[AD5732], }, > + { "ad5752", (kernel_ulong_t)&ad5754_chip_info_data[AD5752], }, > + { "ad5724", (kernel_ulong_t)&ad5754_chip_info_data[AD5724], }, > + { "ad5734", (kernel_ulong_t)&ad5754_chip_info_data[AD5734], }, > + { "ad5754", (kernel_ulong_t)&ad5754_chip_info_data[AD5754], }, > + { "ad5722r", (kernel_ulong_t)&ad5754_chip_info_data[AD5722R], }, > + { "ad5732r", (kernel_ulong_t)&ad5754_chip_info_data[AD5732R], }, > + { "ad5752r", (kernel_ulong_t)&ad5754_chip_info_data[AD5752R], }, > + { "ad5724r", (kernel_ulong_t)&ad5754_chip_info_data[AD5724R], }, > + { "ad5734r", (kernel_ulong_t)&ad5754_chip_info_data[AD5734R], }, > + { "ad5754r", (kernel_ulong_t)&ad5754_chip_info_data[AD5754R], }, > + {}, Nice to drop commas on null terminators as nothing can ever come after them anyway. > +}; > + > +static const struct of_device_id ad5754_dt_id[] = { > + { > + .compatible = "adi,ad5722", > + .data = &ad5754_chip_info_data[AD5722], > + }, > + { If there was ever a case for the acceptable option of }, { this is it :) > + .compatible = "adi,ad5732", > + .data = &ad5754_chip_info_data[AD5732], > + }, > + { > + .compatible = "adi,ad5752", > + .data = &ad5754_chip_info_data[AD5752], > + }, > + { > + .compatible = "adi,ad5724", > + .data = &ad5754_chip_info_data[AD5724], > + }, > + { > + .compatible = "adi,ad5734", > + .data = &ad5754_chip_info_data[AD5734], > + }, > + { > + .compatible = "adi,ad5754", > + .data = &ad5754_chip_info_data[AD5754], > + }, > + { > + .compatible = "adi,ad5722r", > + .data = &ad5754_chip_info_data[AD5722R], > + }, > + { > + .compatible = "adi,ad5732r", > + .data = &ad5754_chip_info_data[AD5732R], > + }, > + { > + .compatible = "adi,ad5752r", > + .data = &ad5754_chip_info_data[AD5752R], > + }, > + { > + .compatible = "adi,ad5724r", > + .data = &ad5754_chip_info_data[AD5724R], > + }, > + { > + .compatible = "adi,ad5734r", > + .data = &ad5754_chip_info_data[AD5734R], > + }, > + { > + .compatible = "adi,ad5754r", > + .data = &ad5754_chip_info_data[AD5754R], > + }, > + {}, No comma on NULL terminators. > +}; > +MODULE_DEVICE_TABLE(of, ad5754_dt_id); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-09 17:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-04 7:18 [PATCH 0/2] Add support for the AD5754 DAC Ciprian Regus 2022-10-04 7:18 ` [PATCH 1/2] dt-bindings: iio: dac: add adi,ad5754.yaml Ciprian Regus 2022-10-04 11:26 ` Krzysztof Kozlowski 2022-10-04 7:18 ` [PATCH 2/2] drivers: iio: dac: Add AD5754 DAC driver Ciprian Regus 2022-10-04 8:40 ` Marcus Folkesson 2022-10-04 8:50 ` Andy Shevchenko 2022-10-09 17:09 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox