Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S376232AbXECRIK (ORCPT ); Thu, 3 May 2007 13:08:10 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1031360AbXECRIK (ORCPT ); Thu, 3 May 2007 13:08:10 -0400 Received: from opal.biophys.uni-duesseldorf.de ([134.99.176.7]:37917 "EHLO opal.biophys.uni-duesseldorf.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031363AbXECRII (ORCPT ); Thu, 3 May 2007 13:08:08 -0400 Date: Thu, 3 May 2007 19:07:03 +0200 (CEST) From: Michael Schmitz To: Dmitry Torokhov cc: Geert Uytterhoeven , Linus Torvalds , Andrew Morton , linux-m68k@vger.kernel.org, linux-kernel@vger.kernel.org, Michael Schmitz , Roman Zippel Subject: Re: [patch 04/33] m68k: Atari keyboard and mouse support. In-Reply-To: Message-ID: References: <20070501195052.390551603@mail.of.borg> <20070501195129.842338339@mail.of.borg> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3432 Lines: 121 > > +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/