* [PATCH v2 0/2] leds: trigger: pattern: notify usespace if pattern finished
@ 2022-11-21 12:38 Martin Kurbanov
2022-11-21 12:38 ` [PATCH v2 1/2] leds: trigger: pattern: minor code style changes Martin Kurbanov
2022-11-21 12:38 ` [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
0 siblings, 2 replies; 7+ messages in thread
From: Martin Kurbanov @ 2022-11-21 12:38 UTC (permalink / raw)
To: Pavel Machek, Raphael Teysseyre, Andy Shevchenko
Cc: linux-kernel, linux-leds, kernel, Martin Kurbanov
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] leds: trigger: pattern: minor code style changes
2022-11-21 12:38 [PATCH v2 0/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
@ 2022-11-21 12:38 ` Martin Kurbanov
2022-11-21 13:28 ` Andy Shevchenko
2022-11-21 12:38 ` [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
1 sibling, 1 reply; 7+ messages in thread
From: Martin Kurbanov @ 2022-11-21 12:38 UTC (permalink / raw)
To: Pavel Machek, Raphael Teysseyre, Andy Shevchenko
Cc: linux-kernel, linux-leds, kernel, Martin Kurbanov
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>
---
drivers/leds/trigger/ledtrig-pattern.c | 29 +++++++-------------------
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c
index 43a265dc4696..354304b404aa 100644
--- a/drivers/leds/trigger/ledtrig-pattern.c
+++ b/drivers/leds/trigger/ledtrig-pattern.c
@@ -155,7 +155,7 @@ static ssize_t repeat_show(struct device *dev, struct device_attribute *attr,
mutex_unlock(&data->lock);
- return scnprintf(buf, PAGE_SIZE, "%d\n", repeat);
+ return sysfs_emit(buf, "%d\n", repeat);
}
static ssize_t repeat_store(struct device *dev, struct device_attribute *attr,
@@ -192,7 +192,6 @@ static ssize_t repeat_store(struct device *dev, struct device_attribute *attr,
mutex_unlock(&data->lock);
return err < 0 ? err : count;
}
-
static DEVICE_ATTR_RW(repeat);
static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data,
@@ -207,13 +206,13 @@ static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data,
goto out;
for (i = 0; i < data->npatterns; i++) {
- count += scnprintf(buf + count, PAGE_SIZE - count,
- "%d %u ",
- data->patterns[i].brightness,
- data->patterns[i].delta_t);
+ count += sysfs_emit_at(buf, count,
+ "%d %u ",
+ data->patterns[i].brightness,
+ data->patterns[i].delta_t);
}
- buf[count - 1] = '\n';
+ sysfs_emit_at(buf, count - 1, "\n");
out:
mutex_unlock(&data->lock);
@@ -307,7 +306,6 @@ static ssize_t pattern_store(struct device *dev, struct device_attribute *attr,
return pattern_trig_store_patterns(led_cdev, buf, NULL, count, false);
}
-
static DEVICE_ATTR_RW(pattern);
static ssize_t hw_pattern_show(struct device *dev,
@@ -327,7 +325,6 @@ static ssize_t hw_pattern_store(struct device *dev,
return pattern_trig_store_patterns(led_cdev, buf, NULL, count, true);
}
-
static DEVICE_ATTR_RW(hw_pattern);
static umode_t pattern_trig_attrs_mode(struct kobject *kobj,
@@ -443,19 +440,7 @@ static struct led_trigger pattern_led_trigger = {
.deactivate = pattern_trig_deactivate,
.groups = pattern_trig_groups,
};
-
-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);
MODULE_AUTHOR("Raphael Teysseyre <rteysseyre@gmail.com>");
MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org>");
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished
2022-11-21 12:38 [PATCH v2 0/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
2022-11-21 12:38 ` [PATCH v2 1/2] leds: trigger: pattern: minor code style changes Martin Kurbanov
@ 2022-11-21 12:38 ` Martin Kurbanov
2022-11-21 12:44 ` Martin Kurbanov
2022-11-21 13:26 ` Andy Shevchenko
1 sibling, 2 replies; 7+ messages in thread
From: Martin Kurbanov @ 2022-11-21 12:38 UTC (permalink / raw)
To: Pavel Machek, Raphael Teysseyre, Andy Shevchenko
Cc: linux-kernel, linux-leds, kernel, Martin Kurbanov
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 implements such an event
using sysfs node and sysfs_notify_dirent() call.
Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
---
.../testing/sysfs-class-led-trigger-pattern | 11 ++++
drivers/leds/trigger/ledtrig-pattern.c | 63 ++++++++++++++++++-
2 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
index 8c57d2780554..b2564123b54b 100644
--- a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
@@ -38,3 +38,14 @@ Description:
It should be noticed that some leds, like EL15203000 may
only support indefinitely patterns, so they always store -1.
+
+What: /sys/class/leds/<led>/is_running
+Date: October 2022
+KernelVersion: 6.1
+Description:
+ Indicates running of a software pattern for the LED.
+
+ This file is read only and supports poll() to detect when the
+ software pattern ended.
+
+ 1 means pattern is running and number 0 are finish or not run.
diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c
index 354304b404aa..19a6b5dcd7af 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 is_running;
struct timer_list timer;
+ struct kernfs_node *is_running_kn;
};
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->is_running = false;
+
+ if (data->is_running_kn)
+ sysfs_notify_dirent(data->is_running_kn);
+
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->is_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->is_running = false;
if (data->is_hw_pattern)
led_cdev->pattern_clear(led_cdev);
@@ -267,6 +277,7 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev,
mutex_lock(&data->lock);
del_timer_sync(&data->timer);
+ data->is_running = false;
if (data->is_hw_pattern)
led_cdev->pattern_clear(led_cdev);
@@ -327,6 +338,16 @@ static ssize_t hw_pattern_store(struct device *dev,
}
static DEVICE_ATTR_RW(hw_pattern);
+static ssize_t is_running_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 sysfs_emit(buf, "%d\n", data->is_running);
+}
+static DEVICE_ATTR_RO(is_running);
+
static umode_t pattern_trig_attrs_mode(struct kobject *kobj,
struct attribute *attr, int index)
{
@@ -382,9 +403,41 @@ static void pattern_init(struct led_classdev *led_cdev)
kfree(pattern);
}
+static int pattern_trig_add_is_running(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_is_running);
+ if (ret) {
+ dev_err(dev,
+ "Error creating is_running (%pe)\n", ERR_PTR(ret));
+ return ret;
+ }
+
+ data->is_running_kn = sysfs_get_dirent(dev->kobj.sd, "is_running");
+ if (!data->is_running_kn) {
+ dev_err(dev, "Error getting is_running kernelfs\n");
+ device_remove_file(dev, &dev_attr_is_running);
+ return -ENXIO;
+ }
+
+ return 0;
+}
+
+static void pattern_trig_remove_is_running(struct led_classdev *led_cdev)
+{
+ struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
+
+ sysfs_put(data->is_running_kn);
+ device_remove_file(led_cdev->dev, &dev_attr_is_running);
+}
+
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)
@@ -403,6 +456,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_is_running(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) {
@@ -428,6 +488,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_is_running(led_cdev);
led_set_brightness(led_cdev, LED_OFF);
kfree(data);
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished
2022-11-21 12:38 ` [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
@ 2022-11-21 12:44 ` Martin Kurbanov
2022-11-21 13:24 ` Andy Shevchenko
2022-11-21 13:26 ` Andy Shevchenko
1 sibling, 1 reply; 7+ messages in thread
From: Martin Kurbanov @ 2022-11-21 12:44 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, kernel,
Pavel Machek, Raphael Teysseyre
On 21.11.2022 15:38, Martin Kurbanov wrote:
> 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 implements such an event
> using sysfs node and sysfs_notify_dirent() call.
>
> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
> ---
> .../testing/sysfs-class-led-trigger-pattern | 11 ++++
> drivers/leds/trigger/ledtrig-pattern.c | 63 ++++++++++++++++++-
> 2 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> index 8c57d2780554..b2564123b54b 100644
> --- a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> @@ -38,3 +38,14 @@ Description:
>
> It should be noticed that some leds, like EL15203000 may
> only support indefinitely patterns, so they always store -1.
> +
> +What: /sys/class/leds/<led>/is_running
> +Date: October 2022
> +KernelVersion: 6.1
> +Description:
> + Indicates running of a software pattern for the LED.
> +
> + This file is read only and supports poll() to detect when the
> + software pattern ended.
> +
> + 1 means pattern is running and number 0 are finish or not run.
> diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c
> index 354304b404aa..19a6b5dcd7af 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 is_running;
> struct timer_list timer;
> + struct kernfs_node *is_running_kn;
> };
>
> 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->is_running = false;
> +
> + if (data->is_running_kn)
> + sysfs_notify_dirent(data->is_running_kn);
> +
> 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->is_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->is_running = false;
>
> if (data->is_hw_pattern)
> led_cdev->pattern_clear(led_cdev);
> @@ -267,6 +277,7 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev,
> mutex_lock(&data->lock);
>
> del_timer_sync(&data->timer);
> + data->is_running = false;
>
> if (data->is_hw_pattern)
> led_cdev->pattern_clear(led_cdev);
> @@ -327,6 +338,16 @@ static ssize_t hw_pattern_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(hw_pattern);
>
> +static ssize_t is_running_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 sysfs_emit(buf, "%d\n", data->is_running);
> +}
> +static DEVICE_ATTR_RO(is_running);
> +
> static umode_t pattern_trig_attrs_mode(struct kobject *kobj,
> struct attribute *attr, int index)
> {
> @@ -382,9 +403,41 @@ static void pattern_init(struct led_classdev *led_cdev)
> kfree(pattern);
> }
>
> +static int pattern_trig_add_is_running(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_is_running);
> + if (ret) {
> + dev_err(dev,
> + "Error creating is_running (%pe)\n", ERR_PTR(ret));
> + return ret;
> + }
> +
> + data->is_running_kn = sysfs_get_dirent(dev->kobj.sd, "is_running");
> + if (!data->is_running_kn) {
> + dev_err(dev, "Error getting is_running kernelfs\n");
> + device_remove_file(dev, &dev_attr_is_running);
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static void pattern_trig_remove_is_running(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> +
> + sysfs_put(data->is_running_kn);
> + device_remove_file(led_cdev->dev, &dev_attr_is_running);
> +}
> +
> 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)
> @@ -403,6 +456,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_is_running(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) {
> @@ -428,6 +488,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_is_running(led_cdev);
>
> led_set_brightness(led_cdev, LED_OFF);
> kfree(data);
Hello Andy,
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.
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished
2022-11-21 12:44 ` Martin Kurbanov
@ 2022-11-21 13:24 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2022-11-21 13:24 UTC (permalink / raw)
To: Martin Kurbanov
Cc: linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, kernel,
Pavel Machek, Raphael Teysseyre
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?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished
2022-11-21 12:38 ` [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
2022-11-21 12:44 ` Martin Kurbanov
@ 2022-11-21 13:26 ` Andy Shevchenko
1 sibling, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2022-11-21 13:26 UTC (permalink / raw)
To: Martin Kurbanov
Cc: Pavel Machek, Raphael Teysseyre, linux-kernel, linux-leds, kernel
On Mon, Nov 21, 2022 at 2:39 PM Martin Kurbanov
<mmkurbanov@sberdevices.ru> wrote:
>
> In the current moment, userspace caller can schedule LED pattern with
a LED pattern
> appropriate parameters, but it doesn't have ability to listen to any
> events indicated pattern finished. This patch implements such an event
> using sysfs node and sysfs_notify_dirent() call.
...
> +Date: October 2022
> +KernelVersion: 6.1
It can't be. Have you read my previous comments?
...
> + 1 means pattern is running and number 0 are finish or not run.
are finished
...
> +static void pattern_trig_remove_is_running(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_get_trigger_data(led_cdev);
> +
> + sysfs_put(data->is_running_kn);
> + device_remove_file(led_cdev->dev, &dev_attr_is_running);
> +}
If the file is opened at this time, what would happen during execution
of this function?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] leds: trigger: pattern: minor code style changes
2022-11-21 12:38 ` [PATCH v2 1/2] leds: trigger: pattern: minor code style changes Martin Kurbanov
@ 2022-11-21 13:28 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2022-11-21 13:28 UTC (permalink / raw)
To: Martin Kurbanov
Cc: Pavel Machek, Raphael Teysseyre, linux-kernel, linux-leds, kernel
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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-21 13:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-21 12:38 [PATCH v2 0/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
2022-11-21 12:38 ` [PATCH v2 1/2] leds: trigger: pattern: minor code style changes Martin Kurbanov
2022-11-21 13:28 ` Andy Shevchenko
2022-11-21 12:38 ` [PATCH v2 2/2] leds: trigger: pattern: notify usespace if pattern finished Martin Kurbanov
2022-11-21 12:44 ` Martin Kurbanov
2022-11-21 13:24 ` Andy Shevchenko
2022-11-21 13:26 ` 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 read-only IMAP folder(s) and NNTP newsgroup(s).