public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: dac: mcp4821: add configurable gain support
@ 2026-03-19 16:52 Nikhil Gautam
  2026-03-21 18:07 ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Nikhil Gautam @ 2026-03-19 16:52 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, dlechner, anshulusr, Nikhil Gautam

Add support for configuring the DAC gain using the GA bit.
Expose gain control via IIO_CHAN_INFO_CALIBSCALE.

Scale is updated dynamically based on selected gain:
- 1x gain → 2.048V full-scale
- 2x gain → 4.096V full-scale

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

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 748bdca9a964..f9f97917bc9c 100644
--- a/drivers/iio/dac/mcp4821.c
+++ b/drivers/iio/dac/mcp4821.c
@@ -12,26 +12,43 @@
  *	MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
  *
  * TODO:
- *	- Configurable gain
  *	- Regulator control
  */
 
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/types.h>
-
 #include <linux/unaligned.h>
 
 #define MCP4821_ACTIVE_MODE BIT(12)
 #define MCP4802_SECOND_CHAN BIT(15)
+#define MCP4821_GAIN_ENABLE BIT(13)
 
-/* 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
 
-enum mcp4821_supported_drvice_ids {
+/*
+ * 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,
 	ID_MCP4802,
 	ID_MCP4811,
@@ -43,6 +60,7 @@ enum mcp4821_supported_drvice_ids {
 struct mcp4821_state {
 	struct spi_device *spi;
 	u16 dac_value[2];
+	bool gain_1x;
 };
 
 struct mcp4821_chip_info {
@@ -51,16 +69,19 @@ struct mcp4821_chip_info {
 	const struct iio_chan_spec channels[2];
 };
 
-#define MCP4821_CHAN(channel_id, resolution)                          \
-	{                                                             \
-		.type = IIO_VOLTAGE, .output = 1, .indexed = 1,       \
-		.channel = (channel_id),                              \
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),         \
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
-		.scan_type = {                                        \
-			.realbits = (resolution),                     \
-			.shift = 12 - (resolution),                   \
-		},                                                    \
+#define MCP4821_CHAN(channel_id, resolution)					\
+	{															\
+		.type = IIO_VOLTAGE, .output = 1, .indexed = 1,			\
+		.channel = (channel_id),								\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+			BIT(IIO_CHAN_INFO_CALIBSCALE),						\
+	    .info_mask_shared_by_type_available =					\
+			BIT(IIO_CHAN_INFO_CALIBSCALE),						\
+		.scan_type = {											\
+			.realbits = (resolution),							\
+			.shift = 12 - (resolution),							\
+		},														\
 	}
 
 static const struct mcp4821_chip_info mcp4821_chip_info_table[6] = {
@@ -122,13 +143,25 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 		state = iio_priv(indio_dev);
 		*val = state->dac_value[chan->channel];
 		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
-		*val = MCP4821_2X_GAIN_VREF_MV;
+		state = iio_priv(indio_dev);
+		if (state->gain_1x)
+			*val = MCP4821_VREF_MV;
+		else
+			*val = MCP4821_VREF_MV * 2;
 		*val2 = chan->scan_type.realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
+
+	case IIO_CHAN_INFO_CALIBSCALE:
+		state = iio_priv(indio_dev);
+		*val = state->gain_1x ? 1 : 2;
+		return IIO_VAL_INT;
+
 	default:
-		return -EINVAL;
+		break;
 	}
+	return -EINVAL;
 }
 
 static int mcp4821_write_raw(struct iio_dev *indio_dev,
@@ -140,34 +173,68 @@ static int mcp4821_write_raw(struct iio_dev *indio_dev,
 	__be16 write_buffer;
 	int ret;
 
-	if (val2 != 0)
-		return -EINVAL;
+	switch (mask) {
 
-	if (val < 0 || val >= BIT(chan->scan_type.realbits))
-		return -EINVAL;
+	case IIO_CHAN_INFO_RAW:
 
-	if (mask != IIO_CHAN_INFO_RAW)
-		return -EINVAL;
+		if (val2 != 0)
+			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;
+		if (state->gain_1x)
+			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_CALIBSCALE:
+		if (val == 1)
+			state->gain_1x = true;
+		else if (val == 2)
+			state->gain_1x = false;
+		else
+			return -EINVAL;
+		return 0;
+	default:
+		break;
 	}
+	return -EINVAL;
+}
 
-	state->dac_value[chan->channel] = val;
+static const int mcp4821_gain_avail[] = {1, 2};
 
-	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_CALIBSCALE:
+		*vals = mcp4821_gain_avail;
+		*type = IIO_VAL_INT;
+		*length = 2;
+		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,6 +249,7 @@ static int mcp4821_probe(struct spi_device *spi)
 
 	state = iio_priv(indio_dev);
 	state->spi = spi;
+	state->gain_1x = false; /* default 2x */
 
 	info = spi_get_device_match_data(spi);
 	indio_dev->name = info->name;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: dac: mcp4821: add configurable gain support
  2026-03-19 16:52 [PATCH] iio: dac: mcp4821: add configurable gain support Nikhil Gautam
@ 2026-03-21 18:07 ` Jonathan Cameron
  2026-03-25  6:51   ` Nikhil Gautam
  2026-03-26  7:12   ` [PATCH v2] " Nikhil Gautam
  0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-03-21 18:07 UTC (permalink / raw)
  To: Nikhil Gautam; +Cc: linux-iio, dlechner, anshulusr

On Thu, 19 Mar 2026 22:22:16 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:

> Add support for configuring the DAC gain using the GA bit.
> Expose gain control via IIO_CHAN_INFO_CALIBSCALE.
> 
> Scale is updated dynamically based on selected gain:
> - 1x gain → 2.048V full-scale
> - 2x gain → 4.096V full-scale
> 
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
> ---
>  drivers/iio/dac/mcp4821.c | 140 ++++++++++++++++++++++++++++----------
>  1 file changed, 104 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
> index 748bdca9a964..f9f97917bc9c 100644
> --- a/drivers/iio/dac/mcp4821.c
> +++ b/drivers/iio/dac/mcp4821.c
> @@ -12,26 +12,43 @@
>   *	MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
>   *
>   * TODO:
> - *	- Configurable gain
>   *	- Regulator control
>   */
>  
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/spi/spi.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/types.h>
> -
Hi Nikhil,

Don't do code recorganization in a patch doing anything else.
Also, it's fairly common convention to have subsystem specific headers
in a block at the end for a driver in that subsystem.
>  #include <linux/unaligned.h>
This is the oddity.  Was a result of a mass change from
asm/unaligned.h to this that didn't include reordering headers.

Given you are touching the driver anyway, it's fine to move that up to
under spi.h, but should still be a seperate patch.


>  
>  #define MCP4821_ACTIVE_MODE BIT(12)
>  #define MCP4802_SECOND_CHAN BIT(15)
> +#define MCP4821_GAIN_ENABLE BIT(13)

Put this in order. So above MCP4802_SECOND_CHAN
>  
> -/* 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
>  
> -enum mcp4821_supported_drvice_ids {
> +/*
> + * 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,
>  	ID_MCP4802,
>  	ID_MCP4811,
> @@ -43,6 +60,7 @@ enum mcp4821_supported_drvice_ids {
>  struct mcp4821_state {
>  	struct spi_device *spi;
>  	u16 dac_value[2];
> +	bool gain_1x;
>  };
>  
>  struct mcp4821_chip_info {
> @@ -51,16 +69,19 @@ struct mcp4821_chip_info {
>  	const struct iio_chan_spec channels[2];
>  };
>  
> -#define MCP4821_CHAN(channel_id, resolution)                          \
> -	{                                                             \
> -		.type = IIO_VOLTAGE, .output = 1, .indexed = 1,       \
> -		.channel = (channel_id),                              \
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),         \
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> -		.scan_type = {                                        \
> -			.realbits = (resolution),                     \
> -			.shift = 12 - (resolution),                   \
> -		},                                                    \
> +#define MCP4821_CHAN(channel_id, resolution)					\
> +	{															\
> +		.type = IIO_VOLTAGE, .output = 1, .indexed = 1,			\
> +		.channel = (channel_id),								\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +			BIT(IIO_CHAN_INFO_CALIBSCALE),						\
> +	    .info_mask_shared_by_type_available =					\
> +			BIT(IIO_CHAN_INFO_CALIBSCALE),						\
> +		.scan_type = {											\
> +			.realbits = (resolution),							\
> +			.shift = 12 - (resolution),							\
> +		},														\
>  	}
No idea what went wrong formatting wise here, but that needs to go back to normal!

Why is it calibscale as opposed to scale?  A x2 multiplier presumably affects the
scale userspace should apply? calibbscale is for adjusting due to minor device differences
not this sort of major range adjustment.

>  
>  static const struct mcp4821_chip_info mcp4821_chip_info_table[6] = {
> @@ -122,13 +143,25 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
>  		state = iio_priv(indio_dev);
>  		*val = state->dac_value[chan->channel];
>  		return IIO_VAL_INT;
> +
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = MCP4821_2X_GAIN_VREF_MV;
> +		state = iio_priv(indio_dev);
> +		if (state->gain_1x)
> +			*val = MCP4821_VREF_MV;
> +		else
> +			*val = MCP4821_VREF_MV * 2;
>  		*val2 = chan->scan_type.realbits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		state = iio_priv(indio_dev);
> +		*val = state->gain_1x ? 1 : 2;
> +		return IIO_VAL_INT;
> +
>  	default:
> -		return -EINVAL;
> +		break;
>  	}
> +	return -EINVAL;

Why?  Original code was at least as good if not better.

>  }
>  
>  static int mcp4821_write_raw(struct iio_dev *indio_dev,
> @@ -140,34 +173,68 @@ static int mcp4821_write_raw(struct iio_dev *indio_dev,
>  	__be16 write_buffer;
>  	int ret;
>  
> -	if (val2 != 0)
> -		return -EINVAL;
> +	switch (mask) {
>  
> -	if (val < 0 || val >= BIT(chan->scan_type.realbits))
> -		return -EINVAL;
> +	case IIO_CHAN_INFO_RAW:
>  
> -	if (mask != IIO_CHAN_INFO_RAW)
> -		return -EINVAL;
> +		if (val2 != 0)
> +			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;
> +		if (state->gain_1x)
> +			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_CALIBSCALE:
> +		if (val == 1)
> +			state->gain_1x = true;
> +		else if (val == 2)
> +			state->gain_1x = false;
> +		else
> +			return -EINVAL;
> +		return 0;
> +	default:
> +		break;
>  	}
> +	return -EINVAL;
Move that up into the default. 

> +}

Thanks,

Jonathan


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: dac: mcp4821: add configurable gain support
  2026-03-21 18:07 ` Jonathan Cameron
@ 2026-03-25  6:51   ` Nikhil Gautam
  2026-03-26  7:12   ` [PATCH v2] " Nikhil Gautam
  1 sibling, 0 replies; 7+ messages in thread
From: Nikhil Gautam @ 2026-03-25  6:51 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, dlechner, anshulusr

On Sat, Mar 21, 2026 at 06:07:07PM +0000, Jonathan Cameron wrote:
> On Thu, 19 Mar 2026 22:22:16 +0530
> Nikhil Gautam <nikhilgtr@gmail.com> wrote:
> 

Hi Jonathan,

Thank you for the detailed review, that’s very helpful.

> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/types.h>
> >  #include <linux/module.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/spi/spi.h>
> > -
> > -#include <linux/iio/iio.h>
> > -#include <linux/iio/types.h>
> > -
> Hi Nikhil,
> 
> Don't do code recorganization in a patch doing anything else.
> Also, it's fairly common convention to have subsystem specific headers
> in a block at the end for a driver in that subsystem.
> >  #include <linux/unaligned.h>
> This is the oddity.  Was a result of a mass change from
> asm/unaligned.h to this that didn't include reordering headers.
> 
> Given you are touching the driver anyway, it's fine to move that up to
> under spi.h, but should still be a seperate patch.
> 
Understood. I agree this wasn’t appropriate to mix with functional changes.
I’ll drop the header reordering from this patch, and if needed,
will send a separate cleanup patch that moves linux/unaligned.h to a more sensible location.

> 
> >  
> >  #define MCP4821_ACTIVE_MODE BIT(12)
> >  #define MCP4802_SECOND_CHAN BIT(15)
> > +#define MCP4821_GAIN_ENABLE BIT(13)
> 
> Put this in order. So above MCP4802_SECOND_CHAN
> >  
Agreed. I’ll reorder the defines so the gain bit is placed logically alongside the other command bits,
before MCP4802_SECOND_CHAN. 

> > -#define MCP4821_CHAN(channel_id, resolution)                          \
> > -	{                                                             \
> > -		.type = IIO_VOLTAGE, .output = 1, .indexed = 1,       \
> > -		.channel = (channel_id),                              \
> > -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),         \
> > -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > -		.scan_type = {                                        \
> > -			.realbits = (resolution),                     \
> > -			.shift = 12 - (resolution),                   \
> > -		},                                                    \
> > +#define MCP4821_CHAN(channel_id, resolution)					\
> > +	{															\
> > +		.type = IIO_VOLTAGE, .output = 1, .indexed = 1,			\
> > +		.channel = (channel_id),								\
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> > +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> > +			BIT(IIO_CHAN_INFO_CALIBSCALE),						\
> > +	    .info_mask_shared_by_type_available =					\
> > +			BIT(IIO_CHAN_INFO_CALIBSCALE),						\
> > +		.scan_type = {											\
> > +			.realbits = (resolution),							\
> > +			.shift = 12 - (resolution),							\
> > +		},														\
> >  	}
> No idea what went wrong formatting wise here, but that needs to go back to normal!
>
Yes, that was accidental and slipped through during editing. I’ll restore the original formatting style

> Why is it calibscale as opposed to scale?  A x2 multiplier presumably affects the
> scale userspace should apply? calibbscale is for adjusting due to minor device differences
> not this sort of major range adjustment.
>
Thanks for the clarification. You’re right exposing the gain selection via calibscale was incorrect.
The GA bit changes the voltage‑per‑LSB and therefore the scale userspace should apply.
I will switch this to use selectable IIO_CHAN_INFO_SCALE values with scale_available, and remove all use of calibscale.

> > +
> >  	default:
> > -		return -EINVAL;
> > +		break;
> >  	}
> > +	return -EINVAL;
> 
> Why?  Original code was at least as good if not better.
>
Agreed. The original structure was clearer.
I’ll move the EINVAL returns back into the default case and avoid the extra break.

> >  }
> >  
> > +	default:
> > +		break;
> >  	}
> > +	return -EINVAL;
> Move that up into the default. 
> 
Sure, will move that up.

I’ll post a revised version addressing all of the above.

Thanks again for the review.

Best regards,
Nikhil

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] iio: dac: mcp4821: add configurable gain support
  2026-03-21 18:07 ` Jonathan Cameron
  2026-03-25  6:51   ` Nikhil Gautam
@ 2026-03-26  7:12   ` Nikhil Gautam
  2026-03-26 20:38     ` Jonathan Cameron
  1 sibling, 1 reply; 7+ messages in thread
From: Nikhil Gautam @ 2026-03-26  7:12 UTC (permalink / raw)
  To: jic23; +Cc: anshulusr, dlechner, linux-iio, Nikhil Gautam

Add support for configuring the DAC gain using the GA bit.
Expose gain control via IIO_CHAN_INFO_CALIBSCALE.

Scale is updated dynamically based on selected gain:
- 1x gain → 2.048V full-scale
- 2x gain → 4.096V full-scale

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
Changes in v2:
- Drop header reordering
- Move MCP4821_GAIN_ENABLE define before MCP4802_SECOND_CHAN
- Use IIO_CHAN_INFO_SCALE instead of CALIBSCALE since full scale changes
- Restore original -EINVAL handling in default cases
---
 drivers/iio/dac/mcp4821.c | 105 ++++++++++++++++++++++++++++++--------
 1 file changed, 83 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 748bdca9a964..362ca12988dc 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,12 +25,32 @@
 #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
 
-enum mcp4821_supported_drvice_ids {
+/*
+ * 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,
 	ID_MCP4802,
 	ID_MCP4811,
@@ -43,6 +62,7 @@ enum mcp4821_supported_drvice_ids {
 struct mcp4821_state {
 	struct spi_device *spi;
 	u16 dac_value[2];
+	bool gain_1x;
 };
 
 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,13 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 		state = iio_priv(indio_dev);
 		*val = state->dac_value[chan->channel];
 		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
-		*val = MCP4821_2X_GAIN_VREF_MV;
+		state = iio_priv(indio_dev);
+		if (state->gain_1x)
+			*val = MCP4821_VREF_MV;
+		else
+			*val = MCP4821_VREF_MV * 2;
 		*val2 = chan->scan_type.realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
@@ -140,34 +166,66 @@ 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;
+		if (state->gain_1x)
+			write_val |= MCP4821_GAIN_ENABLE;
 
-	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;
+		}
 
-	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 == 1)
+			state->gain_1x = true;
+		else if (val == 2)
+			state->gain_1x = false;
+		else
+			return -EINVAL;
+		return 0;
+	default:
+		return -EINVAL;
 	}
+}
 
-	state->dac_value[chan->channel] = val;
+static const int mcp4821_gain_avail[] = {1, 2};
 
-	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_INT;
+		*length = 2;
+		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)
@@ -177,12 +235,15 @@ static int mcp4821_probe(struct spi_device *spi)
 	const struct mcp4821_chip_info *info;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
-	if (indio_dev == NULL)
+	if (!indio_dev)
 		return -ENOMEM;
 
 	state = iio_priv(indio_dev);
 	state->spi = spi;
 
+	/* default gain is 2x as GA bit is active low*/
+	state->gain_1x = false;
+
 	info = spi_get_device_match_data(spi);
 	indio_dev->name = info->name;
 	indio_dev->info = &mcp4821_info;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio: dac: mcp4821: add configurable gain support
  2026-03-26  7:12   ` [PATCH v2] " Nikhil Gautam
@ 2026-03-26 20:38     ` Jonathan Cameron
  2026-03-27  5:55       ` Nikhil Gautam
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-03-26 20:38 UTC (permalink / raw)
  To: Nikhil Gautam; +Cc: anshulusr, dlechner, linux-iio

On Thu, 26 Mar 2026 12:42:42 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:

> Add support for configuring the DAC gain using the GA bit.
> Expose gain control via IIO_CHAN_INFO_CALIBSCALE.
> 
> Scale is updated dynamically based on selected gain:
> - 1x gain → 2.048V full-scale
> - 2x gain → 4.096V full-scale
> 
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
Just one small thing from me.

>  static int mcp4821_probe(struct spi_device *spi)
> @@ -177,12 +235,15 @@ static int mcp4821_probe(struct spi_device *spi)
>  	const struct mcp4821_chip_info *info;
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> -	if (indio_dev == NULL)
> +	if (!indio_dev)

Not related to rest of the patch. So doesn't belong in here.
Also generally not worth the noise of changing these.

>  		return -ENOMEM;
>  
>  	state = iio_priv(indio_dev);
>  	state->spi = spi;
>  
> +	/* default gain is 2x as GA bit is active low*/
> +	state->gain_1x = false;
> +
>  	info = spi_get_device_match_data(spi);
>  	indio_dev->name = info->name;
>  	indio_dev->info = &mcp4821_info;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio: dac: mcp4821: add configurable gain support
  2026-03-26 20:38     ` Jonathan Cameron
@ 2026-03-27  5:55       ` Nikhil Gautam
  2026-03-27  6:18         ` [PATCH v3] " Nikhil Gautam
  0 siblings, 1 reply; 7+ messages in thread
From: Nikhil Gautam @ 2026-03-27  5:55 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: anshulusr, dlechner, linux-iio

On Thu, Mar 26, 2026 at 08:38:24PM +0000, Jonathan Cameron wrote:
> On Thu, 26 Mar 2026 12:42:42 +0530
> Nikhil Gautam <nikhilgtr@gmail.com> wrote:
> 
> > Add support for configuring the DAC gain using the GA bit.
> > Expose gain control via IIO_CHAN_INFO_CALIBSCALE.
> > 
> > Scale is updated dynamically based on selected gain:
> > - 1x gain → 2.048V full-scale
> > - 2x gain → 4.096V full-scale
> > 
> > Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
> Just one small thing from me.
> 
> >  static int mcp4821_probe(struct spi_device *spi)
> > @@ -177,12 +235,15 @@ static int mcp4821_probe(struct spi_device *spi)
> >  	const struct mcp4821_chip_info *info;
> >  
> >  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> > -	if (indio_dev == NULL)
> > +	if (!indio_dev)
> 
> Not related to rest of the patch. So doesn't belong in here.
> Also generally not worth the noise of changing these.

Sure will remove this change in v3,
will avoid these kind of changes in my future patches
> 
> >  		return -ENOMEM;
> >  
> >  	state = iio_priv(indio_dev);
> >  	state->spi = spi;
> >  
> > +	/* default gain is 2x as GA bit is active low*/
> > +	state->gain_1x = false;
> > +
> >  	info = spi_get_device_match_data(spi);
> >  	indio_dev->name = info->name;
> >  	indio_dev->info = &mcp4821_info;
> 

Thanks,
Nikhil

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3] iio: dac: mcp4821: add configurable gain support
  2026-03-27  5:55       ` Nikhil Gautam
@ 2026-03-27  6:18         ` Nikhil Gautam
  0 siblings, 0 replies; 7+ messages in thread
From: Nikhil Gautam @ 2026-03-27  6:18 UTC (permalink / raw)
  To: jic23; +Cc: anshulusr, dlechner, linux-iio, Nikhil Gautam

Add support for configuring the DAC gain using the GA bit.
Expose gain control via IIO_CHAN_INFO_CALIBSCALE.

Scale is updated dynamically based on selected gain:
- 1x gain → 2.048V full-scale
- 2x gain → 4.096V full-scale

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
Changes in v2:
- Drop header reordering
- Move MCP4821_GAIN_ENABLE define before MCP4802_SECOND_CHAN
- Use IIO_CHAN_INFO_SCALE instead of CALIBSCALE since full scale changes
- Restore original -EINVAL handling in default cases

Changes in v3:
- Restored NULL check in indio_dev alloc
---
 drivers/iio/dac/mcp4821.c | 103 ++++++++++++++++++++++++++++++--------
 1 file changed, 82 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/dac/mcp4821.c b/drivers/iio/dac/mcp4821.c
index 748bdca9a964..9160812219f9 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,12 +25,32 @@
 #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
 
-enum mcp4821_supported_drvice_ids {
+/*
+ * 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,
 	ID_MCP4802,
 	ID_MCP4811,
@@ -43,6 +62,7 @@ enum mcp4821_supported_drvice_ids {
 struct mcp4821_state {
 	struct spi_device *spi;
 	u16 dac_value[2];
+	bool gain_1x;
 };
 
 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,13 @@ static int mcp4821_read_raw(struct iio_dev *indio_dev,
 		state = iio_priv(indio_dev);
 		*val = state->dac_value[chan->channel];
 		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
-		*val = MCP4821_2X_GAIN_VREF_MV;
+		state = iio_priv(indio_dev);
+		if (state->gain_1x)
+			*val = MCP4821_VREF_MV;
+		else
+			*val = MCP4821_VREF_MV * 2;
 		*val2 = chan->scan_type.realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
@@ -140,34 +166,66 @@ 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;
+		if (state->gain_1x)
+			write_val |= MCP4821_GAIN_ENABLE;
 
-	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;
+		}
 
-	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 == 1)
+			state->gain_1x = true;
+		else if (val == 2)
+			state->gain_1x = false;
+		else
+			return -EINVAL;
+		return 0;
+	default:
+		return -EINVAL;
 	}
+}
 
-	state->dac_value[chan->channel] = val;
+static const int mcp4821_gain_avail[] = {1, 2};
 
-	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_INT;
+		*length = 2;
+		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 +241,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_1x = false;
+
 	info = spi_get_device_match_data(spi);
 	indio_dev->name = info->name;
 	indio_dev->info = &mcp4821_info;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-03-27  6:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-19 16:52 [PATCH] iio: dac: mcp4821: add configurable gain support Nikhil Gautam
2026-03-21 18:07 ` Jonathan Cameron
2026-03-25  6:51   ` Nikhil Gautam
2026-03-26  7:12   ` [PATCH v2] " Nikhil Gautam
2026-03-26 20:38     ` Jonathan Cameron
2026-03-27  5:55       ` Nikhil Gautam
2026-03-27  6:18         ` [PATCH v3] " Nikhil Gautam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox