On Mon, Dec 14, 2009 at 05:00:59PM -0500, Chaogui Zhang wrote:
> On Sat, Dec 12, 2009 at 03:32:59PM -0800, Dmitry Torokhov wrote:
> > Hi Chaogui,
> >
> > On Sat, Dec 12, 2009 at 02:01:43PM -0500, Chaogui Zhang wrote:
> > > +/* Check the inital AGC burst and space value to match the NEC protocol */
> > > +static inline int is_nec(int leadpulse, int leadspace)
> > > +{
> > > + /* leadpulse should be 9 ms = 9000 us and leadspace should be
> > > + * 4.5 ms = 4500 us. We allow +/- 200 microseconds for both.
> > > + * Time is measured in units of 50 microseconds.
> > > + * 170 == 8800/50, 184 == 9200/50,
> > > + * 86 == 4300/50, 94 == 4700/50.
> > > + */
> > > + return (leadpulse >= 170 && leadpulse <= 184)
> > > + && (leadspace >= 86 && leadspace <= 94);
> >
> > That surely should be '||'. I prefer it at the end too.
>
> I think it should be && as the protocol specifies both the 9ms AGC burst
> and the 4.5ms space following it.
>
Ah, yes, I misread the statement and did not see that it involved 2
different variables: leadpulse and leadspace.
> + { KE_KEY, 0x28, { KEY_1 } },
> + { KE_KEY, 0x29, { KEY_2 } },
> + { KE_KEY, 0x2a, { KEY_3 } },
> + { KE_KEY, 0x2b, { KEY_4 } },
> + { KE_KEY, 0x2c, { KEY_5 } },
> + { KE_KEY, 0x2d, { KEY_6 } },
> + { KE_KEY, 0x2e, { KEY_7 } },
> + { KE_KEY, 0x2f, { KEY_8 } },
> + { KE_KEY, 0x30, { KEY_9 } },
> + { KE_KEY, 0x31, { KEY_0 } },
Hm, I forgot to mention this in my previous mail - I am trying to move
RC and phones to KEY_NUMERIC_X keycodes which are supposed to be
independent on state of shift and NumLock. I will change it locally -
and if users want old definitions - why, EVIOSKEYCODE now works ;)
> + { KE_KEY, 0x32, { KEY_CLEAR } },
> + { KE_KEY, 0x33, { KEY_ENTER } },
> + { KE_KEY, 0x34, { KEY_VIDEO } }, /* TV Input */
> + { KE_KEY, 0x36, { KEY_EPG } }, /* Guide */
> + { KE_KEY, 0x44, { KEY_ZOOM } }, /* Aspect */
> + { KE_KEY, 0x48, { KEY_STOP } },
> + { KE_KEY, 0x4a, { KEY_DVD } }, /* DVD Menu */
> + { KE_KEY, 0x5f, { KEY_CYCLEWINDOWS } }, /* Window */
> + { KE_END, 0}
> +};
> +
> +/* table of devices that work with this driver */
> +static struct usb_device_id tivoir_table[] = {
> + {USB_DEVICE(USB_TIVOIR_VENDOR_ID, USB_TIVOIR_PRODUCT_ID)},
> + {} /* Terminating entry */
> +};
> +
> +/* Structure to hold all of our driver specific stuff */
> +struct usb_tivoir {
> + char name[128];
> + char phys[64];
> + struct usb_device *udev;
> + struct input_dev *input;
> + struct usb_interface *interface;
> + struct usb_endpoint_descriptor *in_endpoint;
> + struct urb *irq_urb;
> + dma_addr_t in_dma;
> + unsigned char *in_buffer;
> +
> + /* variables used to parse messages from remote. */
> + int stage;
> + int pulse;
> + int space;
> + u32 code; /* 32 bit raw code from the remote */
> + int repeat;
> + int bitcount;
> +};
> +
> +static struct usb_driver tivoir_driver;
> +
> +/*
> + * Debug routine that prints out what we've received from the remote.
> + */
> +static void tivoir_print_packet(struct usb_tivoir *remote)
> +{
> + u8 codes[4 * TIVOIR_RECV_SIZE];
> + int i, length;
> +
> + /* The lower 5 bits of the first byte of each packet indicates the size
> + * of the transferred buffer, not including the first byte itself.
> + */
> +
> + length = (remote->in_buffer[0]) & 0x1f;
> + for (i = 0; i <= length; i++)
> + snprintf(codes + i * 3, 4, "%02x ", remote->in_buffer[i]);
> +
> + /* 0x80 at the end of a regular packet or in a separate packet
> + indicates key release */
> +
> + if (i < TIVOIR_RECV_SIZE && remote->in_buffer[i] == 0x80)
> + snprintf(codes + i * 3, 4, "%02x ", remote->in_buffer[i]);
> +
> + dev_printk(KERN_DEBUG, &remote->udev->dev, "%s: %s\n", __func__, codes);
> +}
> +
> +static inline u16 code_address(u32 code)
> +{
> + /* Higher 16 bits of the code is the remote address */
> + return code >> 16;
> +}
> +
> +static inline u8 code_command(u32 code)
> +{
> + /* Lower 8 bits of the code is the command */
> + return code & 0xff;
> +}
> +
> +static void tivoir_report_key(struct usb_tivoir *remote)
> +{
> + struct input_dev *input = remote->input;
> + struct key_entry *key;
> +
> +
> + dev_dbg(&remote->udev->dev, "%s: address = 0x%04x, command = 0x%02x\n",
> + __func__, remote->code >> 16, remote->code & 0xff);
> +
> + if (code_address(remote->code) == TIVO_REMOTE_ADDR) {
> + key = sparse_keymap_entry_from_scancode(input,
> + code_command(remote->code));
> + if (!key) { /* invalid code, do nothing */
> + remote->code = 0;
> + return;
> + }
> + sparse_keymap_report_entry(input, key, 1, !remote->repeat);
Should not this be
sparse_keymap_report_entry(input, key, remote->repeat, false);
?
In fact remote->repeat is more like "button down", not necessarily
repeat, right? I also do not see the readon for keeping it in the device
structure, you can simply pass it in tivoir_report_key(), can't you?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
