Hi Dmitry,

Sorry for the slow response.

On Dec 23, 2007 6:29 PM, Dmitry Baryshkov [off-list ref] wrote:

quoted

From b213bef91e3f98c6104138050ad4eaa0a54e765d Mon Sep 17 00:00:00 2001

From: Dmitry Baryshkov <redacted>
Date: Mon, 24 Dec 2007 02:15:34 +0300
Subject: [PATCH] Tosa keyboard support

Support keyboard on tosa (Sharp Zaurus SL-6000x).
Largely based on patches by Dirk Opfer.
This is the final version that supports switching between
full keycode range and range available to the console and
keyb Kdrive driver.

The driver looks very nice, just a couple minor comments:

+
+#ifdef CONFIG_PM
+static int tosakbd_suspend(struct platform_device *dev, pm_message_t state)
+{
+       /* Nothing yet */

Stop the polling timer here?

+
+       return 0;
+}
+

...

+
+       tosakbd->input = input_dev;
+
+       input_dev->private = tosakbd;

Please use input_set_drvdata(). I am about to commit the change
removing private from input_dev structure.

+       input_dev->name = "Tosa Keyboard";
+       input_dev->phys = "tosakbd/input0";
+       input_dev->cdev.dev = &pdev->dev;

Please use input_dev->dev.parent. cdev is going away.

+
+       input_dev->id.bustype = BUS_HOST;
+       input_dev->id.vendor = 0x0001;
+       input_dev->id.product = 0x0001;
+       input_dev->id.version = 0x0100;
+
+       input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
+       input_dev->keycode = tosakbd->keycode;
+       input_dev->keycodesize = sizeof(unsigned int);
+       input_dev->keycodemax = ARRAY_SIZE(tosakbd_keycode);
+
+       memcpy(tosakbd->keycode, tosakbd_keycode, sizeof(tosakbd_keycode));
+
+       for (i = 0; i < ARRAY_SIZE(tosakbd_keycode); i++)
+               set_bit(tosakbd->keycode[i], input_dev->keybit);
+       clear_bit(0, input_dev->keybit);

Might want to switch to __set_bit to save couple of bytes. We dont
need atomicity here.

+
+#define TOSA_KEY_SYNC          KEY_102ND /* ??? */
+
+
+#ifndef CONFIG_KEYBOARD_TOSA_USE_EXT_KEYCODES
+#define TOSA_KEY_RECORD                KEY_YEN
+#define TOSA_KEY_ADDRESSBOOK   KEY_KATAKANA
+#define TOSA_KEY_CANCEL                KEY_ESC
+#define TOSA_KEY_CENTER                KEY_HIRAGANA
+#define TOSA_KEY_OK            KEY_HENKAN
+#define TOSA_KEY_CALENDAR      KEY_KATAKANAHIRAGANA
+#define TOSA_KEY_HOMEPAGE      KEY_HANGEUL
+#define TOSA_KEY_LIGHT         KEY_MUHENKAN
+#define TOSA_KEY_MENU          KEY_HANJA
+#define TOSA_KEY_FN            KEY_RIGHTALT
+#define TOSA_KEY_MAIL          KEY_ZENKAKUHANKAKU
+#else
+#define TOSA_KEY_RECORD                KEY_RECORD
+#define TOSA_KEY_ADDRESSBOOK   KEY_ADDRESSBOOK
+#define TOSA_KEY_CANCEL                KEY_CANCEL
+#define TOSA_KEY_CENTER                KEY_SELECT /* ??? */
+#define TOSA_KEY_OK            KEY_OK
+#define TOSA_KEY_CALENDAR      KEY_CALENDAR
+#define TOSA_KEY_HOMEPAGE      KEY_HOMEPAGE
+#define TOSA_KEY_LIGHT         KEY_SWITCHVIDEOMODE /* ??? */

KEY_ILLUMTOGGLE maybe? SWITCHWIDEOMODE is for cycling between outputs...

Also, I'd put these defines rigth in tosakbd.c.

Thanks.

-- 
Dmitry