-----Original Message-----
From: David Lechner <dlechner@baylibre.com>
Sent: Monday, March 16, 2026 5:51 PM
To: Sabau, Radu bogdan <Radu.Sabau@analog.com>; Andy Shevchenko
[off-list ref]
Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
[off-list ref]; Jonathan Cameron [off-list ref];
Sa, Nuno [off-list ref]; Andy Shevchenko [off-list ref];
Rob Herring [off-list ref]; Krzysztof Kozlowski [off-list ref];
Conor Dooley [off-list ref]; Uwe Kleine-König
[off-list ref]; Liam Girdwood [off-list ref]; Mark Brown
[off-list ref]; Linus Walleij [off-list ref]; Bartosz
Golaszewski [off-list ref]; Philipp Zabel [off-list ref];
linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
kernel@vger.kernel.org; linux-pwm@vger.kernel.org; linux-
gpio@vger.kernel.org
Subject: Re: [PATCH v3 2/4] iio: adc: ad4691: add initial driver for AD4691
family

[External]

On 3/16/26 10:29 AM, Sabau, Radu bogdan wrote:

quoted

quoted

-----Original Message-----
From: Andy Shevchenko <redacted>
Sent: Friday, March 13, 2026 12:58 PM
To: Sabau, Radu bogdan <Radu.Sabau@analog.com>

quoted

+	u32 acc_mask[2] = { mask & 0xFF, mask >> 8 };

This looks quite wrong. Is it for sure like two 32-bit stances per each mask
byte? If not, this should be __le16 acc_mask = cpu_to_le16(~BIT(...));

Hi Andy,

Each acc_mask has its own register, therefore the u32 acc_mask[2] is
intentional - since the regmap is configured with val_bits=32 - the 4-byte
stride matches what regmap reads. However, I understand how this
can be confusing for anyone reading the code, therefore I propose
two ways for this :

1. Keep regmap_bulk_write and add a comment above acc_mask explaining
why u32 is used, although these register values are 8 bits.
2. Switch to regmap_multi_reg_write, which takes explicit (reg, value) pairs
and sidesteps the ambiguity entirely.

Do you have a preference?

Since we already have a custom read/write functions to handle different
register sizes and the chip can read more than one consecutive register
at once, can we just call this a single register and add a special case
to ad4691_reg_read/write() to handle it? Then we can just do a regular
regmap_read/write() functions to access it as a single 16-bit value.

This sounds even better! I will have this in the next version!