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

[Takashi Iwai]

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 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
> >>> 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