On Sat, 20 Mar 2021 10:17:57 +0100, Hans de Goede wrote: > > Hi, > > On 3/20/21 8:41 AM, Takashi Iwai wrote: > > 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. > > Yes setting the new LED-access flags from the machine driver(s) would > work too for the 3 devices (spanning 2 machine drivers) on which > I'm trying to get the mute-LED to work, assuming Mark is going to be > ok with that approach. But those are pretty simple devices. > > There also is the recently posted Dell privacy stuff which is > also using LED-triggers on a "full-blown" Intel core series laptop, > which use codecs in much more varied ways. And I've the feeling that > we will see more of this stuff coming up and in those cases the extra > flexibility which going through UCM gives us would be good I think. > > I believe that that Dell privacy stuff is actually the reason why > Jaroslav started this whole series, right Jaroslav ? > > I'm just piggy backing along with my own use-cases which I had > on my wishlist / itches-list for a while now :) > > >> 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? > > Right. > > > Would other OS can follow a similar pattern? Let's check that first > > (although I myself think this should be feasible). > > With other OS you mean e.g. Android? Android has device-specific > init-scripts which can either call alsactl or directly do the > echo-s. Also ChromeOS. I'd like to get a general consensus before moving forward. Takashi