| Message ID | 20221121123833.164614-1-mmkurbanov@sberdevices.ru |
|---|---|
| Headers | show |
| Series | leds: trigger: pattern: notify usespace if pattern finished | expand |
On Mon, Nov 21, 2022 at 2:44 PM Martin Kurbanov <MMKurbanov@sberdevices.ru> wrote: > On 21.11.2022 15:38, Martin Kurbanov wrote: > In the previous patch series feedback you mentioned two main problems: > sysfs node creation time and life cycle, and sysfs node creation method. > Let me explain why I didn't fix the above items. > > 1) About sysfs node creation time and its life cycle. In my opinion, > sysfs node should exist only when user has activated pattern explicitly; > otherwise, it will mislead potential user in the case when pattern is > not activated, but sysfs node existed. OK. > 2) About pattern_trig_attrs. We need to use sysfs_notify_dirent() > instead of sysfs_notify(), cause sysfs_notify() can sleep on the lock, > but it's not acceptable for the pattern code in the timer context. > Considering this, we have to create sysfs node in the > pattern_trig_activate() directly and retrieve kernfs_node for required > sysfs_notify_dirent() routine. Is there a guarantee that nobody is using the removed node? If no, what would be the problems with that if any?
On Mon, Nov 21, 2022 at 2:39 PM Martin Kurbanov <mmkurbanov@sberdevices.ru> wrote: > > This patch adds some minor code style changes: > - remove a blank line before DEVICE_ATTR_RW declarations > - convert sysfs scnprintf() to sysfs_emit()/sysfs_emit_at() > - use module_led_trigger instead of pattern_trig_init/exit > > Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru> ... > + count += sysfs_emit_at(buf, count, > + "%d %u ", With this getting shorter you may put these to one line. > + data->patterns[i].brightness, > + data->patterns[i].delta_t); ... > -static int __init pattern_trig_init(void) > -{ > - return led_trigger_register(&pattern_led_trigger); > -} > - > -static void __exit pattern_trig_exit(void) > -{ > - led_trigger_unregister(&pattern_led_trigger); > -} > - > -module_init(pattern_trig_init); > -module_exit(pattern_trig_exit); > +module_led_trigger(pattern_led_trigger); This should be a separate patch. ... Otherwise this looks good to me.
In the current moment, userspace caller can schedule LED pattern with appropriate parameters, but it doesn't have ability to listen to any events indicated pattern finished. This patch series add support for a 'is_running' attribute to signal LED pattern state to userspace. For example of user space code how to use the feature: https://gist.github.com/m-kurbanov/775408521c436a371c3640d49808d08d Changes v1 -> v2: - code style fixes (Andy Shevchenko) - typo fixes (Andy Shevchenko) - add ABI documentation (Andy Shevchenko) - added example on the gist (Andy Shevchenko) Martin Kurbanov (2): leds: trigger: pattern: minor code style changes leds: trigger: pattern: notify usespace if pattern finished .../testing/sysfs-class-led-trigger-pattern | 11 +++ drivers/leds/trigger/ledtrig-pattern.c | 92 ++++++++++++++----- 2 files changed, 80 insertions(+), 23 deletions(-) -- 2.38.1