Messages in this thread |  | | From | Rodrigo Alencar <> | | Date | Wed, 29 Apr 2026 15:21:26 +0100 | | Subject | Re: [PATCH v4 04/13] iio: dac: ad5686: fix powerdown control on dual-channel devices |
| |
On 26/04/29 05:03PM, Andy Shevchenko wrote: > On Wed, Apr 29, 2026 at 02:07:34PM +0100, Rodrigo Alencar via B4 Relay wrote: > > > 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. > > ... > > > + /* Initialize masks to all ones provided the max shift (last channel) */ > > + shift = ad5686_pd_mask_shift(&st->chip_info->channels[st->chip_info->num_channels - 1]); > > + st->pwr_down_mask = GENMASK(shift + 1, 0); > > + st->pwr_down_mode = GENMASK(shift + 1, 0); > > Why do we care about upper bits being cleared? Can we simply use here
With this weird dual-channel use case, the shift of the second channel is 6 and unused bits in-between needs to be one (probably because the dual channel hardware is the same as the quad-channel (but using the first and the last channel) and it will consume more power if those are zero). Then for this case we need the masks initialized to 0b00111100 in the end, so we can have those "ghost" channels disabled.
> st->pwr_down_mask = GENMASK($MAX_CHAN * 2 + 1, 0); > // or even ~0, U32_MAX, et cetera > st->pwr_down_mask = ~0; > > st->pwr_down_mode = ...same...
setting it all to ones, breaks the single-channel case, because of the weird logic in ad5686_write_dac_powerdown(), I get to tidy this up only when introducing the control_sync() function.
-- Kind regards,
Rodrigo Alencar
|  |