Hi, On 3/2/21 10:14 PM, Jaroslav Kysela wrote: > Dne 01. 03. 21 v 22:26 Hans de Goede napsal(a): >> Hi, >> >> On 3/1/21 9:43 PM, Mark Brown wrote: >>> On Mon, Mar 01, 2021 at 08:49:34PM +0100, Hans de Goede wrote: >>>> On 3/1/21 8:15 PM, Mark Brown wrote: >>> >>>>> Off the top of my head something like writing a control name into a >>>>> sysfs file might work, it doesn't scale if you need to use multiple >>>>> controls as rt5640 does though. >>> >>>> Currently ALSA/UCM does not use sysfs files for anything, so this >>>> feels very inconsistent with how all the rest of this currently works. >>> >>> Yes, you'd really want to add string controls in ALSA. >> >> Hmm, we already have SNDRV_CTL_ELEM_TYPE_BYTES controls. I think that will >> work nicely actually, we can have the UCM conf file send a 0 terminated >> string to the driver that way. It would be nice to have some syntactic >> sugar on the UCM side to be able to actually specify a string instead >> of an array of bytes, but I don't think we need any new userspace API >> for this. > > The LEDs are controlled per machine not per card. So do we need to create the 'Speaker/Mic LED Control' control for all cards? > > Also, this change sounds really generic. The interface may be implemented in my proposed control led kernel module, not in the codec drivers. > > The Mark's sysfs idea is not bad in my opinion. The sequences may be extended in UCM, we have already 'exec' command. Yes, this command is a little heavy for the sysfs writes, but we can add command like 'sysset' or so for sysfs like: Okay, so this would be a sysfs file per card then? Sol we would have for example 2 new sysfs files like this show up when your module is loaded: /sys/class/sounds/card0/spk_mute_led_control /sys/class/sounds/card0/mic_mute_led_control And reading would iterate over all mixer-elements of the card and print the names of those which have the relevant access LED flag set, where as a write would be taken as a control-name to add the access LED flag too? And an empty write would be special and clear the flag on all controls? I guess we don't strictly need that if we only set things up at boot once, but it might still be handy to force things to a clean state. > > # detach all speaker LED controls for card 1 > # similar to 'echo -n "card=1,*" > /sysfs/devices/virtual/sound/ctl-led/speaker/detach' > sysset "devices/virtual/sound/ctl-led/speaker/detach:card=1,*" > > # attach the 'Speaker Playback Switch',10 control to speaker LED trigger in card 1 > # similar to 'echo -n "card=1,iface=MIXER,name='Speaker Playback Switch',index=10" > /sysfs/devices/virtual/sound/ctl-led/speaker/attach > sysset "devices/virtual/sound/ctl-led/speaker/attach:card=1,iface=MIXER,name='Speaker Playback Switch',index=10" I think a sysfs file per card would work better, that would certainly be a lot more inline with how sysfs is normally used... Also do we need the iface=MIXER part ? > Security: The LED-control bindings should be handled only in the boot / init phase (thus in UCM BootSequence section) and the sysfs interface files should be read-only for normal users. Yes that make sense, but it will require some extra helper to that, I guess it could be an extra flag to the alsactl restore command which already gets run at boot, or an extra alsactl command ? Regards, Hans
