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

[Phil Sutter <phil@nwl.cc>]

Hi Pavel,

On Mon, Aug 09, 2021 at 08:11:18PM +0200, Pavel Machek wrote:
> > 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.
> 
> > +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