Re: [PATCH] [media] imon: Fix internal key table for 15c2:0034

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mauro,

Am 23.07.2014 22:55, schrieb Mauro Carvalho Chehab:
Hi Ulrich,

Em Sat, 15 Mar 2014 17:02:30 +0100
Ulrich Eckhardt <uli-lirc@xxxxxxxxxxxxxxx> escreveu:

First of all, your emailer mangled the patch. It added extra spaces,
converted tab into spaces, etc. The patch can't be applied like that.

Upps, sorry that was not the problem of the mailer, the problem was
located in front of the keyboard. I will recreate the patches against the latest git kernel.

+struct imon_usb_dev_descr {
+       __u16 flags;
+#define IMON_NO_FLAGS 0
+#define IMON_NEED_20MS_PKT_DELAY 1
+       struct imon_panel_key_table key_table[];
+};
+

It seems that you're merging two different issues at the same patch.
Please split it into one patch per logical change.

My first attempt, was to get the front panel to work. But this change would have break other iMON remotes. So I decided to extend the current design to support the definition of key tables for each USB device indifferently.

I will create a first patch for extending the design and a second patch for the iMON DH102 specifics.

-static u32 imon_panel_key_lookup(u64 code)
+static u32 imon_panel_key_lookup(struct imon_context *ictx, u64 code)
    {
-       int i;
+       int i = 0;
+       struct imon_panel_key_table *key_table = ictx->dev_descr->key_table;
           u32 keycode = KEY_RESERVED;

-       for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++) {
-               if (imon_panel_key_table[i].hw_code == (code | 0xffee)) {
-                       keycode = imon_panel_key_table[i].keycode;
+       while (key_table[i].hw_code != 0) {

Why to convert a for into a while? using a for() is more Kernel style
for loops like this one.

Due to this change, the imon_panel_key_table is not an array of a defined size anymore. Depending on the device, more or less definitions for the key table are necessary. So AFAIK I can't use the ARRAY_SIZE macro for this.

@@ -1591,8 +1733,8 @@ static void imon_incoming_packet(struct
           spin_lock_irqsave(&ictx->kc_lock, flags);

           do_gettimeofday(&t);
-       /* KEY_MUTE repeats from knob need to be suppressed */
-       if (ictx->kc == KEY_MUTE && ictx->kc == ictx->last_keycode) {
+       /* repeats from panel need to be suppressed */
+       if (ictx->kc == ictx->last_keycode) {


Why did you remove KEY_MUTE? Again, best to put its removal on a
separate patch, clearly stating why this change is needed.

Other keys of the front panel will also send repeats, leading to strange behavior. I will put this in the third patch with a detailed description.
--
Ulrich Eckhardt                  http://www.uli-eckhardt.de

Ein Blitzableiter auf dem Kirchturm ist das denkbar stärkste
Misstrauensvotum gegen den lieben Gott. (Karl Krauss)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux