Thread (4 messages) 4 messages, 2 authors, 2021-08-10

Re: [PATCH] leds: trigger: Add invert attribute to ledtrig-audio

From: Phil Sutter <phil@nwl.cc>
Date: 2021-08-10 09:22:59

Hi Pavel,

On Mon, Aug 09, 2021 at 08:11:18PM +0200, Pavel Machek wrote:
quoted
Inverting micmute LED used to be possible via a mixer setting, but
conversion to LEDs class (probably) killed it. Re-establish the old
functionality via sysfs attribute in audio LED triggers.
So we have both invert and inverted attributes. Fun :-).
Hmm! :)

Are you talking about LED_BLINK_INVERT flag? I see a few triggers allow
inversion but didn't find LED drivers exporting such a property.
See sysfs-class-led and sysfs-class-led-trigger-oneshot.
I think I "copied" from oneshot trigger when writing this patch.
We definitely want this documented. We probably want this for most
triggers, maybe it should get one implementation in library somewhere?
Should this be an implicit attribute of simple triggers only or all? In
the latter case (which could simplify some triggers) I guess the value
inversion has to take place in led_set_brightness_nopm(), the lowest
level function triggers may use.

How does inversion work, actually? LED_OFF <-> LED_ON is trivial, but
what about LED_HALF and LED_FULL? Leaving LED_HALF as-is seems logical,
but the opposite of LED_OFF might be LED_ON or LED_FULL. Does
max_brightness determine that?
Otherwise it makes sense.
quoted
+static ssize_t do_invert_store(enum led_audio type,
+			       const char *buf, size_t size)
+{
+	unsigned long state;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &state);
+	if (ret)
+		return ret;
+
+	led_invert[type] = !!state;
+	ledtrig_audio_set(type, audio_state[type]);
Accepting 42 as valid value sounds wrong. Anyway, this should do what
oneshot trigger does.
Similarities to oneshot are not a coincidence. :)

Cheers, Phil
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help