On Fri, 19 Mar 2021 23:08:33 +0100, Hans de Goede wrote: > > Hi, > > On 3/19/21 6:22 PM, Takashi Iwai wrote: > > 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@xxxxxxxx> > >> > >> 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 > > doesn't fit. OTOH, if it's 0666, it's accessible to all users even > > remotely, which is worse than the access with the normal sound device > > file. Or if it's supposed to be changed via udev stuff or systemd? > > Or is it just for debugging? > > > > 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. > > > > 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. > > The flexibility offered by this new sysfs API is necessary for the ASoC > codec drivers, because Mark does not want to have which controls are > tied to the LED triggers hard-coded inside the codec drivers. The hard-coded mapping itself isn't always bad things, IMO. Of course, it's a question whether to be done in the codec driver in a fixed routing. A machine driver would fit well, instead; i.e. instead of the control-access bit flag, just bind statically from the machine driver after instantiating the kctl objects like sysfs does. > So as Jaroslav mentions in his reply, the plan is to have the UCM > profiles contain commands to setup the LED triggers to this new > sysfs API. IIUC, this won't be only UCM but also the combination of udev + alsactl + UCM, right? Would other OS can follow a similar pattern? Let's check that first (although I myself think this should be feasible). thanks, Takashi
