[PATCH v4 0/3] add support for ad777x family

[PATCH v4 0/3] add support for ad777x family

[Ramona Alexandra Nechita]

v4:
	* updated commit description
	* removed misc indents in private structure
	* removed "_filter" from available filters
	* updated calibscale/bias functions to use get/put unaligned
	* switched to iio_device_claim_scoped
	* added comments exaplaining necessary timings/buffers
	* misc line breaks/wrap fixes
	* switched to iio_trigger_generic_data_poll
	* removed writing to indio_dev masklength
	* switched to kernel_ulong_t for driver data

Ramona Alexandra Nechita (3):
  dt-bindings: iio: adc: add a7779 doc
  Documentation: ABI: added filter mode doc in sysfs-bus-iio
  drivers: iio: adc: add support for ad777x family

 Documentation/ABI/testing/sysfs-bus-iio       |  22 +
 .../ABI/testing/sysfs-bus-iio-adc-ad4130      |  46 -
 .../bindings/iio/adc/adi,ad7779.yaml          |  85 ++
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad7779.c                      | 952 ++++++++++++++++++
 6 files changed, 1071 insertions(+), 46 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
 create mode 100644 drivers/iio/adc/ad7779.c

-- 
2.43.0

[PATCH v4 1/3] dt-bindings: iio: adc: add a7779 doc

[Ramona Alexandra Nechita]

Add dt bindings for adc ad7779.

Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
---
 .../bindings/iio/adc/adi,ad7779.yaml          | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
new file mode 100644
index 000000000000..10a67644e915
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7779.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD777X family 8-Channel, 24-Bit, Simultaneous Sampling ADCs
+
+maintainers:
+  - Ramona Nechita <ramona.nechita@analog.com>
+
+description: |
+  The AD777X family consist of 8-channel, simultaneous sampling analog-to-
+  digital converter (ADC). Eight full Σ-Δ ADCs are on-chip. The
+  AD7771 provides an ultralow input current to allow direct sensor
+  connection. Each input channel has a programmable gain stage
+  allowing gains of 1, 2, 4, and 8 to map lower amplitude sensor
+  outputs into the full-scale ADC input range, maximizing the
+  dynamic range of the signal chain.
+
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7770.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7771.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7779.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7770
+      - adi,ad7771
+      - adi,ad7779
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vref-supply:
+    description:
+      The regulator to use as an external reference. If it does not exists the
+      internal reference will be used.
+
+  start-gpios:
+    description:
+      Pin that controls start synchronization pulse.
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+          compatible = "adi,ad7779";
+          reg = <0>;
+          vref-supply = <&vref>;
+          start-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
+          reset-gpios = <&gpio0 93 GPIO_ACTIVE_LOW>;
+          clocks = <&adc_clk>;
+        };
+    };
+...
-- 
2.43.0

[PATCH v4 2/3] Documentation: ABI: added filter mode doc in sysfs-bus-iio

[Ramona Alexandra Nechita]

The filter mode / filter type property is used for ad4130
and ad7779 drivers, therefore the ABI doc file for ad4130
was removed, merging both of them in the sysfs-bus-iio

Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio       | 22 +++++++++
 .../ABI/testing/sysfs-bus-iio-adc-ad4130      | 46 -------------------
 2 files changed, 22 insertions(+), 46 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 2e6d5ebfd3c7..31b1cafec8b5 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2225,6 +2225,28 @@ Description:
 		An example format is 16-bytes, 2-digits-per-byte, HEX-string
 		representing the sensor unique ID number.
 
+What:		/sys/bus/iio/devices/iio:deviceX/filter_type_available
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
+KernelVersion:	6.1
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Reading returns a list with the possible filter modes. Options
+		for the attribute:
+			* "sinc3"	- The digital sinc3 filter. Moderate 1st conversion time.
+		    Good noise performance.
+			* "sinc4"       - Sinc 4. Excellent noise performance. Long
+			1st conversion time.
+			* "sinc5"	- The digital sinc5 filter. Excellent noise performance
+			* "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
+		    time.
+			* "sinc3+rej60" - Sinc3 + 60Hz rejection.
+			* "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion
+		    time.
+			* "sinc3+pf1"   - Sinc3 + device specific Post Filter 1.
+			* "sinc3+pf2"   - Sinc3 + device specific Post Filter 2.
+			* "sinc3+pf3"   - Sinc3 + device specific Post Filter 3.
+			* "sinc3+pf4"   - Sinc3 + device specific Post Filter 4.
+
 What:		/sys/.../events/in_proximity_thresh_either_runningperiod
 KernelVersion:	6.6
 Contact:	linux-iio@vger.kernel.org
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
deleted file mode 100644
index f24ed6687e90..000000000000
--- a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
+++ /dev/null
@@ -1,46 +0,0 @@
-What:		/sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
-KernelVersion:  6.2
-Contact:	linux-iio@vger.kernel.org
-Description:
-		Reading returns a list with the possible filter modes.
-
-		  * "sinc4"       - Sinc 4. Excellent noise performance. Long
-                    1st conversion time. No natural 50/60Hz rejection.
-
-		  * "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
-		    time.
-
-		  * "sinc3"	      - Sinc3. Moderate 1st conversion time.
-		    Good noise performance.
-
-		  * "sinc3+rej60" - Sinc3 + 60Hz rejection. At a sampling
-		    frequency of 50Hz, achieves simultaneous 50Hz and 60Hz
-		    rejection.
-
-		  * "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion
-		    time. Best used with a sampling frequency of at least
-		    216.19Hz.
-
-		  * "sinc3+pf1"   - Sinc3 + Post Filter 1. 53dB rejection @
-		    50Hz, 58dB rejection @ 60Hz.
-
-		  * "sinc3+pf2"   - Sinc3 + Post Filter 2. 70dB rejection @
-		    50Hz, 70dB rejection @ 60Hz.
-
-		  * "sinc3+pf3"   - Sinc3 + Post Filter 3. 99dB rejection @
-		    50Hz, 103dB rejection @ 60Hz.
-
-		  * "sinc3+pf4"   - Sinc3 + Post Filter 4. 103dB rejection @
-		    50Hz, 109dB rejection @ 60Hz.
-
-What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
-KernelVersion:  6.2
-Contact:	linux-iio@vger.kernel.org
-Description:
-		Set the filter mode of the differential channel. When the filter
-		mode changes, the in_voltageY-voltageZ_sampling_frequency and
-		in_voltageY-voltageZ_sampling_frequency_available attributes
-		might also change to accommodate the new filter mode.
-		If the current sampling frequency is out of range for the new
-		filter mode, the sampling frequency will be changed to the
-		closest valid one.
-- 
2.43.0

[PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Ramona Alexandra Nechita]

Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
sending out data both on DOUT lines interface,as on the SDO line.
The driver currently implements only theSDO data streaming mode. SPI
communication is used alternatively foraccessingregisters and streaming
data. Register access are protected by crc8.

Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
---
 drivers/iio/adc/Kconfig  |  11 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7779.c | 952 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 964 insertions(+)
 create mode 100644 drivers/iio/adc/ad7779.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 0d9282fa67f5..05d3719c2458 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -206,6 +206,17 @@ config AD7768_1
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad7768-1.
 
+config AD7779
+	tristate "Analog Devices AD7779 ADC driver"
+	depends on SPI
+	select IIO_BUFFER
+	help
+	  Say yes here to build support for Analog Devices AD777X family
+	  (AD7770, AD7771, AD7779) analog to digital converter (ADC).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad7779.
+
 config AD7780
 	tristate "Analog Devices AD7780 and similar ADCs driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index b3c434722364..e25997e926bb 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
 obj-$(CONFIG_AD7606) += ad7606.o
 obj-$(CONFIG_AD7766) += ad7766.o
 obj-$(CONFIG_AD7768_1) += ad7768-1.o
+obj-$(CONFIG_AD7779) += ad7779.o
 obj-$(CONFIG_AD7780) += ad7780.o
 obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
new file mode 100644
index 000000000000..7a83977fd00c
--- /dev/null
+++ b/drivers/iio/adc/ad7779.c
@@ -0,0 +1,952 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7770, AD7771, AD7779 ADC
+ *
+ * Copyright 2023-2024 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+#include <asm/unaligned.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define AD7779_SPI_READ_CMD			BIT(7)
+
+#define AD7779_DISABLE_SD			BIT(7)
+
+#define AD7779_REG_CH_DISABLE			0x08
+#define AD7779_REG_CH_SYNC_OFFSET(ch)		(0x09 + (ch))
+#define AD7779_REG_CH_CONFIG(ch)		(0x00 + (ch))
+#define AD7779_REG_GENERAL_USER_CONFIG_1	0x11
+#define AD7779_REG_GENERAL_USER_CONFIG_2	0x12
+#define AD7779_REG_GENERAL_USER_CONFIG_3	0x13
+#define AD7779_REG_DOUT_FORMAT			0x14
+#define AD7779_REG_ADC_MUX_CONFIG		0x15
+#define AD7779_REG_GPIO_CONFIG			0x17
+#define AD7779_REG_BUFFER_CONFIG_1		0x19
+#define AD7779_REG_GLOBAL_MUX_CONFIG		0x16
+#define AD7779_REG_BUFFER_CONFIG_2		0x1A
+#define AD7779_REG_GPIO_DATA			0x18
+#define AD7779_REG_CH_OFFSET_UPPER_BYTE(ch)	(0x1C + (ch) * 6)
+#define AD7779_REG_CH_OFFSET_LOWER_BYTE(ch)	(0x1E + (ch) * 6)
+#define AD7779_REG_CH_GAIN_UPPER_BYTE(ch)	(0x1F + (ch) * 6)
+#define AD7779_REG_CH_OFFSET_MID_BYTE(ch)	(0x1D + (ch) * 6)
+#define AD7779_REG_CH_GAIN_MID_BYTE(ch)		(0x20 + (ch) * 6)
+#define AD7779_REG_CH_ERR_REG(ch)		(0x4C + (ch))
+#define AD7779_REG_CH0_1_SAT_ERR		0x54
+#define AD7779_REG_CH_GAIN_LOWER_BYTE(ch)	(0x21 + (ch) * 6)
+#define AD7779_REG_CH2_3_SAT_ERR		0x55
+#define AD7779_REG_CH4_5_SAT_ERR		0x56
+#define AD7779_REG_CH6_7_SAT_ERR		0x57
+#define AD7779_REG_CHX_ERR_REG_EN		0x58
+#define AD7779_REG_GEN_ERR_REG_1		0x59
+#define AD7779_REG_GEN_ERR_REG_1_EN		0x5A
+#define AD7779_REG_GEN_ERR_REG_2		0x5B
+#define AD7779_REG_GEN_ERR_REG_2_EN		0x5C
+#define AD7779_REG_STATUS_REG_1			0x5D
+#define AD7779_REG_STATUS_REG_2			0x5E
+#define AD7779_REG_STATUS_REG_3			0x5F
+#define AD7779_REG_SRC_N_MSB			0x60
+#define AD7779_REG_SRC_N_LSB			0x61
+#define AD7779_REG_SRC_IF_MSB			0x62
+#define AD7779_REG_SRC_IF_LSB			0x63
+#define AD7779_REG_SRC_UPDATE			0x64
+
+#define AD7779_FILTER_MSK			BIT(6)
+#define AD7779_MOD_POWERMODE_MSK		BIT(6)
+#define AD7779_MOD_PDB_REFOUT_MSK		BIT(4)
+#define AD7779_MOD_SPI_EN_MSK			BIT(4)
+
+/* AD7779_REG_DOUT_FORMAT */
+#define AD7779_DOUT_FORMAT_MSK			GENMASK(7, 6)
+#define AD7779_DOUT_HEADER_FORMAT		BIT(5)
+#define AD7779_DCLK_CLK_DIV_MSK			GENMASK(3, 1)
+
+#define AD7779_REFMUX_CTRL_MSK			GENMASK(7, 6)
+#define AD7779_SPI_CRC_EN_MSK			BIT(0)
+
+#define AD7779_MAXCLK_LOWPOWER			4096000
+#define AD7779_NUM_CHANNELS			8
+#define AD7779_RESET_BUF_SIZE			8
+
+#define AD7779_LOWPOWER_DIV			512
+#define AD7779_HIGHPOWER_DIV			2048
+
+#define AD7779_SINC3_MAXFREQ			(16 * HZ_PER_KHZ)
+#define AD7779_SINC5_MAXFREQ			(128 * HZ_PER_KHZ)
+
+#define AD7779_DEFAULT_SAMPLING_FREQ		(8 * HZ_PER_KHZ)
+#define AD7779_DEFAULT_SAMPLING_2LINE		(4 * HZ_PER_KHZ)
+#define AD7779_DEFAULT_SAMPLING_1LINE		(2 * HZ_PER_KHZ)
+
+#define AD7779_SPIMODE_MAX_SAMP_FREQ		(16 * HZ_PER_KHZ)
+
+#define GAIN_REL				0x555555
+#define AD7779_FREQ_MSB_MSK			GENMASK(15, 8)
+#define AD7779_FREQ_LSB_MSK			GENMASK(7, 0)
+#define AD7779_UPPER				GENMASK(23, 16)
+#define AD7779_MID				GENMASK(15, 8)
+#define AD7779_LOWER				GENMASK(7, 0)
+
+#define AD7779_REG_MSK		GENMASK(6, 0)
+
+#define AD7779_CRC8_POLY			0x07
+DECLARE_CRC8_TABLE(ad7779_crc8_table);
+
+enum ad7779_filter {
+	AD7779_SINC3,
+	AD7779_SINC5,
+};
+
+enum ad7779_variant {
+	ad7770,
+	ad7771,
+	AD7779,
+};
+
+enum ad7779_power_mode {
+	AD7779_LOW_POWER,
+	AD7779_HIGH_POWER,
+};
+
+struct ad7779_chip_info {
+	const char *name;
+	struct iio_chan_spec const *channels;
+};
+
+struct ad7779_state {
+	struct spi_device *spi;
+	const struct ad7779_chip_info *chip_info;
+	struct clk *mclk;
+	struct iio_trigger *trig;
+	struct completion completion;
+	unsigned int sampling_freq;
+	enum ad7779_power_mode power_mode;
+	enum ad7779_filter filter_enabled;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	u8			reg_rx_buf[3] __aligned(IIO_DMA_MINALIGN);
+	u8			reg_tx_buf[3];
+	u32			spidata_rx[8];
+	u32			spidata_tx[8];
+	u8			reset_buf[8];
+};
+
+static const char * const ad7779_filter_type[] = {
+	[AD7779_SINC3] = "sinc3",
+	[AD7779_SINC5] = "sinc5",
+};
+
+static int ad7779_spi_read(struct ad7779_state *st, u8 reg, u8 *rbuf)
+{
+	int ret;
+	int length = 3;
+	u8 crc_buf[2];
+	u8 exp_crc = 0;
+	struct spi_transfer reg_read_tr[] = {
+		{
+			.tx_buf = st->reg_tx_buf,
+			.rx_buf = st->reg_rx_buf,
+		},
+	};
+
+	if (reg == AD7779_REG_GEN_ERR_REG_1_EN)
+		length = 2;
+	reg_read_tr[0].len = length;
+
+	st->reg_tx_buf[0] = AD7779_SPI_READ_CMD | FIELD_GET(AD7779_REG_MSK, reg);
+	st->reg_tx_buf[1] = 0;
+	st->reg_tx_buf[2] = crc8(ad7779_crc8_table, st->reg_tx_buf, 2, 0);
+
+	ret = spi_sync_transfer(st->spi, reg_read_tr, ARRAY_SIZE(reg_read_tr));
+	if (ret)
+		return ret;
+
+	crc_buf[0] = AD7779_SPI_READ_CMD | FIELD_GET(AD7779_REG_MSK, reg);
+	crc_buf[1] = st->reg_rx_buf[1];
+	exp_crc = crc8(ad7779_crc8_table, crc_buf, 2, 0);
+	if (reg != AD7779_REG_GEN_ERR_REG_1_EN && exp_crc != st->reg_rx_buf[2]) {
+		dev_err(&st->spi->dev, "Bad CRC %x, expected %x",
+			st->reg_rx_buf[2], exp_crc);
+		return -EINVAL;
+	}
+	*rbuf = st->reg_rx_buf[1];
+
+	return 0;
+}
+
+static int ad7779_spi_write(struct ad7779_state *st, u8 reg, u8 val)
+{
+	int length = 3;
+	struct spi_transfer reg_write_tr[] = {
+		{
+			.tx_buf = st->reg_tx_buf,
+		},
+	};
+
+	if (reg == AD7779_REG_GEN_ERR_REG_1_EN)
+		length = 2;
+	reg_write_tr[0].len = length;
+
+	st->reg_tx_buf[0] = FIELD_GET(AD7779_REG_MSK, reg);
+	st->reg_tx_buf[1] = val;
+	st->reg_tx_buf[2] = crc8(ad7779_crc8_table, st->reg_tx_buf, 2, 0);
+
+	return spi_sync_transfer(st->spi, reg_write_tr, ARRAY_SIZE(reg_write_tr));
+}
+
+static int ad7779_spi_write_mask(struct ad7779_state *st, u8 reg, u8 mask,
+				 u8 val)
+{
+	int ret;
+	u8 regval, data;
+
+	ret = ad7779_spi_read(st, reg, &data);
+	if (ret)
+		return ret;
+
+	regval = data;
+	regval &= ~mask;
+	regval |= val;
+
+	if (regval == data)
+		return 0;
+
+	return ad7779_spi_write(st, reg, regval);
+}
+
+static int ad7779_reg_access(struct iio_dev *indio_dev,
+			     unsigned int reg,
+			     unsigned int writeval,
+			     unsigned int *readval)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return ad7779_spi_read(st, reg, (u8 *)readval);
+
+	return ad7779_spi_write(st, reg, writeval);
+}
+
+static int ad7779_set_sampling_frequency(struct ad7779_state *st,
+					 unsigned int sampling_freq)
+{
+	int ret;
+	unsigned int dec;
+	unsigned int div;
+	unsigned int decimal;
+	int temp;
+	unsigned int kfreq;
+
+	if (st->filter_enabled == AD7779_SINC3 &&
+	    sampling_freq > AD7779_SINC3_MAXFREQ)
+		return -EINVAL;
+
+	if (st->filter_enabled == AD7779_SINC5 &&
+	    sampling_freq > AD7779_SINC5_MAXFREQ)
+		return -EINVAL;
+
+	if (sampling_freq > AD7779_SPIMODE_MAX_SAMP_FREQ)
+		return -EINVAL;
+
+	if (st->power_mode == AD7779_LOW_POWER)
+		div = AD7779_LOWPOWER_DIV;
+	else
+		div = AD7779_HIGHPOWER_DIV;
+
+	kfreq = sampling_freq / KILO;
+	dec = div / kfreq;
+
+	ret = ad7779_spi_write(st, AD7779_REG_SRC_N_MSB,
+						FIELD_GET(AD7779_FREQ_MSB_MSK, dec));
+	if (ret)
+		return ret;
+	ret = ad7779_spi_write(st, AD7779_REG_SRC_N_LSB,
+						FIELD_GET(AD7779_FREQ_LSB_MSK, dec));
+	if (ret)
+		return ret;
+
+	if (div % kfreq) {
+		temp = (div * KILO) / kfreq;
+		decimal = ((temp -  dec * KILO) << 16) / KILO;
+		ret = ad7779_spi_write(st, AD7779_REG_SRC_N_MSB,
+						FIELD_GET(AD7779_FREQ_MSB_MSK, decimal));
+		if (ret)
+			return ret;
+		ret = ad7779_spi_write(st, AD7779_REG_SRC_N_LSB,
+						FIELD_GET(AD7779_FREQ_LSB_MSK, decimal));
+		if (ret)
+			return ret;
+	} else {
+		ret = ad7779_spi_write(st, AD7779_REG_SRC_N_MSB,
+						   FIELD_GET(AD7779_FREQ_MSB_MSK, 0x0));
+		if (ret)
+			return ret;
+		ret = ad7779_spi_write(st, AD7779_REG_SRC_N_LSB,
+							FIELD_GET(AD7779_FREQ_LSB_MSK, 0x0));
+		if (ret)
+			return ret;
+	}
+	ret = ad7779_spi_write(st, AD7779_REG_SRC_UPDATE, 0x1);
+	if (ret)
+		return ret;
+
+	/* SRC update settling time */
+	fsleep(15);
+	ret = ad7779_spi_write(st, AD7779_REG_SRC_UPDATE, 0x0);
+	if (ret)
+		return ret;
+
+	/* SRC update settling time */
+	fsleep(15);
+
+	st->sampling_freq = sampling_freq;
+
+	return 0;
+}
+
+static int ad7779_get_filter(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+	u8 temp;
+	int ret;
+
+	ret = ad7779_spi_read(st, AD7779_REG_GENERAL_USER_CONFIG_2, &temp);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(AD7779_FILTER_MSK, temp);
+}
+
+static int ad7779_set_filter(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     unsigned int mode)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad7779_spi_write_mask(st,
+				    AD7779_REG_GENERAL_USER_CONFIG_2,
+				    AD7779_FILTER_MSK,
+				    FIELD_PREP(AD7779_FILTER_MSK, mode));
+	if (ret < 0)
+		return ret;
+
+	ret = ad7779_set_sampling_frequency(st, st->sampling_freq);
+	if (ret < 0)
+		return ret;
+
+	st->filter_enabled = mode;
+
+	return 0;
+}
+
+static int ad7779_get_calibscale(struct ad7779_state *st, int channel)
+{
+	int ret;
+	u8 calibscale[3];
+	// u8 low, mid, high;
+	ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_LOWER_BYTE(channel), &calibscale[0]);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_MID_BYTE(channel), &calibscale[1]);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_UPPER_BYTE(channel), &calibscale[2]);
+	if (ret)
+		return ret;
+
+	// return FIELD_PREP(AD7779_UPPER, high) | FIELD_PREP(AD7779_MID, mid) |
+	//        FIELD_PREP(AD7779_LOWER, low);
+	return get_unaligned_be24(calibscale);
+}
+
+static int ad7779_set_calibscale(struct ad7779_state *st, int channel, int val)
+{
+	int ret;
+	unsigned int gain;
+	unsigned long long tmp;
+	u8 gain_bytes[3];
+
+	tmp = val * 5592405LL;
+	gain = DIV_ROUND_CLOSEST_ULL(tmp, MEGA);
+	put_unaligned_be24(gain, gain_bytes);
+	ret = ad7779_spi_write(st,
+			       AD7779_REG_CH_GAIN_UPPER_BYTE(channel),
+			       gain_bytes[0]);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write(st,
+			       AD7779_REG_CH_GAIN_MID_BYTE(channel),
+			       gain_bytes[1]);
+	if (ret)
+		return ret;
+
+	return ad7779_spi_write(st,
+				AD7779_REG_CH_GAIN_LOWER_BYTE(channel),
+				gain_bytes[2]);
+}
+
+static int ad7779_get_calibbias(struct ad7779_state *st, int channel)
+{
+	int ret;
+	u8 calibbias[3];
+	u8 low, mid, high;
+
+	ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
+			      &calibbias[0]);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_MID_BYTE(channel),
+				  &calibbias[1]);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_read(st,
+			      AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
+			      &calibbias[2]);
+	if (ret)
+		return ret;
+
+	return get_unaligned_be24(calibbias);
+}
+
+static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
+{
+	int ret;
+	u8 calibbias[3];
+	u8 msb, mid, lsb;
+
+	msb = FIELD_GET(AD7779_UPPER, val);
+	mid = FIELD_GET(AD7779_MID, val);
+	lsb = FIELD_GET(AD7779_LOWER, val);
+	put_unaligned_be24(val, calibbias);
+	ret = ad7779_spi_write(st,
+			       AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
+			       calibbias[0]);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write(st,
+			       AD7779_REG_CH_OFFSET_MID_BYTE(channel),
+			       calibbias[1]);
+	if (ret)
+		return ret;
+
+	return ad7779_spi_write(st,
+				AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
+				calibbias[2]);
+}
+
+static int ad7779_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long mask)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+
+	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+		switch (mask) {
+		case IIO_CHAN_INFO_CALIBSCALE:
+			*val = ad7779_get_calibscale(st, chan->channel);
+			iio_device_release_direct_mode(indio_dev);
+			if (*val < 0)
+				return -EINVAL;
+			*val2 = GAIN_REL;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_CHAN_INFO_CALIBBIAS:
+			*val = ad7779_get_calibbias(st, chan->channel);
+			iio_device_release_direct_mode(indio_dev);
+			if (*val < 0)
+				return -EINVAL;
+			return IIO_VAL_INT;
+		case IIO_CHAN_INFO_SAMP_FREQ:
+			*val = st->sampling_freq;
+			iio_device_release_direct_mode(indio_dev);
+			if (*val < 0)
+				return -EINVAL;
+			return IIO_VAL_INT;
+		}
+		return -EINVAL;
+	}
+	unreachable();
+}
+
+static int ad7779_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val,
+			    int val2,
+			    long mask)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return ad7779_set_calibscale(st, chan->channel, val2);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		return ad7779_set_calibbias(st, chan->channel, val);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad7779_set_sampling_frequency(st, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7779_buffer_preenable(struct iio_dev *indio_dev)
+{
+	int ret;
+	struct ad7779_state *st = iio_priv(indio_dev);
+
+	ret = ad7779_spi_write_mask(st,
+				    AD7779_REG_GENERAL_USER_CONFIG_3,
+				    AD7779_MOD_SPI_EN_MSK,
+				    FIELD_PREP(AD7779_MOD_SPI_EN_MSK, 1));
+	if (ret)
+		return ret;
+ 
+	/* DRDY output cannot be disabled at device level
+	 * therefore we mask the irq at host end.
+	 */
+	enable_irq(st->spi->irq);
+
+	return 0;
+}
+
+static int ad7779_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+
+	disable_irq(st->spi->irq);
+
+	return ad7779_spi_write(st, AD7779_REG_GENERAL_USER_CONFIG_3,
+			       AD7779_DISABLE_SD);
+}
+
+static irqreturn_t ad7779_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+	int bit;
+	int k = 0;
+	/* Each channel shifts out HEADER + 24 bits of data
+	 * therefore 8 * u32 for the data and 64 bits for
+	 * the timestamp
+	 */
+	u32 tmp[10]; 
+
+	struct spi_transfer sd_readback_tr[] = {
+		{
+			.rx_buf = st->spidata_rx,
+			.tx_buf = st->spidata_tx,
+			.len = 32,
+		}
+	};
+
+	if (!iio_buffer_enabled(indio_dev)){
+		iio_trigger_notify_done(indio_dev->trig);
+		return IRQ_HANDLED;
+	}
+
+	st->spidata_tx[0] = AD7779_SPI_READ_CMD;
+	ret = spi_sync_transfer(st->spi, sd_readback_tr,
+				ARRAY_SIZE(sd_readback_tr));
+	if (ret) {
+		dev_err(&st->spi->dev,
+			"spi transfer error in irq handler");
+		iio_trigger_notify_done(indio_dev->trig);
+		return IRQ_HANDLED;
+	}
+
+	for_each_set_bit(bit, indio_dev->active_scan_mask, AD7779_NUM_CHANNELS - 1)
+		tmp[k++] = st->spidata_rx[bit];
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &tmp[0], pf->timestamp);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int ad7779_reset(struct iio_dev *indio_dev, struct gpio_desc *reset_gpio)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+	struct spi_transfer reg_read_tr[] = {
+		{
+			.tx_buf = st->reset_buf,
+			.len = 8,
+		},
+	};
+
+	memset(st->reset_buf, 0xff, sizeof(st->reset_buf));
+
+	if (reset_gpio) {
+		/* Delay for reset to occur is 225 microseconds*/
+		gpiod_set_value(reset_gpio, 1);
+		fsleep(230);
+		return 0;
+	}
+
+	ret = spi_sync_transfer(st->spi, reg_read_tr,
+				ARRAY_SIZE(reg_read_tr));
+	if (ret)
+		return ret;
+
+	/* Delay for reset to occur is 225 microseconds*/
+	fsleep(230);
+
+	return 0;
+}
+
+static const struct iio_info ad7779_info = {
+	.read_raw = ad7779_read_raw,
+	.write_raw = ad7779_write_raw,
+	.debugfs_reg_access = &ad7779_reg_access,
+};
+
+static const struct iio_enum ad7779_filter_enum = {
+	.items = ad7779_filter_type,
+	.num_items = ARRAY_SIZE(ad7779_filter_type),
+	.get = ad7779_get_filter,
+	.set = ad7779_set_filter,
+};
+
+static const struct iio_chan_spec_ext_info ad7779_ext_filter[] = {
+	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad7779_filter_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
+				  &ad7779_filter_enum),
+	{ }
+};
+
+#define AD777x_CHAN_S(index, _ext_info)					       \
+	{								       \
+		.type = IIO_VOLTAGE,					       \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE)  |	       \
+				      BIT(IIO_CHAN_INFO_CALIBBIAS),	       \
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),       \
+		.address = (index),					       \
+		.indexed = 1,						       \
+		.channel = (index),					       \
+		.scan_index = (index),					       \
+		.ext_info = (_ext_info),				       \
+		.scan_type = {						       \
+			.sign = 's',					       \
+			.realbits = 24,					       \
+			.storagebits = 32,				       \
+			.endianness = IIO_BE,				   \
+		},							       		   \
+	}
+
+#define AD777x_CHAN_NO_FILTER_S(index)					       \
+	AD777x_CHAN_S(index, NULL)
+
+#define AD777x_CHAN_FILTER_S(index)					       \
+	AD777x_CHAN_S(index, ad7779_ext_filter)
+
+static const struct iio_chan_spec ad7779_channels[] = {
+	AD777x_CHAN_NO_FILTER_S(0),
+	AD777x_CHAN_NO_FILTER_S(1),
+	AD777x_CHAN_NO_FILTER_S(2),
+	AD777x_CHAN_NO_FILTER_S(3),
+	AD777x_CHAN_NO_FILTER_S(4),
+	AD777x_CHAN_NO_FILTER_S(5),
+	AD777x_CHAN_NO_FILTER_S(6),
+	AD777x_CHAN_NO_FILTER_S(7),
+	IIO_CHAN_SOFT_TIMESTAMP(8),
+};
+
+static const struct iio_chan_spec ad7779_channels_filter[] = {
+	AD777x_CHAN_FILTER_S(0),
+	AD777x_CHAN_FILTER_S(1),
+	AD777x_CHAN_FILTER_S(2),
+	AD777x_CHAN_FILTER_S(3),
+	AD777x_CHAN_FILTER_S(4),
+	AD777x_CHAN_FILTER_S(5),
+	AD777x_CHAN_FILTER_S(6),
+	AD777x_CHAN_FILTER_S(7),
+	IIO_CHAN_SOFT_TIMESTAMP(8),
+};
+
+static const struct iio_buffer_setup_ops ad7779_buffer_setup_ops = {
+	.preenable = ad7779_buffer_preenable,
+	.postdisable = ad7779_buffer_postdisable,
+};
+
+static const struct iio_trigger_ops ad7779_trigger_ops = {
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static int ad7779_powerup(struct ad7779_state *st, struct gpio_desc *start_gpio)
+{
+	int ret;
+	
+	ret = ad7779_spi_write_mask(st, AD7779_REG_GEN_ERR_REG_1_EN,
+				    AD7779_SPI_CRC_EN_MSK,
+				    FIELD_PREP(AD7779_SPI_CRC_EN_MSK, 1));
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+				    AD7779_MOD_POWERMODE_MSK,
+				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK, 1));
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+				    AD7779_MOD_PDB_REFOUT_MSK,
+				    FIELD_PREP(AD7779_MOD_PDB_REFOUT_MSK, 1));
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
+				    AD7779_DCLK_CLK_DIV_MSK,
+				    FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 1));
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_ADC_MUX_CONFIG,
+				    AD7779_REFMUX_CTRL_MSK,
+				    FIELD_PREP(AD7779_REFMUX_CTRL_MSK, 1));
+	if (ret)
+		return ret;
+
+
+	st->power_mode = AD7779_HIGH_POWER;
+	ret = ad7779_set_sampling_frequency(st, AD7779_DEFAULT_SAMPLING_FREQ);
+	if (ret)
+		return ret;
+
+	gpiod_set_value(start_gpio, 0);
+	/* Start setup time */
+	fsleep(15);
+	gpiod_set_value(start_gpio, 1);
+	fsleep(15);
+	gpiod_set_value(start_gpio, 0);
+	fsleep(15);
+
+	return 0;
+}
+
+static int ad7779_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ad7779_state *st;
+	struct gpio_desc *reset_gpio, *start_gpio;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
+	if (IS_ERR(st->mclk))
+		return PTR_ERR(st->mclk);
+
+	if (!spi->irq)
+		return dev_err_probe(&spi->dev, ret,
+				     "DRDY irq not present\n"); 
+
+	reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset_gpio))
+		return PTR_ERR(reset_gpio);
+
+	start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
+	if (IS_ERR(start_gpio))
+		return PTR_ERR(start_gpio);
+
+	crc8_populate_msb(ad7779_crc8_table, AD7779_CRC8_POLY);
+	st->spi = spi;
+
+	st->chip_info = spi_get_device_match_data(spi);
+	if (!st->chip_info)
+		return -ENODEV;
+
+	ret = ad7779_reset(indio_dev, start_gpio);
+	if (ret)
+		return ret;
+
+	ad7779_powerup(st, start_gpio);
+	if (ret)
+		return ret;
+
+	indio_dev->name = st->chip_info->name;
+	indio_dev->info = &ad7779_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = st->chip_info->channels;
+	indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
+
+	st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+					  		indio_dev->name, iio_device_id(indio_dev));
+	if (!st->trig)
+		return -ENOMEM;
+
+	st->trig->ops= &ad7779_trigger_ops;
+	st->trig->dev.parent = &spi->dev;
+
+	iio_trigger_set_drvdata(st->trig, st);
+
+	ret = devm_request_irq(&spi->dev, spi->irq,
+						   iio_trigger_generic_data_rdy_poll,
+						   IRQF_ONESHOT | IRQF_NO_AUTOEN,
+						   indio_dev->name, st->trig);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "request irq %d failed\n",
+				     st->spi->irq);
+
+	ret = devm_iio_trigger_register(&spi->dev, st->trig);
+	if (ret)
+		return ret;
+
+	indio_dev->trig = iio_trigger_get(st->trig);
+
+	init_completion(&st->completion);
+
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+										  &iio_pollfunc_store_time,
+										  &ad7779_trigger_handler,
+										  &ad7779_buffer_setup_ops);
+	if (ret)
+		return ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
+				    AD7779_DCLK_CLK_DIV_MSK,
+				    FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static int __maybe_unused ad7779_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+				    AD7779_MOD_POWERMODE_MSK,
+				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
+					       AD7779_LOW_POWER));
+	if (ret)
+		return ret;
+
+	st->power_mode = AD7779_LOW_POWER;
+	return 0;
+}
+
+static int __maybe_unused ad7779_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
+				    AD7779_MOD_POWERMODE_MSK,
+				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
+					       AD7779_HIGH_POWER));
+	if (ret)
+		return ret;
+
+	st->power_mode = AD7779_HIGH_POWER;
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ad7779_pm_ops, ad7779_suspend, ad7779_resume);
+
+static const struct ad7779_chip_info ad7770_chip_info = {
+	.name = "ad7770",
+	.channels = ad7779_channels,
+};
+
+static const struct ad7779_chip_info ad7771_chip_info = {
+	.name = "ad7771",
+	.channels = ad7779_channels_filter,
+};
+
+static const struct ad7779_chip_info AD7779_chip_info = {
+	.name = "AD7779",
+	.channels = ad7779_channels,
+};
+
+static const struct spi_device_id ad7779_id[] = {
+	{
+		.name = "ad7770",
+		.driver_data = (kernel_ulong_t)&ad7770_chip_info
+	},
+	{
+		.name = "ad7771",
+		.driver_data = (kernel_ulong_t)&ad7771_chip_info
+	},
+	{
+		.name = "AD7779",
+		.driver_data = (kernel_ulong_t)&AD7779_chip_info
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad7779_id);
+
+static const struct of_device_id ad7779_of_table[] = {
+	{
+		.compatible = "adi,ad7770",
+		.data = &ad7770_chip_info,
+	},
+	{
+		.compatible = "adi,ad7771",
+		.data = &ad7771_chip_info,
+	},
+	{
+		.compatible = "adi,AD7779",
+		.data = &AD7779_chip_info,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7779_of_table);
+
+static struct spi_driver ad7779_driver = {
+	.driver = {
+		.name = "ad7779",
+		.pm = pm_sleep_ptr(&ad7779_pm_ops),
+		.of_match_table = ad7779_of_table,
+	},
+	.probe = ad7779_probe,
+	.id_table = ad7779_id,
+};
+module_spi_driver(ad7779_driver);
+
+MODULE_AUTHOR("Ramona Alexandra Nechita <ramona.nechita@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7779 ADC");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v4 1/3] dt-bindings: iio: adc: add a7779 doc

[Krzysztof Kozlowski]

On 24/07/2024 17:54, Ramona Alexandra Nechita wrote:
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

Best regards,
Krzysztof

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Krzysztof Kozlowski]

On 24/07/2024 17:54, Ramona Alexandra Nechita wrote:
> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
> sending out data both on DOUT lines interface,as on the SDO line.
> The driver currently implements only theSDO data streaming mode. SPI
> communication is used alternatively foraccessingregisters and streaming

Typo, please run spell check.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

There is no "drivers".

> data. Register access are protected by crc8.
> 
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
> ---
>  drivers/iio/adc/Kconfig  |  11 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad7779.c | 952 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 964 insertions(+)
>  create mode 100644 drivers/iio/adc/ad7779.c
> 

The driver has several trivial style issues. Please be sure such
trivialities are fixed. Get internal review on this. You do need to ask
community to tell you that you must run checkpatch. Or to tell them that
indentation/alignment is entirely broken. Grab some colleague of yours
and perform internal review first. This applies to entire Analog,
because there is increased amount of contributions from Analog and not
all of them look like passing basic sanity checks.

By sending code full of silly trivialities, community reviewers might
feel overwhelmed and quite grumpy.

> +
> +	indio_dev->trig = iio_trigger_get(st->trig);
> +
> +	init_completion(&st->completion);
> +
> +	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> +										  &iio_pollfunc_store_time,
> +										  &ad7779_trigger_handler,
> +										  &ad7779_buffer_setup_ops);

Sorry, tha's not a Linux coding style. Checkpatch will tell you that.

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> +				    AD7779_DCLK_CLK_DIV_MSK,
> +				    FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static int __maybe_unused ad7779_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> +				    AD7779_MOD_POWERMODE_MSK,
> +				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> +					       AD7779_LOW_POWER));
> +	if (ret)
> +		return ret;
> +
> +	st->power_mode = AD7779_LOW_POWER;
> +	return 0;
> +}
> +
> +static int __maybe_unused ad7779_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> +				    AD7779_MOD_POWERMODE_MSK,
> +				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> +					       AD7779_HIGH_POWER));
> +	if (ret)
> +		return ret;
> +
> +	st->power_mode = AD7779_HIGH_POWER;
> +
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ad7779_pm_ops, ad7779_suspend, ad7779_resume);
> +
> +static const struct ad7779_chip_info ad7770_chip_info = {
> +	.name = "ad7770",
> +	.channels = ad7779_channels,
> +};
> +
> +static const struct ad7779_chip_info ad7771_chip_info = {
> +	.name = "ad7771",
> +	.channels = ad7779_channels_filter,
> +};
> +
> +static const struct ad7779_chip_info AD7779_chip_info = {
> +	.name = "AD7779",

Hm...

> +	.channels = ad7779_channels,
> +};
> +
> +static const struct spi_device_id ad7779_id[] = {
> +	{
> +		.name = "ad7770",
> +		.driver_data = (kernel_ulong_t)&ad7770_chip_info
> +	},
> +	{
> +		.name = "ad7771",
> +		.driver_data = (kernel_ulong_t)&ad7771_chip_info
> +	},
> +	{
> +		.name = "AD7779",

Eh...

> +		.driver_data = (kernel_ulong_t)&AD7779_chip_info
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, ad7779_id);
> +
> +static const struct of_device_id ad7779_of_table[] = {
> +	{
> +		.compatible = "adi,ad7770",
> +		.data = &ad7770_chip_info,
> +	},
> +	{
> +		.compatible = "adi,ad7771",
> +		.data = &ad7771_chip_info,
> +	},
> +	{
> +		.compatible = "adi,AD7779",

That's not a valid compatible. Only lower-case are allowed.

> +		.data = &AD7779_chip_info,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7779_of_table);
> +

Best regards,
Krzysztof

Re: [PATCH v4 2/3] Documentation: ABI: added filter mode doc in sysfs-bus-iio

[kernel test robot]

Hi Ramona,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.10 next-20240724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240724155517.12470-4-ramona.nechita%40analog.com
patch subject: [PATCH v4 2/3] Documentation: ABI: added filter mode doc in sysfs-bus-iio
reproduce: (https://download.01.org/0day-ci/archive/20240725/202407250236.nzmLshy5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407250236.nzmLshy5-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
   Warning: Documentation/devicetree/bindings/regulator/siliconmitus,sm5703-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/siliconmitus,sm5703.yaml
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/reserved-memory/qcom
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/display/exynos/
   Using alabaster theme

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[kernel test robot]

Hi Ramona,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.10 next-20240724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240724155517.12470-5-ramona.nechita%40analog.com
patch subject: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family
config: xtensa-allyesconfig (https://download.01.org/0day-ci/archive/20240725/202407250808.qn23hGFg-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407250808.qn23hGFg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407250808.qn23hGFg-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/iio/adc/ad7779.c: In function 'ad7779_get_calibbias':
>> drivers/iio/adc/ad7779.c:420:22: warning: unused variable 'high' [-Wunused-variable]
     420 |         u8 low, mid, high;
         |                      ^~~~
>> drivers/iio/adc/ad7779.c:420:17: warning: unused variable 'mid' [-Wunused-variable]
     420 |         u8 low, mid, high;
         |                 ^~~
>> drivers/iio/adc/ad7779.c:420:12: warning: unused variable 'low' [-Wunused-variable]
     420 |         u8 low, mid, high;
         |            ^~~
   drivers/iio/adc/ad7779.c: In function 'ad7779_set_calibbias':
>> drivers/iio/adc/ad7779.c:445:22: warning: variable 'lsb' set but not used [-Wunused-but-set-variable]
     445 |         u8 msb, mid, lsb;
         |                      ^~~
>> drivers/iio/adc/ad7779.c:445:17: warning: variable 'mid' set but not used [-Wunused-but-set-variable]
     445 |         u8 msb, mid, lsb;
         |                 ^~~
>> drivers/iio/adc/ad7779.c:445:12: warning: variable 'msb' set but not used [-Wunused-but-set-variable]
     445 |         u8 msb, mid, lsb;
         |            ^~~
   drivers/iio/adc/ad7779.c: In function 'ad7779_read_raw':
>> drivers/iio/adc/ad7779.c:475:13: warning: unused variable 'ret' [-Wunused-variable]
     475 |         int ret;
         |             ^~~


vim +/high +420 drivers/iio/adc/ad7779.c

   415	
   416	static int ad7779_get_calibbias(struct ad7779_state *st, int channel)
   417	{
   418		int ret;
   419		u8 calibbias[3];
 > 420		u8 low, mid, high;
   421	
   422		ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
   423				      &calibbias[0]);
   424		if (ret)
   425			return ret;
   426	
   427		ret = ad7779_spi_read(st, AD7779_REG_CH_OFFSET_MID_BYTE(channel),
   428					  &calibbias[1]);
   429		if (ret)
   430			return ret;
   431	
   432		ret = ad7779_spi_read(st,
   433				      AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
   434				      &calibbias[2]);
   435		if (ret)
   436			return ret;
   437	
   438		return get_unaligned_be24(calibbias);
   439	}
   440	
   441	static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
   442	{
   443		int ret;
   444		u8 calibbias[3];
 > 445		u8 msb, mid, lsb;
   446	
   447		msb = FIELD_GET(AD7779_UPPER, val);
   448		mid = FIELD_GET(AD7779_MID, val);
   449		lsb = FIELD_GET(AD7779_LOWER, val);
   450		put_unaligned_be24(val, calibbias);
   451		ret = ad7779_spi_write(st,
   452				       AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
   453				       calibbias[0]);
   454		if (ret)
   455			return ret;
   456	
   457		ret = ad7779_spi_write(st,
   458				       AD7779_REG_CH_OFFSET_MID_BYTE(channel),
   459				       calibbias[1]);
   460		if (ret)
   461			return ret;
   462	
   463		return ad7779_spi_write(st,
   464					AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
   465					calibbias[2]);
   466	}
   467	
   468	static int ad7779_read_raw(struct iio_dev *indio_dev,
   469				   struct iio_chan_spec const *chan,
   470				   int *val,
   471				   int *val2,
   472				   long mask)
   473	{
   474		struct ad7779_state *st = iio_priv(indio_dev);
 > 475		int ret;
   476	
   477		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
   478			switch (mask) {
   479			case IIO_CHAN_INFO_CALIBSCALE:
   480				*val = ad7779_get_calibscale(st, chan->channel);
   481				iio_device_release_direct_mode(indio_dev);
   482				if (*val < 0)
   483					return -EINVAL;
   484				*val2 = GAIN_REL;
   485				return IIO_VAL_FRACTIONAL;
   486			case IIO_CHAN_INFO_CALIBBIAS:
   487				*val = ad7779_get_calibbias(st, chan->channel);
   488				iio_device_release_direct_mode(indio_dev);
   489				if (*val < 0)
   490					return -EINVAL;
   491				return IIO_VAL_INT;
   492			case IIO_CHAN_INFO_SAMP_FREQ:
   493				*val = st->sampling_freq;
   494				iio_device_release_direct_mode(indio_dev);
   495				if (*val < 0)
   496					return -EINVAL;
   497				return IIO_VAL_INT;
   498			}
   499			return -EINVAL;
   500		}
   501		unreachable();
   502	}
   503	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[kernel test robot]

Hi Ramona,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.10 next-20240724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240724155517.12470-5-ramona.nechita%40analog.com
patch subject: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240725/202407251042.LUZ78skF-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407251042.LUZ78skF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407251042.LUZ78skF-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/iio/adc/ad7779.c:420:5: warning: unused variable 'low' [-Wunused-variable]
     420 |         u8 low, mid, high;
         |            ^~~
   drivers/iio/adc/ad7779.c:420:10: warning: unused variable 'mid' [-Wunused-variable]
     420 |         u8 low, mid, high;
         |                 ^~~
   drivers/iio/adc/ad7779.c:420:15: warning: unused variable 'high' [-Wunused-variable]
     420 |         u8 low, mid, high;
         |                      ^~~~
   drivers/iio/adc/ad7779.c:445:5: warning: variable 'msb' set but not used [-Wunused-but-set-variable]
     445 |         u8 msb, mid, lsb;
         |            ^
   drivers/iio/adc/ad7779.c:445:10: warning: variable 'mid' set but not used [-Wunused-but-set-variable]
     445 |         u8 msb, mid, lsb;
         |                 ^
   drivers/iio/adc/ad7779.c:445:15: warning: variable 'lsb' set but not used [-Wunused-but-set-variable]
     445 |         u8 msb, mid, lsb;
         |                      ^
   drivers/iio/adc/ad7779.c:475:6: warning: unused variable 'ret' [-Wunused-variable]
     475 |         int ret;
         |             ^~~
>> drivers/iio/adc/ad7779.c:779:35: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     779 |                 return dev_err_probe(&spi->dev, ret,
         |                                                 ^~~
   drivers/iio/adc/ad7779.c:766:9: note: initialize the variable 'ret' to silence this warning
     766 |         int ret;
         |                ^
         |                 = 0
   8 warnings generated.


vim +/ret +779 drivers/iio/adc/ad7779.c

   760	
   761	static int ad7779_probe(struct spi_device *spi)
   762	{
   763		struct iio_dev *indio_dev;
   764		struct ad7779_state *st;
   765		struct gpio_desc *reset_gpio, *start_gpio;
   766		int ret;
   767	
   768		indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
   769		if (!indio_dev)
   770			return -ENOMEM;
   771	
   772		st = iio_priv(indio_dev);
   773	
   774		st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
   775		if (IS_ERR(st->mclk))
   776			return PTR_ERR(st->mclk);
   777	
   778		if (!spi->irq)
 > 779			return dev_err_probe(&spi->dev, ret,
   780					     "DRDY irq not present\n"); 
   781	
   782		reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
   783		if (IS_ERR(reset_gpio))
   784			return PTR_ERR(reset_gpio);
   785	
   786		start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
   787		if (IS_ERR(start_gpio))
   788			return PTR_ERR(start_gpio);
   789	
   790		crc8_populate_msb(ad7779_crc8_table, AD7779_CRC8_POLY);
   791		st->spi = spi;
   792	
   793		st->chip_info = spi_get_device_match_data(spi);
   794		if (!st->chip_info)
   795			return -ENODEV;
   796	
   797		ret = ad7779_reset(indio_dev, start_gpio);
   798		if (ret)
   799			return ret;
   800	
   801		ad7779_powerup(st, start_gpio);
   802		if (ret)
   803			return ret;
   804	
   805		indio_dev->name = st->chip_info->name;
   806		indio_dev->info = &ad7779_info;
   807		indio_dev->modes = INDIO_DIRECT_MODE;
   808		indio_dev->channels = st->chip_info->channels;
   809		indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
   810	
   811		st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
   812						  		indio_dev->name, iio_device_id(indio_dev));
   813		if (!st->trig)
   814			return -ENOMEM;
   815	
   816		st->trig->ops= &ad7779_trigger_ops;
   817		st->trig->dev.parent = &spi->dev;
   818	
   819		iio_trigger_set_drvdata(st->trig, st);
   820	
   821		ret = devm_request_irq(&spi->dev, spi->irq,
   822							   iio_trigger_generic_data_rdy_poll,
   823							   IRQF_ONESHOT | IRQF_NO_AUTOEN,
   824							   indio_dev->name, st->trig);
   825		if (ret)
   826			return dev_err_probe(&spi->dev, ret, "request irq %d failed\n",
   827					     st->spi->irq);
   828	
   829		ret = devm_iio_trigger_register(&spi->dev, st->trig);
   830		if (ret)
   831			return ret;
   832	
   833		indio_dev->trig = iio_trigger_get(st->trig);
   834	
   835		init_completion(&st->completion);
   836	
   837		ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
   838											  &iio_pollfunc_store_time,
   839											  &ad7779_trigger_handler,
   840											  &ad7779_buffer_setup_ops);
   841		if (ret)
   842			return ret;
   843	
   844		ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
   845					    AD7779_DCLK_CLK_DIV_MSK,
   846					    FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
   847		if (ret)
   848			return ret;
   849	
   850		return devm_iio_device_register(&spi->dev, indio_dev);
   851	}
   852	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Krzysztof Kozlowski]

On 24/07/2024 18:14, Krzysztof Kozlowski wrote:
> On 24/07/2024 17:54, Ramona Alexandra Nechita wrote:
>> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
>> sending out data both on DOUT lines interface,as on the SDO line.
>> The driver currently implements only theSDO data streaming mode. SPI
>> communication is used alternatively foraccessingregisters and streaming
> 
> Typo, please run spell check.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> There is no "drivers".
> 
>> data. Register access are protected by crc8.
>>
>> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
>> ---
>>  drivers/iio/adc/Kconfig  |  11 +
>>  drivers/iio/adc/Makefile |   1 +
>>  drivers/iio/adc/ad7779.c | 952 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 964 insertions(+)
>>  create mode 100644 drivers/iio/adc/ad7779.c
>>
> 
> The driver has several trivial style issues. Please be sure such
> trivialities are fixed. Get internal review on this. You do need to ask
> community to tell you that you must run checkpatch. Or to tell them that
> indentation/alignment is entirely broken. Grab some colleague of yours
> and perform internal review first. This applies to entire Analog,
> because there is increased amount of contributions from Analog and not
> all of them look like passing basic sanity checks.
> 
> By sending code full of silly trivialities, community reviewers might
> feel overwhelmed and quite grumpy.

The amount of trivial warnings pointed out by compilation in separate
email is as well disappointing. You do not need community to check if
all variables are used - compilers tell it, so use them. Toolchains and
static checkers will point some issues without any need of using
community reviewers. Look, isn't it great? Instead of using human force
you can use tools...

This applies to all Analog contributions - *you must check* your code
with W=1, sparse, smatch and coccinelle.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: iio: adc: add a7779 doc

[Jonathan Cameron]

On Wed, 24 Jul 2024 18:54:39 +0300
Ramona Alexandra Nechita <ramona.nechita@analog.com> wrote:

> Add dt bindings for adc ad7779.
> 
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad7779.yaml          | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
> new file mode 100644
> index 000000000000..10a67644e915
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7779.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD777X family 8-Channel, 24-Bit, Simultaneous Sampling ADCs
> +
> +maintainers:
> +  - Ramona Nechita <ramona.nechita@analog.com>
> +
> +description: |
> +  The AD777X family consist of 8-channel, simultaneous sampling analog-to-
> +  digital converter (ADC). Eight full Σ-Δ ADCs are on-chip. The
> +  AD7771 provides an ultralow input current to allow direct sensor
> +  connection. Each input channel has a programmable gain stage
> +  allowing gains of 1, 2, 4, and 8 to map lower amplitude sensor
> +  outputs into the full-scale ADC input range, maximizing the
> +  dynamic range of the signal chain.
> +
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7770.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7771.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7779.pdf
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7770
> +      - adi,ad7771
> +      - adi,ad7779
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  clocks:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +

I missed this earlier, but a device that only has a reference supply is 
novel.  A quick glance at the datasheet shows a bunch of others power supplies.
They all need to be documented and any that are required for the device to function
must be listed as required.

Also seems to be at least one plausible interrupt line (alert)
that needs documenting whether or not the driver supports it yet.



> +  vref-supply:
> +    description:
> +      The regulator to use as an external reference. If it does not exists the
> +      internal reference will be used.
> +
> +  start-gpios:
> +    description:
> +      Pin that controls start synchronization pulse.
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +          compatible = "adi,ad7779";
> +          reg = <0>;
> +          vref-supply = <&vref>;
> +          start-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> +          reset-gpios = <&gpio0 93 GPIO_ACTIVE_LOW>;
> +          clocks = <&adc_clk>;
> +        };
> +    };
> +...

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Jonathan Cameron]

On Wed, 24 Jul 2024 18:54:41 +0300
Ramona Alexandra Nechita <ramona.nechita@analog.com> wrote:

> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
> sending out data both on DOUT lines interface,as on the SDO line.
> The driver currently implements only theSDO data streaming mode. SPI
> communication is used alternatively foraccessingregisters and streaming
> data. Register access are protected by crc8.
> 
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
Hi Ramona,

Various comments inline.  Key one though is make sure you read your
own code before posting as there is a bunch of left over stuff
from updates still in the code.

Jonathan

> diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
> new file mode 100644
> index 000000000000..7a83977fd00c
> --- /dev/null
> +++ b/drivers/iio/adc/ad7779.c

> +
> +static int ad7779_spi_write(struct ad7779_state *st, u8 reg, u8 val)
> +{
> +	int length = 3;
> +	struct spi_transfer reg_write_tr[] = {
> +		{
> +			.tx_buf = st->reg_tx_buf,
> +		},
> +	};
> +
> +	if (reg == AD7779_REG_GEN_ERR_REG_1_EN)
> +		length = 2;
> +	reg_write_tr[0].len = length;
> +
> +	st->reg_tx_buf[0] = FIELD_GET(AD7779_REG_MSK, reg);
> +	st->reg_tx_buf[1] = val;
> +	st->reg_tx_buf[2] = crc8(ad7779_crc8_table, st->reg_tx_buf, 2, 0);
> +
> +	return spi_sync_transfer(st->spi, reg_write_tr, ARRAY_SIZE(reg_write_tr));

spi_write()?

> +}


> +static int ad7779_set_sampling_frequency(struct ad7779_state *st,
> +					 unsigned int sampling_freq)
> +{
...

> +
> +	if (st->power_mode == AD7779_LOW_POWER)

At the moment we only end up in low power mode via suspend I think.
So this logic and indeed the tracking of power mode isn't yet needed.

> +		div = AD7779_LOWPOWER_DIV;
> +	else
> +		div = AD7779_HIGHPOWER_DIV;
> +


> +
> +static int ad7779_get_calibscale(struct ad7779_state *st, int channel)
> +{
> +	int ret;
> +	u8 calibscale[3];
> +	// u8 low, mid, high;
> +	ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_LOWER_BYTE(channel), &calibscale[0]);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_MID_BYTE(channel), &calibscale[1]);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_read(st, AD7779_REG_CH_GAIN_UPPER_BYTE(channel), &calibscale[2]);
> +	if (ret)
> +		return ret;
> +
> +	// return FIELD_PREP(AD7779_UPPER, high) | FIELD_PREP(AD7779_MID, mid) |
> +	//        FIELD_PREP(AD7779_LOWER, low);

Remember to cleanup before posting.  Always read through your own patch for this
sort of leftover stuff or get someone else to do it before posting (as can be hard
to spot in your own code).


> +	return get_unaligned_be24(calibscale);
> +}

> +
> +static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
> +{
> +	int ret;
> +	u8 calibbias[3];
> +	u8 msb, mid, lsb;
> +
> +	msb = FIELD_GET(AD7779_UPPER, val);
> +	mid = FIELD_GET(AD7779_MID, val);
> +	lsb = FIELD_GET(AD7779_LOWER, val);
? left over code?
> +	put_unaligned_be24(val, calibbias);
> +	ret = ad7779_spi_write(st,
> +			       AD7779_REG_CH_OFFSET_UPPER_BYTE(channel),
> +			       calibbias[0]);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_write(st,
> +			       AD7779_REG_CH_OFFSET_MID_BYTE(channel),
> +			       calibbias[1]);
> +	if (ret)
> +		return ret;
> +
> +	return ad7779_spi_write(st,
> +				AD7779_REG_CH_OFFSET_LOWER_BYTE(channel),
> +				calibbias[2]);
> +}
> +
> +static int ad7779_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long mask)
> +{
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +		switch (mask) {
> +		case IIO_CHAN_INFO_CALIBSCALE:
> +			*val = ad7779_get_calibscale(st, chan->channel);
> +			iio_device_release_direct_mode(indio_dev);
> +			if (*val < 0)
> +				return -EINVAL;
> +			*val2 = GAIN_REL;
> +			return IIO_VAL_FRACTIONAL;
> +		case IIO_CHAN_INFO_CALIBBIAS:
> +			*val = ad7779_get_calibbias(st, chan->channel);
> +			iio_device_release_direct_mode(indio_dev);
> +			if (*val < 0)
> +				return -EINVAL;
> +			return IIO_VAL_INT;
> +		case IIO_CHAN_INFO_SAMP_FREQ:
> +			*val = st->sampling_freq;
> +			iio_device_release_direct_mode(indio_dev);
> +			if (*val < 0)
> +				return -EINVAL;
> +			return IIO_VAL_INT;
> +		}
> +		return -EINVAL;
> +	}
> +	unreachable();
> +}
> +
> +static int ad7779_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val,
> +			    int val2,
No need to wrap so much.  Stick a few of these on one line.

> +			    long mask)
> +{
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		return ad7779_set_calibscale(st, chan->channel, val2);

We have to take the direct_mode lock to read them but not write?
That seems backwards. Should always be able to read, but write
should be stopped if buffered capture in flight.


> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		return ad7779_set_calibbias(st, chan->channel, val);
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ad7779_set_sampling_frequency(st, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad7779_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	int ret;
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +
> +	ret = ad7779_spi_write_mask(st,
> +				    AD7779_REG_GENERAL_USER_CONFIG_3,
> +				    AD7779_MOD_SPI_EN_MSK,
> +				    FIELD_PREP(AD7779_MOD_SPI_EN_MSK, 1));
> +	if (ret)
> +		return ret;
> + 
> +	/* DRDY output cannot be disabled at device level

Comment syntax needs fixing.

> +	 * therefore we mask the irq at host end.
> +	 */
> +	enable_irq(st->spi->irq);
> +
> +	return 0;
> +}
> +
> +static int ad7779_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +
> +	disable_irq(st->spi->irq);
> +
> +	return ad7779_spi_write(st, AD7779_REG_GENERAL_USER_CONFIG_3,
> +			       AD7779_DISABLE_SD);
> +}
> +
> +static irqreturn_t ad7779_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +	int bit;
> +	int k = 0;
> +	/* Each channel shifts out HEADER + 24 bits of data

Wrong comment syntax, and also early than necessary line wrap.

> +	 * therefore 8 * u32 for the data and 64 bits for
> +	 * the timestamp
> +	 */
> +	u32 tmp[10]; 
> +
> +	struct spi_transfer sd_readback_tr[] = {
> +		{
> +			.rx_buf = st->spidata_rx,
> +			.tx_buf = st->spidata_tx,
> +			.len = 32,

Why 32?  Good to add some maths or a comment on the sizing.

> +		}
> +	};
> +
> +	if (!iio_buffer_enabled(indio_dev)){
> +		iio_trigger_notify_done(indio_dev->trig);
> +		return IRQ_HANDLED;

use a goto.

> +	}
> +
> +	st->spidata_tx[0] = AD7779_SPI_READ_CMD;
> +	ret = spi_sync_transfer(st->spi, sd_readback_tr,
> +				ARRAY_SIZE(sd_readback_tr));
> +	if (ret) {
> +		dev_err(&st->spi->dev,
> +			"spi transfer error in irq handler");
goto.

> +		iio_trigger_notify_done(indio_dev->trig);
> +		return IRQ_HANDLED;
> +	}
> +
> +	for_each_set_bit(bit, indio_dev->active_scan_mask, AD7779_NUM_CHANNELS - 1)
> +		tmp[k++] = st->spidata_rx[bit];

Update this to use Nuno's new macros for iterating over the scan mask.

> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, &tmp[0], pf->timestamp);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}

> +static int ad7779_powerup(struct ad7779_state *st, struct gpio_desc *start_gpio)
> +{
> +	int ret;
> +	
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GEN_ERR_REG_1_EN,
> +				    AD7779_SPI_CRC_EN_MSK,
> +				    FIELD_PREP(AD7779_SPI_CRC_EN_MSK, 1));
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> +				    AD7779_MOD_POWERMODE_MSK,
> +				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK, 1));

Do these need to be done separately?  Or can a single write be used to write
both fields?

> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> +				    AD7779_MOD_PDB_REFOUT_MSK,
> +				    FIELD_PREP(AD7779_MOD_PDB_REFOUT_MSK, 1));
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> +				    AD7779_DCLK_CLK_DIV_MSK,
> +				    FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 1));
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_ADC_MUX_CONFIG,
> +				    AD7779_REFMUX_CTRL_MSK,
> +				    FIELD_PREP(AD7779_REFMUX_CTRL_MSK, 1));
> +	if (ret)
> +		return ret;
> +
> +
> +	st->power_mode = AD7779_HIGH_POWER;
> +	ret = ad7779_set_sampling_frequency(st, AD7779_DEFAULT_SAMPLING_FREQ);
> +	if (ret)
> +		return ret;
> +
> +	gpiod_set_value(start_gpio, 0);
> +	/* Start setup time */
> +	fsleep(15);
> +	gpiod_set_value(start_gpio, 1);
> +	fsleep(15);
> +	gpiod_set_value(start_gpio, 0);
> +	fsleep(15);
> +
> +	return 0;
> +}
> +
> +static int ad7779_probe(struct spi_device *spi)
> +{

...

> +	indio_dev->name = st->chip_info->name;
> +	indio_dev->info = &ad7779_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = st->chip_info->channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
> +
> +	st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> +					  		indio_dev->name, iio_device_id(indio_dev));

As has already been commented on. Fix all this alignment stuff.

> +	if (!st->trig)
> +		return -ENOMEM;
> +
> +	st->trig->ops= &ad7779_trigger_ops;
> +	st->trig->dev.parent = &spi->dev;

devm_iio_trigger_alloc does this for you.
https://elixir.bootlin.com/linux/v6.10/source/drivers/iio/industrialio-trigger.c#L563

> +
> +	iio_trigger_set_drvdata(st->trig, st);
> +
> +	ret = devm_request_irq(&spi->dev, spi->irq,
> +						   iio_trigger_generic_data_rdy_poll,
> +						   IRQF_ONESHOT | IRQF_NO_AUTOEN,
> +						   indio_dev->name, st->trig);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret, "request irq %d failed\n",
> +				     st->spi->irq);

...

> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static int __maybe_unused ad7779_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> +				    AD7779_MOD_POWERMODE_MSK,
> +				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> +					       AD7779_LOW_POWER));
> +	if (ret)
> +		return ret;
> +
> +	st->power_mode = AD7779_LOW_POWER;
As above. There should be no need to track this as you only change it
in suspend / resume.

> +	return 0;
> +}
> +
> +static int __maybe_unused ad7779_resume(struct device *dev)

Don't need __maybe_unused as DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr() ensure
these functions are visible to the compiler, but that it them removes them
if they aren't in use.

> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
> +				    AD7779_MOD_POWERMODE_MSK,
> +				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
> +					       AD7779_HIGH_POWER));
> +	if (ret)
> +		return ret;
> +
> +	st->power_mode = AD7779_HIGH_POWER;
> +
> +	return 0;
> +}

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Dan Carpenter]

Hi Ramona,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ramona-Alexandra-Nechita/dt-bindings-iio-adc-add-a7779-doc/20240725-000001
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240724155517.12470-5-ramona.nechita%40analog.com
patch subject: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family
config: hexagon-randconfig-r081-20240728 (https://download.01.org/0day-ci/archive/20240729/202407290151.pCUNfKeG-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project ccae7b461be339e717d02f99ac857cf0bc7d17fc)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202407290151.pCUNfKeG-lkp@intel.com/

smatch warnings:
drivers/iio/adc/ad7779.c:779 ad7779_probe() error: uninitialized symbol 'ret'.

vim +/ret +779 drivers/iio/adc/ad7779.c

ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  761  static int ad7779_probe(struct spi_device *spi)
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  762  {
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  763  	struct iio_dev *indio_dev;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  764  	struct ad7779_state *st;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  765  	struct gpio_desc *reset_gpio, *start_gpio;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  766  	int ret;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  767  
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  768  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  769  	if (!indio_dev)
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  770  		return -ENOMEM;
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  771  
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  772  	st = iio_priv(indio_dev);
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  773  
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  774  	st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  775  	if (IS_ERR(st->mclk))
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  776  		return PTR_ERR(st->mclk);
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  777  
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  778  	if (!spi->irq)
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24 @779  		return dev_err_probe(&spi->dev, ret,
                                                                                                        ^^^
s/ret/-EINVAL/

ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  780  				     "DRDY irq not present\n"); 
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  781  
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  782  	reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
ed8e3886f0d748 Ramona Alexandra Nechita 2024-07-24  783  	if (IS_ERR(reset_gpio))

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 2/3] Documentation: ABI: added filter mode doc in sysfs-bus-iio

[Andy Shevchenko]

On Wed, Jul 24, 2024 at 06:54:40PM +0300, Ramona Alexandra Nechita wrote:
> The filter mode / filter type property is used for ad4130
> and ad7779 drivers, therefore the ABI doc file for ad4130
> was removed, merging both of them in the sysfs-bus-iio

Missed grammatical period at the end.

...

> +What:		/sys/bus/iio/devices/iio:deviceX/filter_type_available
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
> +KernelVersion:	6.1

> -What:		/sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
> -KernelVersion:  6.2

> -What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
> -KernelVersion:  6.2

So, the commit message is silent about version downgrade. Was it a typo
or on purpose?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Andy Shevchenko]

On Wed, Jul 24, 2024 at 06:14:52PM +0200, Krzysztof Kozlowski wrote:
> On 24/07/2024 17:54, Ramona Alexandra Nechita wrote:

...

> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.

I would be more precise

  `git log --oneline --no-merges -- DIRECTORY_OR_FILE`

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Nechita, Ramona]

Hi all,

I implemented most of the feedback from previous emails and I will be ready to send a new patch soon. One minor question inline.

Thank you,
Ramona
-----Original Message-----
From: Jonathan Cameron <jic23@kernel.org> 
Sent: Saturday, July 27, 2024 6:41 PM
To: Nechita, Ramona <Ramona.Nechita@analog.com>
Cc: linux-iio@vger.kernel.org; Lars-Peter Clausen <lars@metafoo.de>; Tanislav, Cosmin <Cosmin.Tanislav@analog.com>; Hennerich, Michael <Michael.Hennerich@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Sa, Nuno <Nuno.Sa@analog.com>; Schmitt, Marcelo <Marcelo.Schmitt@analog.com>; Marius Cristea <marius.cristea@microchip.com>; Ivan Mikhaylov <fr0st61te@gmail.com>; Mike Looijmans <mike.looijmans@topic.nl>; Marcus Folkesson <marcus.folkesson@gmail.com>; Liam Beguin <liambeguin@gmail.com>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[>> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of 
>> sending out data both on DOUT lines interface,as on the SDO line.
>> The driver currently implements only theSDO data streaming mode. SPI 
>> communication is used alternatively foraccessingregisters and 
>> streaming data. Register access are protected by crc8.
>> 
>> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
>Hi Ramona,
>
>Various comments inline.  Key one though is make sure you read your own code before posting as there is a bunch of left over stuff from updates still in the code.
>
>Jonathan
>
>> diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c new 
>> file mode 100644 index 000000000000..7a83977fd00c
>> --- /dev/null
>> +++ b/drivers/iio/adc/ad7779.c
>
>> +
>.....
>> +static irqreturn_t ad7779_trigger_handler(int irq, void *p)
>> +{
>> +	struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct ad7779_state *st = iio_priv(indio_dev);
>> +	int ret;
>> +	int bit;
>> +	int k = 0;
>> +	/* Each channel shifts out HEADER + 24 bits of data
>
>Wrong comment syntax, and also early than necessary line wrap.
>
>> +	 * therefore 8 * u32 for the data and 64 bits for
>> +	 * the timestamp
>> +	 */
>> +	u32 tmp[10]; 
>> +
>> +	struct spi_transfer sd_readback_tr[] = {
>> +		{
>> +			.rx_buf = st->spidata_rx,
>> +			.tx_buf = st->spidata_tx,
>> +			.len = 32,
>
>Why 32?  Good to add some maths or a comment on the sizing.
>
>> +		}
>> +	};
>> +
>> +	if (!iio_buffer_enabled(indio_dev)){
>> +		iio_trigger_notify_done(indio_dev->trig);
>> +		return IRQ_HANDLED;
>
>use a goto.
>
>> +	}
>> +
>> +	st->spidata_tx[0] = AD7779_SPI_READ_CMD;
>> +	ret = spi_sync_transfer(st->spi, sd_readback_tr,
>> +				ARRAY_SIZE(sd_readback_tr));
>> +	if (ret) {
>> +		dev_err(&st->spi->dev,
>> +			"spi transfer error in irq handler");
>goto.
>
>> +		iio_trigger_notify_done(indio_dev->trig);
>> +		return IRQ_HANDLED;
>> +	}
>> +
>> +	for_each_set_bit(bit, indio_dev->active_scan_mask, AD7779_NUM_CHANNELS - 1)
>> +		tmp[k++] = st->spidata_rx[bit];
>
>Update this to use Nuno's new macros for iterating over the scan mask.

Does this refer to iio_for_each_active_channel ? I checked and noticed that the patch containing this macro is not upstream yet, should I wait for it to be merged before sending out a new patch?

>
>> +
>> +	iio_push_to_buffers_with_timestamp(indio_dev, &tmp[0], pf->timestamp);
>> +
>> +	iio_trigger_notify_done(indio_dev->trig);
>> +
>> +	return IRQ_HANDLED;
>> +}
>......
>
>Don't need __maybe_unused as DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr() ensure
>these functions are visible to the compiler, but that it them removes them
>if they aren't in use.
>
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +	struct ad7779_state *st = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	ret = ad7779_spi_write_mask(st, AD7779_REG_GENERAL_USER_CONFIG_1,
>> +				    AD7779_MOD_POWERMODE_MSK,
>> +				    FIELD_PREP(AD7779_MOD_POWERMODE_MSK,
>> +					       AD7779_HIGH_POWER));
>> +	if (ret)
>> +		return ret;
>> +
>> +	st->power_mode = AD7779_HIGH_POWER;
>> +
>> +	return 0;
>> +}

Re: [PATCH v4 3/3] drivers: iio: adc: add support for ad777x family

[Andy Shevchenko]

On Tue, Sep 03, 2024 at 02:26:41PM +0000, Nechita, Ramona wrote:
> From: Jonathan Cameron <jic23@kernel.org> 
> Sent: Saturday, July 27, 2024 6:41 PM
> To: Nechita, Ramona <Ramona.Nechita@analog.com>

...

> >> +	for_each_set_bit(bit, indio_dev->active_scan_mask, AD7779_NUM_CHANNELS - 1)
> >> +		tmp[k++] = st->spidata_rx[bit];
> >
> >Update this to use Nuno's new macros for iterating over the scan mask.
> 
> Does this refer to iio_for_each_active_channel ? I checked and noticed that
> the patch containing this macro is not upstream yet, should I wait for it to
> be merged before sending out a new patch?

It's in the maintainer's tree, which your patch should be based on.
So I don't see any issues here. Do you?

-- 
With Best Regards,
Andy Shevchenko