All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Kim Seer Paller" <kimseer.paller@analog.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v4 3/6] iio: dac: ad3530r: Convert sw_ldac_trig_reg to a function pointer
Date: Fri, 03 Jul 2026 10:23:23 +0000	[thread overview]
Message-ID: <20260703102323.A096F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260703-iio-ad3532r-support-v4-3-69d9a336f4e8@analog.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Writing to the highest channel (channel 7 on AD3530/R, channel 15 on AD3532/R) fails with -EINVAL because max_register excludes the second byte of the 16-bit DAC value.
--

commit 744847f13949cc21f1d21acc0a27b2cff1da3ef3
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad3530r: Convert sw_ldac_trig_reg to a function pointer

This commit converts the sw_ldac_trig_reg fixed register address into a
function pointer that returns the trigger register for a given channel.
This supports multi-bank register architectures where the trigger
register depends on the channel being updated.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 9d4545e2c9b33..7841546f0c07b 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c

[ ... ]

> @@ -215,7 +225,7 @@ static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
>  	if (st->ldac_gpio)
>  		return ad3530r_trigger_hw_ldac(st->ldac_gpio);
>  
> -	return regmap_set_bits(st->regmap, st->chip_info->sw_ldac_trig_reg,
> +	return regmap_set_bits(st->regmap, st->chip_info->sw_ldac_trig_reg(chan),
>  			       AD3530R_SLD_TRIG_A);
>  }

[Severity: High]
This is a pre-existing issue, but looking at ad3530r_dac_write(), the
function writes a 16-bit value using regmap_bulk_write() just before
triggering the LDAC:

	ret = regmap_bulk_write(st->regmap, st->chip_info->input_ch_reg(chan),
				&st->buf, sizeof(st->buf));

When writing to the highest channel (channel 7 for AD3530/R, or channel 15
for AD3532/R), the base register address is calculated as 0xF9 or 0x30F9.
Because the DAC value is 2 bytes and val_bits is 8, regmap will try to
write to both 0xF9 and 0xFA (or 0x30F9 and 0x30FA).

However, AD3530R_MAX_REG_ADDR is defined as 0xF9 and AD3532R_MAX_REG_ADDR
is defined as 0x30F9. Does this cause writes to the highest channel to
unconditionally fail with -EINVAL since the second byte's address exceeds
the max_register limit?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-iio-ad3532r-support-v4-0-69d9a336f4e8@analog.com?part=3

  reply	other threads:[~2026-07-03 10:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03 10:10 [PATCH v4 0/6] Add support for AD3532R/AD3532 Kim Seer Paller
2026-07-03 10:10 ` [PATCH v4 1/6] iio: ABI: add DAC 10kohm_to_gnd powerdown mode Kim Seer Paller
2026-07-03 10:10 ` [PATCH v4 2/6] dt-bindings: iio: dac: add support for AD3532R/AD3532 Kim Seer Paller
2026-07-03 10:10 ` [PATCH v4 3/6] iio: dac: ad3530r: Convert sw_ldac_trig_reg to a function pointer Kim Seer Paller
2026-07-03 10:23   ` sashiko-bot [this message]
2026-07-03 12:25   ` Andy Shevchenko
2026-07-03 10:10 ` [PATCH v4 4/6] iio: dac: ad3530r: Refactor setup to table-driven register banks Kim Seer Paller
2026-07-03 12:27   ` Andy Shevchenko
2026-07-03 10:10 ` [PATCH v4 5/6] iio: dac: ad3530r: Make regmap_config selectable per chip Kim Seer Paller
2026-07-03 12:29   ` Andy Shevchenko
2026-07-03 10:10 ` [PATCH v4 6/6] iio: dac: ad3530r: Add support for AD3532R/AD3532 Kim Seer Paller
2026-07-03 10:30   ` sashiko-bot
2026-07-03 12:39   ` Andy Shevchenko
2026-07-04 23:53 ` [PATCH v4 0/6] " 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=20260703102323.A096F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kimseer.paller@analog.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.