[PATCH v5 0/3] iio: dac: mcp4821: add configurable gain support

[PATCH v5 0/3] iio: dac: mcp4821: add configurable gain support

[Nikhil Gautam]

This series updates the MCP4821 DAC driver to support configurable gain.

Patch 1 fixes a spelling issue in an enum name.
Patch 2 performs a small refactor to simplify state handling.
Patch 3 adds configurable gain support using the GA bit and
adds support for writing to the scale attribute to select the gain.

The scale attribute is used to control gain selection. Writing supported
scale values selects the corresponding gain setting. The implementation
supports all variants of the MCP48xx family (8, 10, and 12-bit devices).

Changes in v5:
- Fix commit message to remove incorrect "fix scale" wording
- Clarify that scale is used to control gain
- _Scale_available is only getting the supported scales
- Make scale write handling generic for all supported resolutions
- Address review comments from David Lechner

Changes in v4:
- Split changes into separate patches as suggested
- Ensure scale_available matches scale
- Handle sysfs write inputs correctly

Changes in v3:
- Restore NULL check in indio_dev allocation

Changes in v2:
- Use IIO_CHAN_INFO_SCALE instead of CALIBSCALE
- Fix error handling and cleanup

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
Nikhil Gautam (3):
  iio: dac: mcp4821: fix spelling mistake in enum name
  iio: dac: mcp4821: move state initialization outside switch
  iio: dac: mcp4821: add configurable gain support

 drivers/iio/dac/mcp4821.c | 137 +++++++++++++++++++++++++++++++-------
 1 file changed, 114 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH v5 1/3] iio: dac: mcp4821: fix spelling mistake in enum name

[Nikhil Gautam]

Fix a typo in the enum name mcp4821_supported_drvice_ids
by renaming it to mcp4821_supported_device_ids.

This improves code readability and consistency.

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 drivers/iio/dac/mcp4821.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 748bdca9a964..29187f2a9d3c 100644
--- a/drivers/iio/dac/mcp4821.c
+++ b/drivers/iio/dac/mcp4821.c
@@ -31,7 +31,7 @@
 /* DAC uses an internal Voltage reference of 4.096V at a gain of 2x */
 #define MCP4821_2X_GAIN_VREF_MV 4096
 
-enum mcp4821_supported_drvice_ids {
+enum mcp4821_supported_device_ids {
 	ID_MCP4801,
 	ID_MCP4802,
 	ID_MCP4811,
-- 
2.43.0

[PATCH v5 2/3] iio: dac: mcp4821: move state initialization outside switch

[Nikhil Gautam]

Move the iio_priv() call outside the switch statement in
mcp4821_read_raw() to avoid repeating it in multiple cases.

This is a cleanup change with no functional impact.

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 drivers/iio/dac/mcp4821.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 29187f2a9d3c..102ae3718514 100644
--- a/drivers/iio/dac/mcp4821.c
+++ b/drivers/iio/dac/mcp4821.c
@@ -115,11 +115,10 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan, int *val,
 			    int *val2, long mask)
 {
-	struct mcp4821_state *state;
+	struct mcp4821_state *state = iio_priv(indio_dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		state = iio_priv(indio_dev);
 		*val = state->dac_value[chan->channel];
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-- 
2.43.0

Re: [PATCH v5 2/3] iio: dac: mcp4821: move state initialization outside switch

[David Lechner]

On 4/14/26 4:22 AM, Nikhil Gautam wrote:
> Move the iio_priv() call outside the switch statement in
> mcp4821_read_raw() to avoid repeating it in multiple cases.
> 
> This is a cleanup change with no functional impact.

This is still not a "cleanup". It is preparing for the next patch.

> 
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
> ---
>  drivers/iio/dac/mcp4821.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
> index 29187f2a9d3c..102ae3718514 100644
> --- a/drivers/iio/dac/mcp4821.c
> +++ b/drivers/iio/dac/mcp4821.c
> @@ -115,11 +115,10 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan, int *val,
>  			    int *val2, long mask)
>  {
> -	struct mcp4821_state *state;
> +	struct mcp4821_state *state = iio_priv(indio_dev);
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		state = iio_priv(indio_dev);
>  		*val = state->dac_value[chan->channel];
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:

[PATCH v5 3/3] iio: dac: mcp4821: add configurable gain support

[Nikhil Gautam]

Add support for configuring the DAC gain using the GA bit and
update scale handling

The MCP4821 supports two gain settings:
- 1x gain → 2.048V full-scale
- 2x gain → 4.096V full-scale

Scale write support is added in the IIO interface. Only scale
values advertised via the scale_available attribute are accepted,
ensuring consistency between the configured gain and exposed
scale values.

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 drivers/iio/dac/mcp4821.c | 132 ++++++++++++++++++++++++++++++++------
 1 file changed, 112 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 102ae3718514..c30c320984ce 100644
--- a/drivers/iio/dac/mcp4821.c
+++ b/drivers/iio/dac/mcp4821.c
@@ -12,7 +12,6 @@
  *	MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
  *
  * TODO:
- *	- Configurable gain
  *	- Regulator control
  */
 
@@ -26,10 +25,30 @@
 #include <linux/unaligned.h>
 
 #define MCP4821_ACTIVE_MODE BIT(12)
+#define MCP4821_GAIN_ENABLE BIT(13)
 #define MCP4802_SECOND_CHAN BIT(15)
 
-/* DAC uses an internal Voltage reference of 4.096V at a gain of 2x */
-#define MCP4821_2X_GAIN_VREF_MV 4096
+/* DAC uses an internal Voltage reference of 2.048V */
+#define MCP4821_VREF_MV 2048
+
+/*
+ * MCP48xx DAC output:
+ *
+ * Vout = (Vref * D / 2^N) * G
+ *
+ * where:
+ *  - Vref = 2.048V (internal reference)
+ *  - N = DAC resolution (12 bits for MCP4821)
+ *  - G = gain selection:
+ *        1x when GA bit = 1
+ *        2x when GA bit = 0 (default)
+ *
+ * Therefore full-scale voltage is:
+ *  - 1x gain: 2.048V
+ *  - 2x gain: 4.096V
+ *
+ * Scale = Vfull-scale / 2^N
+ */
 
 enum mcp4821_supported_device_ids {
 	ID_MCP4801,
@@ -40,9 +59,16 @@ enum mcp4821_supported_device_ids {
 	ID_MCP4822,
 };
 
+enum gain_modes {
+	MCP4821_GAIN_X1 = 1,
+	MCP4821_GAIN_X2 = 2,
+};
+
 struct mcp4821_state {
 	struct spi_device *spi;
 	u16 dac_value[2];
+	int gain;
+	int avail_gain[4];
 };
 
 struct mcp4821_chip_info {
@@ -57,6 +83,7 @@ struct mcp4821_chip_info {
 		.channel = (channel_id),                              \
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),         \
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE), \
 		.scan_type = {                                        \
 			.realbits = (resolution),                     \
 			.shift = 12 - (resolution),                   \
@@ -121,8 +148,9 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		*val = state->dac_value[chan->channel];
 		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
-		*val = MCP4821_2X_GAIN_VREF_MV;
+		*val = MCP4821_VREF_MV * state->gain;
 		*val2 = chan->scan_type.realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
@@ -130,6 +158,17 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static void mcp4821_calc_scale(int vref_mv, int resolution,
+				int *val, int *val2)
+{
+	s64 tmp;
+	int micro;
+
+	tmp = (s64)vref_mv * 1000000LL >> resolution;
+	*val = div_s64_rem(tmp, 1000000LL, &micro);
+	*val2 = micro;
+}
+
 static int mcp4821_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan, int val,
 			     int val2, long mask)
@@ -138,35 +177,85 @@ static int mcp4821_write_raw(struct iio_dev *indio_dev,
 	u16 write_val;
 	__be16 write_buffer;
 	int ret;
+	int v, v2;
 
-	if (val2 != 0)
-		return -EINVAL;
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
 
-	if (val < 0 || val >= BIT(chan->scan_type.realbits))
-		return -EINVAL;
+		if (val2 != 0)
+			return -EINVAL;
 
-	if (mask != IIO_CHAN_INFO_RAW)
-		return -EINVAL;
+		if (val < 0 || val >= BIT(chan->scan_type.realbits))
+			return -EINVAL;
 
-	write_val = MCP4821_ACTIVE_MODE | val << chan->scan_type.shift;
-	if (chan->channel)
-		write_val |= MCP4802_SECOND_CHAN;
+		write_val = MCP4821_ACTIVE_MODE | val << chan->scan_type.shift;
+		if (chan->channel)
+			write_val |= MCP4802_SECOND_CHAN;
 
-	write_buffer = cpu_to_be16(write_val);
-	ret = spi_write(state->spi, &write_buffer, sizeof(write_buffer));
-	if (ret) {
-		dev_err(&state->spi->dev, "Failed to write to device: %d", ret);
-		return ret;
+		/* GA bit = 1 -> 1x gain */
+		if (state->gain == MCP4821_GAIN_X1)
+			write_val |= MCP4821_GAIN_ENABLE;
+
+		write_buffer = cpu_to_be16(write_val);
+		ret = spi_write(state->spi, &write_buffer, sizeof(write_buffer));
+		if (ret) {
+			dev_err(&state->spi->dev, "Failed to write to device: %d", ret);
+			return ret;
+		}
+
+		state->dac_value[chan->channel] = val;
+		return 0;
+
+	case IIO_CHAN_INFO_SCALE:
+		mcp4821_calc_scale(MCP4821_VREF_MV, chan->scan_type.realbits, &v, &v2);
+		if (val == v && val2 == v2) {
+			state->gain = MCP4821_GAIN_X1;
+			return 0;
+		}
+
+		mcp4821_calc_scale(MCP4821_VREF_MV * MCP4821_GAIN_X2,
+				chan->scan_type.realbits, &v, &v2);
+		if (val == v && val2 == v2) {
+			state->gain = MCP4821_GAIN_X2;
+			return 0;
+		}
+		return -EINVAL;
+	default:
+		return -EINVAL;
 	}
+}
 
-	state->dac_value[chan->channel] = val;
+static inline void mcp4821_init_avail_gain(struct mcp4821_state *state,
+				int resolution)
+{
+	state->avail_gain[0] = MCP4821_VREF_MV;
+	state->avail_gain[1] = resolution;
+	state->avail_gain[2] = MCP4821_VREF_MV * MCP4821_GAIN_X2;
+	state->avail_gain[3] = resolution;
+}
+
+static int mcp4821_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long info)
+{
+	struct mcp4821_state *state = iio_priv(indio_dev);
 
-	return 0;
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = state->avail_gain;
+		*type = IIO_VAL_FRACTIONAL_LOG2;
+		*length = ARRAY_SIZE(state->avail_gain);
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
 }
 
 static const struct iio_info mcp4821_info = {
 	.read_raw = &mcp4821_read_raw,
 	.write_raw = &mcp4821_write_raw,
+	.read_avail = &mcp4821_read_avail,
 };
 
 static int mcp4821_probe(struct spi_device *spi)
@@ -182,12 +271,15 @@ static int mcp4821_probe(struct spi_device *spi)
 	state = iio_priv(indio_dev);
 	state->spi = spi;
 
+	/* default gain is 2x as GA bit is active low*/
+	state->gain = MCP4821_GAIN_X2;
 	info = spi_get_device_match_data(spi);
 	indio_dev->name = info->name;
 	indio_dev->info = &mcp4821_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = info->channels;
 	indio_dev->num_channels = info->num_channels;
+	mcp4821_init_avail_gain(state, info->channels[0].scan_type.realbits);
 
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
-- 
2.43.0

Re: [PATCH v5 3/3] iio: dac: mcp4821: add configurable gain support

[David Lechner]

On 4/14/26 4:22 AM, Nikhil Gautam wrote:
> Add support for configuring the DAC gain using the GA bit and
> update scale handling
> 

...

> +enum gain_modes {
> +	MCP4821_GAIN_X1 = 1,
> +	MCP4821_GAIN_X2 = 2,

We try to avoid making enums that just map the number 1 to 1
and so on.

> +};
> +
>  struct mcp4821_state {
>  	struct spi_device *spi;
>  	u16 dac_value[2];
> +	int gain;
> +	int avail_gain[4];

Would make more sense to call this scale_avail.

>  };
>  
>  struct mcp4821_chip_info {
> @@ -57,6 +83,7 @@ struct mcp4821_chip_info {
>  		.channel = (channel_id),                              \
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),         \
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE), \
>  		.scan_type = {                                        \
>  			.realbits = (resolution),                     \
>  			.shift = 12 - (resolution),                   \
> @@ -121,8 +148,9 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_RAW:
>  		*val = state->dac_value[chan->channel];
>  		return IIO_VAL_INT;
> +
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = MCP4821_2X_GAIN_VREF_MV;
> +		*val = MCP4821_VREF_MV * state->gain;
>  		*val2 = chan->scan_type.realbits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
> @@ -130,6 +158,17 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static void mcp4821_calc_scale(int vref_mv, int resolution,
> +				int *val, int *val2)
> +{
> +	s64 tmp;
> +	int micro;
> +
> +	tmp = (s64)vref_mv * 1000000LL >> resolution;

Use MICRO instead of writing lots of 0s.

> +	*val = div_s64_rem(tmp, 1000000LL, &micro);
> +	*val2 = micro;

Technically, IIO_VAL_FRACTIONAL_LOG2 can print up to 9 digits after
the decimal, so this would be safer to do nano instead of micro.

For that, we need to also implement a write_raw_get_fmt() callback.

> +}
> +
>  static int mcp4821_write_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan, int val,
>  			     int val2, long mask)

...

> @@ -182,12 +271,15 @@ static int mcp4821_probe(struct spi_device *spi)
>  	state = iio_priv(indio_dev);
>  	state->spi = spi;
>  
> +	/* default gain is 2x as GA bit is active low*/

Needs space before ending */

Saying "active low" is a bit odd. I think the main point of selecting
this value is so we don't break userspace that may already be depending
on it being the default.

> +	state->gain = MCP4821_GAIN_X2;
>  	info = spi_get_device_match_data(spi);
>  	indio_dev->name = info->name;
>  	indio_dev->info = &mcp4821_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = info->channels;
>  	indio_dev->num_channels = info->num_channels;
> +	mcp4821_init_avail_gain(state, info->channels[0].scan_type.realbits);
>  
>  	return devm_iio_device_register(&spi->dev, indio_dev);
>  }

Re: [PATCH v5 3/3] iio: dac: mcp4821: add configurable gain support

[Nikhil Gautam]

On Tue, Apr 14, 2026 at 08:57:35AM -0500, David Lechner wrote:
> On 4/14/26 4:22 AM, Nikhil Gautam wrote:
> 
> > +	*val = div_s64_rem(tmp, 1000000LL, &micro);
> > +	*val2 = micro;
> 
> Technically, IIO_VAL_FRACTIONAL_LOG2 can print up to 9 digits after
> the decimal, so this would be safer to do nano instead of micro.
> 
> For that, we need to also implement a write_raw_get_fmt() callback.
>

Thanks for the suggestion.

Currently the driver uses IIO_VAL_FRACTIONAL_LOG2 for the scale
attribute this format is sufficient for all supported chips
(8, 10 and 12-bit), the resulting scale values
are exact and can be represented without loss of precision.

The write path validates the scale values based on exact matches of the
expected values, so additional precision (e.g. nano) is not required.

Using INT_PLUS_NANO would also require changing the existing format and
implementing write_raw_get_fmt(), which does not seem necessary here.

So keeping IIO_VAL_FRACTIONAL_LOG2 for both read and write paths.