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

[Jaroslav Kysela]

Dne 19. 03. 21 v 18:22 Takashi Iwai napsal(a):
> On Fri, 19 Mar 2021 17:34:39 +0100,
> Hans de Goede wrote:
>>
>> 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 at 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.
> 
> IMO, that's the question: how we'll enable this in future.  If the
> binding of the control/mute mapping is provided via UCM, it's supposed
> to be changeable by each user.  Then the current sysfs permission

Nope. We have two UCM boot sequences which are called from "alsactl init" only
now. So, respecting the security concerns, only root should "fiddle" with this
settings. So yes, udev + alsactl (or any script) executed as root.

> Through a quick glance over the series, I'm fine to take those
> patches, but the only concern is the sysfs entries.  Basically, once
> when we use sysfs entries, it's set in stone.  So we should be very
> clear about our strategy how to deploy the control/mute mapping
> regarding using those sysfs entries.
> 
> OTOH, if the interface is thought for debugging or development
> purpose, it could be done in debugfs, which we can keep playing in
> further development, too.

We need attach / detach (reset and list operations are optional, but nice to
have). If you have any other idea, let me know.

> And, BTW, the mute LED mode setup doesn't have to be sysfs entries;
> we'd need primarily only the flags for inverted LED behavior, and
> those are only two, so it could be simply module options.  Then it's
> even easier for users to set up than tweaking sysfs entries.

I don't insist to control this over sysfs. But if sysfs is in the game, it's
nice to have the runtime control for this. The module parameter may be added
to modify the default value.

						Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.