Re: [PATCH v3 2/4] iio: light: add support for veml6031x00 ALS series

[Javier Carrasco]

On Tue May 26, 2026 at 8:05 PM CEST, Jonathan Cameron wrote:
> On Tue, 26 May 2026 18:59:33 +0100
> Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
>> On Sun, 24 May 2026 23:53:56 +0200
>> Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote:
>>
>> > These sensors provide two light channels (ALS and IR), I2C communication
>> > and a multiplexed interrupt line to signal data ready and configurable
>> > threshold alarms.
>> >
>> > This first implementation provides basic functionality (measurement
>> > configuration, raw reads and ID validation) and defines the different
>> > register regions in preparation for extended features in the subsequent
>> > patches of the series.
>> >
>> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>
>>
>> A few things inline.  Biggest one is that device driver specific state needs
>> local well documented locking. Here the whole complex gain handling means there
>> are a bunch of register field where the accesses to each set of them need
>> to appear atomic.
> The sashiko comment on underflowing pm counters also looks plausible
> so check that as well.

Hi Jonathan, thanks for your review to this and the other patches of the
series. I will go through all your comments within the next days.

I replied to its comment on the PM underflow, but as I only replied to it,
and then I forwarded the message, it might have gone unseen.

I believe Sashiko is misinterpreting how this pattern works (not
introduced by me, it is already available upstream in
drivers/spi/atmel-quadspi.c). Decrementing the usage counter is fine
because the devres action uses pm_runtime_put_noidle(), which in turns uses
atomic_add_unless(usage_count, -1, 0) to avoid decrementing the usage_count
if it is already 0. Otherwise the devm_ variant would be unusable
because it would not be possible to put the device in autosuspend for
the same reason Sashiko is complaining.

Best regards,
Javier