Linux IIO development
 help / color / mirror / Atom feed
From: "Nuno Sá" <noname.nuno@gmail.com>
To: Kim Seer Paller <kimseer.paller@analog.com>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux@analog.com, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 3/4] iio: dac: ad3530r: parameterize DAC resolution
Date: Tue, 28 Jul 2026 17:40:01 +0100	[thread overview]
Message-ID: <amjbOIMuS3MFpesz@nsa> (raw)
In-Reply-To: <20260721-iio-ad5710r-upstream-v2-3-324949dc72da@analog.com>

On Tue, Jul 21, 2026 at 04:47:12PM +0800, Kim Seer Paller wrote:
> Add a per-chip resolution field and use it in the raw read/write and
> scale paths instead of assuming 16 bits. Drop AD3530R_REG_VAL_MASK and
> AD3530R_DAC_MAX_VAL, which hardcoded the 16-bit width and are now
> unused.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---

You could have stated in the commit that this in preparation of a
follow up commit. Anyways:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/dac/ad3530r.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 06b7f280f61b..97483534f8c3 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c
> @@ -61,14 +61,12 @@
>  #define AD3530R_SLD_TRIG_A			BIT(7)
>  #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
>  #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)
> -#define AD3530R_REG_VAL_MASK			GENMASK(15, 0)
>  #define AD3530R_OP_MODE_CHAN_MSK(chan)		(GENMASK(1, 0) << 2 * (chan))
>  
>  #define AD3530R_SW_RESET			(BIT(7) | BIT(0))
>  #define AD3530R_INTERNAL_VREF_mV		2500
>  #define AD3530R_LDAC_PULSE_US			100
>  
> -#define AD3530R_DAC_MAX_VAL			GENMASK(15, 0)
>  #define AD3530R_CH_PER_REG			4
>  #define AD3530R_CH_PER_BANK			8
>  #define AD3531R_MAX_CHANNELS			4
> @@ -99,6 +97,7 @@ struct ad3530r_chip_info {
>  	unsigned int num_channels;
>  	unsigned int num_banks;
>  	unsigned int num_op_mode_regs;
> +	unsigned int resolution;
>  	bool internal_ref_support;
>  };
>  
> @@ -310,7 +309,7 @@ static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
>  	int ret;
>  
>  	guard(mutex)(&st->lock);
> -	st->buf = cpu_to_be16(val);
> +	st->buf = cpu_to_be16(val << (16 - st->chip_info->resolution));
>  
>  	ret = regmap_bulk_write(st->regmap, st->chip_info->input_ch_reg(chan),
>  				&st->buf, sizeof(st->buf));
> @@ -340,12 +339,12 @@ static int ad3530r_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  
> -		*val = FIELD_GET(AD3530R_REG_VAL_MASK, be16_to_cpu(st->buf));
> +		*val = be16_to_cpu(st->buf) >> (16 - st->chip_info->resolution);
>  
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = st->vref_mV;
> -		*val2 = 16;
> +		*val2 = st->chip_info->resolution;
>  
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
> @@ -361,7 +360,7 @@ static int ad3530r_write_raw(struct iio_dev *indio_dev,
>  
>  	switch (info) {
>  	case IIO_CHAN_INFO_RAW:
> -		if (val < 0 || val > AD3530R_DAC_MAX_VAL)
> +		if (val < 0 || val > (1 << st->chip_info->resolution) - 1)
>  			return -EINVAL;
>  
>  		return ad3530r_dac_write(st, chan->channel, val);
> @@ -525,6 +524,7 @@ static const struct regmap_config ad3532r_regmap_config = {
>  
>  static const struct ad3530r_chip_info ad3530_chip = {
>  	.name = "ad3530",
> +	.resolution = 16,
>  	.channels = ad3530r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3530r_channels),
> @@ -541,6 +541,7 @@ static const struct ad3530r_chip_info ad3530_chip = {
>  
>  static const struct ad3530r_chip_info ad3530r_chip = {
>  	.name = "ad3530r",
> +	.resolution = 16,
>  	.channels = ad3530r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3530r_channels),
> @@ -557,6 +558,7 @@ static const struct ad3530r_chip_info ad3530r_chip = {
>  
>  static const struct ad3530r_chip_info ad3531_chip = {
>  	.name = "ad3531",
> +	.resolution = 16,
>  	.channels = ad3531r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3531r_channels),
> @@ -573,6 +575,7 @@ static const struct ad3530r_chip_info ad3531_chip = {
>  
>  static const struct ad3530r_chip_info ad3531r_chip = {
>  	.name = "ad3531r",
> +	.resolution = 16,
>  	.channels = ad3531r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3531r_channels),
> @@ -589,6 +592,7 @@ static const struct ad3530r_chip_info ad3531r_chip = {
>  
>  static const struct ad3530r_chip_info ad3532_chip = {
>  	.name = "ad3532",
> +	.resolution = 16,
>  	.channels = ad3532r_channels,
>  	.regmap_config = &ad3532r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3532r_channels),
> @@ -605,6 +609,7 @@ static const struct ad3530r_chip_info ad3532_chip = {
>  
>  static const struct ad3530r_chip_info ad3532r_chip = {
>  	.name = "ad3532r",
> +	.resolution = 16,
>  	.channels = ad3532r_channels,
>  	.regmap_config = &ad3532r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3532r_channels),
> 
> -- 
> 2.34.1
> 

  reply	other threads:[~2026-07-28 16:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21  8:47 [PATCH v2 0/4] Add support for AD5710R/AD5711R DAC Kim Seer Paller
2026-07-21  8:47 ` [PATCH v2 1/4] iio: ABI: Add DAC current powerdown attributes and 15kohm_to_gnd mode Kim Seer Paller
2026-07-21  8:47 ` [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml Kim Seer Paller
2026-07-21 15:52   ` Conor Dooley
2026-07-24 21:57     ` Jonathan Cameron
2026-07-28 15:41       ` Conor Dooley
2026-07-28 20:44         ` Jonathan Cameron
2026-07-24 22:10   ` Jonathan Cameron
2026-07-21  8:47 ` [PATCH v2 3/4] iio: dac: ad3530r: parameterize DAC resolution Kim Seer Paller
2026-07-28 16:40   ` Nuno Sá [this message]
2026-07-21  8:47 ` [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R Kim Seer Paller
2026-07-21 10:38   ` Andy Shevchenko
2026-07-23  4:54     ` Kim Seer Paller
2026-07-24 22:26   ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amjbOIMuS3MFpesz@nsa \
    --to=noname.nuno@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=kimseer.paller@analog.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox