Messages in this thread | | | Date | Thu, 3 May 2007 19:07:03 +0200 (CEST) | From | Michael Schmitz <> | Subject | Re: [patch 04/33] m68k: Atari keyboard and mouse support. |
| |
> > +static unsigned char rep_scancode; > > +static struct timer_list atakeyb_rep_timer = { > > + .function = atakeyb_rep, > > +}; > > Is there a problem with repeat implementation in input core that > requres custom-made repeater here?
Apparently not - the homebrew repeater has not been active pretty much since I started to work on Atari in 2.6... time to kill that code.
> > + > > + // handle_scancode(scancode, !break_flag); > > The code looks a bit dirty (old code commented out, C++ style comments).
You got that right.
> > + > > + > > +int atari_kbdrate(struct kbd_repeat *k) > > +{ > > + if (k->delay > 0) { > > + /* convert from msec to jiffies */ > > + key_repeat_delay = (k->delay * HZ + 500) / 1000; > > If this is really needed - msecs_to_jiffies().
That has to go as well (together with the repeater).
> > > > > +config KEYBOARD_ATARI > > + tristate "Atari keyboard" > > + depends on ATARI > > + select ATARI_KBD_CORE > > + help > > + Say Y here if you are running Linux on any Atari and have a keyboard > > + attached. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called atakbd. > > Can we spell it out: atarikbd or atari_kbd?
Could do that, yes. Christoph suggested to fold everythig into atakeyb.c so modularization would go away here.
> > > + > > + if (!(atakbd_dev = input_allocate_device())) > > + return -ENOMEM; > > + > > + // need to init core driver if not already done so > > + if (atari_keyb_init()) > > Memory leak
How so? If the core has been initialized already this will just return ...
> > + for (i = 1; i < 0x72; i++) { > > + atakbd_keycode[i] = i; > > + set_bit(atakbd_keycode[i], atakbd_dev->keybit); > > It looks like this driver is not using standard input event codes. If > Roman does not want to adjust keymaps on Amiga and Atari that should > be handled in legacy keyboard driver (drivers/char/keyboard.c). As it > is programs using /dev/input/eventX have no chance of working.
The translation map should not have been overwritten like above, is that what you mean? My original patch didn't have that bit; scancodes were translated to input keycodes using atakbd_keycode[scancode] instead. I'll have that reverted...
> > + } > > + > > + input_register_device(atakbd_dev); > > Error handling.
Oops.
> > > + > > +static int mouse_threshold[2] = {2,2}; > > + > > +#ifdef __MODULE__ > > +MODULE_PARM(mouse_threshold, "2i"); > > MODULE_PARM is so 20th century...
Never tested, sorry.
> > + > > +static int atamouse_open(struct input_dev *dev) > > +{ > > + if (atamouse_used++) > > + return 0; > > No need to count, input core takes care of this.
OK; I'll delete this.
> > + > > + printk(KERN_INFO "input: %s at keyboard ACIA\n", atamouse_dev->name); > > Input core already logs every input device registered, do we need to > repeat it in the driver?
Not really ... I'll merge atakeyb.c, atakbd.c and atamouse.c and send a new patch to Geert.
Thanks,
Michael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |