From: Samuel Thibault <samuel.thibault@ens-lyon.org> To: David Herrmann <dh.herrmann@gmail.com> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>, Pavel Machek <pavel@ucw.cz>, Andrew Morton <akpm@linux-foundation.org>, jslaby@suse.cz, rpurdie@rpsys.net, linux-kernel <linux-kernel@vger.kernel.org>, Evan Broder <evan@ebroder.net>, Arnaud Patard <arnaud.patard@rtp-net.org>, Peter Korsgaard <jacmet@sunsite.dk>, Sascha Hauer <s.hauer@pengutronix.de>, Matt Sealey <matt@genesi-usa.com>, Rob Clark <robdclark@gmail.com>, Niels de Vos <devos@fedoraproject.org>, linux-arm-kernel@lists.infradead.org, Steev Klimaszewski <steev@genesi-usa.com> Subject: Re: [PATCH] Route kbd LEDs through the generic LEDs layer Date: Mon, 15 Jul 2013 21:08:03 +0200 [thread overview] Message-ID: <20130715190803.GG11600@type.youpi.perso.aquilenet.fr> (raw) In-Reply-To: <CANq1E4TRAOp+=w4oqPt4sjV7iLyDygxxpwrZy7dSB8UK288aNA@mail.gmail.com> Hello, David Herrmann, le Mon 15 Jul 2013 17:03:08 +0200, a écrit : > > @@ -13,6 +13,10 @@ > > bool "Virtual terminal" if EXPERT > > depends on !S390 && !UML > > select INPUT > > + select NEW_LEDS > > + select LEDS_CLASS > > + select LEDS_TRIGGERS > > + select INPUT_LEDS > > This looks odd. Any dependency changes in the input-layer will break > this. But I guess that's what we get for a non-recursive "select". Hmm Yes, that's the issue. > I also think that the macros don't really simplify this. So: > [VC_SHIFTLOCK] = { .name = "shiftlock", .activate = > kbd_lockstate_trigger_activate }, > isn't much longer than: > DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK, "shiftlock"), That makes it more than 80 columns, at least. > > + for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) > > + led_trigger_register(&ledtrig_ledstate[i]); > > + for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) > > + led_trigger_register(&ledtrig_lockstate[i]); > > + > > This returns "int", are you sure no error-handling is needed? ATM only registering the same name several times can happen, and shouldn't ever happen. But better warn than be sorry, indeed. > > + * Keyboard LEDs are propagated by default like the following example: > > + * > > + * VT keyboard numlock trigger > > + * -> vt::numl VT LED > > + * -> vt-numl VT trigger > > Nitpick: What's the reason for the syntax difference? "vt-numl" vs. "vt::numl" vt::numl is a clear LED classification convention. For triggers, I don't see a clear convention, but at least I have never seen "::", only "-". That makes me realize that for keyboard triggers I haven't introduced a prefix, and only used "scrollock", "numlock", etc. Perhaps "kbd-scrollock" etc. or (in phase with LEDs), "kbd::scrollock" etc.? Samuel
WARNING: multiple messages have this Message-ID (diff)
From: samuel.thibault@ens-lyon.org (Samuel Thibault) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] Route kbd LEDs through the generic LEDs layer Date: Mon, 15 Jul 2013 21:08:03 +0200 [thread overview] Message-ID: <20130715190803.GG11600@type.youpi.perso.aquilenet.fr> (raw) In-Reply-To: <CANq1E4TRAOp+=w4oqPt4sjV7iLyDygxxpwrZy7dSB8UK288aNA@mail.gmail.com> Hello, David Herrmann, le Mon 15 Jul 2013 17:03:08 +0200, a ?crit : > > @@ -13,6 +13,10 @@ > > bool "Virtual terminal" if EXPERT > > depends on !S390 && !UML > > select INPUT > > + select NEW_LEDS > > + select LEDS_CLASS > > + select LEDS_TRIGGERS > > + select INPUT_LEDS > > This looks odd. Any dependency changes in the input-layer will break > this. But I guess that's what we get for a non-recursive "select". Hmm Yes, that's the issue. > I also think that the macros don't really simplify this. So: > [VC_SHIFTLOCK] = { .name = "shiftlock", .activate = > kbd_lockstate_trigger_activate }, > isn't much longer than: > DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK, "shiftlock"), That makes it more than 80 columns, at least. > > + for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) > > + led_trigger_register(&ledtrig_ledstate[i]); > > + for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) > > + led_trigger_register(&ledtrig_lockstate[i]); > > + > > This returns "int", are you sure no error-handling is needed? ATM only registering the same name several times can happen, and shouldn't ever happen. But better warn than be sorry, indeed. > > + * Keyboard LEDs are propagated by default like the following example: > > + * > > + * VT keyboard numlock trigger > > + * -> vt::numl VT LED > > + * -> vt-numl VT trigger > > Nitpick: What's the reason for the syntax difference? "vt-numl" vs. "vt::numl" vt::numl is a clear LED classification convention. For triggers, I don't see a clear convention, but at least I have never seen "::", only "-". That makes me realize that for keyboard triggers I haven't introduced a prefix, and only used "scrollock", "numlock", etc. Perhaps "kbd-scrollock" etc. or (in phase with LEDs), "kbd::scrollock" etc.? Samuel
next prev parent reply other threads:[~2013-07-15 19:08 UTC|newest] Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <201011112205.oABM5KVJ005298@imap1.linux-foundation.org> [not found] ` <201011111440.07882.dmitry.torokhov@gmail.com> [not found] ` <20110102090935.GV32469@atrey.karlin.mff.cuni.cz> [not found] ` <20110102103210.GA25662@core.coreip.homeip.net> [not found] ` <20110102225741.GX5480@const.famille.thibault.fr> [not found] ` <20110112182702.GA9168@core.coreip.homeip.net> 2011-01-15 19:09 ` [patch 20/35] leds: route kbd LEDs through the generic LEDs layer Samuel Thibault 2011-11-14 4:06 ` Samuel Thibault 2012-02-06 14:19 ` Pavel Machek 2012-11-28 22:06 ` Samuel Thibault 2012-12-21 0:34 ` [PATCH] Route " Samuel Thibault 2012-12-21 0:34 ` Samuel Thibault 2012-12-21 0:34 ` Samuel Thibault 2013-07-07 10:10 ` Samuel Thibault 2013-07-07 10:10 ` Samuel Thibault 2013-07-12 11:36 ` Pavel Machek 2013-07-12 11:36 ` Pavel Machek 2013-07-12 12:42 ` Samuel Thibault 2013-07-12 12:42 ` Samuel Thibault 2013-07-12 23:33 ` Pavel Machek 2013-07-12 23:33 ` Pavel Machek 2013-07-13 9:35 ` Samuel Thibault 2013-07-13 9:35 ` Samuel Thibault 2013-07-15 9:12 ` Pavel Machek 2013-07-15 9:12 ` Pavel Machek 2014-03-16 10:16 ` Pali Rohár 2014-03-16 10:19 ` Samuel Thibault 2014-03-27 1:08 ` Pali Rohár 2014-03-28 7:01 ` 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) Pavel Machek 2014-03-28 7:01 ` Pavel Machek 2014-03-28 7:17 ` Greg KH 2014-03-28 7:17 ` Greg KH 2014-04-06 9:43 ` Pali Rohár 2014-04-06 9:43 ` Pali Rohár 2014-04-06 9:55 ` Sebastian Reichel 2014-03-28 8:08 ` Samuel Thibault 2014-03-28 8:08 ` Samuel Thibault 2013-07-15 9:27 ` [PATCH] Route kbd LEDs through the generic LEDs layer Peter Korsgaard 2013-07-15 9:27 ` Peter Korsgaard 2013-07-15 15:03 ` David Herrmann 2013-07-15 15:03 ` David Herrmann 2013-07-15 19:08 ` Samuel Thibault [this message] 2013-07-15 19:08 ` Samuel Thibault 2013-07-17 15:14 ` David Herrmann 2013-07-17 15:14 ` David Herrmann 2010-02-24 1:20 [PATCH] Route kbd leds through the generic leds layer Samuel Thibault -- strict thread matches above, loose matches on Subject: below -- 2010-02-24 1:20 Samuel Thibault
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20130715190803.GG11600@type.youpi.perso.aquilenet.fr \ --to=samuel.thibault@ens-lyon.org \ --cc=akpm@linux-foundation.org \ --cc=arnaud.patard@rtp-net.org \ --cc=devos@fedoraproject.org \ --cc=dh.herrmann@gmail.com \ --cc=dmitry.torokhov@gmail.com \ --cc=evan@ebroder.net \ --cc=jacmet@sunsite.dk \ --cc=jslaby@suse.cz \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=matt@genesi-usa.com \ --cc=pavel@ucw.cz \ --cc=robdclark@gmail.com \ --cc=rpurdie@rpsys.net \ --cc=s.hauer@pengutronix.de \ --cc=steev@genesi-usa.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.