Re: [PATCHv4] INPUT: Route keyboard LEDs through the generic LEDs layer.

[Samuel Thibault]

Dmitry Torokhov, le Fri 19 Dec 2014 15:07:21 -0800, a écrit :
> On Sat, Dec 20, 2014 at 12:02:53AM +0100, Pavel Machek wrote:
> > On Fri 2014-12-19 23:59:33, Samuel Thibault wrote:
> > > Andrew Morton, le Fri 19 Dec 2014 14:46:41 -0800, a écrit :
> > > > > Changed in this version:
> > > > > - Use kcalloc instead of kzalloc
> > > > > - to avoid any mutex order violation, defer LED update into a work callback.
> > > > 
> > > > Confused.  This patch is identical to the one that's presently in -mm.
> > > 
> > > Well, yes: I'm submitting the latest version of this patch to Dmitry,
> > > i.e. including the fixes that happened in the -mm tree.
> > 
> > Andrew, do you think you could send the patch to Dmitry? He has been
> > ignoring it for way too long... and it should be slightly harder for
> > him to ignore patch from you...
> 
> I'll keep ignoring the patch until someone will figure out how to make new way
> of handling LEDs play well with the current way (via writing to evdev). That
> means not treating access to evdev as "raw" and ignoring LED trigger remapping
> done in LED layer. So far I just heard that it is not a problem and nobody uses
> evdev interface...

?! There must have been a misunderstanding, and the absence of response
to my last mail (April 17th) didn't help to understand what you were
expecting.  I'm sorry, but things can't work if there are not efforts to
discuss on both sides.

I never said that nobody uses the evdev interface.

I just said that I wasn't sure to understand the scenario you were
proposing.  I was asking for details, not barring discussion...  I'm
fine with reworking my patch yet again, but I can't do that if I don't
understand precisely the use-case you are expecting.

So, to recap what we apparently want:

(1) console-setup wants to be able to change which keyboard modifier is
shown on the capslock LED
(2) embedded people want to be able to plug the keyboard LED state to misc
LEDs
- people will quickly want to be able to repurpose their keyboard LEDs,
for instance:
  (3) make the numlock LED show disk activity
  (4) exchange capslock and numlock LEDs for some reason.

And the question is: how all of that should be mixed?

ATM, we have this:

kbd state -> kbd led -> input led
  ^^            ^^          ^^
KDSKBLED     KDSETLED     EV_LED

What I had proposed and implemented in my current patch is the following
pipeline:

kbd state -> kbd led -> kbd trigger -> vt led -> vt trigger -> input led
  ^^           ^^                   ^^                      ^^        ^^
KDSKBLED   KDSETLED   vt::capsl/trigger   input*::capsl/trigger   EV_LED

Which leaves the evdev users keeping raw control, unmodified.  In
your mail from April 12th, you seemed to want that (4) have effect on
evdev, I asked for confirmation, but never got an answer.  Your above
paragraph seems to clearly show that you want evdev users to get at
least the (4) remapping.  That wasn't obvious to me, because I had
always considered evdev as a way to drive physical hardware, without any
software abstraction.  But OK, let's go with that.

AIUI, what you would be OK with is the following:

kbd state -> kbd trigger -> kbd led -> input led -> input rawled
   ^^                    ^^   ^^         ^^      ^^
KDSKBLED  vt::capsl/trigger  KDSETLED  EV_LED  input*::capsl/trigger

Which, instead of introducing a "VT led", separates the input led into
the part exposed to other systems (kbd, evdev) and the lowlevel part.  I
have here also fixed the behavior of KDSETLED: in my patch proposal it
would get redirected by console-setup, which I believe we don't want.

So for (1) console-setup would use vt::capsl/trigger to make it use
another modifier, and this would only have effect for actual capslock
keypresses and KDSKBLED ioctls, which I believe is what we want
(again, correct me if I'm wrong, but please detail, otherwise I can't
know how I'm wrong); for (2) people would use input*::capsl/trigger*,
which would now indeed have the nice effect of making e.g. Xorg
getting the good behavior along the way; for (3) people would also use
input*::capsl/trigger*, and that would now indeed prevent spurious
changes from e.g. Xorg.

Now, there is (4).  I don't know why people would want that, but well,
you mentioned it in some mail, so why not.  Ideally it would go through
input*::capsl/trigger*, but what to put there?  In my earlier proposal,
that would have been vt-capsl, but you want the exchange to work on
evdev access too.  A possibility would be to introduce a LED trigger for
each input LED, like this:

kbd state -> kbd trigger -> kbd led -> input led -> input led trigger -> input rawled
   ^^                    ^^   ^^          ^^                          ^^        
KDSKBLED  vt::capsl/trigger  KDSETLED   EV_LED              input*::capsl/trigger

Which seems quite hairy: for each input LED we'd create both a trigger
(gathering both the kbd led events and the evdev led events) and a LED
(the actual physical LED), but why not.

To exchange capsl and numl, one would then have to do something like
this:

for i in input*
do
	echo $i-numl > /sys/class/leds/$i::capsl/trigger
	echo $i-capsl > /sys/class/leds/$i::numl/trigger
done

That also opens the possibility of odd things like this:

echo input0-numl > /sys/class/leds/input1::capsl/trigger

i.e. all evdev numl events on input0 will actually also drive input1's
capsl.  Why not.

Is it looking more like what you want?

Samuel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/