Skip to contentSkip to conversation

dt-bindings: iio: amplifiers: add ADA4255

PATCH In review Cosmin Tanislav 3 participants 5 messages Jan 20, 2025 – Jan 25, 2025

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 dt-bindings: iio: amplifiers: add ADA4255

The ADA4255 is a precision programmable gain instrumentation amplifier (PGIA) with integrated bipolar charge pumps.

With its integrated charge pumps, the ADA4255 internally produces the high voltage bipolar supplies needed to achieve a wide input voltage range (38V typical with VDDCP = 5V) without lowering input impedance.

The charge pump topology of the ADA4255 allows channels to be isolated with only low voltage components, reducing complexity, size, and implementation time in industrial and process control systems.

---
 .../bindings/iio/amplifiers/adi,ada4255.yaml  | 83 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++
 2 files changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/amplifiers/adi,ada4255.yaml
Documentation/devicetree/bindings/iio/amplifiers/adi,ada4255.yaml new file +83 -0
new file mode 100644
index 0000000000000..7ae752a63b111
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/amplifiers/adi,ada4255.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADA4255 Programmable Gain Instrumentation Amplifier
+
+maintainers:
+  - Cosmin Tanislav <[email protected]>
+
+description: |
+  Zero Drift, High Voltage, Programmable Gain Instrumentation Amplifiers.
+
+  ADA4254
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ada4254.pdf
+
+  ADA4255
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ada4255.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ada4254
+      - adi,ada4255
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: mclk
+
+  clock-output-names:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  avdd-supply: true
+  dvdd-supply: true
+  vddcp-supply: true
+  vocm-supply: true
+
+  adi,excitation-current-microamp:
+    description: Excitation current to apply to IOUT.
+    enum: [0, 100, 200, 300, 400, 500, 600, 700, 800,
+           900, 1000, 1100, 1200, 1300, 1400, 1500]
+    default: 0
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,ada4255
+    then:
+      properties:
+        adi,charge-pump-freq-hz:
+          description: Frequency at which to run the charge pumps.
+          enum: [8000000, 16000000]
+          default: 16000000
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      amplifier@0{
+        compatible = "adi,ada4255";
+        reg = <0>;
+      };
+    };
+...
MAINTAINERS +7 -0
index e8e67cd31961e..be46db0866011 100644
@@ -1405,6 +1405,13 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ada4250.yaml
 F:	drivers/iio/amplifiers/ada4250.c
 
+ANALOG DEVICES INC ADA4255 DRIVER
+M:	Cosmin Tanislav <[email protected]>
+L:	[email protected]
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ada4255.yaml
+
 ANALOG DEVICES INC ADF4377 DRIVER
 M:	Antoniu Miclaus <[email protected]>
 L:	[email protected]
Signature · 1 line
2.48.1
#1 lore raw

PATCH 2/2 iio: amplifiers: add ADA4255 driver

The ADA4255 is a precision programmable gain instrumentation amplifier (PGIA) with integrated bipolar charge pumps.

With its integrated charge pumps, the ADA4255 internally produces the high voltage bipolar supplies needed to achieve a wide input voltage range (38V typical with VDDCP = 5V) without lowering input impedance.

The charge pump topology of the ADA4255 allows channels to be isolated with only low voltage components, reducing complexity, size, and implementation time in industrial and process control systems.

---
 MAINTAINERS                      |   1 +
 drivers/iio/amplifiers/Kconfig   |  12 +
 drivers/iio/amplifiers/Makefile  |   1 +
 drivers/iio/amplifiers/ada4255.c | 937 +++++++++++++++++++++++++++++++
 4 files changed, 951 insertions(+)
 create mode 100644 drivers/iio/amplifiers/ada4255.c
MAINTAINERS +1 -0
index be46db0866011..f16c0ba60e53f 100644
@@ -1411,6 +1411,7 @@ L:	[email protected]
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ada4255.yaml
+F:	drivers/iio/amplifiers/ada4255.c
 
 ANALOG DEVICES INC ADF4377 DRIVER
 M:	Antoniu Miclaus <[email protected]>
drivers/iio/amplifiers/Kconfig +12 -0
index 55eb16b32f6c9..4fad09f8bddd3 100644
@@ -36,6 +36,18 @@ config ADA4250
 	  To compile this driver as a module, choose M here: the
 	  module will be called ada4250.
 
+config ADA4255
+	tristate "Analog Devices ADA4255 Instrumentation Amplifier"
+	depends on SPI
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for Analog Devices AD4254 and
+	  ADA4255 SPI Instrumentation Amplifiers. The driver provides
+	  direct access via sysfs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ada4255.
+
 config HMC425
 	tristate "Analog Devices HMC425A and similar GPIO Gain Amplifiers"
 	depends on GPIOLIB
drivers/iio/amplifiers/Makefile +1 -0
index 2126331129cf6..3a43a42106d23 100644
@@ -6,4 +6,5 @@
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AD8366) += ad8366.o
 obj-$(CONFIG_ADA4250) += ada4250.o
+obj-$(CONFIG_ADA4255) += ada4255.o
 obj-$(CONFIG_HMC425) += hmc425a.o
drivers/iio/amplifiers/ada4255.c new file +937 -0

937 lines not on this page · raw message

Signature · 1 line
2.48.1
maintainer
#2 lore raw

Re: [PATCH 2/2] iio: amplifiers: add ADA4255 driver

Quoted 2 lines · On Mon, 20 Jan 2025 12:54:25 +0200 Cosmin Tanislav <[email protected]>…

The ADA4255 is a precision programmable gain instrumentation amplifier (PGIA) with integrated bipolar charge pumps.

Good to shout a bit about this including a gpio chip.

Quoted 10 lines

With its integrated charge pumps, the ADA4255 internally produces the high voltage bipolar supplies needed to achieve a wide input voltage range (38V typical with VDDCP = 5V) without lowering input impedance.

The charge pump topology of the ADA4255 allows channels to be isolated with only low voltage components, reducing complexity, size, and implementation time in industrial and process control systems.

A few comments inline, but all superficial stuff. Looks like a nice driver to me

Jonathan

drivers/iio/amplifiers/ada4255.c line 3

Quoted 9 lines
drivers/iio/amplifiers/ada4255.c new file +3 -0
new file mode 100644
index 0000000000000..8d25ffa7baa6c
@@ -0,0 +1,937 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Analog Devices, Inc.

That date wants a -2025 given you will be changing this in response to review anyway.

Quoted 7 lines
drivers/iio/amplifiers/ada4255.c new file +7 -0
+ * Author: Cosmin Tanislav <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>

drivers/iio/amplifiers/ada4255.c line 61

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +9 -0
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
 
+
+#define ADA4255_NAME				"ada4255"

Just use the string inline. These generic 'driver name' defines are unhelpful to reading the code in comparison with the string in the place it's used.

drivers/iio/amplifiers/ada4255.c line 72

Quoted 11 lines
drivers/iio/amplifiers/ada4255.c new file +11 -0
+
+#define ada4255_from_clk_hw(hw) \
+	container_of(hw, struct ada4255_state, int_clk_hw)
+
+struct ada4255_chip_info {
+	const char			*name;
+	bool				has_charge_pump;
+};
+
+struct ada4255_state {
+	struct spi_device		*spi;

only used to get to the dev. So either have a struct device pointer here or just get that from the regmap where needed.

drivers/iio/amplifiers/ada4255.c line 78

Quoted 6 lines
drivers/iio/amplifiers/ada4255.c new file +6 -0
+	struct regmap			*regmap;
+	struct clk			*mclk;
+	const struct ada4255_chip_info	*chip_info;
+
+	/*
+	 * Synchronize consecutive regmap operations.

Be more specific as regmap has it's own locks.
Perhaps a read modify write section that needs to
be locked around?

Quoted 4 lines
drivers/iio/amplifiers/ada4255.c new file +4 -0
+	 */
+	struct mutex			lock;
+	struct gpio_chip		gc;
+	struct notifier_block		ext_clk_nb;

drivers/iio/amplifiers/ada4255.c

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +9 -0
+	struct clk_hw			int_clk_hw;
+
+	bool				gpio4_clk_en;
+	bool				int_clk_out_en;
+};
 
+
+static const unsigned int ada4255_excitation_current_ua_tbl[] = {
+	0, 100, 200, 300, 400, 500, 600, 700, 800,
+	900, 1000, 1100, 1200, 1300, 1400, 1500

9 on 1st line is a bit random. Go with a power of 2. 8 is per line and 1 on the last line probably best choiceh ere.

Quoted 6 lines
drivers/iio/amplifiers/ada4255.c new file +6 -0
+};
+
+static const unsigned int ada4255_clk_cp_sel_hz_tbl[] = {
+	16000000, 8000000
+};
+

drivers/iio/amplifiers/ada4255.c line 201

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+static const unsigned int ada4255_int_clk_rate_hz_tbl[] = {
+	1000000, 125000,
+};
+
+static int _ada4255_find_tbl_index(const unsigned int *tbl, unsigned int tbl_len,
+				   unsigned int val, unsigned int *index)
+{
+	unsigned int i;
+
+	for (i = 0; i < tbl_len; i++)

The kernel style docs are a bit ambiguous IIRC on this, but I'd add {} on basis next bit is multiline.

Quoted 17 lines
drivers/iio/amplifiers/ada4255.c new file +16 -0
+		if (val == tbl[i]) {
+			*index = i;
+			return 0;
+		}
+
+	return -EINVAL;
+}
 
+static int ada4255_set_gain(struct ada4255_state *st, int val, int val2)
+{
+	ssize_t tbl_len = ARRAY_SIZE(ada4255_gain_tbl);
+	unsigned int i, g5, g4, g3_0;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(ada4255_gain_tbl); i++)
+		if (val == ada4255_gain_tbl[i][0] &&
+		    val2 == ada4255_gain_tbl[i][1])

drivers/iio/amplifiers/ada4255.c line 306

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+			break;
+
+	if (i == tbl_len)
+		return -EINVAL;
+
+	g5 = ada4255_gain_reg_tbl[i][0];
+	g4 = ada4255_gain_reg_tbl[i][1];
+	g3_0 = ada4255_gain_reg_tbl[i][2];
+
+	mutex_lock(&st->lock);

As below. guard() helps in this sort of case

Quoted 24 lines
drivers/iio/amplifiers/ada4255.c new file +24 -0
+
+	ret = regmap_update_bits(st->regmap, ADA4255_TEST_MUX_REG,
+				 ADA4255_GAIN_G5_MASK,
+				 FIELD_PREP(ADA4255_GAIN_G5_MASK, g5));
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(st->regmap, ADA4255_GAIN_MUX_REG,
+				 ADA4255_GAIN_G4_MASK | ADA4255_GAIN_G3_0_MASK,
+				 FIELD_PREP(ADA4255_GAIN_G4_MASK, g4) |
+				 FIELD_PREP(ADA4255_GAIN_G3_0_MASK, g3_0));
+
+out:
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int ada4255_get_gain(struct ada4255_state *st, int *val, int *val2)
+{
+	ssize_t tbl_len = ARRAY_SIZE(ada4255_gain_reg_tbl);
+	unsigned int i, gain, test_gain, g5, g4, g3_0;
+	int ret;
+

drivers/iio/amplifiers/ada4255.c line 340

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+	mutex_lock(&st->lock);
+
+	ret = regmap_read(st->regmap, ADA4255_GAIN_MUX_REG, &gain);
+	if (ret)
+		goto out;
+
+	ret = regmap_read(st->regmap, ADA4255_TEST_MUX_REG, &test_gain);
+
+out:
+	mutex_unlock(&st->lock);

scoped_guard(mutex, &st->lock)
is useful in cases like this as can return without having to worry about unlocking by hand.

Quoted 20 lines
drivers/iio/amplifiers/ada4255.c new file +20 -0
+
+	if (ret)
+		return ret;
+
+	g5 = FIELD_GET(ADA4255_GAIN_G5_MASK, test_gain);
+	g4 = FIELD_GET(ADA4255_GAIN_G4_MASK, gain);
+	g3_0 = FIELD_GET(ADA4255_GAIN_G3_0_MASK, gain);
+
+	for (i = 0; i < ARRAY_SIZE(ada4255_gain_reg_tbl); i++)
+		if (g5 == ada4255_gain_reg_tbl[i][0] &&
+		    g4 == ada4255_gain_reg_tbl[i][1] &&
+		    g3_0 == ada4255_gain_reg_tbl[i][2])
+			break;
+
+	if (i == tbl_len)
+		return -EINVAL;
+
+	*val = ada4255_gain_tbl[i][0];
+	*val2 = ada4255_gain_tbl[i][1];
+

drivers/iio/amplifiers/ada4255.c line 384

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +9 -0
+	return IIO_VAL_INT_PLUS_NANO;
+}
 
+static int ada4255_set_ch_en(struct ada4255_state *st, unsigned int ch, int val)
+{
+	unsigned int ch_val = val ? ada4255_ch_input_mux_tbl[ch] : 0;
+	int current_val;
+	int ret;
+
+	mutex_lock(&st->lock);

guard(mutex)(&st->lock);
+ include cleanup.h
so we can have simpler direct returns instead of goto.

Quoted 23 lines
drivers/iio/amplifiers/ada4255.c new file +22 -0
+
+	ret = ada4255_get_ch_en(st, ch, &current_val);
+	if (ret < 0)
+		goto out;
+
+	if (current_val == val) {
+		ret = 0;
+		goto out;
+	}
+
+	ret = regmap_write(st->regmap, ADA4255_INPUT_MUX_REG, ch_val);
+
+out:
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
 
+static unsigned long ada4255_int_clk_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct ada4255_state *st = ada4255_from_clk_hw(hw);
+	struct device *dev = &st->spi->dev;

drivers/iio/amplifiers/ada4255.c

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+	unsigned int i = 0;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADA4255_SYNC_CFG_REG, &i);
+	if (ret)
+		dev_err(dev, "Failed to read internal clock rate: %d\n", ret);
+
+	i = FIELD_GET(ADA4255_INT_CLK_OUT_MASK, i);
+
+	return ada4255_int_clk_rate_hz_tbl[i];

Might as well combine as

	return ada4244_int_clk_rate_hz_tbl[FIELD_GET(ADA4255_INT_CLK_OUT_MASK)];

to avoid confusing reuse of i. Rename i as reg_val or something as well.

drivers/iio/amplifiers/ada4255.c line 668

Quoted 11 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+}
+
 
+static int ada4255_setup_ext_clk(struct ada4255_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	unsigned int divider;
+	unsigned long rate;
+	int ret;
+
+	ret = clk_prepare_enable(st->mclk);

Use get_optional_enabled() below. If we have it want to turn it on and then you don't need to handle tear down yourself.

Quoted 5 lines
drivers/iio/amplifiers/ada4255.c new file +5 -0
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, ada4255_clk_disable_unprepare,
+				       st->mclk);

...

drivers/iio/amplifiers/ada4255.c line 745

Quoted 9 lines
drivers/iio/amplifiers/ada4255.c new file +8 -0
+}
+
 
+
+static int ada4255_setup(struct ada4255_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	unsigned int i;
+	int ret = 0;

Always set, so no need to initialise here.

Quoted 28 lines
drivers/iio/amplifiers/ada4255.c new file +28 -0
+	u32 val;
+
+	st->mclk = devm_clk_get_optional(dev, "mclk");
+	if (IS_ERR(st->mclk))
+		return dev_err_probe(dev, PTR_ERR(st->mclk),
+				     "Failed to get mclk\n");
+
+	if (st->mclk)
+		ret = ada4255_setup_ext_clk(st);
+	else
+		ret = ada4255_setup_int_clk(st);
+
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u32(dev, "adi,excitation-current-microamp", &val);
+	if (!ret) {
+		ret = ada4255_find_tbl_index(ada4255_excitation_current_ua_tbl,
+					     val, &i);
+		if (ret)
+			return ret;
+
+		ret = regmap_update_bits(st->regmap, AD4255_EX_CURRENT_CFG_REG,
+					 AD4255_EX_CURRENT_MASK,
+					 FIELD_PREP(AD4255_EX_CURRENT_MASK, i));
+		if (ret)
+			return ret;
+

drivers/iio/amplifiers/ada4255.c line 783

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+		ret = regmap_update_bits(st->regmap, AD4255_EX_CURRENT_CFG_REG,
+					 AD4255_EX_CURRENT_SEL_MASK,
+					 FIELD_PREP(AD4255_EX_CURRENT_SEL_MASK, 1));
+		if (ret)
+			return ret;
+	}
+
+	ret = device_property_read_u32(dev, "adi,charge-pump-freq-hz", &val);
+	if (!ret) {
+		if (!st->chip_info->has_charge_pump)

This seems backwards. Not up to us to check for extra properties in drivers. So I'd check if we expect one before trying to read the property.

	if (!st->chip_info->has_charge_pump)
		return 0;

	ret = device_property_read_u32(dev, "adi,charge-pump-freq-hz", &val);
	if (ret)
		return ret; // 0 if it is optional.

	etc

drivers/iio/amplifiers/ada4255.c line 795

Quoted 12 lines
drivers/iio/amplifiers/ada4255.c new file +12 -0
+			return dev_err_probe(dev, -EINVAL,
+					     "Unsupported charge pump function\n");
+
+		ret = ada4255_find_tbl_index(ada4255_clk_cp_sel_hz_tbl, val, &i);
+		if (ret)
+			return ret;
+
+		ret = regmap_update_bits(st->regmap, ADA4255_SYNC_CFG_REG,
+					 ADA4255_SYNC_CLK_CP_SEL_MASK,
+					 FIELD_PREP(ADA4255_SYNC_CLK_CP_SEL_MASK, i));
+		if (ret)
+			return ret;

After reorder as above, can return regmap_update_bits() for this last one.

Quoted 9 lines
drivers/iio/amplifiers/ada4255.c new file +8 -0
+	}
+
+	return 0;
+}
 
+static int ada4255_reset(struct ada4255_state *st)
+{
+	int ret;
+

drivers/iio/amplifiers/ada4255.c line 836

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +10 -0
+	ret = regmap_write(st->regmap, ADA4255_RESET_REG,
+			   FIELD_PREP(ADA4255_RESET_REG, 1));
+	if (ret)
+		return ret;
+
+	/*
+	 * A full calibration occurs after a soft reset and it takes
+	 * approximately 85ms.
+	 * See datasheet page 29, Section OUTPUT RIPPLE CALIBRATION
+	 * CONFIGURATION.

I'd wrap this as one paragraph. Looks a bit odd as it is now.
* A full calibration occurs after a soft reset and it takes
* approximately 85ms. See datasheet page 29, Section OUTPUT
* RIPPLE CALIBRATION CONFIGURATION.

drivers/iio/amplifiers/ada4255.c line 849

Quoted 13 lines
drivers/iio/amplifiers/ada4255.c new file +13 -0
+	 */
+	fsleep(85000);
+
+	return 0;
+}
+
+static int ada4255_probe(struct spi_device *spi)
+{
+	static const char * const regulator_names[] = {
+		"avdd",
+		"dvdd",
+		"vddcp",
+		"vocm",

Fine to put these 4 on one line. We aren't likely to see more added later and it will be a fairly short line.

Quoted 39 lines
drivers/iio/amplifiers/ada4255.c new file +39 -0
+	};
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ada4255_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->chip_info = spi_get_device_match_data(spi);
+	if (!st->chip_info)
+		return -EINVAL;
+
+	indio_dev->info = &ada4255_info;
+	indio_dev->name = st->chip_info->name;
+	indio_dev->channels = ada4255_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ada4255_channels);
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
+					     regulator_names);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+	st->regmap = devm_regmap_init_spi(spi, &ada4255_regmap_config);
+	if (IS_ERR(st->regmap))
+		return PTR_ERR(st->regmap);
+
+	ret = ada4255_reset(st);
+	if (ret)
+		return ret;
+
+	ret = ada4255_setup(st);
+	if (ret)
+		return ret;
+
+	ret = ada4255_setup_gpio_chip(st);

drivers/iio/amplifiers/ada4255.c line 927

Quoted 10 lines
drivers/iio/amplifiers/ada4255.c new file +9 -0
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
 
+static struct spi_driver ada4255_driver = {
+	.driver = {
+			.name = ADA4255_NAME,
+			.of_match_table = ada4255_of_match,

Indent is 1 tab more than standard

drivers/iio/amplifiers/ada4255.c

Quoted 1 line
drivers/iio/amplifiers/ada4255.c new file +1 -0
+		},

Also for this.

drivers/iio/amplifiers/ada4255.c line 932

Quoted 4 lines
drivers/iio/amplifiers/ada4255.c new file +4 -0
+	.probe = ada4255_probe,
+	.id_table = ada4255_id,
+};
+

A nice pattern that people use is to group the module_spi_driver() with it's one parameter by not putting a blank line inbetween. Hence I would delete this one.

Quoted 1 line

+module_spi_driver(ada4255_driver);

THanks,

Jonathan

#3 lore raw
In reply to Cosmin Tanislav

Documentation/devicetree/bindings/iio/amplifiers/adi,ada4255.yaml line 69

Quoted 8 lines · Cosmin Tanislav
Documentation/devicetree/bindings/iio/amplifiers/adi,ada4255.yaml new file +7 -0
+  avdd-supply: true
+  dvdd-supply: true
+  vddcp-supply: true
+  vocm-supply: true
 
+required:
+  - compatible
+  - reg

Why are the supplies not marked as required? I would imagine that at least the first two are needed for operation?

maintainer
#4 lore raw
In reply to Cosmin Tanislav
Quoted 7 lines · On Mon, 20 Jan 2025 12:54:24 +0200 Cosmin Tanislav <[email protected]>…

The ADA4255 is a precision programmable gain instrumentation amplifier (PGIA) with integrated bipolar charge pumps.

With its integrated charge pumps, the ADA4255 internally produces the high voltage bipolar supplies needed to achieve a wide input voltage range (38V typical with VDDCP = 5V) without lowering input impedance.

Quoted 5 lines

The charge pump topology of the ADA4255 allows channels to be isolated with only low voltage components, reducing complexity, size, and implementation time in industrial and process control systems.

Hi Cosmin,

In general please add a cover letter to all patches. Along with providing some general info and anything you particular want to highlight, it provides somewhere for others comment on the whole series + it gives a nice name in patchwork!

I've nothing else to add to what Cosmin said on this patch.

Jonathan