[PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02

Ariana Lazar posted 2 patches 1 month, 2 weeks ago
[PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Ariana Lazar 1 month, 2 weeks ago
This is the iio driver for Microchip MCP48FxBy1/2/4/8 series of buffered
voltage output Digital-to-Analog Converters with nonvolatile or volatile
memory and an SPI Interface.

The families support up to 8 output channels.

The devices can be 8-bit, 10-bit and 12-bit.

Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
 MAINTAINERS                  |    1 +
 drivers/iio/dac/Kconfig      |   20 +
 drivers/iio/dac/Makefile     |    1 +
 drivers/iio/dac/mcp48feb02.c | 1243 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1265 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ed24fd2758ad0103dbc5191d0ec180f8ee5e8298..c861001ff9a94af2ff98c9c697e40909a4c69f6d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14950,6 +14950,7 @@ M:	Ariana Lazar <ariana.lazar@microchip.com>
 L:	linux-iio@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/iio/dac/microchip,mcp48feb02.yaml
+F:	drivers/iio/dac/mcp48feb02.c
 
 MCR20A IEEE-802.15.4 RADIO DRIVER
 M:	Stefan Schmidt <stefan@datenfreihafen.org>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index e0996dc014a3d538ab6b4e0d50ff54ede50f1527..56fb14abb36c6955348ca0b1ad72ce47a4af4e0d 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -519,6 +519,26 @@ config MCP4821
 	  To compile this driver as a module, choose M here: the module
 	  will be called mcp4821.
 
+config MCP48FEB02
+	tristate "MCP48FxBy1/2/4/8 driver"
+	depends on SPI
+	help
+	  Say yes here if you want to build the driver for the Microchip:
+	  - 8-bit DAC:
+	    MCP48FEB01, MCP48FEB02, MCP48FEB04, MCP48FEB08,
+	    MCP48FVB01, MCP48FVB02, MCP48FVB04, MCP48FVB08
+          - 10-bit DAC:
+	    MCP48FEB11, MCP48FEB12, MCP48FEB14, MCP48FEB18,
+	    MCP48FVB11, MCP48FVB12, MCP48FVB14, MCP48FVB18
+          - 12-bit DAC:
+	    MCP48FEB21, MCP48FEB22, MCP48FEB24, MCP48FEB28,
+	    MCP48FVB21, MCP48FVB22, MCP48FVB24, MCP48FVB28
+	  having 1 to 8 channels, 8/10/12-bit digital-to-analog converter
+	  (DAC) with SPI interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp48feb02.
+
 config MCP4922
 	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
 	depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 3684cd52b7fa9bc0ad9f855323dcbb2e4965c404..77b473823bdcc265797169a2bcc263b110092faa 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_MAX5821) += max5821.o
 obj-$(CONFIG_MCP4725) += mcp4725.o
 obj-$(CONFIG_MCP4728) += mcp4728.o
 obj-$(CONFIG_MCP4821) += mcp4821.o
+obj-$(CONFIG_MCP48FEB02) += mcp48feb02.o
 obj-$(CONFIG_MCP4922) += mcp4922.o
 obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
 obj-$(CONFIG_STM32_DAC) += stm32-dac.o
diff --git a/drivers/iio/dac/mcp48feb02.c b/drivers/iio/dac/mcp48feb02.c
new file mode 100644
index 0000000000000000000000000000000000000000..20955f77053329a9c385f55c7314032eb6b04dfd
--- /dev/null
+++ b/drivers/iio/dac/mcp48feb02.c
@@ -0,0 +1,1243 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IIO driver for MCP48FEB02 Multi-Channel DAC with SPI interface
+ *
+ * Copyright (C) 2026 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Ariana Lazar <ariana.lazar@microchip.com>
+ *
+ * Datasheet links:
+ * [MCP48FEBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005429B.pdf
+ * [MCP48FVBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005466A.pdf
+ * [MCP48FxBx4/8] https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP48FXBX4-8-Family-Data-Sheet-DS20006362A.pdf
+ */
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/kstrtox.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+#include <linux/time64.h>
+#include <linux/types.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+
+/* Register addresses must be left shifted with 3 positions in order to append command mask */
+#define MCP48FEB02_DAC0_REG_ADDR			0x00
+#define MCP48FEB02_VREF_REG_ADDR			0x40
+#define MCP48FEB02_POWER_DOWN_REG_ADDR			0x48
+#define MCP48FEB02_DAC_CTRL_MASK			GENMASK(1, 0)
+
+#define MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR		0x50
+#define MCP48FEB02_GAIN_BIT_MASK			BIT(0)
+#define MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK		BIT(6)
+#define MCP48FEB02_GAIN_BITS_MASK			GENMASK(15, 8)
+
+#define MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR		0x58
+
+#define MCP48FEB02_NV_DAC0_REG_ADDR			0x80
+#define MCP48FEB02_NV_VREF_REG_ADDR			0xC0
+#define MCP48FEB02_NV_POWER_DOWN_REG_ADDR		0xC8
+#define MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR	0xD0
+#define MCP48FEB02_NV_I2C_SLAVE_ADDR_MASK		GENMASK(7, 0)
+
+/* Voltage reference, Power-Down control register and DAC Wiperlock status register fields */
+#define DAC_CTRL_MASK(ch)				(GENMASK(1, 0) << (2 * (ch)))
+#define DAC_CTRL_VAL(ch, val)				((val) << (2 * (ch)))
+
+/* Gain Control and I2C Slave Address Reguster fields */
+#define DAC_GAIN_MASK(ch)				(BIT(0) << (8 + (ch)))
+#define DAC_GAIN_VAL(ch, val)				((val) << (8 + (ch)))
+
+#define REG_ADDR(reg)					((reg) << 3)
+#define NV_REG_ADDR(reg)				((NV_DAC_ADDR_OFFSET + (reg)) << 3)
+#define READFLAG_MASK					GENMASK(2, 1)
+
+#define MCP48FEB02_MAX_CH				8
+#define MCP48FEB02_MAX_SCALES_CH			3
+#define MCP48FEB02_DAC_WIPER_UNLOCKED			0
+#define MCP48FEB02_NORMAL_OPERATION			0
+#define MCP48FEB02_INTERNAL_BAND_GAP_uV			2440000
+#define NV_DAC_ADDR_OFFSET				0x10
+
+enum mcp48feb02_vref_mode {
+	MCP48FEB02_VREF_VDD = 0,
+	MCP48FEB02_INTERNAL_BAND_GAP = 1,
+	MCP48FEB02_EXTERNAL_VREF_UNBUFFERED = 2,
+	MCP48FEB02_EXTERNAL_VREF_BUFFERED = 3,
+};
+
+enum mcp48feb02_scale {
+	MCP48FEB02_SCALE_VDD = 0,
+	MCP48FEB02_SCALE_GAIN_X1 = 1,
+	MCP48FEB02_SCALE_GAIN_X2 = 2,
+};
+
+enum mcp48feb02_gain_bit_mode {
+	MCP48FEB02_GAIN_BIT_X1 = 0,
+	MCP48FEB02_GAIN_BIT_X2 = 1,
+};
+
+static const char * const mcp48feb02_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"open_circuit",
+};
+
+/**
+ * struct mcp48feb02_features - chip specific data
+ * @name: device name
+ * @phys_channels: number of hardware channels
+ * @resolution: DAC resolution
+ * @have_ext_vref1: does the hardware have an the second external voltage reference?
+ * @have_eeprom: does the hardware have an internal eeprom?
+ */
+struct mcp48feb02_features {
+	const char *name;
+	unsigned int phys_channels;
+	unsigned int resolution;
+	bool have_ext_vref1;
+	bool have_eeprom;
+};
+
+static const struct mcp48feb02_features mcp48feb01_chip_features = {
+	.name = "mcp48feb01",
+	.phys_channels = 1,
+	.resolution = 8,
+	.have_ext_vref1 = false,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb02_chip_features = {
+	.name = "mcp48feb02",
+	.phys_channels = 2,
+	.resolution = 8,
+	.have_ext_vref1 = false,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb04_chip_features = {
+	.name = "mcp48feb04",
+	.phys_channels = 4,
+	.resolution = 8,
+	.have_ext_vref1 = true,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb08_chip_features = {
+	.name = "mcp48feb08",
+	.phys_channels = 8,
+	.resolution = 8,
+	.have_ext_vref1 = true,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb11_chip_features = {
+	.name = "mcp48feb11",
+	.phys_channels = 1,
+	.resolution = 10,
+	.have_ext_vref1 = false,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb12_chip_features = {
+	.name = "mcp48feb12",
+	.phys_channels = 2,
+	.resolution = 10,
+	.have_ext_vref1 = false,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb14_chip_features = {
+	.name = "mcp48feb14",
+	.phys_channels = 4,
+	.resolution = 10,
+	.have_ext_vref1 = true,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb18_chip_features = {
+	.name = "mcp48feb18",
+	.phys_channels = 8,
+	.resolution = 10,
+	.have_ext_vref1 = true,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb21_chip_features = {
+	.name = "mcp48feb21",
+	.phys_channels = 1,
+	.resolution = 12,
+	.have_ext_vref1 = false,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb22_chip_features = {
+	.name = "mcp48feb22",
+	.phys_channels = 2,
+	.resolution = 12,
+	.have_ext_vref1 = false,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb24_chip_features = {
+	.name = "mcp48feb24",
+	.phys_channels = 4,
+	.resolution = 12,
+	.have_ext_vref1 = true,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48feb28_chip_features = {
+	.name = "mcp48feb28",
+	.phys_channels = 8,
+	.resolution = 12,
+	.have_ext_vref1 = true,
+	.have_eeprom = true,
+};
+
+static const struct mcp48feb02_features mcp48fvb01_chip_features = {
+	.name = "mcp48fvb01",
+	.phys_channels = 1,
+	.resolution = 8,
+	.have_ext_vref1 = false,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb02_chip_features = {
+	.name = "mcp48fvb02",
+	.phys_channels = 2,
+	.resolution = 8,
+	.have_ext_vref1 = false,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb04_chip_features = {
+	.name = "mcp48fvb04",
+	.phys_channels = 4,
+	.resolution = 8,
+	.have_ext_vref1 = true,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb08_chip_features = {
+	.name = "mcp48fvb08",
+	.phys_channels = 8,
+	.resolution = 8,
+	.have_ext_vref1 = true,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb11_chip_features = {
+	.name = "mcp48fvb11",
+	.phys_channels = 1,
+	.resolution = 10,
+	.have_ext_vref1 = false,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb12_chip_features = {
+	.name = "mcp48fvb12",
+	.phys_channels = 2,
+	.resolution = 10,
+	.have_ext_vref1 = false,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb14_chip_features = {
+	.name = "mcp48fvb14",
+	.phys_channels = 4,
+	.resolution = 10,
+	.have_ext_vref1 = true,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb18_chip_features = {
+	.name = "mcp48fvb18",
+	.phys_channels = 8,
+	.resolution = 10,
+	.have_ext_vref1 = true,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb21_chip_features = {
+	.name = "mcp48fvb21",
+	.phys_channels = 1,
+	.resolution = 12,
+	.have_ext_vref1 = false,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb22_chip_features = {
+	.name = "mcp48fvb22",
+	.phys_channels = 2,
+	.resolution = 12,
+	.have_ext_vref1 = false,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb24_chip_features = {
+	.name = "mcp48fvb24",
+	.phys_channels = 4,
+	.resolution = 12,
+	.have_ext_vref1 = true,
+	.have_eeprom = false,
+};
+
+static const struct mcp48feb02_features mcp48fvb28_chip_features = {
+	.name = "mcp48fvb28",
+	.phys_channels = 8,
+	.resolution = 12,
+	.have_ext_vref1 = true,
+	.have_eeprom = false,
+};
+
+/**
+ * struct mcp48feb02_channel_data - channel configuration
+ * @ref_mode: chosen voltage for reference
+ * @use_2x_gain: output driver gain control
+ * @powerdown: is false if the channel is in normal operation mode
+ * @powerdown_mode: selected power-down mode
+ * @dac_data: dac value
+ */
+struct mcp48feb02_channel_data {
+	u8 ref_mode;
+	bool use_2x_gain;
+	bool powerdown;
+	u8 powerdown_mode;
+	u16 dac_data;
+};
+
+/**
+ * struct mcp48feb02_data - chip configuration
+ * @chdata: options configured for each channel on the device
+ * @lock: prevents concurrent reads/writes to driver's state members
+ * @chip_features: pointer to features struct
+ * @scale_1: scales set on channels that are based on Vref1
+ * @scale: scales set on channels that are based on Vref/Vref0
+ * @active_channels_mask: enabled channels
+ * @regmap: regmap for directly accessing device register
+ * @labels: table with channels labels
+ * @phys_channels: physical channels on the device
+ * @vref1_buffered: Vref1 buffer is enabled
+ * @vref_buffered: Vref/Vref0 buffer is enabled
+ * @use_vref1: vref1-supply is defined
+ * @use_vref: vref-supply is defined
+ */
+struct mcp48feb02_data {
+	struct mcp48feb02_channel_data chdata[MCP48FEB02_MAX_CH];
+	struct mutex lock; /* prevents concurrent reads/writes to driver's state members */
+	const struct mcp48feb02_features *chip_features;
+	int scale_1[2 * MCP48FEB02_MAX_SCALES_CH];
+	int scale[2 * MCP48FEB02_MAX_SCALES_CH];
+	unsigned long active_channels_mask;
+	struct regmap *regmap;
+	const char *labels[MCP48FEB02_MAX_CH];
+	u16 phys_channels;
+	bool vref1_buffered;
+	bool vref_buffered;
+	bool use_vref1;
+	bool use_vref;
+};
+
+static const struct regmap_range mcp48feb02_readable_ranges[] = {
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+	regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR),
+};
+
+static const struct regmap_range mcp48feb02_writable_ranges[] = {
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+	regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR),
+};
+
+static const struct regmap_range mcp48feb02_volatile_ranges[] = {
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+	regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR),
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+	regmap_reg_range(MCP48FEB02_NV_DAC0_REG_ADDR, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR),
+};
+
+static const struct regmap_access_table mcp48feb02_readable_table = {
+	.yes_ranges = mcp48feb02_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(mcp48feb02_readable_ranges),
+};
+
+static const struct regmap_access_table mcp48feb02_writable_table = {
+	.yes_ranges = mcp48feb02_writable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(mcp48feb02_writable_ranges),
+};
+
+static const struct regmap_access_table mcp48feb02_volatile_table = {
+	.yes_ranges = mcp48feb02_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(mcp48feb02_volatile_ranges),
+};
+
+static const struct regmap_config mcp48feb02_regmap_config = {
+	.name = "mcp48feb02_regmap",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.rd_table = &mcp48feb02_readable_table,
+	.wr_table = &mcp48feb02_writable_table,
+	.volatile_table = &mcp48feb02_volatile_table,
+	.max_register = MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR,
+	.read_flag_mask = READFLAG_MASK,
+	.cache_type = REGCACHE_MAPLE,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
+
+/* For devices that doesn't have nonvolatile memory */
+static const struct regmap_range mcp48fvb02_readable_ranges[] = {
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+};
+
+static const struct regmap_range mcp48fvb02_writable_ranges[] = {
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+};
+
+static const struct regmap_range mcp48fvb02_volatile_ranges[] = {
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+	regmap_reg_range(MCP48FEB02_DAC0_REG_ADDR, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR),
+};
+
+static const struct regmap_access_table mcp48fvb02_readable_table = {
+	.yes_ranges = mcp48fvb02_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(mcp48fvb02_readable_ranges),
+};
+
+static const struct regmap_access_table mcp48fvb02_writable_table = {
+	.yes_ranges = mcp48fvb02_writable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(mcp48fvb02_writable_ranges),
+};
+
+static const struct regmap_access_table mcp48fvb02_volatile_table = {
+	.yes_ranges = mcp48fvb02_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(mcp48fvb02_volatile_ranges),
+};
+
+static const struct regmap_config mcp48fvb02_regmap_config = {
+	.name = "mcp48fvb02_regmap",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.rd_table = &mcp48fvb02_readable_table,
+	.wr_table = &mcp48fvb02_writable_table,
+	.volatile_table = &mcp48fvb02_volatile_table,
+	.max_register = MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR,
+	.read_flag_mask = READFLAG_MASK,
+	.cache_type = REGCACHE_MAPLE,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
+
+static int mcp48feb02_write_to_eeprom(struct mcp48feb02_data *data, unsigned int reg,
+				      unsigned int val)
+{
+	int eewa_val, ret;
+
+	ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR,
+				       eewa_val,
+				       !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK),
+				       USEC_PER_MSEC, USEC_PER_MSEC * 5);
+	if (ret)
+		return ret;
+
+	return regmap_write(data->regmap, reg, val);
+}
+
+static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct mcp48feb02_data *data = iio_priv(dev_to_iio_dev(dev));
+	unsigned int i, val, val1, eewa_val;
+	bool state;
+	int ret;
+
+	ret = kstrtobool(buf, &state);
+	if (ret)
+		return ret;
+
+	if (!state)
+		return 0;
+
+	/*
+	 * Wait until the currently occurring EEPROM Write Cycle is completed.
+	 * Only serial commands to the volatile memory are allowed.
+	 */
+	guard(mutex)(&data->lock);
+
+	/*
+	 * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled,
+	 * writing to EEPROM is available.
+	 */
+	ret = regmap_read(data->regmap, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR, &val);
+	if (ret)
+		return ret;
+
+	if (val) {
+		dev_err(dev, "DAC Wiper and DAC Configuration not are unlocked.\n");
+		return -EINVAL;
+	}
+
+	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
+		ret = mcp48feb02_write_to_eeprom(data, NV_REG_ADDR(i),
+						 data->chdata[i].dac_data);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &val);
+	if (ret)
+		return ret;
+
+	ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_VREF_REG_ADDR, val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &val);
+	if (ret)
+		return ret;
+
+	ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_POWER_DOWN_REG_ADDR, val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val,
+				       !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK),
+				       USEC_PER_MSEC, USEC_PER_MSEC * 5);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &val1);
+	if (ret)
+		return ret;
+
+	ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR,
+					 (val1 & MCP48FEB02_GAIN_BITS_MASK) |
+					 (val & MCP48FEB02_NV_I2C_SLAVE_ADDR_MASK));
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR_WO(store_eeprom, 0);
+
+static struct attribute *mcp48feb02_attributes[] = {
+	&iio_dev_attr_store_eeprom.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group mcp48feb02_attribute_group = {
+	.attrs = mcp48feb02_attributes,
+};
+
+static int mcp48feb02_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+	int ret;
+	u8 ch;
+
+	guard(mutex)(&data->lock);
+
+	for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) {
+		u8 pd_mode;
+
+		data->chdata[ch].powerdown = true;
+		pd_mode = data->chdata[ch].powerdown_mode + 1;
+		ret = regmap_update_bits(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR,
+					 DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, pd_mode));
+		if (ret)
+			return ret;
+
+		ret = regmap_write(data->regmap, REG_ADDR(ch), data->chdata[ch].dac_data);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int mcp48feb02_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+	u8 ch;
+
+	guard(mutex)(&data->lock);
+
+	for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) {
+		int ret;
+
+		data->chdata[ch].powerdown = false;
+
+		ret = regmap_write(data->regmap, REG_ADDR(ch), data->chdata[ch].dac_data);
+		if (ret)
+			return ret;
+
+		ret = regmap_update_bits(data->regmap, MCP48FEB02_VREF_REG_ADDR,
+					 DAC_CTRL_MASK(ch),
+					 DAC_CTRL_VAL(ch, data->chdata[ch].ref_mode));
+		if (ret)
+			return ret;
+
+		ret = regmap_update_bits(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR,
+					 DAC_GAIN_MASK(ch),
+					 DAC_GAIN_VAL(ch, data->chdata[ch].use_2x_gain));
+		if (ret)
+			return ret;
+
+		ret = regmap_update_bits(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR,
+					 DAC_CTRL_MASK(ch),
+					 DAC_CTRL_VAL(ch, MCP48FEB02_NORMAL_OPERATION));
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int mcp48feb02_get_powerdown_mode(struct iio_dev *indio_dev,
+					 const struct iio_chan_spec *chan)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+
+	return data->chdata[chan->address].powerdown_mode;
+}
+
+static int mcp48feb02_set_powerdown_mode(struct iio_dev *indio_dev, const struct iio_chan_spec *ch,
+					 unsigned int mode)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+
+	data->chdata[ch->address].powerdown_mode = mode;
+
+	return 0;
+}
+
+static ssize_t mcp48feb02_read_powerdown(struct iio_dev *indio_dev, uintptr_t private,
+					 const struct iio_chan_spec *ch, char *buf)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+
+	/* Print if channel is in a power-down mode or not */
+	return sysfs_emit(buf, "%d\n", data->chdata[ch->address].powerdown);
+}
+
+static ssize_t mcp48feb02_write_powerdown(struct iio_dev *indio_dev, uintptr_t private,
+					  const struct iio_chan_spec *ch, const char *buf,
+					  size_t len)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+	u32 reg = ch->address;
+	u8 tmp_pd_mode;
+	bool state;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	ret = kstrtobool(buf, &state);
+	if (ret)
+		return ret;
+
+	/*
+	 * Set the channel to the specified power-down mode. Exiting power-down mode
+	 * requires writing normal operation mode (0) to the channel-specific register bits.
+	 */
+	tmp_pd_mode = state ? (data->chdata[reg].powerdown_mode + 1) : MCP48FEB02_NORMAL_OPERATION;
+	ret = regmap_update_bits(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR,
+				 DAC_CTRL_MASK(reg), DAC_CTRL_VAL(reg, tmp_pd_mode));
+	if (ret)
+		return ret;
+
+	data->chdata[reg].powerdown = state;
+
+	return len;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(mcp48feb02_pm_ops, mcp48feb02_suspend, mcp48feb02_resume);
+
+static const struct iio_enum mcp48febxx_powerdown_mode_enum = {
+	.items = mcp48feb02_powerdown_modes,
+	.num_items = ARRAY_SIZE(mcp48feb02_powerdown_modes),
+	.get = mcp48feb02_get_powerdown_mode,
+	.set = mcp48feb02_set_powerdown_mode,
+};
+
+static const struct iio_chan_spec_ext_info mcp48feb02_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = mcp48feb02_read_powerdown,
+		.write = mcp48feb02_write_powerdown,
+		.shared = IIO_SEPARATE,
+	},
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp48febxx_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE, &mcp48febxx_powerdown_mode_enum),
+	{ }
+};
+
+static const struct iio_chan_spec mcp48febxx_ch_template = {
+	.type = IIO_VOLTAGE,
+	.output = 1,
+	.indexed = 1,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+	.ext_info = mcp48feb02_ext_info,
+};
+
+static void mcp48feb02_init_scale(struct mcp48feb02_data *data, enum mcp48feb02_scale scale,
+				  int vref_uV, int scale_avail[])
+{
+	u32 value_micro, value_int;
+	u64 tmp;
+
+	/* vref_uV should not be negative */
+	tmp = (u64)vref_uV * MILLI >> data->chip_features->resolution;
+	value_int = div_u64_rem(tmp, MICRO, &value_micro);
+	scale_avail[scale * 2] = value_int;
+	scale_avail[scale * 2 + 1] = value_micro;
+}
+
+static int mcp48feb02_init_scales_avail(struct mcp48feb02_data *data, int vdd_uV,
+					int vref_uV, int vref1_uV)
+{
+	int tmp_vref;
+
+	mcp48feb02_init_scale(data, MCP48FEB02_SCALE_VDD, vdd_uV, data->scale);
+
+	tmp_vref = data->use_vref ? vref_uV : MCP48FEB02_INTERNAL_BAND_GAP_uV;
+	mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X1, tmp_vref, data->scale);
+	mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X2, tmp_vref * 2, data->scale);
+
+	if (data->phys_channels >= 4) {
+		mcp48feb02_init_scale(data, MCP48FEB02_SCALE_VDD, vdd_uV, data->scale_1);
+
+		if (data->use_vref1)
+			tmp_vref = vref1_uV;
+		else
+			tmp_vref = MCP48FEB02_INTERNAL_BAND_GAP_uV;
+
+		mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X1,
+				      tmp_vref, data->scale_1);
+		mcp48feb02_init_scale(data, MCP48FEB02_SCALE_GAIN_X2,
+				      tmp_vref * 2, data->scale_1);
+	}
+
+	return 0;
+}
+
+static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *ch,
+				 const int **vals, int *type, int *length, long info)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		switch (ch->type) {
+		case IIO_VOLTAGE:
+			if (data->phys_channels >= 4 && (ch->address % 2))
+				*vals = data->scale_1;
+			else
+				*vals = data->scale;
+
+			*length = 2 * MCP48FEB02_MAX_SCALES_CH;
+			*type = IIO_VAL_INT_PLUS_MICRO;
+			return IIO_AVAIL_LIST;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static void mcp48feb02_get_scale(int ch, struct mcp48feb02_data *data, int *val, int *val2)
+{
+	enum mcp48feb02_scale current_scale;
+
+	if (data->chdata[ch].ref_mode == MCP48FEB02_VREF_VDD)
+		current_scale = MCP48FEB02_SCALE_VDD;
+	else if (data->chdata[ch].use_2x_gain)
+		current_scale = MCP48FEB02_SCALE_GAIN_X2;
+	else
+		current_scale = MCP48FEB02_SCALE_GAIN_X1;
+
+	if (data->phys_channels >= 4 && (ch % 2)) {
+		*val = data->scale_1[current_scale * 2];
+		*val2 = data->scale_1[current_scale * 2 + 1];
+	} else {
+		*val = data->scale[current_scale * 2];
+		*val2 = data->scale[current_scale * 2 + 1];
+	}
+}
+
+static int mcp48feb02_check_scale(struct mcp48feb02_data *data, int val, int val2, int scale[])
+{
+	unsigned int i;
+
+	for (i = 0; i < MCP48FEB02_MAX_SCALES_CH; i++) {
+		if (scale[i * 2] == val && scale[i * 2 + 1] == val2)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static int mcp48feb02_ch_scale(struct mcp48feb02_data *data, int ch, int scale)
+{
+	int tmp_val, ret;
+
+	if (scale == MCP48FEB02_SCALE_VDD) {
+		tmp_val = MCP48FEB02_VREF_VDD;
+	} else if (data->phys_channels >= 4 && (ch % 2)) {
+		if (data->use_vref1) {
+			if (data->vref1_buffered)
+				tmp_val = MCP48FEB02_EXTERNAL_VREF_BUFFERED;
+			else
+				tmp_val = MCP48FEB02_EXTERNAL_VREF_UNBUFFERED;
+		} else {
+			tmp_val = MCP48FEB02_INTERNAL_BAND_GAP;
+		}
+	} else if (data->use_vref) {
+		if (data->vref_buffered)
+			tmp_val = MCP48FEB02_EXTERNAL_VREF_BUFFERED;
+		else
+			tmp_val = MCP48FEB02_EXTERNAL_VREF_UNBUFFERED;
+	} else {
+		tmp_val = MCP48FEB02_INTERNAL_BAND_GAP;
+	}
+
+	ret = regmap_update_bits(data->regmap, MCP48FEB02_VREF_REG_ADDR,
+				 DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, tmp_val));
+	if (ret)
+		return ret;
+
+	data->chdata[ch].ref_mode = tmp_val;
+
+	return 0;
+}
+
+/*
+ * Setting the scale in order to choose between VDD and (Vref or Band Gap) from the user
+ * space. The VREF pin is either an input or an output, therefore the user cannot
+ * simultaneously connect an external voltage reference to the pin and select the
+ * internal Band Gap.
+ * When the DAC’s voltage reference is configured as the VREF pin, the pin is an input.
+ * When the DAC’s voltage reference is configured as the internal Band Gap,
+ * the VREF pin is an output.
+ * If Vref/Vref1 voltage is not available, then the internal Band Gap will be used
+ * to calculate the values for the scale.
+ */
+static int mcp48feb02_set_scale(struct mcp48feb02_data *data, int ch, int scale)
+{
+	int tmp_val, ret;
+
+	ret = mcp48feb02_ch_scale(data, ch, scale);
+	if (ret)
+		return ret;
+
+	if (scale == MCP48FEB02_SCALE_GAIN_X2)
+		tmp_val = MCP48FEB02_GAIN_BIT_X2;
+	else
+		tmp_val = MCP48FEB02_GAIN_BIT_X1;
+
+	ret = regmap_update_bits(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR,
+				 DAC_GAIN_MASK(ch), DAC_GAIN_VAL(ch, tmp_val));
+	if (ret)
+		return ret;
+
+	data->chdata[ch].use_2x_gain = tmp_val;
+
+	return 0;
+}
+
+static int mcp48feb02_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *ch,
+			       int *val, int *val2, long mask)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(data->regmap, REG_ADDR(ch->address), val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		mcp48feb02_get_scale(ch->address, data, val, val2);
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mcp48feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *ch,
+				int val, int val2, long mask)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+	int *tmp_scale, ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_write(data->regmap, REG_ADDR(ch->address), val);
+		if (ret)
+			return ret;
+
+		data->chdata[ch->address].dac_data = val;
+		return 0;
+	case IIO_CHAN_INFO_SCALE:
+		if (data->phys_channels >= 4 && (ch->address % 2))
+			tmp_scale = data->scale_1;
+		else
+			tmp_scale = data->scale;
+
+		ret = mcp48feb02_check_scale(data, val, val2, tmp_scale);
+		if (ret < 0)
+			return ret;
+
+		return mcp48feb02_set_scale(data, ch->address, ret);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mcp48feb02_read_label(struct iio_dev *indio_dev, struct iio_chan_spec const *ch,
+				 char *label)
+{
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+
+	return sysfs_emit(label, "%s\n", data->labels[ch->address]);
+}
+
+static const struct iio_info mcp48feb02_info = {
+	.read_raw = mcp48feb02_read_raw,
+	.write_raw = mcp48feb02_write_raw,
+	.read_label = mcp48feb02_read_label,
+	.read_avail = &mcp48feb02_read_avail,
+	.attrs = &mcp48feb02_attribute_group,
+};
+
+static const struct iio_info mcp48fvb02_info = {
+	.read_raw = mcp48feb02_read_raw,
+	.write_raw = mcp48feb02_write_raw,
+	.read_label = mcp48feb02_read_label,
+	.read_avail = &mcp48feb02_read_avail,
+};
+
+static int mcp48feb02_parse_fw(struct iio_dev *indio_dev,
+			       const struct mcp48feb02_features *chip_features)
+{
+	struct iio_chan_spec chanspec = mcp48febxx_ch_template;
+	struct mcp48feb02_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	struct iio_chan_spec *channels;
+	u32 num_channels;
+	u8 chan_idx = 0;
+
+	guard(mutex)(&data->lock);
+
+	num_channels = device_get_child_node_count(dev);
+	if (num_channels > chip_features->phys_channels)
+		return dev_err_probe(dev, -EINVAL, "More channels than the chip supports\n");
+
+	if (!num_channels)
+		return dev_err_probe(dev, -EINVAL, "No channel specified in the devicetree.\n");
+
+	channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
+
+	device_for_each_child_node_scoped(dev, child) {
+		u32 reg = 0;
+		int ret;
+
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			return dev_err_probe(dev, ret, "Invalid channel number\n");
+
+		if (reg >= chip_features->phys_channels)
+			return dev_err_probe(dev, -EINVAL,
+					     "The index of the channels does not match the chip\n");
+
+		set_bit(reg, &data->active_channels_mask);
+
+		ret = fwnode_property_read_string(child, "label", &data->labels[reg]);
+		if (ret)
+			return dev_err_probe(dev, ret, "%pfw: invalid label\n",
+					     fwnode_get_name(child));
+
+		chanspec.address = reg;
+		chanspec.channel = reg;
+		channels[chan_idx] = chanspec;
+		chan_idx++;
+	}
+
+	indio_dev->num_channels = num_channels;
+	indio_dev->channels = channels;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	data->phys_channels = chip_features->phys_channels;
+
+	data->vref_buffered = device_property_read_bool(dev, "microchip,vref-buffered");
+
+	if (chip_features->have_ext_vref1)
+		data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1-buffered");
+
+	return 0;
+}
+
+static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data)
+{
+	unsigned int i, vref_ch, gain_ch, pd_ch;
+	int ret;
+
+	ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch);
+	if (ret)
+		return ret;
+
+	gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK;
+	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
+		struct device *dev = regmap_get_device(data->regmap);
+		unsigned int pd_tmp;
+
+		data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK;
+		data->chdata[i].use_2x_gain = (gain_ch >> i)  & MCP48FEB02_GAIN_BIT_MASK;
+
+		/*
+		 * Inform the user that the current voltage reference read from the volatile
+		 * register of the chip is different from the one specified in the device tree.
+		 * Considering that the user cannot have an external voltage reference connected
+		 * to the pin and select the internal Band Gap at the same time, in order to avoid
+		 * miscofiguring the reference voltage, the volatile register will not be written.
+		 * In order to overwrite the setting from volatile register with the one from the
+		 * device tree, the user needs to write the chosen scale.
+		 */
+		switch (data->chdata[i].ref_mode) {
+		case MCP48FEB02_INTERNAL_BAND_GAP:
+			if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) {
+				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
+				dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i);
+				break;
+			}
+			if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) &&
+			    data->use_vref) {
+				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
+				dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i);
+				break;
+			}
+			break;
+		case MCP48FEB02_EXTERNAL_VREF_UNBUFFERED:
+		case MCP48FEB02_EXTERNAL_VREF_BUFFERED:
+			if (data->phys_channels >= 4 && (i % 2) && !data->use_vref1) {
+				dev_dbg(dev, "ch[%u]: was configured to use VREF1", i);
+				dev_dbg(dev,
+					"ch[%u]: reference voltage set to internal band gap", i);
+				break;
+			}
+			if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) &&
+			    !data->use_vref) {
+				dev_dbg(dev, "ch[%u]: was configured to use VREF", i);
+				dev_dbg(dev,
+					"ch[%u]: reference voltage set to internal band gap", i);
+				break;
+			}
+			break;
+		}
+
+		pd_tmp = (pd_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK;
+		data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp;
+		data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode);
+	}
+
+	return 0;
+}
+
+static int mcp48feb02_init_ch_scales(struct mcp48feb02_data *data, int vdd_uV,
+				     int vref_uV, int vref1_uV)
+{
+	unsigned int i;
+
+	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
+		struct device *dev = regmap_get_device(data->regmap);
+		int ret;
+
+		ret = mcp48feb02_init_scales_avail(data, vdd_uV, vref_uV, vref1_uV);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to init scales for ch %u\n", i);
+	}
+
+	return 0;
+}
+
+static int mcp48feb02_probe(struct spi_device *spi)
+{
+	const struct mcp48feb02_features *chip_features;
+	struct device *dev = &spi->dev;
+	struct mcp48feb02_data *data;
+	struct iio_dev *indio_dev;
+	int vref1_uV = 0;
+	int vref_uV = 0;
+	int vdd_uV;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+
+	chip_features = spi_get_device_match_data(spi);
+	if (!chip_features)
+		return -EINVAL;
+
+	data->chip_features = chip_features;
+
+	if (chip_features->have_eeprom) {
+		data->regmap = devm_regmap_init_spi(spi, &mcp48feb02_regmap_config);
+		indio_dev->info = &mcp48feb02_info;
+	} else {
+		data->regmap = devm_regmap_init_spi(spi, &mcp48fvb02_regmap_config);
+		indio_dev->info = &mcp48fvb02_info;
+	}
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing spi regmap\n");
+
+	indio_dev->name = chip_features->name;
+
+	ret = mcp48feb02_parse_fw(indio_dev, chip_features);
+	if (ret)
+		return dev_err_probe(dev, ret, "Error parsing firmware data\n");
+
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "vdd");
+	if (ret < 0)
+		return ret;
+
+	vdd_uV = ret;
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
+	if (ret > 0) {
+		vref_uV = ret;
+		data->use_vref = true;
+	} else {
+		dev_dbg(dev, "using internal band gap as voltage reference.\n");
+		dev_dbg(dev, "External Vref is unavailable.\n");
+	}
+
+	if (chip_features->have_ext_vref1) {
+		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
+		if (ret > 0) {
+			vref1_uV = ret;
+			data->use_vref1 = true;
+		} else {
+			dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
+			dev_dbg(dev, "External Vref1 is unavailable.\n");
+		}
+	}
+
+	ret = mcp48feb02_init_ctrl_regs(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Error initialising vref register\n");
+
+	ret = mcp48feb02_init_ch_scales(data, vdd_uV, vref_uV, vref1_uV);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct spi_device_id mcp48feb02_id[] = {
+	{ "mcp48feb01", (kernel_ulong_t)&mcp48feb01_chip_features },
+	{ "mcp48feb02", (kernel_ulong_t)&mcp48feb02_chip_features },
+	{ "mcp48feb04", (kernel_ulong_t)&mcp48feb04_chip_features },
+	{ "mcp48feb08", (kernel_ulong_t)&mcp48feb08_chip_features },
+	{ "mcp48feb11", (kernel_ulong_t)&mcp48feb11_chip_features },
+	{ "mcp48feb12", (kernel_ulong_t)&mcp48feb12_chip_features },
+	{ "mcp48feb14", (kernel_ulong_t)&mcp48feb14_chip_features },
+	{ "mcp48feb18", (kernel_ulong_t)&mcp48feb18_chip_features },
+	{ "mcp48feb21", (kernel_ulong_t)&mcp48feb21_chip_features },
+	{ "mcp48feb22", (kernel_ulong_t)&mcp48feb22_chip_features },
+	{ "mcp48feb24", (kernel_ulong_t)&mcp48feb24_chip_features },
+	{ "mcp48feb28", (kernel_ulong_t)&mcp48feb28_chip_features },
+	{ "mcp48fvb01", (kernel_ulong_t)&mcp48fvb01_chip_features },
+	{ "mcp48fvb02", (kernel_ulong_t)&mcp48fvb02_chip_features },
+	{ "mcp48fvb04", (kernel_ulong_t)&mcp48fvb04_chip_features },
+	{ "mcp48fvb08", (kernel_ulong_t)&mcp48fvb08_chip_features },
+	{ "mcp48fvb11", (kernel_ulong_t)&mcp48fvb11_chip_features },
+	{ "mcp48fvb12", (kernel_ulong_t)&mcp48fvb12_chip_features },
+	{ "mcp48fvb14", (kernel_ulong_t)&mcp48fvb14_chip_features },
+	{ "mcp48fvb18", (kernel_ulong_t)&mcp48fvb18_chip_features },
+	{ "mcp48fvb21", (kernel_ulong_t)&mcp48fvb21_chip_features },
+	{ "mcp48fvb22", (kernel_ulong_t)&mcp48fvb22_chip_features },
+	{ "mcp48fvb24", (kernel_ulong_t)&mcp48fvb24_chip_features },
+	{ "mcp48fvb28", (kernel_ulong_t)&mcp48fvb28_chip_features },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, mcp48feb02_id);
+
+static const struct of_device_id mcp48feb02_of_match[] = {
+	{ .compatible = "microchip,mcp48feb01", .data = &mcp48feb01_chip_features },
+	{ .compatible = "microchip,mcp48feb02", .data = &mcp48feb02_chip_features },
+	{ .compatible = "microchip,mcp48feb04", .data = &mcp48feb04_chip_features },
+	{ .compatible = "microchip,mcp48feb08", .data = &mcp48feb08_chip_features },
+	{ .compatible = "microchip,mcp48feb11", .data = &mcp48feb11_chip_features },
+	{ .compatible = "microchip,mcp48feb12", .data = &mcp48feb12_chip_features },
+	{ .compatible = "microchip,mcp48feb14", .data = &mcp48feb14_chip_features },
+	{ .compatible = "microchip,mcp48feb18", .data = &mcp48feb18_chip_features },
+	{ .compatible = "microchip,mcp48feb21", .data = &mcp48feb21_chip_features },
+	{ .compatible = "microchip,mcp48feb22", .data = &mcp48feb22_chip_features },
+	{ .compatible = "microchip,mcp48feb24", .data = &mcp48feb24_chip_features },
+	{ .compatible = "microchip,mcp48feb28", .data = &mcp48feb28_chip_features },
+	{ .compatible = "microchip,mcp48fvb01", .data = &mcp48fvb01_chip_features },
+	{ .compatible = "microchip,mcp48fvb02", .data = &mcp48fvb02_chip_features },
+	{ .compatible = "microchip,mcp48fvb04", .data = &mcp48fvb04_chip_features },
+	{ .compatible = "microchip,mcp48fvb08", .data = &mcp48fvb08_chip_features },
+	{ .compatible = "microchip,mcp48fvb11", .data = &mcp48fvb11_chip_features },
+	{ .compatible = "microchip,mcp48fvb12", .data = &mcp48fvb12_chip_features },
+	{ .compatible = "microchip,mcp48fvb14",	.data = &mcp48fvb14_chip_features },
+	{ .compatible = "microchip,mcp48fvb18", .data = &mcp48fvb18_chip_features },
+	{ .compatible = "microchip,mcp48fvb21", .data = &mcp48fvb21_chip_features },
+	{ .compatible = "microchip,mcp48fvb22", .data = &mcp48fvb22_chip_features },
+	{ .compatible = "microchip,mcp48fvb24", .data = &mcp48fvb24_chip_features },
+	{ .compatible = "microchip,mcp48fvb28", .data = &mcp48fvb28_chip_features },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mcp48feb02_of_match);
+
+static struct spi_driver mcp48feb02_driver = {
+	.driver = {
+		.name	= "mcp48feb02",
+		.of_match_table = mcp48feb02_of_match,
+		.pm	= pm_sleep_ptr(&mcp48feb02_pm_ops),
+	},
+	.probe		= mcp48feb02_probe,
+	.id_table	= mcp48feb02_id,
+};
+module_spi_driver(mcp48feb02_driver);
+
+MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>");
+MODULE_DESCRIPTION("IIO driver for MCP48FEB02 Multi-Channel DAC with SPI interface");
+MODULE_LICENSE("GPL");

-- 
2.43.0

Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Thu, 12 Feb 2026 14:48:35 +0200
Ariana Lazar <ariana.lazar@microchip.com> wrote:

> This is the iio driver for Microchip MCP48FxBy1/2/4/8 series of buffered
> voltage output Digital-to-Analog Converters with nonvolatile or volatile
> memory and an SPI Interface.
> 
> The families support up to 8 output channels.
> 
> The devices can be 8-bit, 10-bit and 12-bit.
> 
> Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
Hi Ariana,
Given the much larger outstanding question on whether this can share a driver
with other similar parts I only took a very quick look at this version.

Comments inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/dac/mcp48feb02.c b/drivers/iio/dac/mcp48feb02.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..20955f77053329a9c385f55c7314032eb6b04dfd
> --- /dev/null
> +++ b/drivers/iio/dac/mcp48feb02.c

> +
> +static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data)
> +{
> +	unsigned int i, vref_ch, gain_ch, pd_ch;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch);
> +	if (ret)
> +		return ret;
> +
> +	gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK;
> +	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
> +		struct device *dev = regmap_get_device(data->regmap);
> +		unsigned int pd_tmp;
> +
> +		data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK;
> +		data->chdata[i].use_2x_gain = (gain_ch >> i)  & MCP48FEB02_GAIN_BIT_MASK;
> +
> +		/*
> +		 * Inform the user that the current voltage reference read from the volatile
> +		 * register of the chip is different from the one specified in the device tree.
> +		 * Considering that the user cannot have an external voltage reference connected
> +		 * to the pin and select the internal Band Gap at the same time, in order to avoid
> +		 * miscofiguring the reference voltage, the volatile register will not be written.

Spell check comments.  misconfiguring

> +		 * In order to overwrite the setting from volatile register with the one from the
> +		 * device tree, the user needs to write the chosen scale.

I'm a little unsure of why we need this extra gate on updating things to match
the device tree provided config.  Why should the volatile register at this point
match what DT says?  If it does seems to me we should be noisier about it than dev_dbg()


> +		 */
> +		switch (data->chdata[i].ref_mode) {
> +		case MCP48FEB02_INTERNAL_BAND_GAP:
> +			if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) {
> +				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
> +				dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i);
> +				break;
> +			}
> +			if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) &&
> +			    data->use_vref) {
> +				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
> +				dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i);
> +				break;
> +			}
Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Thu, Feb 12, 2026 at 02:48:35PM +0200, Ariana Lazar wrote:
> This is the iio driver for Microchip MCP48FxBy1/2/4/8 series of buffered
> voltage output Digital-to-Analog Converters with nonvolatile or volatile
> memory and an SPI Interface.
> 
> The families support up to 8 output channels.
> 
> The devices can be 8-bit, 10-bit and 12-bit.

...

> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>

> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

I would split this group...

> +#include <linux/kstrtox.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/spi/spi.h>
> +#include <linux/time64.h>
> +#include <linux/types.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +

...to be here as other IIO drivers do.

...

> +/* Gain Control and I2C Slave Address Reguster fields */
> +#define DAC_GAIN_MASK(ch)				(BIT(0) << (8 + (ch)))

Just BIT(8 + (ch)) should suffice.

> +#define DAC_GAIN_VAL(ch, val)				((val) << (8 + (ch)))

For the sake of consistency this may be also rewritten to

#define DAC_GAIN_VAL(ch, val)				((val) * BIT(8 + (ch)))

...

> +/**
> + * struct mcp48feb02_channel_data - channel configuration
> + * @ref_mode: chosen voltage for reference
> + * @use_2x_gain: output driver gain control
> + * @powerdown: is false if the channel is in normal operation mode
> + * @powerdown_mode: selected power-down mode
> + * @dac_data: dac value

DAC

> + */
> +struct mcp48feb02_channel_data {
> +	u8 ref_mode;
> +	bool use_2x_gain;
> +	bool powerdown;
> +	u8 powerdown_mode;
> +	u16 dac_data;

Wondering if the following arrangement is slightly better:

	u16 dac_data;
	u8 ref_mode;
	u8 powerdown_mode;
	bool powerdown;
	bool use_2x_gain;

> +};

...

> +/**
> + * struct mcp48feb02_data - chip configuration
> + * @chdata: options configured for each channel on the device
> + * @lock: prevents concurrent reads/writes to driver's state members
> + * @chip_features: pointer to features struct
> + * @scale_1: scales set on channels that are based on Vref1
> + * @scale: scales set on channels that are based on Vref/Vref0
> + * @active_channels_mask: enabled channels
> + * @regmap: regmap for directly accessing device register
> + * @labels: table with channels labels
> + * @phys_channels: physical channels on the device
> + * @vref1_buffered: Vref1 buffer is enabled
> + * @vref_buffered: Vref/Vref0 buffer is enabled
> + * @use_vref1: vref1-supply is defined
> + * @use_vref: vref-supply is defined
> + */
> +struct mcp48feb02_data {
> +	struct mcp48feb02_channel_data chdata[MCP48FEB02_MAX_CH];
> +	struct mutex lock; /* prevents concurrent reads/writes to driver's state members */
> +	const struct mcp48feb02_features *chip_features;
> +	int scale_1[2 * MCP48FEB02_MAX_SCALES_CH];
> +	int scale[2 * MCP48FEB02_MAX_SCALES_CH];

I would name it scale1 and scale0. This will increase readability to see that
the sizes are equal and that the first digit is the part of the name.

> +	unsigned long active_channels_mask;
> +	struct regmap *regmap;
> +	const char *labels[MCP48FEB02_MAX_CH];
> +	u16 phys_channels;
> +	bool vref1_buffered;
> +	bool vref_buffered;
> +	bool use_vref1;
> +	bool use_vref;
> +};

...

> +static int mcp48feb02_write_to_eeprom(struct mcp48feb02_data *data, unsigned int reg,
> +				      unsigned int val)
> +{
> +	int eewa_val, ret;

Is it okay that the eewa_val is signed?

> +	ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR,
> +				       eewa_val,
> +				       !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK),
> +				       USEC_PER_MSEC, USEC_PER_MSEC * 5);

I would rather put it as

				       1 * USEC_PER_MSEC, 5 * USEC_PER_MSEC);

This follows the natural (from physics) reading — 1ms, 5ms.

> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(data->regmap, reg, val);
> +}

...

> +static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct mcp48feb02_data *data = iio_priv(dev_to_iio_dev(dev));
> +	unsigned int i, val, val1, eewa_val;
> +	bool state;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &state);
> +	if (ret)
> +		return ret;
> +
> +	if (!state)
> +		return 0;
> +
> +	/*
> +	 * Wait until the currently occurring EEPROM Write Cycle is completed.
> +	 * Only serial commands to the volatile memory are allowed.
> +	 */
> +	guard(mutex)(&data->lock);
> +
> +	/*
> +	 * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled,
> +	 * writing to EEPROM is available.
> +	 */
> +	ret = regmap_read(data->regmap, MCP48FEB02_WIPERLOCK_STATUS_REG_ADDR, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val) {
> +		dev_err(dev, "DAC Wiper and DAC Configuration not are unlocked.\n");

"are not"

> +		return -EINVAL;
> +	}
> +
> +	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
> +		ret = mcp48feb02_write_to_eeprom(data, NV_REG_ADDR(i),
> +						 data->chdata[i].dac_data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_VREF_REG_ADDR, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_POWER_DOWN_REG_ADDR, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read_poll_timeout(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val,
> +				       !(eewa_val & MCP48FEB02_GAIN_BIT_STATUS_EEWA_MASK),
> +				       USEC_PER_MSEC, USEC_PER_MSEC * 5);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &val1);
> +	if (ret)
> +		return ret;
> +
> +	ret = mcp48feb02_write_to_eeprom(data, MCP48FEB02_NV_GAIN_CTRL_I2C_SLAVE_REG_ADDR,
> +					 (val1 & MCP48FEB02_GAIN_BITS_MASK) |
> +					 (val & MCP48FEB02_NV_I2C_SLAVE_ADDR_MASK));
> +	if (ret)
> +		return ret;
> +
> +	return len;
> +}

> +

Unneeded blank line.

> +static IIO_DEVICE_ATTR_WO(store_eeprom, 0);

...

> +static void mcp48feb02_init_scale(struct mcp48feb02_data *data, enum mcp48feb02_scale scale,
> +				  int vref_uV, int scale_avail[])
> +{
> +	u32 value_micro, value_int;
> +	u64 tmp;
> +
> +	/* vref_uV should not be negative */
> +	tmp = (u64)vref_uV * MILLI >> data->chip_features->resolution;

If vref_uV is guaranteed to be less than ~33V, this code can be transformed to
avoid 64-bit division. Hints: resolution is always great than 3; MILLI equals
to 2³*5³.

> +	value_int = div_u64_rem(tmp, MICRO, &value_micro);
> +	scale_avail[scale * 2] = value_int;
> +	scale_avail[scale * 2 + 1] = value_micro;
> +}

Since it's kinda a common stuff, perhaps one wants to add a helper
to include/linux/math.h.

...

> +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *ch,
> +				 const int **vals, int *type, int *length, long info)
> +{
> +	struct mcp48feb02_data *data = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (ch->type) {
> +		case IIO_VOLTAGE:
> +			if (data->phys_channels >= 4 && (ch->address % 2))
> +				*vals = data->scale_1;
> +			else
> +				*vals = data->scale;

Actually, if you put the scales as

	int scales[2][2 * MCP48FEB02_MAX_SCALES_CH];

this will become as simple as

			if (data->phys_channels >= 4)
				*vals = data->scales[ch->address];
			else
				*vals = data->scales[0];

OTOH, I am not sure if it can be always as

			*vals = data->scales[ch->address];

which would be the best approach.

> +			*length = 2 * MCP48FEB02_MAX_SCALES_CH;
> +			*type = IIO_VAL_INT_PLUS_MICRO;
> +			return IIO_AVAIL_LIST;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static void mcp48feb02_get_scale(int ch, struct mcp48feb02_data *data, int *val, int *val2)
> +{
> +	enum mcp48feb02_scale current_scale;
> +
> +	if (data->chdata[ch].ref_mode == MCP48FEB02_VREF_VDD)
> +		current_scale = MCP48FEB02_SCALE_VDD;
> +	else if (data->chdata[ch].use_2x_gain)
> +		current_scale = MCP48FEB02_SCALE_GAIN_X2;
> +	else
> +		current_scale = MCP48FEB02_SCALE_GAIN_X1;
> +
> +	if (data->phys_channels >= 4 && (ch % 2)) {
> +		*val = data->scale_1[current_scale * 2];
> +		*val2 = data->scale_1[current_scale * 2 + 1];
> +	} else {
> +		*val = data->scale[current_scale * 2];
> +		*val2 = data->scale[current_scale * 2 + 1];
> +	}

Ditto. I.o.w. you can avoid (ch % 2) for good.

> +}

...

> +static int mcp48feb02_set_scale(struct mcp48feb02_data *data, int ch, int scale)
> +{
> +	int tmp_val, ret;

Why is 'tmp_val' signed?

> +	ret = mcp48feb02_ch_scale(data, ch, scale);
> +	if (ret)
> +		return ret;
> +
> +	if (scale == MCP48FEB02_SCALE_GAIN_X2)
> +		tmp_val = MCP48FEB02_GAIN_BIT_X2;
> +	else
> +		tmp_val = MCP48FEB02_GAIN_BIT_X1;
> +
> +	ret = regmap_update_bits(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR,
> +				 DAC_GAIN_MASK(ch), DAC_GAIN_VAL(ch, tmp_val));
> +	if (ret)
> +		return ret;
> +
> +	data->chdata[ch].use_2x_gain = tmp_val;
> +
> +	return 0;
> +}

...

> +static int mcp48feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *ch,
> +				int val, int val2, long mask)
> +{
> +	struct mcp48feb02_data *data = iio_priv(indio_dev);
> +	int *tmp_scale, ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_write(data->regmap, REG_ADDR(ch->address), val);
> +		if (ret)
> +			return ret;
> +
> +		data->chdata[ch->address].dac_data = val;
> +		return 0;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (data->phys_channels >= 4 && (ch->address % 2))
> +			tmp_scale = data->scale_1;
> +		else
> +			tmp_scale = data->scale;

Same, (ch->address % 2) can be avoided.

> +		ret = mcp48feb02_check_scale(data, val, val2, tmp_scale);
> +		if (ret < 0)
> +			return ret;
> +
> +		return mcp48feb02_set_scale(data, ch->address, ret);
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static int mcp48feb02_parse_fw(struct iio_dev *indio_dev,
> +			       const struct mcp48feb02_features *chip_features)
> +{
> +	struct iio_chan_spec chanspec = mcp48febxx_ch_template;
> +	struct mcp48feb02_data *data = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(data->regmap);
> +	struct iio_chan_spec *channels;
> +	u32 num_channels;

> +	u8 chan_idx = 0;

Assignments like this are harder to maintain and prone to subtle mistakes in
case the variable gets reused. Please, split it...

> +	guard(mutex)(&data->lock);
> +
> +	num_channels = device_get_child_node_count(dev);
> +	if (num_channels > chip_features->phys_channels)
> +		return dev_err_probe(dev, -EINVAL, "More channels than the chip supports\n");
> +
> +	if (!num_channels)

While this is standard pattern, I find == 0 is more explicit when we compare
counters, but it's up to you and maintainers.

> +		return dev_err_probe(dev, -EINVAL, "No channel specified in the devicetree.\n");
> +
> +	channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;

...to be here as

	chan_idx = 0;

> +	device_for_each_child_node_scoped(dev, child) {
> +		u32 reg = 0;

Redundant assignment. "reg" is a mandatory property AFAICS from the below code.

> +		int ret;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Invalid channel number\n");
> +
> +		if (reg >= chip_features->phys_channels)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "The index of the channels does not match the chip\n");

> +		set_bit(reg, &data->active_channels_mask);

Is atomic bit operation required here?

> +		ret = fwnode_property_read_string(child, "label", &data->labels[reg]);
> +		if (ret)

> +			return dev_err_probe(dev, ret, "%pfw: invalid label\n",
> +					     fwnode_get_name(child));

Something is really wrong here. Please, fix accordingly.

> +		chanspec.address = reg;
> +		chanspec.channel = reg;
> +		channels[chan_idx] = chanspec;
> +		chan_idx++;
> +	}
> +
> +	indio_dev->num_channels = num_channels;
> +	indio_dev->channels = channels;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	data->phys_channels = chip_features->phys_channels;
> +
> +	data->vref_buffered = device_property_read_bool(dev, "microchip,vref-buffered");

> +	if (chip_features->have_ext_vref1)
> +		data->vref1_buffered = device_property_read_bool(dev, "microchip,vref1-buffered");

Alternatively can be

	if (device_property_read_bool(dev, "microchip,vref1-buffered"))
		data->vref1_buffered = chip_features->have_ext_vref1;

the difference is that vref1_buffered can be filled with "false", but I don't see
if it can be true before that. You may stick with your variant to avoid this side
effect.

> +	return 0;
> +}

...

> +static int mcp48feb02_init_ctrl_regs(struct mcp48feb02_data *data)
> +{
> +	unsigned int i, vref_ch, gain_ch, pd_ch;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_VREF_REG_ADDR, &vref_ch);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_GAIN_CTRL_STATUS_REG_ADDR, &gain_ch);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, MCP48FEB02_POWER_DOWN_REG_ADDR, &pd_ch);
> +	if (ret)
> +		return ret;
> +
> +	gain_ch = gain_ch & MCP48FEB02_GAIN_BITS_MASK;
> +	for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) {
> +		struct device *dev = regmap_get_device(data->regmap);
> +		unsigned int pd_tmp;
> +
> +		data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK;
> +		data->chdata[i].use_2x_gain = (gain_ch >> i)  & MCP48FEB02_GAIN_BIT_MASK;
> +
> +		/*
> +		 * Inform the user that the current voltage reference read from the volatile
> +		 * register of the chip is different from the one specified in the device tree.
> +		 * Considering that the user cannot have an external voltage reference connected
> +		 * to the pin and select the internal Band Gap at the same time, in order to avoid
> +		 * miscofiguring the reference voltage, the volatile register will not be written.
> +		 * In order to overwrite the setting from volatile register with the one from the
> +		 * device tree, the user needs to write the chosen scale.
> +		 */
> +		switch (data->chdata[i].ref_mode) {
> +		case MCP48FEB02_INTERNAL_BAND_GAP:
> +			if (data->phys_channels >= 4 && (i % 2) && data->use_vref1) {
> +				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
> +				dev_dbg(dev, "ch[%u]: reference voltage set to VREF1", i);
> +				break;

> +			}
> +			if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) &&
> +			    data->use_vref) {

I don't see how these two conditionals can be run both.

> +				dev_dbg(dev, "ch[%u]: was configured to use internal band gap", i);
> +				dev_dbg(dev, "ch[%u]: reference voltage set to VREF", i);
> +				break;
> +			}

With that in mind, the above can be simplified a bit.

			if (data->use_vref && ((data->phys_channels >= 4 && !(i % 2)) ||
					       (data->phys_channels < 4))) {
				dev_dbg(dev, "ch[%u]: was configured to use internal band gap\n", i);
				dev_dbg(dev, "ch[%u]: reference voltage set to Vref\n", i);
			} else if (data->use_vref1 && data->phys_channels >= 4 && (i % 2)) {
				dev_dbg(dev, "ch[%u]: was configured to use internal band gap\n", i);
				dev_dbg(dev, "ch[%u]: reference voltage set to Vref1\n", i);
			}

The conditionals were reshuffled to make it shorter and easier to compare
(yes, there is a pair of unneeded parentheses for the sake of good looking
 code, a.k.a. readability).

Also note, the messages were missing trailing '\n'; I lowered REF --> ref
in them.

> +			break;
> +		case MCP48FEB02_EXTERNAL_VREF_UNBUFFERED:
> +		case MCP48FEB02_EXTERNAL_VREF_BUFFERED:
> +			if (data->phys_channels >= 4 && (i % 2) && !data->use_vref1) {
> +				dev_dbg(dev, "ch[%u]: was configured to use VREF1", i);
> +				dev_dbg(dev,
> +					"ch[%u]: reference voltage set to internal band gap", i);
> +				break;
> +			}
> +			if ((data->phys_channels < 4 || (data->phys_channels >= 4 && !(i % 2))) &&
> +			    !data->use_vref) {
> +				dev_dbg(dev, "ch[%u]: was configured to use VREF", i);
> +				dev_dbg(dev,
> +					"ch[%u]: reference voltage set to internal band gap", i);
> +				break;
> +			}
> +			break;

Ditto.

> +		}
> +
> +		pd_tmp = (pd_ch >> (2 * i)) & MCP48FEB02_DAC_CTRL_MASK;
> +		data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp;
> +		data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode);
> +	}
> +
> +	return 0;
> +}

...

> +static int mcp48feb02_probe(struct spi_device *spi)
> +{
> +	const struct mcp48feb02_features *chip_features;
> +	struct device *dev = &spi->dev;
> +	struct mcp48feb02_data *data;
> +	struct iio_dev *indio_dev;

> +	int vref1_uV = 0;
> +	int vref_uV = 0;

Please, split the assignments (the rationale was given somewhere above).

> +	int vdd_uV;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +
> +	chip_features = spi_get_device_match_data(spi);
> +	if (!chip_features)
> +		return -EINVAL;
> +
> +	data->chip_features = chip_features;
> +
> +	if (chip_features->have_eeprom) {
> +		data->regmap = devm_regmap_init_spi(spi, &mcp48feb02_regmap_config);
> +		indio_dev->info = &mcp48feb02_info;
> +	} else {
> +		data->regmap = devm_regmap_init_spi(spi, &mcp48fvb02_regmap_config);
> +		indio_dev->info = &mcp48fvb02_info;
> +	}
> +	if (IS_ERR(data->regmap))
> +		return dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing spi regmap\n");

SPI

> +	indio_dev->name = chip_features->name;
> +
> +	ret = mcp48feb02_parse_fw(indio_dev, chip_features);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Error parsing firmware data\n");
> +
> +	ret = devm_mutex_init(dev, &data->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "vdd");
> +	if (ret < 0)
> +		return ret;
> +
> +	vdd_uV = ret;
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
> +	if (ret > 0) {
> +		vref_uV = ret;
> +		data->use_vref = true;
> +	} else {
> +		dev_dbg(dev, "using internal band gap as voltage reference.\n");
> +		dev_dbg(dev, "External Vref is unavailable.\n");
> +	}
> +
> +	if (chip_features->have_ext_vref1) {
> +		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
> +		if (ret > 0) {
> +			vref1_uV = ret;
> +			data->use_vref1 = true;
> +		} else {
> +			dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
> +			dev_dbg(dev, "External Vref1 is unavailable.\n");
> +		}
> +	}
> +
> +	ret = mcp48feb02_init_ctrl_regs(data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Error initialising vref register\n");
> +
> +	ret = mcp48feb02_init_ch_scales(data, vdd_uV, vref_uV, vref1_uV);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}


-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Ariana.Lazar@microchip.com 1 month, 2 weeks ago
Hello Andy,

please see my comments below


> 
> > +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct
> > iio_chan_spec const *ch,
> > +                              const int **vals, int *type, int
> > *length, long info)
> > +{
> > +     struct mcp48feb02_data *data = iio_priv(indio_dev);
> > +
> > +     switch (info) {
> > +     case IIO_CHAN_INFO_SCALE:
> > +             switch (ch->type) {
> > +             case IIO_VOLTAGE:
> > +                     if (data->phys_channels >= 4 && (ch->address
> > % 2))
> > +                             *vals = data->scale_1;
> > +                     else
> > +                             *vals = data->scale;
> 
> Actually, if you put the scales as
> 
>         int scales[2][2 * MCP48FEB02_MAX_SCALES_CH];
> 
> this will become as simple as
> 
>                         if (data->phys_channels >= 4)
>                                 *vals = data->scales[ch->address];
>                         else
>                                 *vals = data->scales[0];
> 
> OTOH, I am not sure if it can be always as
> 
>                         *vals = data->scales[ch->address];
> 
> which would be the best approach.
> 
> 
> 

> 
I am not quite sure I have understood your point of view. In order to
remove the channel parity check, I would have to declare int
scales[MCP48FEB02_MAX_CH][2 * MCP48FEB02_MAX_SCALES_CH] (int
scales[8][6])
regardless of device's number of channels and number of voltage
references. This will be quite a lot unnecessary space allocated
compared to using only two arrays of [2 * MCP48FEB02_MAX_SCALES_CH].

Another way to avoid these checks is to use a dynamically allocated
array of scales. Each member points to an array of [2 *
MCP48FEB02_MAX_SCALES_CH]
and stores corresponding scale values for each channel, while allowing
to allocate the actual number of channels the device has rather than
the maximum.

Please tell me which version you prefer.

Best regards,
Ariana
Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Mon, Feb 16, 2026 at 02:29:19PM +0000, Ariana.Lazar@microchip.com wrote:

...

> > > +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct
> > > iio_chan_spec const *ch,
> > > +                              const int **vals, int *type, int
> > > *length, long info)
> > > +{
> > > +     struct mcp48feb02_data *data = iio_priv(indio_dev);
> > > +
> > > +     switch (info) {
> > > +     case IIO_CHAN_INFO_SCALE:
> > > +             switch (ch->type) {
> > > +             case IIO_VOLTAGE:
> > > +                     if (data->phys_channels >= 4 && (ch->address
> > > % 2))
> > > +                             *vals = data->scale_1;
> > > +                     else
> > > +                             *vals = data->scale;
> > 
> > Actually, if you put the scales as
> > 
> >         int scales[2][2 * MCP48FEB02_MAX_SCALES_CH];
> > 
> > this will become as simple as
> > 
> >                         if (data->phys_channels >= 4)
> >                                 *vals = data->scales[ch->address];
> >                         else
> >                                 *vals = data->scales[0];
> > 
> > OTOH, I am not sure if it can be always as
> > 
> >                         *vals = data->scales[ch->address];
> > 
> > which would be the best approach.
> 
> I am not quite sure I have understood your point of view. In order to
> remove the channel parity check, I would have to declare int
> scales[MCP48FEB02_MAX_CH][2 * MCP48FEB02_MAX_SCALES_CH] (int
> scales[8][6])
> regardless of device's number of channels and number of voltage
> references. This will be quite a lot unnecessary space allocated
> compared to using only two arrays of [2 * MCP48FEB02_MAX_SCALES_CH].
> 
> Another way to avoid these checks is to use a dynamically allocated
> array of scales. Each member points to an array of [2 *
> MCP48FEB02_MAX_SCALES_CH]
> and stores corresponding scale values for each channel, while allowing
> to allocate the actual number of channels the device has rather than
> the maximum.
> 
> Please tell me which version you prefer.

I think something like a second variant. But as it seems going to be an
agreement that this driver is not needed and rather we need to refactor
existing one to add the support for SPI chips, these comments won't make
much value.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Ariana.Lazar@microchip.com 1 month, 2 weeks ago
Hello Andy,


> I think something like a second variant. But as it seems going to be
> an
> agreement that this driver is not needed and rather we need to
> refactor
> existing one to add the support for SPI chips, these comments won't
> make
> much value.
> 
> --
Thank you fo the review. I think these comments are revelent because
that code is common for both drivers, the SPI and I2C.
I will refactor both drivers to be merged into a single one and in
order to avoid multiple code refactoring, if you agree, I will
implement those changes as well.

Best regards,
Ariana

> With Best Regards,
> Andy Shevchenko
> 

Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Tue, Feb 17, 2026 at 11:38:10AM +0000, Ariana.Lazar@microchip.com wrote:

> > I think something like a second variant. But as it seems going to be
> > an
> > agreement that this driver is not needed and rather we need to
> > refactor
> > existing one to add the support for SPI chips, these comments won't
> > make
> > much value.
> > 
> > --
> Thank you fo the review. I think these comments are revelent because
> that code is common for both drivers, the SPI and I2C.
> I will refactor both drivers to be merged into a single one and in
> order to avoid multiple code refactoring, if you agree, I will
> implement those changes as well.

Sure, if you have time for that. I'm not insisting...

-- 
With Best Regards,
Andy Shevchenko