linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
@ 2024-02-13  7:30 Heiner Kallweit
  2024-02-13  7:31 ` [PATCH 1/4] leds: trigger: Store brightness set by led_trigger_event() Heiner Kallweit
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-13  7:30 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer
  Cc: linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
	linux-mips

If a simple trigger is assigned to a LED, then the LED may be off until
the next led_trigger_event() call. This may be an issue for simple
triggers with rare led_trigger_event() calls, e.g. power supply
charging indicators (drivers/power/supply/power_supply_leds.c).
Therefore persist the brightness value of the last led_trigger_event()
call and use this value if the trigger is assigned to a LED.
This change allows to use simple triggers in more cases.
As a first use case simplify handling of the mute audio trigger.

This series touches few subsystems. I'd propose to handle it via
the LED subsystem.

Heiner Kallweit (4):
  leds: trigger: Store brightness set by led_trigger_event()
  ALSA: control-led: Integrate mute led trigger
  Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
  leds: trigger: audio: Remove this trigger

 arch/mips/configs/ci20_defconfig     |  1 -
 drivers/input/input-leds.c           |  8 +---
 drivers/leds/led-triggers.c          |  6 ++-
 drivers/leds/trigger/Kconfig         |  7 ---
 drivers/leds/trigger/Makefile        |  1 -
 drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
 include/linux/leds.h                 | 29 ++++++------
 sound/core/Kconfig                   |  1 -
 sound/core/control_led.c             | 20 +++++++--
 9 files changed, 37 insertions(+), 103 deletions(-)
 delete mode 100644 drivers/leds/trigger/ledtrig-audio.c

-- 
2.43.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/4] leds: trigger: Store brightness set by led_trigger_event()
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
@ 2024-02-13  7:31 ` Heiner Kallweit
  2024-02-13  7:32 ` [PATCH 2/4] ALSA: control-led: Integrate mute led trigger Heiner Kallweit
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-13  7:31 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer
  Cc: linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
	linux-mips

If a simple trigger is assigned to a LED, then the LED may be off until
the next led_trigger_event() call. This may be an issue for simple
triggers with rare led_trigger_event() calls, e.g. power supply
charging indicators (drivers/power/supply/power_supply_leds.c).
Therefore persist the brightness value of the last led_trigger_event()
call and use this value if the trigger is assigned to a LED.
In addition add a getter for the trigger brightness value.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/leds/led-triggers.c |  6 ++++--
 include/linux/leds.h        | 15 +++++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 0f5ac3005..b1b323b19 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -194,11 +194,11 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 		spin_unlock(&trig->leddev_list_lock);
 		led_cdev->trigger = trig;
 
+		ret = 0;
 		if (trig->activate)
 			ret = trig->activate(led_cdev);
 		else
-			ret = 0;
-
+			led_set_brightness(led_cdev, trig->brightness);
 		if (ret)
 			goto err_activate;
 
@@ -387,6 +387,8 @@ void led_trigger_event(struct led_trigger *trig,
 	if (!trig)
 		return;
 
+	trig->brightness = brightness;
+
 	rcu_read_lock();
 	list_for_each_entry_rcu(led_cdev, &trig->led_cdevs, trig_list)
 		led_set_brightness(led_cdev, brightness);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 7598d4729..48fff5980 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -455,6 +455,9 @@ struct led_trigger {
 	int		(*activate)(struct led_classdev *led_cdev);
 	void		(*deactivate)(struct led_classdev *led_cdev);
 
+	/* Brightness set by led_trigger_event */
+	enum led_brightness brightness;
+
 	/* LED-private triggers have this set */
 	struct led_hw_trigger_type *trigger_type;
 
@@ -508,6 +511,12 @@ static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
 	return led_cdev->trigger_data;
 }
 
+static inline enum led_brightness
+led_trigger_get_brightness(const struct led_trigger *trigger)
+{
+	return trigger ? trigger->brightness : LED_OFF;
+}
+
 #define module_led_trigger(__led_trigger) \
 	module_driver(__led_trigger, led_trigger_register, \
 		      led_trigger_unregister)
@@ -544,6 +553,12 @@ static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
 	return NULL;
 }
 
+static inline enum led_brightness
+led_trigger_get_brightness(const struct led_trigger *trigger)
+{
+	return LED_OFF;
+}
+
 #endif /* CONFIG_LEDS_TRIGGERS */
 
 /* Trigger specific enum */
-- 
2.43.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/4] ALSA: control-led: Integrate mute led trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
  2024-02-13  7:31 ` [PATCH 1/4] leds: trigger: Store brightness set by led_trigger_event() Heiner Kallweit
@ 2024-02-13  7:32 ` Heiner Kallweit
  2024-02-13  7:33 ` [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER Heiner Kallweit
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-13  7:32 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer
  Cc: linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
	linux-mips

This driver is the only one calling ledtrig_audio_set(), therefore
the LED audio trigger isn't usable standalone. So it makes sense
to fully integrate LED audio triger handling here.

The module aliases ensure that the driver is auto-loaded (if built
as module) if a LED device has one of the two audio triggers as
default trigger.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 sound/core/control_led.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sound/core/control_led.c b/sound/core/control_led.c
index a78eb4892..b9d650380 100644
--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -53,6 +53,7 @@ struct snd_ctl_led_ctl {
 
 static DEFINE_MUTEX(snd_ctl_led_mutex);
 static bool snd_ctl_led_card_valid[SNDRV_CARDS];
+static struct led_trigger *snd_ctl_ledtrig_audio[NUM_AUDIO_LEDS];
 static struct snd_ctl_led snd_ctl_leds[MAX_LED] = {
 	{
 		.name = "speaker",
@@ -176,8 +177,11 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
 	case MODE_FOLLOW_ROUTE:	if (route >= 0) route ^= 1; break;
 	case MODE_FOLLOW_MUTE:	/* noop */ break;
 	}
-	if (route >= 0)
-		ledtrig_audio_set(led->trigger_type, route ? LED_OFF : LED_ON);
+	if (route >= 0) {
+		struct led_trigger *trig = snd_ctl_ledtrig_audio[led->trigger_type];
+
+		led_trigger_event(trig, route ? LED_OFF : LED_ON);
+	}
 }
 
 static struct snd_ctl_led_ctl *snd_ctl_led_find(struct snd_kcontrol *kctl, unsigned int ioff)
@@ -442,8 +446,9 @@ static ssize_t brightness_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
 	struct snd_ctl_led *led = container_of(dev, struct snd_ctl_led, dev);
+	struct led_trigger *trig = snd_ctl_ledtrig_audio[led->trigger_type];
 
-	return sysfs_emit(buf, "%u\n", ledtrig_audio_get(led->trigger_type));
+	return sysfs_emit(buf, "%u\n", led_trigger_get_brightness(trig));
 }
 
 static DEVICE_ATTR_RW(mode);
@@ -736,6 +741,9 @@ static int __init snd_ctl_led_init(void)
 	struct snd_ctl_led *led;
 	unsigned int group;
 
+	led_trigger_register_simple("audio-mute", &snd_ctl_ledtrig_audio[LED_AUDIO_MUTE]);
+	led_trigger_register_simple("audio-micmute", &snd_ctl_ledtrig_audio[LED_AUDIO_MICMUTE]);
+
 	device_initialize(&snd_ctl_led_dev);
 	snd_ctl_led_dev.class = &sound_class;
 	snd_ctl_led_dev.release = snd_ctl_led_dev_release;
@@ -788,7 +796,13 @@ static void __exit snd_ctl_led_exit(void)
 	}
 	device_unregister(&snd_ctl_led_dev);
 	snd_ctl_led_clean(NULL);
+
+	led_trigger_unregister_simple(snd_ctl_ledtrig_audio[LED_AUDIO_MUTE]);
+	led_trigger_unregister_simple(snd_ctl_ledtrig_audio[LED_AUDIO_MICMUTE]);
 }
 
 module_init(snd_ctl_led_init)
 module_exit(snd_ctl_led_exit)
+
+MODULE_ALIAS("ledtrig:audio-mute");
+MODULE_ALIAS("ledtrig:audio-micmute");
-- 
2.43.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
  2024-02-13  7:31 ` [PATCH 1/4] leds: trigger: Store brightness set by led_trigger_event() Heiner Kallweit
  2024-02-13  7:32 ` [PATCH 2/4] ALSA: control-led: Integrate mute led trigger Heiner Kallweit
@ 2024-02-13  7:33 ` Heiner Kallweit
  2024-02-23 23:23   ` Dmitry Torokhov
  2024-02-13  7:34 ` [PATCH 4/4] leds: trigger: audio: Remove this trigger Heiner Kallweit
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-13  7:33 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer
  Cc: linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
	linux-mips

In a follow-up patch handling of the LED audio trigger will be changed,
including removal of config symbol LEDS_AUDIO_TRIGGER. Therefore set
the default trigger unconditionally to "audio-mute". It does no harm
if a default trigger doesn't exist.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/input/input-leds.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
index b16fc8194..176f1da7f 100644
--- a/drivers/input/input-leds.c
+++ b/drivers/input/input-leds.c
@@ -18,12 +18,6 @@
 #define VT_TRIGGER(_name)	.trigger = NULL
 #endif
 
-#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
-#define AUDIO_TRIGGER(_name)	.trigger = _name
-#else
-#define AUDIO_TRIGGER(_name)	.trigger = NULL
-#endif
-
 static const struct {
 	const char *name;
 	const char *trigger;
@@ -35,7 +29,7 @@ static const struct {
 	[LED_KANA]	= { "kana", VT_TRIGGER("kbd-kanalock") },
 	[LED_SLEEP]	= { "sleep" } ,
 	[LED_SUSPEND]	= { "suspend" },
-	[LED_MUTE]	= { "mute", AUDIO_TRIGGER("audio-mute") },
+	[LED_MUTE]	= { "mute", "audio-mute" },
 	[LED_MISC]	= { "misc" },
 	[LED_MAIL]	= { "mail" },
 	[LED_CHARGING]	= { "charging" },
-- 
2.43.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/4] leds: trigger: audio: Remove this trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
                   ` (2 preceding siblings ...)
  2024-02-13  7:33 ` [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER Heiner Kallweit
@ 2024-02-13  7:34 ` Heiner Kallweit
  2024-02-15 12:29 ` [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Takashi Iwai
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-13  7:34 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer
  Cc: linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
	linux-mips

Now that the audio trigger is fully integrated in
sound/core/control_led.c, we can remove it here.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 arch/mips/configs/ci20_defconfig     |  1 -
 drivers/leds/trigger/Kconfig         |  7 ---
 drivers/leds/trigger/Makefile        |  1 -
 drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
 include/linux/leds.h                 | 14 ------
 sound/core/Kconfig                   |  1 -
 6 files changed, 91 deletions(-)
 delete mode 100644 drivers/leds/trigger/ledtrig-audio.c

diff --git a/arch/mips/configs/ci20_defconfig b/arch/mips/configs/ci20_defconfig
index cdf2a782d..7827b2b39 100644
--- a/arch/mips/configs/ci20_defconfig
+++ b/arch/mips/configs/ci20_defconfig
@@ -152,7 +152,6 @@ CONFIG_LEDS_TRIGGER_CAMERA=m
 CONFIG_LEDS_TRIGGER_PANIC=y
 CONFIG_LEDS_TRIGGER_NETDEV=y
 CONFIG_LEDS_TRIGGER_PATTERN=y
-CONFIG_LEDS_TRIGGER_AUDIO=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_JZ4740=y
 CONFIG_DMADEVICES=y
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index d11d80176..31576952e 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -136,13 +136,6 @@ config LEDS_TRIGGER_PATTERN
 	  which is a series of tuples, of brightness and duration (ms).
 	  If unsure, say N
 
-config LEDS_TRIGGER_AUDIO
-	tristate "Audio Mute LED Trigger"
-	help
-	  This allows LEDs to be controlled by audio drivers for following
-	  the audio mute and mic-mute changes.
-	  If unsure, say N
-
 config LEDS_TRIGGER_TTY
 	tristate "LED Trigger for TTY devices"
 	depends on TTY
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 25c4db97c..242f6c4e3 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -14,5 +14,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
 obj-$(CONFIG_LEDS_TRIGGER_PANIC)	+= ledtrig-panic.o
 obj-$(CONFIG_LEDS_TRIGGER_NETDEV)	+= ledtrig-netdev.o
 obj-$(CONFIG_LEDS_TRIGGER_PATTERN)	+= ledtrig-pattern.o
-obj-$(CONFIG_LEDS_TRIGGER_AUDIO)	+= ledtrig-audio.o
 obj-$(CONFIG_LEDS_TRIGGER_TTY)		+= ledtrig-tty.o
diff --git a/drivers/leds/trigger/ledtrig-audio.c b/drivers/leds/trigger/ledtrig-audio.c
deleted file mode 100644
index 2ecd4b760..000000000
--- a/drivers/leds/trigger/ledtrig-audio.c
+++ /dev/null
@@ -1,67 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-//
-// Audio Mute LED trigger
-//
-
-#include <linux/kernel.h>
-#include <linux/leds.h>
-#include <linux/module.h>
-#include "../leds.h"
-
-static enum led_brightness audio_state[NUM_AUDIO_LEDS];
-
-static int ledtrig_audio_mute_activate(struct led_classdev *led_cdev)
-{
-	led_set_brightness_nosleep(led_cdev, audio_state[LED_AUDIO_MUTE]);
-	return 0;
-}
-
-static int ledtrig_audio_micmute_activate(struct led_classdev *led_cdev)
-{
-	led_set_brightness_nosleep(led_cdev, audio_state[LED_AUDIO_MICMUTE]);
-	return 0;
-}
-
-static struct led_trigger ledtrig_audio[NUM_AUDIO_LEDS] = {
-	[LED_AUDIO_MUTE] = {
-		.name     = "audio-mute",
-		.activate = ledtrig_audio_mute_activate,
-	},
-	[LED_AUDIO_MICMUTE] = {
-		.name     = "audio-micmute",
-		.activate = ledtrig_audio_micmute_activate,
-	},
-};
-
-enum led_brightness ledtrig_audio_get(enum led_audio type)
-{
-	return audio_state[type];
-}
-EXPORT_SYMBOL_GPL(ledtrig_audio_get);
-
-void ledtrig_audio_set(enum led_audio type, enum led_brightness state)
-{
-	audio_state[type] = state;
-	led_trigger_event(&ledtrig_audio[type], state);
-}
-EXPORT_SYMBOL_GPL(ledtrig_audio_set);
-
-static int __init ledtrig_audio_init(void)
-{
-	led_trigger_register(&ledtrig_audio[LED_AUDIO_MUTE]);
-	led_trigger_register(&ledtrig_audio[LED_AUDIO_MICMUTE]);
-	return 0;
-}
-module_init(ledtrig_audio_init);
-
-static void __exit ledtrig_audio_exit(void)
-{
-	led_trigger_unregister(&ledtrig_audio[LED_AUDIO_MUTE]);
-	led_trigger_unregister(&ledtrig_audio[LED_AUDIO_MICMUTE]);
-}
-module_exit(ledtrig_audio_exit);
-
-MODULE_DESCRIPTION("LED trigger for audio mute control");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("ledtrig:audio-mute");
-MODULE_ALIAS("ledtrig:audio-micmute");
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 48fff5980..d2668b427 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -705,18 +705,4 @@ enum led_audio {
 	NUM_AUDIO_LEDS
 };
 
-#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
-enum led_brightness ledtrig_audio_get(enum led_audio type);
-void ledtrig_audio_set(enum led_audio type, enum led_brightness state);
-#else
-static inline enum led_brightness ledtrig_audio_get(enum led_audio type)
-{
-	return LED_OFF;
-}
-static inline void ledtrig_audio_set(enum led_audio type,
-				     enum led_brightness state)
-{
-}
-#endif
-
 #endif		/* __LINUX_LEDS_H_INCLUDED */
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 8077f481d..b970a1734 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -262,6 +262,5 @@ config SND_CTL_LED
 	tristate
 	select NEW_LEDS if SND_CTL_LED
 	select LEDS_TRIGGERS if SND_CTL_LED
-	select LEDS_TRIGGER_AUDIO if SND_CTL_LED
 
 source "sound/core/seq/Kconfig"
-- 
2.43.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
                   ` (3 preceding siblings ...)
  2024-02-13  7:34 ` [PATCH 4/4] leds: trigger: audio: Remove this trigger Heiner Kallweit
@ 2024-02-15 12:29 ` Takashi Iwai
  2024-02-16 12:18   ` Heiner Kallweit
  2024-02-23 15:45 ` Lee Jones
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2024-02-15 12:29 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
	linux-sound, open list:HID CORE LAYER, linux-mips

On Tue, 13 Feb 2024 08:30:30 +0100,
Heiner Kallweit wrote:
> 
> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
> 
> This series touches few subsystems. I'd propose to handle it via
> the LED subsystem.
> 
> Heiner Kallweit (4):
>   leds: trigger: Store brightness set by led_trigger_event()
>   ALSA: control-led: Integrate mute led trigger
>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
>   leds: trigger: audio: Remove this trigger

LGTM.

Reviewed-by: Takashi Iwai <tiwai@suse.de>

One thing I'm not 100% sure is the movement from ledtrig:audio-mute
and ledtrig:audio-micmute alias into snd-ctl-led module.  Who would
use/process those aliases?  I don't think this would be a problem, but
it might change the loading order.


Thanks!

Takashi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-15 12:29 ` [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Takashi Iwai
@ 2024-02-16 12:18   ` Heiner Kallweit
  0 siblings, 0 replies; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-16 12:18 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
	linux-sound, open list:HID CORE LAYER, linux-mips

On 15.02.2024 13:29, Takashi Iwai wrote:
> On Tue, 13 Feb 2024 08:30:30 +0100,
> Heiner Kallweit wrote:
>>
>> If a simple trigger is assigned to a LED, then the LED may be off until
>> the next led_trigger_event() call. This may be an issue for simple
>> triggers with rare led_trigger_event() calls, e.g. power supply
>> charging indicators (drivers/power/supply/power_supply_leds.c).
>> Therefore persist the brightness value of the last led_trigger_event()
>> call and use this value if the trigger is assigned to a LED.
>> This change allows to use simple triggers in more cases.
>> As a first use case simplify handling of the mute audio trigger.
>>
>> This series touches few subsystems. I'd propose to handle it via
>> the LED subsystem.
>>
>> Heiner Kallweit (4):
>>   leds: trigger: Store brightness set by led_trigger_event()
>>   ALSA: control-led: Integrate mute led trigger
>>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
>>   leds: trigger: audio: Remove this trigger
> 
> LGTM.
> 
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> 
> One thing I'm not 100% sure is the movement from ledtrig:audio-mute
> and ledtrig:audio-micmute alias into snd-ctl-led module.  Who would
> use/process those aliases?  I don't think this would be a problem, but
> it might change the loading order.
> 
The ledtrig:% aliases are used when a LED device is registered that has
a default trigger. Like in the case here with the input leds (patch 3).
There might also be DT-defined LEDs with a audio mute default trigger.
snd-ctl-led has a dependency on snd, so at least wrt snd the load order
doesn't change.

> 
> Thanks!
> 
> Takashi

Heiner

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
                   ` (4 preceding siblings ...)
  2024-02-15 12:29 ` [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Takashi Iwai
@ 2024-02-23 15:45 ` Lee Jones
  2024-02-23 15:47   ` Takashi Iwai
  2024-02-23 16:05 ` Lee Jones
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Lee Jones @ 2024-02-23 15:45 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On Tue, 13 Feb 2024, Heiner Kallweit wrote:

> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
> 
> This series touches few subsystems. I'd propose to handle it via
> the LED subsystem.
> 
> Heiner Kallweit (4):
>   leds: trigger: Store brightness set by led_trigger_event()
>   ALSA: control-led: Integrate mute led trigger
>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
>   leds: trigger: audio: Remove this trigger
> 
>  arch/mips/configs/ci20_defconfig     |  1 -
>  drivers/input/input-leds.c           |  8 +---
>  drivers/leds/led-triggers.c          |  6 ++-
>  drivers/leds/trigger/Kconfig         |  7 ---
>  drivers/leds/trigger/Makefile        |  1 -
>  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
>  include/linux/leds.h                 | 29 ++++++------
>  sound/core/Kconfig                   |  1 -
>  sound/core/control_led.c             | 20 +++++++--
>  9 files changed, 37 insertions(+), 103 deletions(-)
>  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c

Are the sound maintainers on-board with this?

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-23 15:45 ` Lee Jones
@ 2024-02-23 15:47   ` Takashi Iwai
  2024-02-23 16:04     ` Lee Jones
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2024-02-23 15:47 UTC (permalink / raw)
  To: Lee Jones
  Cc: Heiner Kallweit, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
	linux-sound, open list:HID CORE LAYER, linux-mips

On Fri, 23 Feb 2024 16:45:59 +0100,
Lee Jones wrote:
> 
> On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> 
> > If a simple trigger is assigned to a LED, then the LED may be off until
> > the next led_trigger_event() call. This may be an issue for simple
> > triggers with rare led_trigger_event() calls, e.g. power supply
> > charging indicators (drivers/power/supply/power_supply_leds.c).
> > Therefore persist the brightness value of the last led_trigger_event()
> > call and use this value if the trigger is assigned to a LED.
> > This change allows to use simple triggers in more cases.
> > As a first use case simplify handling of the mute audio trigger.
> > 
> > This series touches few subsystems. I'd propose to handle it via
> > the LED subsystem.
> > 
> > Heiner Kallweit (4):
> >   leds: trigger: Store brightness set by led_trigger_event()
> >   ALSA: control-led: Integrate mute led trigger
> >   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> >   leds: trigger: audio: Remove this trigger
> > 
> >  arch/mips/configs/ci20_defconfig     |  1 -
> >  drivers/input/input-leds.c           |  8 +---
> >  drivers/leds/led-triggers.c          |  6 ++-
> >  drivers/leds/trigger/Kconfig         |  7 ---
> >  drivers/leds/trigger/Makefile        |  1 -
> >  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> >  include/linux/leds.h                 | 29 ++++++------
> >  sound/core/Kconfig                   |  1 -
> >  sound/core/control_led.c             | 20 +++++++--
> >  9 files changed, 37 insertions(+), 103 deletions(-)
> >  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> 
> Are the sound maintainers on-board with this?

See
  https://lore.kernel.org/r/87zfw1ewrd.wl-tiwai@suse.de


thanks,

Takashi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-23 15:47   ` Takashi Iwai
@ 2024-02-23 16:04     ` Lee Jones
  2024-02-23 16:05       ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: Lee Jones @ 2024-02-23 16:04 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Heiner Kallweit, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
	linux-sound, open list:HID CORE LAYER, linux-mips

On Fri, 23 Feb 2024, Takashi Iwai wrote:

> On Fri, 23 Feb 2024 16:45:59 +0100,
> Lee Jones wrote:
> > 
> > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> > 
> > > If a simple trigger is assigned to a LED, then the LED may be off until
> > > the next led_trigger_event() call. This may be an issue for simple
> > > triggers with rare led_trigger_event() calls, e.g. power supply
> > > charging indicators (drivers/power/supply/power_supply_leds.c).
> > > Therefore persist the brightness value of the last led_trigger_event()
> > > call and use this value if the trigger is assigned to a LED.
> > > This change allows to use simple triggers in more cases.
> > > As a first use case simplify handling of the mute audio trigger.
> > > 
> > > This series touches few subsystems. I'd propose to handle it via
> > > the LED subsystem.
> > > 
> > > Heiner Kallweit (4):
> > >   leds: trigger: Store brightness set by led_trigger_event()
> > >   ALSA: control-led: Integrate mute led trigger
> > >   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > >   leds: trigger: audio: Remove this trigger
> > > 
> > >  arch/mips/configs/ci20_defconfig     |  1 -
> > >  drivers/input/input-leds.c           |  8 +---
> > >  drivers/leds/led-triggers.c          |  6 ++-
> > >  drivers/leds/trigger/Kconfig         |  7 ---
> > >  drivers/leds/trigger/Makefile        |  1 -
> > >  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> > >  include/linux/leds.h                 | 29 ++++++------
> > >  sound/core/Kconfig                   |  1 -
> > >  sound/core/control_led.c             | 20 +++++++--
> > >  9 files changed, 37 insertions(+), 103 deletions(-)
> > >  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> > 
> > Are the sound maintainers on-board with this?
> 
> See
>   https://lore.kernel.org/r/87zfw1ewrd.wl-tiwai@suse.de

Were you happy with Heiner's response?

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
                   ` (5 preceding siblings ...)
  2024-02-23 15:45 ` Lee Jones
@ 2024-02-23 16:05 ` Lee Jones
  2024-02-29 17:26 ` Lee Jones
  2024-03-05 12:08 ` Lee Jones
  8 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2024-02-23 16:05 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On Tue, 13 Feb 2024, Heiner Kallweit wrote:

> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
> 
> This series touches few subsystems. I'd propose to handle it via
> the LED subsystem.
> 
> Heiner Kallweit (4):
>   leds: trigger: Store brightness set by led_trigger_event()
>   ALSA: control-led: Integrate mute led trigger
>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER

Dmitry, you happy too?

>   leds: trigger: audio: Remove this trigger
> 
>  arch/mips/configs/ci20_defconfig     |  1 -
>  drivers/input/input-leds.c           |  8 +---
>  drivers/leds/led-triggers.c          |  6 ++-
>  drivers/leds/trigger/Kconfig         |  7 ---
>  drivers/leds/trigger/Makefile        |  1 -
>  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
>  include/linux/leds.h                 | 29 ++++++------
>  sound/core/Kconfig                   |  1 -
>  sound/core/control_led.c             | 20 +++++++--
>  9 files changed, 37 insertions(+), 103 deletions(-)
>  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> 
> -- 
> 2.43.1
> 

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-23 16:04     ` Lee Jones
@ 2024-02-23 16:05       ` Takashi Iwai
  0 siblings, 0 replies; 21+ messages in thread
From: Takashi Iwai @ 2024-02-23 16:05 UTC (permalink / raw)
  To: Lee Jones
  Cc: Takashi Iwai, Heiner Kallweit, Pavel Machek, Jaroslav Kysela,
	Takashi Iwai, Dmitry Torokhov, Thomas Bogendoerfer,
	linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
	linux-mips

On Fri, 23 Feb 2024 17:04:15 +0100,
Lee Jones wrote:
> 
> On Fri, 23 Feb 2024, Takashi Iwai wrote:
> 
> > On Fri, 23 Feb 2024 16:45:59 +0100,
> > Lee Jones wrote:
> > > 
> > > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> > > 
> > > > If a simple trigger is assigned to a LED, then the LED may be off until
> > > > the next led_trigger_event() call. This may be an issue for simple
> > > > triggers with rare led_trigger_event() calls, e.g. power supply
> > > > charging indicators (drivers/power/supply/power_supply_leds.c).
> > > > Therefore persist the brightness value of the last led_trigger_event()
> > > > call and use this value if the trigger is assigned to a LED.
> > > > This change allows to use simple triggers in more cases.
> > > > As a first use case simplify handling of the mute audio trigger.
> > > > 
> > > > This series touches few subsystems. I'd propose to handle it via
> > > > the LED subsystem.
> > > > 
> > > > Heiner Kallweit (4):
> > > >   leds: trigger: Store brightness set by led_trigger_event()
> > > >   ALSA: control-led: Integrate mute led trigger
> > > >   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > > >   leds: trigger: audio: Remove this trigger
> > > > 
> > > >  arch/mips/configs/ci20_defconfig     |  1 -
> > > >  drivers/input/input-leds.c           |  8 +---
> > > >  drivers/leds/led-triggers.c          |  6 ++-
> > > >  drivers/leds/trigger/Kconfig         |  7 ---
> > > >  drivers/leds/trigger/Makefile        |  1 -
> > > >  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> > > >  include/linux/leds.h                 | 29 ++++++------
> > > >  sound/core/Kconfig                   |  1 -
> > > >  sound/core/control_led.c             | 20 +++++++--
> > > >  9 files changed, 37 insertions(+), 103 deletions(-)
> > > >  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> > > 
> > > Are the sound maintainers on-board with this?
> > 
> > See
> >   https://lore.kernel.org/r/87zfw1ewrd.wl-tiwai@suse.de
> 
> Were you happy with Heiner's response?

Yes.


Takashi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
  2024-02-13  7:33 ` [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER Heiner Kallweit
@ 2024-02-23 23:23   ` Dmitry Torokhov
  2024-02-24  9:31     ` Heiner Kallweit
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Torokhov @ 2024-02-23 23:23 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On Tue, Feb 13, 2024 at 08:33:24AM +0100, Heiner Kallweit wrote:
> In a follow-up patch handling of the LED audio trigger will be changed,
> including removal of config symbol LEDS_AUDIO_TRIGGER. Therefore set
> the default trigger unconditionally to "audio-mute". It does no harm
> if a default trigger doesn't exist.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/input/input-leds.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> index b16fc8194..176f1da7f 100644
> --- a/drivers/input/input-leds.c
> +++ b/drivers/input/input-leds.c
> @@ -18,12 +18,6 @@
>  #define VT_TRIGGER(_name)	.trigger = NULL
>  #endif
>  
> -#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)

Should it be simply changed to CONFIG_SND_CTL_LED?

> -#define AUDIO_TRIGGER(_name)	.trigger = _name
> -#else
> -#define AUDIO_TRIGGER(_name)	.trigger = NULL
> -#endif
> -
>  static const struct {
>  	const char *name;
>  	const char *trigger;
> @@ -35,7 +29,7 @@ static const struct {
>  	[LED_KANA]	= { "kana", VT_TRIGGER("kbd-kanalock") },
>  	[LED_SLEEP]	= { "sleep" } ,
>  	[LED_SUSPEND]	= { "suspend" },
> -	[LED_MUTE]	= { "mute", AUDIO_TRIGGER("audio-mute") },
> +	[LED_MUTE]	= { "mute", "audio-mute" },
>  	[LED_MISC]	= { "misc" },
>  	[LED_MAIL]	= { "mail" },
>  	[LED_CHARGING]	= { "charging" },
> -- 
> 2.43.1
> 
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
  2024-02-23 23:23   ` Dmitry Torokhov
@ 2024-02-24  9:31     ` Heiner Kallweit
  2024-02-26 17:49       ` Dmitry Torokhov
  0 siblings, 1 reply; 21+ messages in thread
From: Heiner Kallweit @ 2024-02-24  9:31 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On 24.02.2024 00:23, Dmitry Torokhov wrote:
> On Tue, Feb 13, 2024 at 08:33:24AM +0100, Heiner Kallweit wrote:
>> In a follow-up patch handling of the LED audio trigger will be changed,
>> including removal of config symbol LEDS_AUDIO_TRIGGER. Therefore set
>> the default trigger unconditionally to "audio-mute". It does no harm
>> if a default trigger doesn't exist.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/input/input-leds.c | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
>> index b16fc8194..176f1da7f 100644
>> --- a/drivers/input/input-leds.c
>> +++ b/drivers/input/input-leds.c
>> @@ -18,12 +18,6 @@
>>  #define VT_TRIGGER(_name)	.trigger = NULL
>>  #endif
>>  
>> -#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
> 
> Should it be simply changed to CONFIG_SND_CTL_LED?
> 
This would be another option. What is better IMO is a matter of
personal taste. Setting the default trigger unconditionally may
cause a negligible runtime overhead when the LED is instantiated,
on the other hand it results in less code to be maintained.
Do you have a preference?

>> -#define AUDIO_TRIGGER(_name)	.trigger = _name
>> -#else
>> -#define AUDIO_TRIGGER(_name)	.trigger = NULL
>> -#endif
>> -
>>  static const struct {
>>  	const char *name;
>>  	const char *trigger;
>> @@ -35,7 +29,7 @@ static const struct {
>>  	[LED_KANA]	= { "kana", VT_TRIGGER("kbd-kanalock") },
>>  	[LED_SLEEP]	= { "sleep" } ,
>>  	[LED_SUSPEND]	= { "suspend" },
>> -	[LED_MUTE]	= { "mute", AUDIO_TRIGGER("audio-mute") },
>> +	[LED_MUTE]	= { "mute", "audio-mute" },
>>  	[LED_MISC]	= { "misc" },
>>  	[LED_MAIL]	= { "mail" },
>>  	[LED_CHARGING]	= { "charging" },
>> -- 
>> 2.43.1
>>
>>
> 
> Thanks.
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
  2024-02-24  9:31     ` Heiner Kallweit
@ 2024-02-26 17:49       ` Dmitry Torokhov
  0 siblings, 0 replies; 21+ messages in thread
From: Dmitry Torokhov @ 2024-02-26 17:49 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On Sat, Feb 24, 2024 at 10:31:17AM +0100, Heiner Kallweit wrote:
> On 24.02.2024 00:23, Dmitry Torokhov wrote:
> > On Tue, Feb 13, 2024 at 08:33:24AM +0100, Heiner Kallweit wrote:
> >> In a follow-up patch handling of the LED audio trigger will be changed,
> >> including removal of config symbol LEDS_AUDIO_TRIGGER. Therefore set
> >> the default trigger unconditionally to "audio-mute". It does no harm
> >> if a default trigger doesn't exist.
> >>
> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> >> ---
> >>  drivers/input/input-leds.c | 8 +-------
> >>  1 file changed, 1 insertion(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> >> index b16fc8194..176f1da7f 100644
> >> --- a/drivers/input/input-leds.c
> >> +++ b/drivers/input/input-leds.c
> >> @@ -18,12 +18,6 @@
> >>  #define VT_TRIGGER(_name)	.trigger = NULL
> >>  #endif
> >>  
> >> -#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
> > 
> > Should it be simply changed to CONFIG_SND_CTL_LED?
> > 
> This would be another option. What is better IMO is a matter of
> personal taste. Setting the default trigger unconditionally may
> cause a negligible runtime overhead when the LED is instantiated,
> on the other hand it results in less code to be maintained.
> Do you have a preference?

I am less concerned about overhead than setting a non-existent default
trigger if the functionality is disabled. I would prefer having the
#ifdef.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
                   ` (6 preceding siblings ...)
  2024-02-23 16:05 ` Lee Jones
@ 2024-02-29 17:26 ` Lee Jones
  2024-03-02 15:09   ` Heiner Kallweit
  2024-03-05 12:08 ` Lee Jones
  8 siblings, 1 reply; 21+ messages in thread
From: Lee Jones @ 2024-02-29 17:26 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On Tue, 13 Feb 2024, Heiner Kallweit wrote:

> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
> 
> This series touches few subsystems. I'd propose to handle it via
> the LED subsystem.
> 
> Heiner Kallweit (4):
>   leds: trigger: Store brightness set by led_trigger_event()
>   ALSA: control-led: Integrate mute led trigger
>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
>   leds: trigger: audio: Remove this trigger
> 
>  arch/mips/configs/ci20_defconfig     |  1 -

>  drivers/input/input-leds.c           |  8 +---

This does not apply.

Please rebase onto v6.8-rc1.

>  drivers/leds/led-triggers.c          |  6 ++-
>  drivers/leds/trigger/Kconfig         |  7 ---
>  drivers/leds/trigger/Makefile        |  1 -
>  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
>  include/linux/leds.h                 | 29 ++++++------
>  sound/core/Kconfig                   |  1 -
>  sound/core/control_led.c             | 20 +++++++--
>  9 files changed, 37 insertions(+), 103 deletions(-)
>  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> 
> -- 
> 2.43.1
> 

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-29 17:26 ` Lee Jones
@ 2024-03-02 15:09   ` Heiner Kallweit
  2024-03-05  9:55     ` Lee Jones
  0 siblings, 1 reply; 21+ messages in thread
From: Heiner Kallweit @ 2024-03-02 15:09 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On 29.02.2024 18:26, Lee Jones wrote:
> On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> 
>> If a simple trigger is assigned to a LED, then the LED may be off until
>> the next led_trigger_event() call. This may be an issue for simple
>> triggers with rare led_trigger_event() calls, e.g. power supply
>> charging indicators (drivers/power/supply/power_supply_leds.c).
>> Therefore persist the brightness value of the last led_trigger_event()
>> call and use this value if the trigger is assigned to a LED.
>> This change allows to use simple triggers in more cases.
>> As a first use case simplify handling of the mute audio trigger.
>>
>> This series touches few subsystems. I'd propose to handle it via
>> the LED subsystem.
>>
>> Heiner Kallweit (4):
>>   leds: trigger: Store brightness set by led_trigger_event()
>>   ALSA: control-led: Integrate mute led trigger
>>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
>>   leds: trigger: audio: Remove this trigger
>>
>>  arch/mips/configs/ci20_defconfig     |  1 -
> 
>>  drivers/input/input-leds.c           |  8 +---
> 
> This does not apply.
> 
> Please rebase onto v6.8-rc1.
> 
Since v6.8-rc1 the following has been added, which is touched by
my series:
698b43780ba2 ("Input: leds - set default-trigger for mute")

Rebasing onto v6.8-rc1 would mean:
- remove the change to input-leds from the series
- resubmit this change via input subsystem

This would affect bisectability, because for the time being
input-leds would reference a config symbol that doesn't exist
any longer.

We'd be fine only if the change to input-leds is applied first.
I think that's the best way to go, if you can't accept a series
based on linux-next.

>>  drivers/leds/led-triggers.c          |  6 ++-
>>  drivers/leds/trigger/Kconfig         |  7 ---
>>  drivers/leds/trigger/Makefile        |  1 -
>>  drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
>>  include/linux/leds.h                 | 29 ++++++------
>>  sound/core/Kconfig                   |  1 -
>>  sound/core/control_led.c             | 20 +++++++--
>>  9 files changed, 37 insertions(+), 103 deletions(-)
>>  delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
>>
>> -- 
>> 2.43.1
>>
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-03-02 15:09   ` Heiner Kallweit
@ 2024-03-05  9:55     ` Lee Jones
  2024-03-05 10:26       ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: Lee Jones @ 2024-03-05  9:55 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
	Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
	open list:HID CORE LAYER, linux-mips

On Sat, 02 Mar 2024, Heiner Kallweit wrote:

> On 29.02.2024 18:26, Lee Jones wrote:
> > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> > 
> >> If a simple trigger is assigned to a LED, then the LED may be off until
> >> the next led_trigger_event() call. This may be an issue for simple
> >> triggers with rare led_trigger_event() calls, e.g. power supply
> >> charging indicators (drivers/power/supply/power_supply_leds.c).
> >> Therefore persist the brightness value of the last led_trigger_event()
> >> call and use this value if the trigger is assigned to a LED.
> >> This change allows to use simple triggers in more cases.
> >> As a first use case simplify handling of the mute audio trigger.
> >>
> >> This series touches few subsystems. I'd propose to handle it via
> >> the LED subsystem.
> >>
> >> Heiner Kallweit (4):
> >>   leds: trigger: Store brightness set by led_trigger_event()
> >>   ALSA: control-led: Integrate mute led trigger
> >>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> >>   leds: trigger: audio: Remove this trigger
> >>
> >>  arch/mips/configs/ci20_defconfig     |  1 -
> > 
> >>  drivers/input/input-leds.c           |  8 +---
> > 
> > This does not apply.
> > 
> > Please rebase onto v6.8-rc1.
> > 
> Since v6.8-rc1 the following has been added, which is touched by
> my series:
> 698b43780ba2 ("Input: leds - set default-trigger for mute")
> 
> Rebasing onto v6.8-rc1 would mean:
> - remove the change to input-leds from the series
> - resubmit this change via input subsystem
> 
> This would affect bisectability, because for the time being
> input-leds would reference a config symbol that doesn't exist
> any longer.
> 
> We'd be fine only if the change to input-leds is applied first.
> I think that's the best way to go, if you can't accept a series
> based on linux-next.

Then it's going to have to wait until v6.10.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-03-05  9:55     ` Lee Jones
@ 2024-03-05 10:26       ` Takashi Iwai
  2024-03-05 10:40         ` Lee Jones
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2024-03-05 10:26 UTC (permalink / raw)
  To: Lee Jones
  Cc: Heiner Kallweit, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
	linux-sound, open list:HID CORE LAYER, linux-mips

On Tue, 05 Mar 2024 10:55:39 +0100,
Lee Jones wrote:
> 
> On Sat, 02 Mar 2024, Heiner Kallweit wrote:
> 
> > On 29.02.2024 18:26, Lee Jones wrote:
> > > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> > > 
> > >> If a simple trigger is assigned to a LED, then the LED may be off until
> > >> the next led_trigger_event() call. This may be an issue for simple
> > >> triggers with rare led_trigger_event() calls, e.g. power supply
> > >> charging indicators (drivers/power/supply/power_supply_leds.c).
> > >> Therefore persist the brightness value of the last led_trigger_event()
> > >> call and use this value if the trigger is assigned to a LED.
> > >> This change allows to use simple triggers in more cases.
> > >> As a first use case simplify handling of the mute audio trigger.
> > >>
> > >> This series touches few subsystems. I'd propose to handle it via
> > >> the LED subsystem.
> > >>
> > >> Heiner Kallweit (4):
> > >>   leds: trigger: Store brightness set by led_trigger_event()
> > >>   ALSA: control-led: Integrate mute led trigger
> > >>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > >>   leds: trigger: audio: Remove this trigger
> > >>
> > >>  arch/mips/configs/ci20_defconfig     |  1 -
> > > 
> > >>  drivers/input/input-leds.c           |  8 +---
> > > 
> > > This does not apply.
> > > 
> > > Please rebase onto v6.8-rc1.
> > > 
> > Since v6.8-rc1 the following has been added, which is touched by
> > my series:
> > 698b43780ba2 ("Input: leds - set default-trigger for mute")
> > 
> > Rebasing onto v6.8-rc1 would mean:
> > - remove the change to input-leds from the series
> > - resubmit this change via input subsystem
> > 
> > This would affect bisectability, because for the time being
> > input-leds would reference a config symbol that doesn't exist
> > any longer.
> > 
> > We'd be fine only if the change to input-leds is applied first.
> > I think that's the best way to go, if you can't accept a series
> > based on linux-next.
> 
> Then it's going to have to wait until v6.10.

Or merging via input tree?
The changes are relatively small and easy, after all.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-03-05 10:26       ` Takashi Iwai
@ 2024-03-05 10:40         ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2024-03-05 10:40 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Heiner Kallweit, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
	linux-sound, open list:HID CORE LAYER, linux-mips

On Tue, 05 Mar 2024, Takashi Iwai wrote:

> On Tue, 05 Mar 2024 10:55:39 +0100,
> Lee Jones wrote:
> > 
> > On Sat, 02 Mar 2024, Heiner Kallweit wrote:
> > 
> > > On 29.02.2024 18:26, Lee Jones wrote:
> > > > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> > > > 
> > > >> If a simple trigger is assigned to a LED, then the LED may be off until
> > > >> the next led_trigger_event() call. This may be an issue for simple
> > > >> triggers with rare led_trigger_event() calls, e.g. power supply
> > > >> charging indicators (drivers/power/supply/power_supply_leds.c).
> > > >> Therefore persist the brightness value of the last led_trigger_event()
> > > >> call and use this value if the trigger is assigned to a LED.
> > > >> This change allows to use simple triggers in more cases.
> > > >> As a first use case simplify handling of the mute audio trigger.
> > > >>
> > > >> This series touches few subsystems. I'd propose to handle it via
> > > >> the LED subsystem.
> > > >>
> > > >> Heiner Kallweit (4):
> > > >>   leds: trigger: Store brightness set by led_trigger_event()
> > > >>   ALSA: control-led: Integrate mute led trigger
> > > >>   Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > > >>   leds: trigger: audio: Remove this trigger
> > > >>
> > > >>  arch/mips/configs/ci20_defconfig     |  1 -
> > > > 
> > > >>  drivers/input/input-leds.c           |  8 +---
> > > > 
> > > > This does not apply.
> > > > 
> > > > Please rebase onto v6.8-rc1.
> > > > 
> > > Since v6.8-rc1 the following has been added, which is touched by
> > > my series:
> > > 698b43780ba2 ("Input: leds - set default-trigger for mute")
> > > 
> > > Rebasing onto v6.8-rc1 would mean:
> > > - remove the change to input-leds from the series
> > > - resubmit this change via input subsystem
> > > 
> > > This would affect bisectability, because for the time being
> > > input-leds would reference a config symbol that doesn't exist
> > > any longer.
> > > 
> > > We'd be fine only if the change to input-leds is applied first.
> > > I think that's the best way to go, if you can't accept a series
> > > based on linux-next.
> > 
> > Then it's going to have to wait until v6.10.
> 
> Or merging via input tree?
> The changes are relatively small and easy, after all.

That's likely to culminate in similar merge-conflicts when further
changes are made to:

  drivers/leds/led-triggers.c
  drivers/leds/trigger/Kconfig
  drivers/leds/trigger/Makefile
  drivers/leds/trigger/ledtrig-audio.c
  include/linux/leds.h

What happens if I take all but the Input change?  If this doesn't cause
build-failures and the Input change will definitely land in v6.9, it
could work.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
  2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
                   ` (7 preceding siblings ...)
  2024-02-29 17:26 ` Lee Jones
@ 2024-03-05 12:08 ` Lee Jones
  8 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2024-03-05 12:08 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
	Dmitry Torokhov, Thomas Bogendoerfer, Heiner Kallweit
  Cc: linux-leds, linux-sound, open list:HID CORE LAYER, linux-mips

On Tue, 13 Feb 2024 08:30:30 +0100, Heiner Kallweit wrote:
> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
> 
> [...]

Applied, thanks!

[1/4] leds: trigger: Store brightness set by led_trigger_event()
      commit: 575129855dee0e364af7df84a77ab5cca54b1442
[2/4] ALSA: control-led: Integrate mute led trigger
      commit: ba8adb1646ee498029ac12b20e792d9d0dd17920
[3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
      (no commit info)
[4/4] leds: trigger: audio: Remove this trigger
      (no commit info)

--
Lee Jones [李琼斯]


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2024-03-05 12:08 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-13  7:30 [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Heiner Kallweit
2024-02-13  7:31 ` [PATCH 1/4] leds: trigger: Store brightness set by led_trigger_event() Heiner Kallweit
2024-02-13  7:32 ` [PATCH 2/4] ALSA: control-led: Integrate mute led trigger Heiner Kallweit
2024-02-13  7:33 ` [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER Heiner Kallweit
2024-02-23 23:23   ` Dmitry Torokhov
2024-02-24  9:31     ` Heiner Kallweit
2024-02-26 17:49       ` Dmitry Torokhov
2024-02-13  7:34 ` [PATCH 4/4] leds: trigger: audio: Remove this trigger Heiner Kallweit
2024-02-15 12:29 ` [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger Takashi Iwai
2024-02-16 12:18   ` Heiner Kallweit
2024-02-23 15:45 ` Lee Jones
2024-02-23 15:47   ` Takashi Iwai
2024-02-23 16:04     ` Lee Jones
2024-02-23 16:05       ` Takashi Iwai
2024-02-23 16:05 ` Lee Jones
2024-02-29 17:26 ` Lee Jones
2024-03-02 15:09   ` Heiner Kallweit
2024-03-05  9:55     ` Lee Jones
2024-03-05 10:26       ` Takashi Iwai
2024-03-05 10:40         ` Lee Jones
2024-03-05 12:08 ` Lee Jones

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).