[PATCH v2 04/11] iio: dac: ad5686: fix powerdown control on dual-channel devices

Rodrigo Alencar via B4 Relay
2026-04-27T11:30:11Z

Fix powerdown control by using a proper bit shift for the powerdown mask
values. During initialization, powerdown bits are initialized so that
unused bits are set to 1 and the correct bit shift is used. Dual-channel
devices use one-hot encoding in the address and that reflects on the
position of the powerdown bits, which are not channel-index based
for that case. Quad-channel devices also use one-hot encoding for the
channel address but the result of log2(address) coincides with the channel
index value. The issue was introduced when first adding support for
dual-channel devices, which overlooked powerdown control differences.

Fixes: 7dc8faeab3e3 ("iio: dac: ad5686: add support for AD5338R")
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index aa17ed6aee43..04a5568f2cbe 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -25,24 +25,35 @@ static const char * const ad5686_powerdown_modes[] = {
"three_state"
};

+static inline unsigned int ad5686_pd_mask_shift(const struct iio_chan_spec *chan)
+{
+ if (chan->channel == chan->address)
+ return chan->channel * 2;
+
+ /* one-hot encoding is used in dual/quad channel devices */
+ return __ffs(chan->address) * 2;
+}
+
static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan)
{
+ unsigned int shift = ad5686_pd_mask_shift(chan);
struct ad5686_state *st = iio_priv(indio_dev);

- return ((st->pwr_down_mode >> (chan->channel * 2)) & 0x3) - 1;
+ return ((st->pwr_down_mode >> shift) & 0x3) - 1;
}

static int ad5686_set_powerdown_mode(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
unsigned int mode)
{
+ unsigned int shift = ad5686_pd_mask_shift(chan);
struct ad5686_state *st = iio_priv(indio_dev);

guard(mutex)(&st->lock);

- st->pwr_down_mode &= ~(0x3 << (chan->channel * 2));
- st->pwr_down_mode |= ((mode + 1) << (chan->channel * 2));
+ st->pwr_down_mode &= ~(0x3 << shift);
+ st->pwr_down_mode |= ((mode + 1) << shift);

return 0;
}
@@ -57,10 +68,10 @@ static const struct iio_enum ad5686_powerdown_mode_enum = {
static ssize_t ad5686_read_dac_powerdown(struct iio_dev *indio_dev,
uintptr_t private, const struct iio_chan_spec *chan, char *buf)
{
+ unsigned int shift = ad5686_pd_mask_shift(chan);
struct ad5686_state *st = iio_priv(indio_dev);

- return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask &
- (0x3 << (chan->channel * 2))));
+ return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask & (0x3 << shift)));
}

static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
@@ -82,9 +93,9 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
guard(mutex)(&st->lock);

if (readin)
- st->pwr_down_mask |= (0x3 << (chan->channel * 2));
+ st->pwr_down_mask |= (0x3 << ad5686_pd_mask_shift(chan));
else
- st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
+ st->pwr_down_mask &= ~(0x3 << ad5686_pd_mask_shift(chan));

switch (st->chip_info->regmap_type) {
case AD5310_REGMAP:
@@ -464,7 +475,7 @@ int ad5686_probe(struct device *dev,
{
struct ad5686_state *st;
struct iio_dev *indio_dev;
- unsigned int val, ref_bit_msk;
+ unsigned int val, ref_bit_msk, shift;
bool has_external_vref;
u8 cmd;
int ret, i;
@@ -489,8 +500,14 @@ int ad5686_probe(struct device *dev,
st->vref_mv = has_external_vref ? ret / 1000 : st->chip_info->int_vref_mv;

/* Set all the power down mode for all channels to 1K pulldown */
- for (i = 0; i < st->chip_info->num_channels; i++)
- st->pwr_down_mode |= (0x01 << (i * 2));
+ st->pwr_down_mode = ~0U;
+ st->pwr_down_mask = ~0U;
+ for (i = 0; i < st->chip_info->num_channels; i++) {
+ shift = ad5686_pd_mask_shift(&st->chip_info->channels[i]);
+ st->pwr_down_mask &= ~(0x3 << shift); /* powered up state */
+ st->pwr_down_mode &= ~(0x3 << shift);
+ st->pwr_down_mode |= (0x01 << shift);
+ }

indio_dev->name = name;
indio_dev->info = &ad5686_info;

--
2.43.0