On Wed, Apr 22, 2026 at 03:45:46PM +0100, Rodrigo Alencar via B4 Relay wrote:
> This patch fixes powerdown control issues by protecting the cached
Please, read Submitting Patches documentation. It clearly says to use
imperative mood.
> powerdown states with mutex access, and 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 encondig 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 encondig for the
> channel address but the result of log2(address) coincides with the channel
> index value.
When I see a word "fix" and no Fixes tag I got confused. Please, align commit
message and tags (either make it just refactor, or a real fix, or explain at
bare minimum in the comments block why there is "fix" w/o Fixes tag).
...
> {
> struct ad5686_state *st = iio_priv(indio_dev);
> + int val, shift = ad5686_pd_mask_shift(chan);
>
> - return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask &
> - (0x3 << (chan->channel * 2))));
> + mutex_lock(&st->lock);
> + val = !!(st->pwr_down_mask & (0x3 << shift));
> + mutex_unlock(&st->lock);
> +
> + return sysfs_emit(buf, "%d\n", val);
> }
Why not use guard()() from day 1?
...
> struct ad5686_state *st;
> - int ret, i;
> + int ret, i, shift;
Do 'i' and 'shift' need to be signed?
--
With Best Regards,
Andy Shevchenko
