Re: [PATCH v7 4/4] iio: light: veml6031x00: add support for events and trigger
"Javier Carrasco" <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Wed Aug 19, 2026 at 6:42 AM CEST, Andy Shevchenko wrote: > On Tue, Aug 18, 2026 at 06:10:51PM +0200, Javier Carrasco wrote: >> On Tue Aug 18, 2026 at 4:01 PM CEST, Andy Shevchenko wrote: > > ... > >> >> + ret = pm_runtime_get_if_active(dev); >> >> + if (ret <= 0) >> > >> > < 0 seems too much to me. If there is disabled runtime PM (and supposedly >> > device is always on) this prevents from getting events. >> >> I am not sure if I get this. A reference is unconditionally acquired >> when events are enabled as well as in buffer_preenable, and also in the >> probe before interrupts are enabled. Runtime PM should be active at this >> point. If not, the interrupt should not come from the device, even if it >> was on (e.g. before autosuspend kicks in). But maybe I am missing >> something? > > The device maybe in these states here: > - powered off (by runtime PM) ret == 0 > - powered on (by some previous activity) ret > 0 > - always on (PM is disabled by user space, for example) ret < 0 > > Are you telling that the third case is impossible? (Note that autosuspend in > this case is irrelevant.) > I followed the execution paths and the third case seems to be impossible. Setting power/control to "on" calls pm_runtime_forbid()[1], which sets runtime_auto to false and increments the usage counter. Note that it does not update disable_depth, which is the variable that pm_runtime_disable() updates and what pm_runtime_get_if_active() checks to return -EINVAL if runtime PM is disabled. Therefore, pm_runtime_get_if_active() should return a positive value under this configuration. Nevertheless, I conducted a small experiment to make sure that theory and praxis match, setting the device to always on and checking what happens: # NOTE: These are the first operations on the DUT after booting. Always on: $ echo on > /sys/bus/i2c/devices/1-0029/power/control $ cat /sys/bus/i2c/devices/1-0029/power/control on # Low value, so I can easily trigger a rising event with more light: $ echo 300 > /sys/bus/iio/devices/iio:device0/events/in_illuminance_thresh_rising_value $ cat /sys/bus/iio/devices/iio:device0/events/in_illuminance_thresh_rising_value 300 # Enable threshold event: $ echo 1 > /sys/bus/iio/devices/iio:device0/events/in_illuminance_thresh_either_en # Monitor events: $ iio_event_monitor /dev/iio:device0 # Good morning! Increment light to trigger the event. # I instrumented the code to log the value: [ 222.644966] veml6031x00 1-0029: pm_runtime_get_if_active() = 1 # The event: Event: time: 1773416327710140541, type: illuminance, channel: 0, evtype: thresh, direction: rising ^C # The configuration is still the one we wanted to test: $ cat /sys/bus/i2c/devices/1-0029/power/runtime_status active cat /sys/bus/i2c/devices/1-0029/power/control on [1] https://kernel.org/doc/html/latest/driver-api/pm/devices.html#sys-devices-power-control-files Best regards, Javier