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

From: Jaroslav Kysela <perex@perex.cz>
To: Hans de Goede <hdegoede@redhat.com>,
	ALSA development <alsa-devel@alsa-project.org>
Cc: Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH v4 6/6] ALSA: led control - add sysfs kcontrol LED marking layer
Date: Fri, 19 Mar 2021 19:11:28 +0100	[thread overview]
Message-ID: <ba480ab5-80b7-d911-052b-53b1e8d43695@perex.cz> (raw)
In-Reply-To: <a3ddb881-6580-cd25-ef3c-734e686e6942@redhat.com>

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. But looking to this command now, it may be better to rename it to "sysw" ("double s" does not look so great).

> Only complaint which I have is the need to add "-n" to the echo commands,
> it would be nice if set_led_id() would check if the copy which it stores in buf2
> ends with "\n" and if it does if it would then strip that from the copy in buf2.

Yes, I will fix that. It's possible to use the shorter string:

echo "Speaker Channel Switch" > /sys/class/sound/card1/controlC1/led-speaker/attach

				Jaroslav

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