Re: [PATCH v4 6/6] ALSA: led control - add sysfs kcontrol LED marking layer

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 3/19/21 7:11 PM, Jaroslav Kysela wrote:
> Dne 19. 03. 21 v 17:34 Hans de Goede napsal(a):
>> Hi,
>>
>> On 3/17/21 6:29 PM, Jaroslav Kysela wrote:
>>> We need to manage the kcontrol entries association for the LED trigger
>>> from the user space. This patch adds a layer to the sysfs tree like:
>>>
>>> /sys/devices/virtual/sound/ctl-led/mic
>>>    + card0
>>>    |  + attach
>>>    |  + detach
>>>    |  ...
>>>    + card1
>>>       + attach
>>>       ...
>>>
>>> Operations:
>>>
>>>   attach and detach
>>>     - amixer style ID is accepted and easy strings for numid and
>>>       simple names
>>>   reset
>>>     - reset all associated kcontrol entries
>>>   list
>>>     - list associated kcontrol entries (numid values only)
>>>
>>> Additional symlinks:
>>>
>>> /sys/devices/virtual/sound/ctl-led/mic/card0/card ->
>>>   /sys/class/sound/card0
>>>
>>> /sys/class/sound/card0/controlC0/led-mic ->
>>>   /sys/devices/virtual/sound/ctl-led/mic/card0
>>>
>>> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
>>
>> Thank you so much for this patch.
>>
>> I've given this new version a try, dropping my sound/soc/codecs/rt56??.c patches to set the access-flags directly.
>>
>> And with these 3 lines in /etc/rc.d/rc.local I get nicely working control of the mute
>> LED build into the (detachable) USB-keyboard's mute hot-key:
>>
>> modprobe snd_ctl_led
>> echo -n name="Speaker Channel Switch" > /sys/class/sound/card1/controlC1/led-speaker/attach
>> echo -n name="HP Channel Switch" > /sys/class/sound/card1/controlC1/led-speaker/attach
>>
>> This needs to be replaced by some UCM profile code doing the equivalent of course,
>> but for a proof-of-concept test of the kernel API this introduces the above will do.
> 
> I added already the FixedBootSequence support to alsa-lib and alsactl and the "sysset" sequence command.

Oh, nice.

I'll look into testing the FixedBootSequence + sysset changes to replace my rc.local hack.

One thing still seems to missing from the puzzle though, what about the "modprobe snd_ctl_led",
I guess we only want to do that on systems where that module is necessary, which means also having
a command for that in the FixedBootSequence ?

I guess we can use the "exec" keyword in the FixedBootSequence to do the modprobe ?

> But looking to this command now, it may be better to rename it to "sysw" ("double s" does not look so great).

"sysset" is fine by me, but so is "sysw" .

Regards,

Hans