hid-sensor-custom: async notification for input-report sysfs attributes? (human presence / attention sensors)
Daniel Paul Perinchery <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-input,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CA+8Hp=M3ynUHzcDshLup5G+HDs0XoWkav0R5k-00m+O7LbOv1w@mail.gmail.com> |
Hi, I've been investigating why userspace can't efficiently watch a hid-sensor-custom input-report value (specifically a human-presence sensor, HID usage 2000e1, on an AMD Sensor Fusion Hub / amd_sfh platform) for changes without polling, and wanted to check whether there's interest in closing this gap before I try to write anything. Setup: Lenovo IdeaPad Pro 5 14AKP10, AMD Ryzen AI 7 350, kernel 7.0.13 (Debian/Parrot, but this is stock hid-sensor-custom.c). Presence and gaze/attention are exposed as separate logical sensors under the same physical hub (HID-SENSOR-2000e1.x.auto), each with the usual auto-generated input-N-<usage>-value sysfs attribute. What I found tracing show_value() in hid-sensor-custom.c: for input reports, it calls sensor_hub_input_attr_get_raw_value(..., report_id, SENSOR_HUB_SYNC, false) -- i.e. every read of the -value file issues a synchronous GET_REPORT to the hub and returns the live answer. There's no cached field backing that attribute, so there's nothing for a notify-on-change mechanism to attach to on that path. Separately, hid_sensor_capture_sample() *is* a genuine async callback fired on hub-pushed input reports, but as far as I can tell it only feeds the raw kfifo behind the driver's misc-device streaming interface (custom_dev), gated on test_bit(0, &sensor_inst->misc_opened). It doesn't appear to update any per-field cached value, so it isn't reachable from the sysfs show_value() path either. Net effect: there's no way for a userspace consumer to poll()/ select() on the -value file and be woken only on real hardware events. The hub itself is already configured for threshold-based reporting (property-reporting-state = 2, i.e. "Threshold Events") in my case, so the inefficiency is entirely on the kernel/sysfs side -- userspace has to fall back to timed re-reads of a synchronous, firmware-round-trip attribute, which is more expensive per-read than a typical cached sysfs value and awkward to size a safe polling interval for. This seems like a real gap for the presence/proximity use case specifically (walk-away lock, auto-dim, etc.), which is inherently event-oriented and is exactly the kind of feature vendors implement natively on Windows via push notifications from the same class of sensor. Before I put together a patch I wanted to ask: is there interest in adding async caching + sysfs_notify() (or sysfs_notify_dirent(), if attrs end up carrying a stored kernfs_node) for input-report values specifically, sourced from hid_sensor_capture_sample()'s existing async path? Rough shape as I see it: - add a cached "last input value" (+ a bool "valid") per relevant field in struct hid_sensor_custom_field - in hid_sensor_capture_sample(), in addition to (or instead of, behind a config/attr) feeding the misc-device fifo, decode and store the value for the matching field, then sysfs_notify() its -value attribute - leave show_value()'s SENSOR_HUB_SYNC path untouched for feature reports and for any input attrs that never get an async report, so nothing regresses for existing consumers Happy to be told this already exists via a different interface I've missed (I did look at whether this should just go through the IIO buffered/trigger interface instead, given some other hid-sensor-* drivers are IIO-backed -- but hid-sensor-custom exposes vendor/custom usages directly via this sysfs shape rather than IIO channels, so I wasn't sure that's the right layer to add it at). Also open to being told the misc-device/fifo interface is meant to be the async consumption path already and userspace tools should be using that instead of polling -value -- if so, is there existing tooling/docs for that path I should be pointing presence-detection use cases at? Not attaching a patch yet since I'd rather get a read on the right approach (and whether it's wanted at all) before writing one. Thanks, Daniel Paul