* [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished
@ 2022-08-24 11:49 Martin Kurbanov
2022-08-24 15:51 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Martin Kurbanov @ 2022-08-24 11:49 UTC (permalink / raw)
To: Pavel Machek, Raphael Teysseyre, Baolin Wang
Cc: linux-leds, linux-kernel, kernel, Martin Kurbanov
In the current moment, userspace caller can schedule led pattern with
appropriate parameters, but it doesn't have ability to listen any events
indicated pattern finished. This patch implements such an event using
sysfs node and sysfs_notify_dirent() call.
Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
---
drivers/leds/trigger/ledtrig-pattern.c | 64 +++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c
index 43a265dc4696..54c4b957052f 100644
--- a/drivers/leds/trigger/ledtrig-pattern.c
+++ b/drivers/leds/trigger/ledtrig-pattern.c
@@ -33,7 +33,9 @@ struct pattern_trig_data {
int delta_t;
bool is_indefinite;
bool is_hw_pattern;
+ bool running;
struct timer_list timer;
+ struct kernfs_node *pattern_ended;
};
static void pattern_trig_update_patterns(struct pattern_trig_data *data)
@@ -76,8 +78,14 @@ static void pattern_trig_timer_function(struct timer_list *t)
struct pattern_trig_data *data = from_timer(data, t, timer);
for (;;) {
- if (!data->is_indefinite && !data->repeat)
+ if (!data->is_indefinite && !data->repeat) {
+ data->running = false;
+
+ if (data->pattern_ended)
+ sysfs_notify_dirent(data->pattern_ended);
+
break;
+ }
if (data->curr->brightness == data->next->brightness) {
/* Step change of brightness */
@@ -137,6 +145,7 @@ static int pattern_trig_start_pattern(struct led_classdev *led_cdev)
data->curr = data->patterns;
data->next = data->patterns + 1;
data->timer.expires = jiffies;
+ data->running = true;
add_timer(&data->timer);
return 0;
@@ -176,6 +185,7 @@ static ssize_t repeat_store(struct device *dev, struct device_attribute *attr,
mutex_lock(&data->lock);
del_timer_sync(&data->timer);
+ data->running = false;
if (data->is_hw_pattern)
led_cdev->pattern_clear(led_cdev);
@@ -268,6 +278,7 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev,
mutex_lock(&data->lock);
del_timer_sync(&data->timer);
+ data->running = false;
if (data->is_hw_pattern)
led_cdev->pattern_clear(led_cdev);
@@ -330,6 +341,17 @@ static ssize_t hw_pattern_store(struct device *dev,
static DEVICE_ATTR_RW(hw_pattern);
+static ssize_t pattern_ended_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !data->running);
+}
+
+static DEVICE_ATTR_RO(pattern_ended);
+
static umode_t pattern_trig_attrs_mode(struct kobject *kobj,
struct attribute *attr, int index)
{
@@ -385,9 +407,41 @@ static void pattern_init(struct led_classdev *led_cdev)
kfree(pattern);
}
+static int pattern_trig_add_pattern_ended(struct led_classdev *led_cdev)
+{
+ struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
+ struct device *dev = led_cdev->dev;
+ int ret;
+
+ ret = device_create_file(dev, &dev_attr_pattern_ended);
+ if (ret) {
+ dev_err(dev,
+ "Error creating pattern_ended (%pe)\n", ERR_PTR(ret));
+ return ret;
+ }
+
+ data->pattern_ended = sysfs_get_dirent(dev->kobj.sd, "pattern_ended");
+ if (!data->pattern_ended) {
+ dev_err(dev, "Error getting pattern_ended kernelfs\n");
+ device_remove_file(dev, &dev_attr_pattern_ended);
+ return -ENXIO;
+ }
+
+ return 0;
+}
+
+static void pattern_trig_remove_pattern_ended(struct led_classdev *led_cdev)
+{
+ struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
+
+ sysfs_put(data->pattern_ended);
+ device_remove_file(led_cdev->dev, &dev_attr_pattern_ended);
+}
+
static int pattern_trig_activate(struct led_classdev *led_cdev)
{
struct pattern_trig_data *data;
+ int err;
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
@@ -406,6 +460,13 @@ static int pattern_trig_activate(struct led_classdev *led_cdev)
data->led_cdev = led_cdev;
led_set_trigger_data(led_cdev, data);
timer_setup(&data->timer, pattern_trig_timer_function, 0);
+
+ err = pattern_trig_add_pattern_ended(led_cdev);
+ if (err)
+ dev_warn(led_cdev->dev,
+ "pattern ended notifications disabled (%pe)\n",
+ ERR_PTR(err));
+
led_cdev->activated = true;
if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
@@ -431,6 +492,7 @@ static void pattern_trig_deactivate(struct led_classdev *led_cdev)
led_cdev->pattern_clear(led_cdev);
del_timer_sync(&data->timer);
+ pattern_trig_remove_pattern_ended(led_cdev);
led_set_brightness(led_cdev, LED_OFF);
kfree(data);
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished
2022-08-24 11:49 [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished Martin Kurbanov
@ 2022-08-24 15:51 ` Andy Shevchenko
2022-08-25 21:16 ` Martin Kurbanov
2022-09-01 22:43 ` Dmitry Rokosov
0 siblings, 2 replies; 5+ messages in thread
From: Andy Shevchenko @ 2022-08-24 15:51 UTC (permalink / raw)
To: Martin Kurbanov
Cc: Pavel Machek, Raphael Teysseyre, Baolin Wang,
Linux LED Subsystem, Linux Kernel Mailing List, kernel
On Wed, Aug 24, 2022 at 3:06 PM Martin Kurbanov
<mmkurbanov@sberdevices.ru> wrote:
>
> In the current moment, userspace caller can schedule led pattern with
LED
> appropriate parameters, but it doesn't have ability to listen any events
listen to
> indicated pattern finished. This patch implements such an event using
> sysfs node and sysfs_notify_dirent() call.
Where is the ABI documentation for that?
Also, any example of user space code (GitHub repository / gist) how to
use the feature?
...
> bool is_indefinite;
> bool is_hw_pattern;
> + bool running;
is_running ?
...
> +static ssize_t pattern_ended_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", !data->running);
sysfs_emit().
> +}
> +
No need for this blank line.
> +static DEVICE_ATTR_RO(pattern_ended);
WRT previous two comments, if the current code is stylic in the above
way, you can convert it first and then add this patch in a series.
...
> +static int pattern_trig_add_pattern_ended(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> + struct device *dev = led_cdev->dev;
> + int ret;
> +
> + ret = device_create_file(dev, &dev_attr_pattern_ended);
> + if (ret) {
> + dev_err(dev,
> + "Error creating pattern_ended (%pe)\n", ERR_PTR(ret));
> + return ret;
> + }
> +
> + data->pattern_ended = sysfs_get_dirent(dev->kobj.sd, "pattern_ended");
> + if (!data->pattern_ended) {
> + dev_err(dev, "Error getting pattern_ended kernelfs\n");
> + device_remove_file(dev, &dev_attr_pattern_ended);
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static void pattern_trig_remove_pattern_ended(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> +
> + sysfs_put(data->pattern_ended);
> + device_remove_file(led_cdev->dev, &dev_attr_pattern_ended);
> +}
I'm wondering if you can always have a file and instead provide a
value there, so user space may use epoll() mechanism on that. It will
simplify your code here.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished
2022-08-24 15:51 ` Andy Shevchenko
@ 2022-08-25 21:16 ` Martin Kurbanov
2022-09-01 22:43 ` Dmitry Rokosov
1 sibling, 0 replies; 5+ messages in thread
From: Martin Kurbanov @ 2022-08-25 21:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Pavel Machek, Raphael Teysseyre, Baolin Wang,
Linux LED Subsystem, Linux Kernel Mailing List, kernel
Hi. Thank you for quick reply.
On 2022-08-24 18:51, Andy Shevchenko wrote:
...
>> +static void pattern_trig_remove_pattern_ended(struct led_classdev *led_cdev)
>> +{
>> + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
>> +
>> + sysfs_put(data->pattern_ended);
>> + device_remove_file(led_cdev->dev, &dev_attr_pattern_ended);
>> +}
>
> I'm wondering if you can always have a file and instead provide a
> value there, so user space may use epoll() mechanism on that. It will
> simplify your code here.
>
Do you mean to add 'dev_attr_pattern_ended' to 'pattern_trig_attrs',
and use 'sysfs_notify' instead of 'sysfs_notify_dirent' call'?
--
Best Regards,
Kurbanov Martin
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished
2022-08-24 15:51 ` Andy Shevchenko
2022-08-25 21:16 ` Martin Kurbanov
@ 2022-09-01 22:43 ` Dmitry Rokosov
2022-09-02 5:35 ` Andy Shevchenko
1 sibling, 1 reply; 5+ messages in thread
From: Dmitry Rokosov @ 2022-09-01 22:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Martin Kurbanov, Pavel Machek, Raphael Teysseyre, Baolin Wang,
Linux LED Subsystem, Linux Kernel Mailing List, kernel
[...]
> > +static int pattern_trig_add_pattern_ended(struct led_classdev *led_cdev)
> > +{
> > + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> > + struct device *dev = led_cdev->dev;
> > + int ret;
> > +
> > + ret = device_create_file(dev, &dev_attr_pattern_ended);
> > + if (ret) {
> > + dev_err(dev,
> > + "Error creating pattern_ended (%pe)\n", ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + data->pattern_ended = sysfs_get_dirent(dev->kobj.sd, "pattern_ended");
> > + if (!data->pattern_ended) {
> > + dev_err(dev, "Error getting pattern_ended kernelfs\n");
> > + device_remove_file(dev, &dev_attr_pattern_ended);
> > + return -ENXIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void pattern_trig_remove_pattern_ended(struct led_classdev *led_cdev)
> > +{
> > + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> > +
> > + sysfs_put(data->pattern_ended);
> > + device_remove_file(led_cdev->dev, &dev_attr_pattern_ended);
> > +}
>
> I'm wondering if you can always have a file and instead provide a
> value there, so user space may use epoll() mechanism on that. It will
> simplify your code here.
Could you please explain what you mean? In the current implementation
userspace can use epoll() already.
--
Thank you,
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished
2022-09-01 22:43 ` Dmitry Rokosov
@ 2022-09-02 5:35 ` Andy Shevchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2022-09-02 5:35 UTC (permalink / raw)
To: Dmitry Rokosov
Cc: Martin Kurbanov, Pavel Machek, Raphael Teysseyre, Baolin Wang,
Linux LED Subsystem, Linux Kernel Mailing List, kernel
On Fri, Sep 2, 2022 at 1:43 AM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
...
> > > +static void pattern_trig_remove_pattern_ended(struct led_classdev *led_cdev)
> > > +{
> > > + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> > > +
> > > + sysfs_put(data->pattern_ended);
> > > + device_remove_file(led_cdev->dev, &dev_attr_pattern_ended);
> > > +}
> >
> > I'm wondering if you can always have a file and instead provide a
> > value there, so user space may use epoll() mechanism on that. It will
> > simplify your code here.
>
> Could you please explain what you mean? In the current implementation
> userspace can use epoll() already.
On the suddenly disappeared file?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-02 5:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-24 11:49 [PATCH v1] leds: trigger: pattern: notify userpace if pattern finished Martin Kurbanov
2022-08-24 15:51 ` Andy Shevchenko
2022-08-25 21:16 ` Martin Kurbanov
2022-09-01 22:43 ` Dmitry Rokosov
2022-09-02 5:35 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).