On Tue, Jun 02, 2026 at 05:33:57PM +0100, Rodrigo Alencar via B4 Relay wrote:
> Implement trigger handler by leveraging the LDAC gpio to update all DAC
> channels at once when it is available. Also, the multiple channel writes
> can be flushed at once with the sync() operation.
...
> +static irqreturn_t ad5686_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct iio_buffer *buffer = indio_dev->buffer;
> + struct ad5686_state *st = iio_priv(indio_dev);
> + u16 val[AD5686_MAX_CHANNELS] = { };
> + int ret, ch, i = 0;
> + bool async_update;
> + u8 cmd;
> +
> + ret = iio_pop_from_buffer(buffer, val);
> + if (ret)
> + goto out;
> +
> + mutex_lock(&st->lock);
> +
> + async_update = st->ldac_gpio &&
> bitmap_weight(indio_dev->active_scan_mask,
> +
> iio_get_masklength(indio_dev)) > 1;
Easier to read on a logical split
async_update = st->ldac_gpio &&
bitmap_weight(indio_dev->active_scan_mask,
iio_get_masklength(indio_dev)) > 1;
Or with a temporary variable where you keep the weight of the bitmap.
> + if (async_update) {
> + /* use ldac to update all channels simultaneously */
> + cmd = AD5686_CMD_WRITE_INPUT_N;
> + gpiod_set_value_cansleep(st->ldac_gpio, 0);
> + } else {
> + cmd = AD5686_CMD_WRITE_INPUT_N_UPDATE_N;
> + }
> +
> + iio_for_each_active_channel(indio_dev, ch) {
> + ret = st->ops->write(st, cmd, indio_dev->channels[ch].address,
> val[i++]);
> + if (ret)
> + goto cleanup;
> + }
> +
> + if (st->ops->sync)
> + ret = st->ops->sync(st); /* flush all pending transfers */
> +cleanup:
Bad label naming. Also may clash in the future with some overflow or wrap traps
(you can find interesting discussion with Linus on the implementation of that
with Kees).
> + if (async_update)
> + gpiod_set_value_cansleep(st->ldac_gpio, 1);
> +
> + mutex_unlock(&st->lock);
> +out:
out_notify_done:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
--
With Best Regards,
Andy Shevchenko
