[PATCH v4 0/3] iio: dac: mcp4821: add gain support and fix scale handling

[PATCH v4 0/3] iio: dac: mcp4821: add gain support and fix scale handling

[Nikhil Gautam]

This series updates the MCP4821 DAC driver to support configurable gain
and ensures scale handling follows the IIO ABI.

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 and corrects scale handling.

The scale is exposed via IIO_CHAN_INFO_SCALE and reflects the selected
gain. The implementation uses IIO_VAL_FRACTIONAL_LOG2 and ensures that
scale_available matches scale. Writes to the scale attribute are validated
and only supported values are accepted.

Changes in v4:
- Split changes into separate patches as suggested
- Fix scale handling to comply with IIO ABI
- Ensure scale_available matches scale
- Handle sysfs write inputs correctly
- Reject invalid scale values

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

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 and fix scale handling

 drivers/iio/dac/mcp4821.c | 107 ++++++++++++++++++++++++++++++--------
 1 file changed, 85 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH v4 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 v4 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 29187f2a9d3c..6b732db1cf49 100644
--- a/drivers/iio/dac/mcp4821.c
+++ b/drivers/iio/dac/mcp4821.c
@@ -116,10 +116,10 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 			    int *val2, long mask)
 {
 	struct mcp4821_state *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 v4 2/3] iio: dac: mcp4821: move state initialization outside switch

[David Lechner]

On 4/13/26 4:44 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.

It isn't a cleanup, it is preparing for the next patch.

> 
> 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 29187f2a9d3c..6b732db1cf49 100644
> --- a/drivers/iio/dac/mcp4821.c
> +++ b/drivers/iio/dac/mcp4821.c
> @@ -116,10 +116,10 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
>  			    int *val2, long mask)
>  {
>  	struct mcp4821_state *state;
> +	state = iio_priv(indio_dev);

This should just be one line.

  	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 v4 3/3] iio: dac: mcp4821: add configurable gain and fix scale handling

[Nikhil Gautam]

Add support for configuring the DAC gain using the GA bit and
update scale handling to follow the IIO ABI.

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

Scale is now exposed via IIO_CHAN_INFO_SCALE and reflects the
selected gain. The driver returns scale using
IIO_VAL_FRACTIONAL_LOG2 and provides scale_available with the
corresponding values.

Writes to the scale attribute are validated and mapped to the
appropriate gain setting. Only supported scale values are accepted,
ensuring consistency between scale and scale_available.

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
v4:
- Split changes into separate patches
- Fix scale handling to comply with IIO ABI
- Use IIO_VAL_FRACTIONAL_LOG2 for scale and scale_available
- Ensure scale_available matches scale
- Handle sysfs write inputs correctly (INT+MICRO)
- Reject invalid scale values

v3:
- Restore NULL check in indio_dev allocation

v2:
- Use IIO_CHAN_INFO_SCALE instead of CALIBSCALE
- Fix error handling and cleanup
---
 drivers/iio/dac/mcp4821.c | 103 ++++++++++++++++++++++++++++++--------
 1 file changed, 83 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 6b732db1cf49..db659b5be8f8 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,
@@ -43,6 +62,7 @@ enum mcp4821_supported_device_ids {
 struct mcp4821_state {
 	struct spi_device *spi;
 	u16 dac_value[2];
+	int gain;
 };
 
 struct mcp4821_chip_info {
@@ -57,6 +77,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),                   \
@@ -122,8 +143,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:
@@ -140,34 +162,72 @@ static int mcp4821_write_raw(struct iio_dev *indio_dev,
 	__be16 write_buffer;
 	int ret;
 
-	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;
+		/* GA bit = 1 -> 1x gain */
+		if (state->gain == 1)
+			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;
+		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:
+
+		if (val == 0 && val2 == 500000)
+			state->gain = 1;
+		else if (val == 1 && val2 == 0)
+			state->gain = 2;
+		else
+			return -EINVAL;
+		return 0;
+
+	default:
+		return -EINVAL;
 	}
+}
 
-	state->dac_value[chan->channel] = val;
+static const int mcp4821_gain_avail[] = {
+	MCP4821_VREF_MV, 12,
+	MCP4821_VREF_MV * 2, 12,};
 
-	return 0;
+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)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = mcp4821_gain_avail;
+		*type = IIO_VAL_FRACTIONAL_LOG2;
+		*length = ARRAY_SIZE(mcp4821_gain_avail);
+		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)
@@ -183,6 +243,9 @@ 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 = 2;
+
 	info = spi_get_device_match_data(spi);
 	indio_dev->name = info->name;
 	indio_dev->info = &mcp4821_info;
-- 
2.43.0

Re: [PATCH v4 3/3] iio: dac: mcp4821: add configurable gain and fix scale handling

[David Lechner]

Subject sounds like two changes in one patch.

If we really are fixing the existing scale handling, it should be
a separate patch. Or if we are not, the subject line shouldn't 
say that.

On 4/13/26 4:44 AM, Nikhil Gautam wrote:
> Add support for configuring the DAC gain using the GA bit and
> update scale handling to follow the IIO ABI.
> 
> The MCP4821 supports two gain settings:
> - 1x gain → 2.048V full-scale
> - 2x gain → 4.096V full-scale
> 
> Scale is now exposed via IIO_CHAN_INFO_SCALE and reflects the

The driver already implemented the scale attribute, so this doesn't
sound right. Maybe you mean scale_available?

> selected gain. The driver returns scale using
> IIO_VAL_FRACTIONAL_LOG2 and provides scale_available with the
> corresponding values.

It already did this.

> 
> Writes to the scale attribute are validated and mapped to the

Writing wasn't supported before, so would be more clear to say
that we added support for writing to the scale attribute.

> appropriate gain setting. Only supported scale values are accepted,
> ensuring consistency between scale and scale_available.
> 
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
> ---

Code looks mostly good now, just one thing...

> @@ -140,34 +162,72 @@ static int mcp4821_write_raw(struct iio_dev *indio_dev,
>  	__be16 write_buffer;
>  	int ret;
>  
> -	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;
> +		/* GA bit = 1 -> 1x gain */
> +		if (state->gain == 1)
> +			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;
> +		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:
> +
> +		if (val == 0 && val2 == 500000)

This only works when realbits == 12. We need to make this more
general so it works for all supported chips in this driver. There
are also 8 and 10-bit chips.

> +			state->gain = 1;
> +		else if (val == 1 && val2 == 0)
> +			state->gain = 2;
> +		else
> +			return -EINVAL;
> +		return 0;
> +
> +	default:
> +		return -EINVAL;
>  	}
> +}
>