2020-07-16 17:18:12

by Marek Behún

[permalink] [raw]
Subject: [PATCH RFC leds + net-next 1/3] leds: trigger: add support for LED-private device triggers

Some LED controllers may come with an internal HW triggering mechanism
for the LED and the ability to switch between SW control and the
internal HW control. This includes most PHYs, various ethernet switches,
the Turris Omnia LED controller or AXP20X PMIC.

This adds support for registering such triggers.

This code is based on work by Pavel Machek <[email protected]> and
Ondřej Jirman <[email protected]>.

Signed-off-by: Marek Behún <[email protected]>
---
drivers/leds/led-triggers.c | 26 ++++++++++++++++++++------
include/linux/leds.h | 10 ++++++++++
2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 79e30d2cb7a5..81e758d5a048 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -27,6 +27,12 @@ LIST_HEAD(trigger_list);

/* Used by LED Class */

+static inline bool
+trigger_relevant(struct led_classdev *led_cdev, struct led_trigger *trig)
+{
+ return !trig->trigger_type || trig->trigger_type == led_cdev->trigger_type;
+}
+
ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t pos, size_t count)
@@ -50,7 +56,7 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,

down_read(&triggers_list_lock);
list_for_each_entry(trig, &trigger_list, next_trig) {
- if (sysfs_streq(buf, trig->name)) {
+ if (sysfs_streq(buf, trig->name) && trigger_relevant(led_cdev, trig)) {
down_write(&led_cdev->trigger_lock);
led_trigger_set(led_cdev, trig);
up_write(&led_cdev->trigger_lock);
@@ -93,8 +99,12 @@ static int led_trigger_format(char *buf, size_t size,
led_cdev->trigger ? "none" : "[none]");

list_for_each_entry(trig, &trigger_list, next_trig) {
- bool hit = led_cdev->trigger &&
- !strcmp(led_cdev->trigger->name, trig->name);
+ bool hit;
+
+ if (!trigger_relevant(led_cdev, trig))
+ continue;
+
+ hit = led_cdev->trigger && !strcmp(led_cdev->trigger->name, trig->name);

len += led_trigger_snprintf(buf + len, size - len,
" %s%s%s", hit ? "[" : "",
@@ -243,7 +253,8 @@ void led_trigger_set_default(struct led_classdev *led_cdev)
down_read(&triggers_list_lock);
down_write(&led_cdev->trigger_lock);
list_for_each_entry(trig, &trigger_list, next_trig) {
- if (!strcmp(led_cdev->default_trigger, trig->name)) {
+ if (!strcmp(led_cdev->default_trigger, trig->name) &&
+ trigger_relevant(led_cdev, trig)) {
led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
led_trigger_set(led_cdev, trig);
break;
@@ -280,7 +291,9 @@ int led_trigger_register(struct led_trigger *trig)
down_write(&triggers_list_lock);
/* Make sure the trigger's name isn't already in use */
list_for_each_entry(_trig, &trigger_list, next_trig) {
- if (!strcmp(_trig->name, trig->name)) {
+ if (!strcmp(_trig->name, trig->name) &&
+ (trig->trigger_type == _trig->trigger_type ||
+ !trig->trigger_type || !_trig->trigger_type)) {
up_write(&triggers_list_lock);
return -EEXIST;
}
@@ -294,7 +307,8 @@ int led_trigger_register(struct led_trigger *trig)
list_for_each_entry(led_cdev, &leds_list, node) {
down_write(&led_cdev->trigger_lock);
if (!led_cdev->trigger && led_cdev->default_trigger &&
- !strcmp(led_cdev->default_trigger, trig->name)) {
+ !strcmp(led_cdev->default_trigger, trig->name) &&
+ trigger_relevant(led_cdev, trig)) {
led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
led_trigger_set(led_cdev, trig);
}
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 2451962d1ec5..6a8d6409c993 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -57,6 +57,10 @@ struct led_init_data {
bool devname_mandatory;
};

+struct led_hw_trigger_type {
+ int dummy;
+};
+
struct led_classdev {
const char *name;
enum led_brightness brightness;
@@ -141,6 +145,9 @@ struct led_classdev {
void *trigger_data;
/* true if activated - deactivate routine uses it to do cleanup */
bool activated;
+
+ /* LEDs that have private triggers have this set */
+ struct led_hw_trigger_type *trigger_type;
#endif

#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
@@ -345,6 +352,9 @@ struct led_trigger {
int (*activate)(struct led_classdev *led_cdev);
void (*deactivate)(struct led_classdev *led_cdev);

+ /* LED-private triggers have this set */
+ struct led_hw_trigger_type *trigger_type;
+
/* LEDs under control by this trigger (for simple triggers) */
rwlock_t leddev_list_lock;
struct list_head led_cdevs;
--
2.26.2


2020-07-20 11:21:25

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH RFC leds + net-next 1/3] leds: trigger: add support for LED-private device triggers

Hi!

> Some LED controllers may come with an internal HW triggering mechanism
> for the LED and the ability to switch between SW control and the
> internal HW control. This includes most PHYs, various ethernet switches,
> the Turris Omnia LED controller or AXP20X PMIC.
>
> This adds support for registering such triggers.
>
> This code is based on work by Pavel Machek <[email protected]> and
> Ondřej Jirman <[email protected]>.
>
> Signed-off-by: Marek Behún <[email protected]>

Looks good to me. I'll likely apply it soon...

Best regards,
Pavel


> ---
> drivers/leds/led-triggers.c | 26 ++++++++++++++++++++------
> include/linux/leds.h | 10 ++++++++++
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index 79e30d2cb7a5..81e758d5a048 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -27,6 +27,12 @@ LIST_HEAD(trigger_list);
>
> /* Used by LED Class */
>
> +static inline bool
> +trigger_relevant(struct led_classdev *led_cdev, struct led_trigger *trig)
> +{
> + return !trig->trigger_type || trig->trigger_type == led_cdev->trigger_type;
> +}
> +
> ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
> struct bin_attribute *bin_attr, char *buf,
> loff_t pos, size_t count)
> @@ -50,7 +56,7 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
>
> down_read(&triggers_list_lock);
> list_for_each_entry(trig, &trigger_list, next_trig) {
> - if (sysfs_streq(buf, trig->name)) {
> + if (sysfs_streq(buf, trig->name) && trigger_relevant(led_cdev, trig)) {
> down_write(&led_cdev->trigger_lock);
> led_trigger_set(led_cdev, trig);
> up_write(&led_cdev->trigger_lock);
> @@ -93,8 +99,12 @@ static int led_trigger_format(char *buf, size_t size,
> led_cdev->trigger ? "none" : "[none]");
>
> list_for_each_entry(trig, &trigger_list, next_trig) {
> - bool hit = led_cdev->trigger &&
> - !strcmp(led_cdev->trigger->name, trig->name);
> + bool hit;
> +
> + if (!trigger_relevant(led_cdev, trig))
> + continue;
> +
> + hit = led_cdev->trigger && !strcmp(led_cdev->trigger->name, trig->name);
>
> len += led_trigger_snprintf(buf + len, size - len,
> " %s%s%s", hit ? "[" : "",
> @@ -243,7 +253,8 @@ void led_trigger_set_default(struct led_classdev *led_cdev)
> down_read(&triggers_list_lock);
> down_write(&led_cdev->trigger_lock);
> list_for_each_entry(trig, &trigger_list, next_trig) {
> - if (!strcmp(led_cdev->default_trigger, trig->name)) {
> + if (!strcmp(led_cdev->default_trigger, trig->name) &&
> + trigger_relevant(led_cdev, trig)) {
> led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
> led_trigger_set(led_cdev, trig);
> break;
> @@ -280,7 +291,9 @@ int led_trigger_register(struct led_trigger *trig)
> down_write(&triggers_list_lock);
> /* Make sure the trigger's name isn't already in use */
> list_for_each_entry(_trig, &trigger_list, next_trig) {
> - if (!strcmp(_trig->name, trig->name)) {
> + if (!strcmp(_trig->name, trig->name) &&
> + (trig->trigger_type == _trig->trigger_type ||
> + !trig->trigger_type || !_trig->trigger_type)) {
> up_write(&triggers_list_lock);
> return -EEXIST;
> }
> @@ -294,7 +307,8 @@ int led_trigger_register(struct led_trigger *trig)
> list_for_each_entry(led_cdev, &leds_list, node) {
> down_write(&led_cdev->trigger_lock);
> if (!led_cdev->trigger && led_cdev->default_trigger &&
> - !strcmp(led_cdev->default_trigger, trig->name)) {
> + !strcmp(led_cdev->default_trigger, trig->name) &&
> + trigger_relevant(led_cdev, trig)) {
> led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
> led_trigger_set(led_cdev, trig);
> }
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 2451962d1ec5..6a8d6409c993 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -57,6 +57,10 @@ struct led_init_data {
> bool devname_mandatory;
> };
>
> +struct led_hw_trigger_type {
> + int dummy;
> +};
> +
> struct led_classdev {
> const char *name;
> enum led_brightness brightness;
> @@ -141,6 +145,9 @@ struct led_classdev {
> void *trigger_data;
> /* true if activated - deactivate routine uses it to do cleanup */
> bool activated;
> +
> + /* LEDs that have private triggers have this set */
> + struct led_hw_trigger_type *trigger_type;
> #endif
>
> #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
> @@ -345,6 +352,9 @@ struct led_trigger {
> int (*activate)(struct led_classdev *led_cdev);
> void (*deactivate)(struct led_classdev *led_cdev);
>
> + /* LED-private triggers have this set */
> + struct led_hw_trigger_type *trigger_type;
> +
> /* LEDs under control by this trigger (for simple triggers) */
> rwlock_t leddev_list_lock;
> struct list_head led_cdevs;

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (5.03 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2020-07-20 19:16:59

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH RFC leds + net-next 1/3] leds: trigger: add support for LED-private device triggers

Hi Marek,

On 7/16/20 7:17 PM, Marek Behún wrote:
> Some LED controllers may come with an internal HW triggering mechanism
> for the LED and the ability to switch between SW control and the
> internal HW control. This includes most PHYs, various ethernet switches,
> the Turris Omnia LED controller or AXP20X PMIC.
>
> This adds support for registering such triggers.
>
> This code is based on work by Pavel Machek <[email protected]> and
> Ondřej Jirman <[email protected]>.
>
> Signed-off-by: Marek Behún <[email protected]>
> ---
> drivers/leds/led-triggers.c | 26 ++++++++++++++++++++------
> include/linux/leds.h | 10 ++++++++++
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index 79e30d2cb7a5..81e758d5a048 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -27,6 +27,12 @@ LIST_HEAD(trigger_list);
>
> /* Used by LED Class */
>
> +static inline bool
> +trigger_relevant(struct led_classdev *led_cdev, struct led_trigger *trig)
> +{
> + return !trig->trigger_type || trig->trigger_type == led_cdev->trigger_type;
> +}
> +
> ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
> struct bin_attribute *bin_attr, char *buf,
> loff_t pos, size_t count)
> @@ -50,7 +56,7 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
>
> down_read(&triggers_list_lock);
> list_for_each_entry(trig, &trigger_list, next_trig) {
> - if (sysfs_streq(buf, trig->name)) {
> + if (sysfs_streq(buf, trig->name) && trigger_relevant(led_cdev, trig)) {
> down_write(&led_cdev->trigger_lock);
> led_trigger_set(led_cdev, trig);
> up_write(&led_cdev->trigger_lock);
> @@ -93,8 +99,12 @@ static int led_trigger_format(char *buf, size_t size,
> led_cdev->trigger ? "none" : "[none]");
>
> list_for_each_entry(trig, &trigger_list, next_trig) {
> - bool hit = led_cdev->trigger &&
> - !strcmp(led_cdev->trigger->name, trig->name);
> + bool hit;
> +
> + if (!trigger_relevant(led_cdev, trig))
> + continue;
> +
> + hit = led_cdev->trigger && !strcmp(led_cdev->trigger->name, trig->name);
>
> len += led_trigger_snprintf(buf + len, size - len,
> " %s%s%s", hit ? "[" : "",
> @@ -243,7 +253,8 @@ void led_trigger_set_default(struct led_classdev *led_cdev)
> down_read(&triggers_list_lock);
> down_write(&led_cdev->trigger_lock);
> list_for_each_entry(trig, &trigger_list, next_trig) {
> - if (!strcmp(led_cdev->default_trigger, trig->name)) {
> + if (!strcmp(led_cdev->default_trigger, trig->name) &&
> + trigger_relevant(led_cdev, trig)) {
> led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
> led_trigger_set(led_cdev, trig);
> break;
> @@ -280,7 +291,9 @@ int led_trigger_register(struct led_trigger *trig)
> down_write(&triggers_list_lock);
> /* Make sure the trigger's name isn't already in use */
> list_for_each_entry(_trig, &trigger_list, next_trig) {
> - if (!strcmp(_trig->name, trig->name)) {
> + if (!strcmp(_trig->name, trig->name) &&
> + (trig->trigger_type == _trig->trigger_type ||
> + !trig->trigger_type || !_trig->trigger_type)) {
> up_write(&triggers_list_lock);
> return -EEXIST;
> }
> @@ -294,7 +307,8 @@ int led_trigger_register(struct led_trigger *trig)
> list_for_each_entry(led_cdev, &leds_list, node) {
> down_write(&led_cdev->trigger_lock);
> if (!led_cdev->trigger && led_cdev->default_trigger &&
> - !strcmp(led_cdev->default_trigger, trig->name)) {
> + !strcmp(led_cdev->default_trigger, trig->name) &&
> + trigger_relevant(led_cdev, trig)) {
> led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
> led_trigger_set(led_cdev, trig);
> }
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 2451962d1ec5..6a8d6409c993 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -57,6 +57,10 @@ struct led_init_data {
> bool devname_mandatory;
> };
>
> +struct led_hw_trigger_type {
> + int dummy;
> +};
> +
> struct led_classdev {
> const char *name;
> enum led_brightness brightness;
> @@ -141,6 +145,9 @@ struct led_classdev {
> void *trigger_data;
> /* true if activated - deactivate routine uses it to do cleanup */
> bool activated;
> +
> + /* LEDs that have private triggers have this set */
> + struct led_hw_trigger_type *trigger_type;
> #endif
>
> #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
> @@ -345,6 +352,9 @@ struct led_trigger {
> int (*activate)(struct led_classdev *led_cdev);
> void (*deactivate)(struct led_classdev *led_cdev);
>
> + /* LED-private triggers have this set */
> + struct led_hw_trigger_type *trigger_type;
> +
> /* LEDs under control by this trigger (for simple triggers) */
> rwlock_t leddev_list_lock;
> struct list_head led_cdevs;

Looks good to me:

Acked-by: Jacek Anaszewski <[email protected]>

--
Best regards,
Jacek Anaszewski

2020-07-21 20:56:36

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH RFC leds + net-next 1/3] leds: trigger: add support for LED-private device triggers

Hi!

> >This adds support for registering such triggers.
> >
> >This code is based on work by Pavel Machek <[email protected]> and
> >Ondřej Jirman <[email protected]>.
> >
> >Signed-off-by: Marek Behún <[email protected]>

> Acked-by: Jacek Anaszewski <[email protected]>


Thanks, applied.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (465.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments