[PATCH 0/5] PXA LX platform support

[PATCH 0/5] PXA LX platform support

[Russell King - ARM Linux]

The following series adds basic support for the LX platform.  This
platform is a portable small netbook style machine, with support
for touch screen LCD, sound, MMC/SD, one serial port, CF, PCMCIA
and MMC.

The entire series is posted to the linux-arm-kernel list; individual
patches are only copied to relevent people/lists.

[PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Russell King]

The PCON device on the LX platform is the heart of the platform,
managing the power, providing battery information, booting, suspend/
resume and keyboard services to the PXA CPU.

Communication between the PXA and PCON is via a multi-master I2C bus,
where both the PCON and PXA behave like an 256 byte EEPROM, and each
reads and writes locations within that namespace.

It has a nasty habbit of resetting the PXA CPU if it sees something
it doesn't like, so we have to be careful how we communicate with the
PCON.  This happens (eg) if we hold the I2C bus for any "unreasonable"
length of time, or ignore the PCON's attempt to address us as a slave.

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
CC: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
CC: <linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 arch/arm/mach-pxa/include/mach/lx-pcon.h |  117 ++++++++
 drivers/i2c/chips/Kconfig                |   10 +
 drivers/i2c/chips/Makefile               |    1 +
 drivers/i2c/chips/lx-pcon.c              |  428 ++++++++++++++++++++++++++++++
 4 files changed, 556 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-pxa/include/mach/lx-pcon.h
 create mode 100644 drivers/i2c/chips/lx-pcon.c

diff --git a/arch/arm/mach-pxa/include/mach/lx-pcon.h b/arch/arm/mach-pxa/include/mach/lx-pcon.h
new file mode 100644
index 0000000..acc410d
--- /dev/null
+++ b/arch/arm/mach-pxa/include/mach/lx-pcon.h
@@ -0,0 +1,117 @@
+/*
+ * PCON addresses
+ */
+#define PCON_KEYBOARD_CMD_ADDR		(0x00)
+#define PCON_KEYBOARD_CMD_VAL_READY	(0)
+#define PCON_KEYBOARD_CMD_VAL_BUSY 	(1)
+#define PCON_POWER_CF_ADDR		(0x02)
+#define PCON_POWER_PCMCIA_ADDR		(0x03)
+#define PCON_POWER_CFG_ADDR		(0x04)
+#define PCON_PWRSTATE_ADDR		(0x05)
+#define PCON_CMD_ADDR			(0x0c)	/* 1 + 8 args */
+
+typedef enum pcon_pwrstate_e {
+	PCON_PWRSTATE_HWReset    = 0,
+	PCON_PWRSTATE_SWReset    = 1,
+	PCON_PWRSTATE_Off        = 2,
+	PCON_PWRSTATE_Suspend    = 3,
+	PCON_PWRSTATE_Sleep      = 4,
+	PCON_PWRSTATE_Booting    = 5,
+	PCON_PWRSTATE_LoadingOS  = 6,
+	PCON_PWRSTATE_Idle       = 7,
+	PCON_PWRSTATE_Run        = 8,
+	PCON_PWRSTATE_Comatose   = 9,
+	PCON_PWRSTATE_PowerFail  = 10
+} pcon_pwrstate_t;
+
+extern void lx_pwr_setstate(pcon_pwrstate_t);
+extern void pcon_pwr_setstate(pcon_pwrstate_t);
+
+typedef enum {
+	PCON_CMD_Ready          = 0,
+	PCON_CMD_ADCRead        = 9,
+	PCON_CMD_GetVersion     = 10,
+	PCON_CMD_SetMMCPower    = 11,
+	PCON_CMD_SetModemPower  = 12,
+	PCON_CMD_SetUSBPower    = 13,
+	PCON_CMD_SetRedLED      = 14,
+	PCON_CMD_SetSerialPower = 15,
+	PCON_CMD_GetSerialCfg   = 17,
+	PCON_CMD_BatteryMonitor = 18,
+	PCON_CMD_GetMMCPower    = 19,
+	PCON_CMD_GetVModemPower = 20,
+	PCON_CMD_GetUSBPower    = 21,
+	PCON_CMD_SetWakeupAlarm = 22,
+} pcon_cmd_t;
+
+/*
+ * Host EEPROM emulation addresses
+ */
+#define PHST_KEYBOARD_START		(0x00)
+#define PHST_KEYBOARD_SIZE		(1)
+#define PHST_SERIAL_START		(0x02)
+#define PHST_SERIAL_SIZE		(1)
+#define PHST_POWER_START		(0x05)
+#define PHST_POWER_SIZE			(1)
+#define PHST_CMDRESP_START		(0x0d)
+#define PHST_CMDRESP_SIZE		(8)
+
+struct lx_i2cslave_watcher {
+	u16 start;
+	u16 size;
+	void (*write)(void *, unsigned int addr, unsigned char newval);
+};
+
+struct lx_i2cslave_watch {
+	struct list_head node;
+	u16 start;
+	u16 end;
+	struct lx_i2cslave_watcher *ops;
+	void *data;
+};
+
+#define I2C_EEPROM_EMU_SIZE (256)
+
+struct lx_eeprom_emu {
+	u16 size;
+	u16 ptr;
+	unsigned int seen_start;
+	struct list_head watch;
+
+	unsigned char bytes[I2C_EEPROM_EMU_SIZE];
+};
+
+extern int lx_i2cslave_addwatcher(struct lx_i2cslave_watcher *, void *data);
+extern void lx_i2cslave_delwatcher(struct lx_i2cslave_watcher *, void *data);
+
+
+
+#define PCON_FLG_ASYNC          (1)
+
+/* exported functions */
+
+extern int pcon_simple_cmd(unsigned int, unsigned int, unsigned int);
+#define pcon_set_usbhost_power(on)	\
+	pcon_simple_cmd(PCON_CMD_SetUSBPower, !!(on), PCON_FLG_ASYNC)
+#define pcon_set_mmc_power(on)		\
+	pcon_simple_cmd(PCON_CMD_SetMMCPower, !!(on), 0)
+#define pcon_set_modem_power(on)	\
+	pcon_simple_cmd(PCON_CMD_SetModemPower, !!(on), 0)
+#define pcon_set_serial_power(on)	\
+	pcon_simple_cmd(PCON_CMD_SetSerialPower, !!(on), PCON_FLG_ASYNC)
+
+#define pcon_monitorbattery(on, flag)	\
+	pcon_simple_cmd(PCON_CMD_BatteryMonitor, !!(on), flag)
+#define pcon_setredled(on)		\
+	pcon_simple_cmd(PCON_CMD_SetRedLED, !!(on), 0)
+
+extern int pcon_simple_write(unsigned int, unsigned int, unsigned int);
+#define pcon_kbd_ack()			\
+	pcon_simple_write(PCON_KEYBOARD_CMD_ADDR, PCON_KEYBOARD_CMD_VAL_READY, PCON_FLG_ASYNC)
+#define pcon_set_pcmcia_power(on)	\
+	pcon_simple_write(PCON_POWER_PCMCIA_ADDR, !!(on), 0)
+#define pcon_set_cf_power(on)		\
+	pcon_simple_write(PCON_POWER_CF_ADDR, !!(on), 0)
+
+struct rtc_time;
+extern int pcon_rtc_wakeup(struct rtc_time *tm);
diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index 8f8c81e..501fb5d 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -89,4 +89,14 @@ config SENSORS_TSL2550
 	  This driver can also be built as a module.  If so, the module
 	  will be called tsl2550.
 
+config LX_PCON
+	bool
+	depends on I2C=y && MACH_NETBOOKPRO
+	default y
+	help
+	  The LX PCON driver is required to communicate with the LX
+	  platform power control device.  This device is the heart
+	  of the system, providing power management and keyboard
+	  handling for the system.
+
 endmenu
diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
index 55a3760..871c8ae 100644
--- a/drivers/i2c/chips/Makefile
+++ b/drivers/i2c/chips/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SENSORS_MAX6875)	+= max6875.o
 obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_PCF8575)		+= pcf8575.o
+obj-$(CONFIG_LX_PCON)		+= lx-pcon.o
 obj-$(CONFIG_SENSORS_TSL2550)	+= tsl2550.o
 
 ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
diff --git a/drivers/i2c/chips/lx-pcon.c b/drivers/i2c/chips/lx-pcon.c
new file mode 100644
index 0000000..67a0a5b
--- /dev/null
+++ b/drivers/i2c/chips/lx-pcon.c
@@ -0,0 +1,428 @@
+/*
+ *  linux/drivers/i2c/chips/lx-pcon.c
+ *
+ *  Driver for LX PCON chip
+ *  Copyright (C) 2004 Simtec Electronics
+ *
+ *  based on linux/drivers/acorn/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+/* This driver provides the low-level i2c sending for the LX's
+ * power-management and general support processor. This allows other parts
+ * of the system to send messages to the chip for processing (including
+ * the userland)
+ *
+ * Note, this code does not provide the i2c client facilities needed by
+ * the PCon to send data to the systems that need information feeding in.
+*/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioctl.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/init.h>
+#include <linux/kthread.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+
+#include <mach/lx-pcon.h>
+
+struct pcon_client {
+	struct i2c_client	*client;
+	struct task_struct	*thread;
+	wait_queue_head_t	thr_wait;
+	struct platform_device	*kbd;
+	struct platform_device	*pwr;
+	void                   *pcon_dev;
+};
+
+/* pcon_cmd_ent
+ *
+ * command entry for the command queue
+*/
+
+struct pcon_cmd_ent {
+	struct pcon_cmd_ent	*next;
+	struct completion	done;
+
+	unsigned int		flags;
+
+#define PCON_FLG__ASYNC		(1 << 0)	/* defined elsewhere */
+#define PCON_FLG_CHKRDY		(1 << 1)
+#define PCON_FLG_RECVMSG	(1 << 2)
+
+	int			sendlen;
+	int			result;
+	unsigned char		sendmsg[16];
+	unsigned char		recvmsg[1];
+};
+
+static struct pcon_cmd_ent *queue;
+static struct pcon_client *cur_client;
+
+#define PCON_I2C_ADDR (0x26 >> 1)
+
+static int
+pcon_do_read(struct pcon_client *pc, int loc, unsigned char *result)
+{
+	struct i2c_client *client = pc->client;
+	unsigned char loc_data[1];
+	struct i2c_msg msgs[2] = {
+		{
+			.addr	= client->addr,
+			.flags	= 0,
+			.len	= 1,
+			.buf	= loc_data,
+		}, {
+			.addr	= client->addr,
+			.flags	= I2C_M_RD,
+			.len	= 1,
+			.buf	= result,
+		}
+	};
+
+	loc_data[0] = loc;
+
+	return i2c_transfer(client->adapter, msgs, 2);
+}
+
+static int pcon_send(struct pcon_client *pc, unsigned char *data, int length)
+{
+	struct i2c_client *client = pc->client;
+	struct i2c_msg msgs[1] = {
+		{
+			.addr	= client->addr,
+			.flags	= 0,
+			.len	= length,
+			.buf	= data
+		}
+	};
+
+	return i2c_transfer(client->adapter, msgs, 1) == 1 ? 1 : 0;
+}
+
+static int pcon_check_ready(struct pcon_client *client)
+{
+	unsigned char inbuff;
+	int ret;
+
+	while (1) {
+		/* the sfi code places this in the way to make sure that the
+		 * friendly pcon isn't going to ack us... dunno if this is
+		 * going to help us or not with our problems with reboot
+		 * after suspend...
+		 *
+		 * yep, this delay fixes the problem.
+		 */
+		msleep(100);
+
+		ret = pcon_do_read(client, PCON_CMD_ADDR, &inbuff);
+
+		if (ret == 2 && inbuff == PCON_CMD_Ready)
+			return 0;
+
+		/* ok, we didn't get an ok response, let us
+		 * wait for a little while and try again
+		 */
+		msleep(100);
+	}
+
+	return 0;
+}
+
+static int pcon_thread(void *param)
+{
+	struct pcon_client *pc = param;
+	unsigned long flags;
+	int ret;
+
+	current->flags |= PF_NOFREEZE;
+
+	while (1) {
+		struct pcon_cmd_ent *cmd;
+
+		wait_event_interruptible(pc->thr_wait, queue ||
+					 kthread_should_stop());
+
+		if (kthread_should_stop())
+			break;
+
+		/*
+		 * remove one item from the queue, and execute it
+		 */
+		local_irq_save(flags);
+		cmd = queue;
+		if (cmd != NULL)
+			queue = cmd->next;
+		local_irq_restore(flags);
+
+		if (!cmd)
+			continue;
+
+		if (cmd->flags & PCON_FLG_CHKRDY)
+			pcon_check_ready(pc);
+
+		if (cmd->flags & PCON_FLG_RECVMSG)
+			ret = pcon_do_read(pc, cmd->sendmsg[0], cmd->recvmsg);
+		else
+			ret = pcon_send(pc, cmd->sendmsg, cmd->sendlen);
+
+		cmd->result = ret;
+
+		if ((cmd->flags & PCON_FLG_ASYNC) == 0) {
+			complete(&cmd->done);
+		} else {
+			kfree(cmd);
+		}
+	}
+
+	return 0;
+}
+
+static int pcon_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct pcon_client *pc;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	dev_info(&client->dev, "LX: pcon found at 0x%02x\n", client->addr);
+
+	pc = kzalloc(sizeof(*pc), GFP_KERNEL);
+	if (pc == NULL) {
+		pr_err("pcon: no memory for pcon client data\n");
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	pc->client = client;
+	i2c_set_clientdata(client, pc);
+
+	pc->kbd = platform_device_alloc("lx-kbd", -1);
+	pc->pwr = platform_device_alloc("lx-pwr", -1);
+	if (!pc->kbd || !pc->pwr) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	pc->kbd->dev.parent = &client->dev;
+	pc->pwr->dev.parent = &client->dev;
+
+	init_waitqueue_head(&pc->thr_wait);
+
+	cur_client = pc;
+
+	pc->thread = kthread_run(pcon_thread, pc, "pcon");
+	if (IS_ERR(pc->thread)) {
+		ret = PTR_ERR(pc->thread);
+		pr_err("pcon: cannot start thread: %d\n", ret);
+		goto done;
+	}
+
+	ret = platform_device_add(pc->kbd);
+	if (ret < 0)
+		goto done;
+
+	ret = platform_device_add(pc->pwr);
+	if (ret < 0) {
+		platform_device_del(pc->kbd);
+		goto done;
+	}
+
+	if (1) {
+		pr_info("Moving power state to Run\n");
+		pcon_pwr_setstate(PCON_PWRSTATE_Run);
+	}
+	ret = 0;
+
+done:
+	if (ret < 0) {
+		if (!IS_ERR(pc->thread) && pc->thread)
+			kthread_stop(pc->thread);
+		platform_device_put(pc->kbd);
+		platform_device_put(pc->pwr);
+		kfree(pc);
+	}
+
+	return ret;
+}
+
+static int pcon_remove(struct i2c_client *client)
+{
+	struct pcon_client *pc = i2c_get_clientdata(client);
+
+	platform_device_unregister(pc->pwr);
+	platform_device_unregister(pc->kbd);
+	kthread_stop(pc->thread);
+	kfree(pc);
+	return 0;
+}
+
+static int
+__pcon_command(struct pcon_client *pc, unsigned int flags, unsigned char *msg,
+		int len, int *result)
+{
+	struct pcon_cmd_ent *ent;
+	unsigned long irq_flags;
+	int ret = 0;
+
+	ent = kmalloc(sizeof(*ent), (flags & PCON_FLG_ASYNC)
+		? GFP_ATOMIC : GFP_KERNEL);
+	if (ent == NULL) {
+		pr_err("%s: no memory for command\n", __func__);
+		return -ENOMEM;
+	}
+
+	init_completion(&ent->done);
+	ent->next    = NULL;
+	ent->flags   = flags;
+	ent->sendlen = len;
+	memcpy(ent->sendmsg, msg, len);
+
+	local_irq_save(irq_flags);
+
+	if (queue == NULL) {
+		queue = ent;
+	} else {
+		struct pcon_cmd_ent *ptr = queue;
+
+		for (; ptr->next != NULL; ptr = ptr->next)
+			;
+
+		ptr->next = ent;
+	}
+
+	local_irq_restore(irq_flags);
+
+	if (pc != NULL)
+		wake_up(&pc->thr_wait);
+
+	if (flags & PCON_FLG_ASYNC) {
+		/* nothing to do, leave it to it. */
+	} else {
+		WARN_ON(pc == NULL);
+		wait_for_completion(&ent->done);
+
+		if (flags & PCON_FLG_RECVMSG) {
+			if (ent->result == 2) {
+				if (result)
+					*result = ent->recvmsg ? 1 : 0;
+				ret = 0;
+			} else
+				ret = ent->result;
+		}
+
+		/* dump this, we've been left with it*/
+		kfree(ent);
+	}
+
+	return ret;
+}
+
+int pcon_simple_cmd(unsigned int cmd, unsigned int val, unsigned int flags)
+{
+	unsigned char sendmsg[3];
+
+	might_sleep_if(flags != PCON_FLG_ASYNC);
+
+	sendmsg[0] = PCON_CMD_ADDR;
+	sendmsg[1] = cmd;
+	sendmsg[2] = val;
+
+	return __pcon_command(cur_client, flags | PCON_FLG_CHKRDY,
+			      sendmsg, sizeof(sendmsg), NULL);
+}
+EXPORT_SYMBOL(pcon_simple_cmd);
+
+int pcon_simple_write(unsigned int addr, unsigned int val, unsigned int flags)
+{
+	unsigned char sendmsg[2];
+
+	might_sleep_if(flags != PCON_FLG_ASYNC);
+
+	sendmsg[0] = addr;
+	sendmsg[1] = val;
+
+	return __pcon_command(cur_client, flags, sendmsg, sizeof(sendmsg),
+			      NULL);
+}
+EXPORT_SYMBOL(pcon_simple_write);
+
+int pcon_rtc_wakeup(struct rtc_time *tm)
+{
+	unsigned char sendmsg[10];
+
+	might_sleep();
+
+	sendmsg[0] = PCON_CMD_ADDR;
+	sendmsg[1] = PCON_CMD_SetWakeupAlarm;
+	sendmsg[2] = tm->tm_sec;
+	sendmsg[3] = tm->tm_min;
+	sendmsg[4] = tm->tm_hour;
+	sendmsg[5] = tm->tm_wday; /* 6 = Saturday */
+	sendmsg[6] = tm->tm_mday; /* 1 = 1st */
+	sendmsg[7] = tm->tm_mon;
+	sendmsg[8] = tm->tm_year >> 8;
+	sendmsg[9] = tm->tm_year & 255;
+
+	return __pcon_command(cur_client, PCON_FLG_CHKRDY, sendmsg,
+			      sizeof(sendmsg), NULL);
+}
+EXPORT_SYMBOL(pcon_rtc_wakeup);
+
+void pcon_pwr_setstate(pcon_pwrstate_t pwrstate)
+{
+	unsigned char sendmsg[2];
+
+	if (cur_client == NULL) {
+		pr_err("%s: no client for write\n", __func__);
+		return;
+	}
+
+	sendmsg[0] = PCON_PWRSTATE_ADDR;
+	sendmsg[1] = pwrstate;
+
+	pcon_send(cur_client, sendmsg, sizeof(sendmsg));
+}
+
+
+static const struct i2c_device_id pcon_id[] = {
+	{ "lxpcon", 0 },
+	{ }
+};
+
+static struct i2c_driver pcon_driver = {
+	.driver		= {
+		.owner		= THIS_MODULE,
+		.name		= "pcon",
+	},
+	.id_table	= pcon_id,
+	.probe		= pcon_probe,
+	.remove		= pcon_remove,
+};
+
+static __init int pcon_init(void)
+{
+	return i2c_add_driver(&pcon_driver);
+}
+
+static __exit void pcon_exit(void)
+{
+	i2c_del_driver(&pcon_driver);
+}
+
+MODULE_AUTHOR("Ben Dooks <ben-Y5A6D6n0/KfQXOPxS62xeg@public.gmane.org>");
+MODULE_DESCRIPTION("Psion Teklogix NetBookPro PCon chip driver");
+
+module_init(pcon_init);
+module_exit(pcon_exit);
-- 
1.6.0.6

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Jean Delvare]

On Sat, 18 Apr 2009 15:20:48 +0100, Russell King wrote:
> The PCON device on the LX platform is the heart of the platform,
> managing the power, providing battery information, booting, suspend/
> resume and keyboard services to the PXA CPU.
> 
> Communication between the PXA and PCON is via a multi-master I2C bus,
> where both the PCON and PXA behave like an 256 byte EEPROM, and each
> reads and writes locations within that namespace.
> 
> It has a nasty habbit of resetting the PXA CPU if it sees something
> it doesn't like, so we have to be careful how we communicate with the
> PCON.  This happens (eg) if we hold the I2C bus for any "unreasonable"
> length of time, or ignore the PCON's attempt to address us as a slave.
> 
> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> CC: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> CC: <linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
>  arch/arm/mach-pxa/include/mach/lx-pcon.h |  117 ++++++++
>  drivers/i2c/chips/Kconfig                |   10 +
>  drivers/i2c/chips/Makefile               |    1 +

Nack. Really, people, please read the files you modify, it isn't that
hard.

-- 
Jean Delvare

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Russell King - ARM Linux]

On Sat, Apr 18, 2009 at 04:29:00PM +0200, Jean Delvare wrote:
> On Sat, 18 Apr 2009 15:20:48 +0100, Russell King wrote:
> > The PCON device on the LX platform is the heart of the platform,
> > managing the power, providing battery information, booting, suspend/
> > resume and keyboard services to the PXA CPU.
> > 
> > Communication between the PXA and PCON is via a multi-master I2C bus,
> > where both the PCON and PXA behave like an 256 byte EEPROM, and each
> > reads and writes locations within that namespace.
> > 
> > It has a nasty habbit of resetting the PXA CPU if it sees something
> > it doesn't like, so we have to be careful how we communicate with the
> > PCON.  This happens (eg) if we hold the I2C bus for any "unreasonable"
> > length of time, or ignore the PCON's attempt to address us as a slave.
> > 
> > Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> > CC: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> > CC: <linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > ---
> >  arch/arm/mach-pxa/include/mach/lx-pcon.h |  117 ++++++++
> >  drivers/i2c/chips/Kconfig                |   10 +
> >  drivers/i2c/chips/Makefile               |    1 +
> 
> Nack. Really, people, please read the files you modify, it isn't that
> hard.

Maybe you can suggest a better place for it?

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Mark Brown]

On Sat, Apr 18, 2009 at 03:42:34PM +0100, Russell King - ARM Linux wrote:
> On Sat, Apr 18, 2009 at 04:29:00PM +0200, Jean Delvare wrote:

> > > The PCON device on the LX platform is the heart of the platform,
> > > managing the power, providing battery information, booting, suspend/
> > > resume and keyboard services to the PXA CPU.

> > Nack. Really, people, please read the files you modify, it isn't that
> > hard.

> Maybe you can suggest a better place for it?

drivers/mfd?

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Russell King - ARM Linux]

On Sat, Apr 18, 2009 at 06:34:06PM +0100, Mark Brown wrote:
> On Sat, Apr 18, 2009 at 03:42:34PM +0100, Russell King - ARM Linux wrote:
> > On Sat, Apr 18, 2009 at 04:29:00PM +0200, Jean Delvare wrote:
> 
> > > > The PCON device on the LX platform is the heart of the platform,
> > > > managing the power, providing battery information, booting, suspend/
> > > > resume and keyboard services to the PXA CPU.
> 
> > > Nack. Really, people, please read the files you modify, it isn't that
> > > hard.
> 
> > Maybe you can suggest a better place for it?
> 
> drivers/mfd?

It's not a multifunction device.  It's a microcontroller.

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Mark Brown]

On Sat, Apr 18, 2009 at 07:10:30PM +0100, Russell King - ARM Linux wrote:
> On Sat, Apr 18, 2009 at 06:34:06PM +0100, Mark Brown wrote:

> > > Maybe you can suggest a better place for it?

> > drivers/mfd?

> It's not a multifunction device.  It's a microcontroller.

Surely that's just an implementation detail?  As I understand your
description the microcontroller is hiding the control interfaces of the
other things it controls so the function is pretty similar.

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Russell King - ARM Linux]

On Sun, Apr 19, 2009 at 12:11:14PM +0100, Mark Brown wrote:
> On Sat, Apr 18, 2009 at 07:10:30PM +0100, Russell King - ARM Linux wrote:
> > On Sat, Apr 18, 2009 at 06:34:06PM +0100, Mark Brown wrote:
> 
> > > > Maybe you can suggest a better place for it?
> 
> > > drivers/mfd?
> 
> > It's not a multifunction device.  It's a microcontroller.
> 
> Surely that's just an implementation detail?  As I understand your
> description the microcontroller is hiding the control interfaces of the
> other things it controls so the function is pretty similar.

No, it's a question of whether drivers/mfd is right.  The "shovel
everything that doesn't fit elsewhere into drivers/mfd" approach
is just creating yet another problem, just like drivers/i2c/chips
is percieved to be.  The only difference is that it becomes someone
elses problem.

I've a good mind to put it in arch/arm/mach-pxa/ purely as a way of
bringing this issue to a close - it's far from ideal having drivers
outside of the drivers/ tree, but if that's the way it has to be to
progress, so be it.

Re: [PATCH 1/5] [ARM] pxa: lx: add PCON i2c driver

[Mark Brown]

On Mon, Apr 20, 2009 at 01:18:05PM +0100, Russell King - ARM Linux wrote:
> On Sun, Apr 19, 2009 at 12:11:14PM +0100, Mark Brown wrote:

> > Surely that's just an implementation detail?  As I understand your
> > description the microcontroller is hiding the control interfaces of the
> > other things it controls so the function is pretty similar.

> No, it's a question of whether drivers/mfd is right.  The "shovel
> everything that doesn't fit elsewhere into drivers/mfd" approach
> is just creating yet another problem, just like drivers/i2c/chips
> is percieved to be.  The only difference is that it becomes someone
> elses problem.

Well, my thinking was that it looks like you've got a bunch of
subsystem-specific drivers for this chip with a core driver managing
shared resources like I/O with the device.  To my mind that's pretty
much the use case for MFD.

As far as general dumping grounds goes we already have drivers/misc.

[PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Russell King]

This adds support for the keyboard support on the LX platform.
We aren't directly connected to the keyboard itself, but the
keyboard is managed by PCON via the system I2C bus.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: <linux-input@vger.kernel.org>
---
 drivers/input/keyboard/Kconfig  |    7 +
 drivers/input/keyboard/Makefile |    1 +
 drivers/input/keyboard/lx-kbd.c |  339 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 347 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/lx-kbd.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..54dc0fa 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -289,6 +289,13 @@ config KEYBOARD_AAED2000
 	  To compile this driver as a module, choose M here: the
 	  module will be called aaed2000_kbd.
 
+config KEYBOARD_LX
+	tristate "LX Keyboard support"
+	depends on MACH_NETBOOKPRO && INPUT && INPUT_KEYBOARD
+	help
+	  Say Y here if you want to use the NetBookPro keyboard on your
+	  system.
+
 config KEYBOARD_GPIO
 	tristate "GPIO Buttons"
 	depends on GENERIC_GPIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..c475f79 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_KEYBOARD_OMAP)		+= omap-keypad.o
 obj-$(CONFIG_KEYBOARD_PXA27x)		+= pxa27x_keypad.o
 obj-$(CONFIG_KEYBOARD_PXA930_ROTARY)	+= pxa930_rotary.o
 obj-$(CONFIG_KEYBOARD_AAED2000)		+= aaed2000_kbd.o
+obj-$(CONFIG_KEYBOARD_LX)		+= lx-kbd.o
 obj-$(CONFIG_KEYBOARD_GPIO)		+= gpio_keys.o
 obj-$(CONFIG_KEYBOARD_HP6XX)		+= jornada680_kbd.o
 obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
diff --git a/drivers/input/keyboard/lx-kbd.c b/drivers/input/keyboard/lx-kbd.c
new file mode 100644
index 0000000..2b2274c
--- /dev/null
+++ b/drivers/input/keyboard/lx-kbd.c
@@ -0,0 +1,339 @@
+/*
+ * drivers/input/keyboard/lx-kbd.c
+ *
+ * (c) 2004 Simtec Electronics
+ * Written by Ben Dooks
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/apm-emulation.h>
+
+#include <mach/lx-pcon.h>
+
+#define LX_NUMKEYCODES (0x80)
+
+static unsigned char normal_map[LX_NUMKEYCODES] = {
+	[0x5b]  = KEY_ESC,
+	[0x5c]  = KEY_1,
+	[0x5d]  = KEY_2,
+	[0x47]  = KEY_3,
+	[0x36]  = KEY_4,
+	[0x37]  = KEY_5,
+	[0x5e]  = KEY_6,
+	[0x2d]  = KEY_7,
+	[0x1d]  = KEY_8,
+	[0x1e]  = KEY_9,
+	[0x11]  = KEY_0,
+	[0x0f]  = KEY_MINUS,
+	[0x0d]  = KEY_BACKSPACE,  /* delete or backspace? */
+
+	[0x02]  = KEY_TAB,
+	[0x4f]  = KEY_Q,
+	[0x53]  = KEY_W,
+	[0x46]  = KEY_E,
+	[0x35]  = KEY_R,
+	[0x38]  = KEY_T,
+	[0x03]  = KEY_Y,
+	[0x2c]  = KEY_U,
+	[0x1c]  = KEY_I,
+	[0x1f]  = KEY_O,
+	[0x12]  = KEY_P,
+	[0x10]  = KEY_EQUAL,
+	[0x00]  = KEY_ENTER,
+
+	[0x3f]  = KEY_BACKSLASH,   /* ? correct */
+	[0x50]  = KEY_A,
+	[0x52]  = KEY_S,
+	[0x45]  = KEY_D,
+	[0x42]  = KEY_F,
+	[0x39]  = KEY_G,
+	[0x61]  = KEY_H,
+	[0x2b]  = KEY_J,
+	[0x1b]  = KEY_K,
+	[0x20]  = KEY_L,
+	[0x13]  = KEY_SEMICOLON,
+	[0x29]  = KEY_APOSTROPHE,
+
+	[0x07]  = KEY_LEFTSHIFT,
+	[0x51]  = KEY_Z,
+	[0x54]  = KEY_X,
+	[0x44]  = KEY_C,
+	[0x43]  = KEY_V,
+	[0x3a]  = KEY_B,
+	[0x06]  = KEY_N,
+	[0x2a]  = KEY_M,
+	[0x28]  = KEY_COMMA,
+	[0x5f]  = KEY_DOT,
+	[0x60]  = KEY_UP,
+	[0x15]  = KEY_RIGHTSHIFT,
+
+	[0x23]  = KEY_LEFTCTRL,
+	[0x41]  = KEY_LEFTALT,
+	[0x31]  = KEY_LEFTMETA,
+	[0x34]  = KEY_SPACE,
+	[0x4d]  = KEY_SLASH,
+	[0x04]  = KEY_LEFT,
+	[0x05]  = KEY_DOWN,
+	[0x01]  = KEY_RIGHT,
+};
+
+/*
+ * Numlock mappings.  This table defines only the special keys
+ * which are affected by the numlock mode.  Other keys are mapped
+ * by the above table.
+ */
+static unsigned char numlck_map[LX_NUMKEYCODES] = {
+	[0x2d]      = KEY_KP7,
+	[0x1d]      = KEY_KP8,
+	[0x1e]      = KEY_KP9,
+	[0x11]      = KEY_KPSLASH,
+
+	[0x2c]      = KEY_KP4,
+	[0x1c]      = KEY_KP5,
+	[0x1f]      = KEY_KP6,
+	[0x12]      = KEY_KPASTERISK,
+
+	[0x2b]      = KEY_KP1,
+	[0x1b]      = KEY_KP2,
+	[0x20]      = KEY_KP3,
+	[0x13]      = KEY_KPMINUS,
+
+	[0x2a]      = KEY_KP0,
+	[0x28]      = KEY_KPDOT,
+	[0x5f]      = KEY_KPPLUS,
+};
+
+/*
+ * mappings for function key - note *ONLY* keys that produce a unique
+ * keycode should be here - others should be handled in userspace as
+ * keymap modifiers , fn is the left meta modifier
+ */
+static unsigned char fn_map[LX_NUMKEYCODES] = {
+	[0x02]	= KEY_CAPSLOCK,
+	[0x41]	= KEY_UNKNOWN,
+	[0x4d]	= KEY_F1,
+	[0x04]	= KEY_HOME,
+	[0x60]	= KEY_PAGEUP,
+	[0x05]	= KEY_PAGEDOWN,
+	[0x01]	= KEY_END,
+	[0x28]	= KEY_BRIGHTNESSDOWN,
+	[0x5f]	= KEY_BRIGHTNESSUP,
+	[0x5b]	= KEY_SUSPEND,
+};
+
+/*
+ * Record the state of key presses, so that we can do the right thing
+ * after the FN key is released.
+ */
+#define KP_NONE		(0)
+#define KP_DOWN		(1 << 0)
+#define KP_FN		(1 << 1)
+#define KP_NUMLCK	(1 << 2)
+
+static unsigned char keystate[LX_NUMKEYCODES];
+
+static int num_lck;
+
+#define key_fn		(keystate[0x31] & KP_DOWN)
+#define key_shift	(keystate[0x15] & KP_DOWN)
+#define key_alt		(keystate[0x41] & KP_DOWN)
+
+static void lx_keyb_process_key(struct input_dev *kbd, unsigned int key)
+{
+	unsigned char *map = normal_map;
+	unsigned int up;
+
+	up = key & 0x80;
+	key &= 0x7f;
+
+	if (key >= LX_NUMKEYCODES)
+		return;
+
+	/*
+	 * Use the Fn map if the function key is pressed and this
+	 * is a key press, or the key is currently in "pressed Fn"
+	 * state.  We do not want to map a press without Fn and a
+	 * subsequent release of the same key with Fn to different
+	 * keycodes.
+	 *
+	 * Ditto for the numlock state.
+	 */
+	if (fn_map[key] && ((key_fn && !up) || keystate[key] & KP_FN)) {
+		keystate[key] |= KP_FN;
+		map = fn_map;
+	} else if (numlck_map[key] &&
+		   ((num_lck && !up) || keystate[key] & KP_NUMLCK)) {
+		keystate[key] |= KP_NUMLCK;
+		map = numlck_map;
+	}
+
+	/*
+	 * The KP_DOWN bit always indicates whether a key is
+	 * pressed or released.  Releases always clear all
+	 * state about the key.
+	 */
+	if (up)
+		keystate[key] = KP_NONE;
+	else
+		keystate[key] |= KP_DOWN;
+
+	/*
+	 * Now handle the special right-shift + alt combination
+	 * for numlock.
+	 */
+	if (key == 0x41 && key_shift && key_alt) /* alt key */
+		num_lck ^= 1;
+
+	/*
+	 * Map the keycode from the LX keyboard code to the input
+	 * subsystem keycode, using the appropriate mapping table.
+	 */
+	key = map[key];
+
+	/*
+	 * Some keys have additional operations after we have
+	 * decoded them.  We don't report these "special" keys
+	 * to the input layer.
+	 */
+	switch (key) {
+	case KEY_BRIGHTNESSUP:
+	case KEY_BRIGHTNESSDOWN:
+		input_event(kbd, EV_PWR, key, !up);
+		break;
+
+	case KEY_SUSPEND:
+		input_event(kbd, EV_PWR, key, !up);
+#if defined(CONFIG_APM_EMULATION) && !defined(CONFIG_INPUT_APMPOWER)
+		if (!up)
+			apm_queue_event(APM_SYS_SUSPEND);
+#endif
+		break;
+
+	default:
+		input_report_key(kbd, key, !up);
+		break;
+	}
+}
+
+static void lx_keyb_reset(struct input_dev *kbd)
+{
+	unsigned char *map;
+	int key;
+
+	for (key = 0; key < LX_NUMKEYCODES; key++)
+		if (keystate[key] & KP_DOWN) {
+			map = normal_map;
+			if (keystate[key] & KP_FN)
+				map = fn_map;
+			else if (keystate[key] & KP_NUMLCK)
+				map = numlck_map;
+			keystate[key] = KP_NONE;
+			input_report_key(kbd, map[key], 0);
+		}
+
+	num_lck = 0;
+}
+
+static void lx_keyb_write(void *data, unsigned int addr, unsigned char val)
+{
+	struct input_dev *kbd = data;
+	pcon_kbd_ack();
+	lx_keyb_process_key(kbd, val);
+}
+
+static struct lx_i2cslave_watcher lx_keyb_watcher = {
+	.start	= PHST_KEYBOARD_START,
+	.size	= PHST_KEYBOARD_SIZE,
+	.write	= lx_keyb_write,
+};
+
+static int lx_keyb_probe(struct platform_device *dev)
+{
+	struct input_dev *kbd;
+	int i, ret;
+
+	kbd = input_allocate_device();
+	if (!kbd)
+		return -ENOMEM;
+
+	kbd->name        = "LX Keyboard";
+	kbd->phys        = "pcon/input0";
+	kbd->dev.parent  = &dev->dev;
+	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
+			   BIT_MASK(EV_PWR);
+	kbd->keycode     = normal_map;
+	kbd->keycodesize = sizeof(unsigned char);
+	kbd->keycodemax  = ARRAY_SIZE(normal_map);
+
+	for (i = 0; i < ARRAY_SIZE(normal_map); i++) {
+		if (normal_map[i])
+			__set_bit(normal_map[i], kbd->keybit);
+	}
+	for (i = 0; i < ARRAY_SIZE(numlck_map); i++) {
+		if (numlck_map[i])
+			__set_bit(numlck_map[i], kbd->keybit);
+	}
+	for (i = 0; i < ARRAY_SIZE(fn_map); i++) {
+		if (fn_map[i])
+			__set_bit(fn_map[i], kbd->keybit);
+	}
+
+	ret = input_register_device(kbd);
+	if (ret == 0) {
+		platform_set_drvdata(dev, kbd);
+		lx_i2cslave_addwatcher(&lx_keyb_watcher, kbd);
+	} else {
+		input_free_device(kbd);
+	}
+	return ret;
+}
+
+static int __devexit lx_keyb_remove(struct platform_device *dev)
+{
+	struct input_dev *kbd = platform_get_drvdata(dev);
+	platform_set_drvdata(dev, NULL);
+	lx_i2cslave_delwatcher(&lx_keyb_watcher, kbd);
+	input_unregister_device(kbd);
+	return 0;
+}
+
+static int lx_keyb_resume(struct platform_device *dev)
+{
+	struct input_dev *kbd = platform_get_drvdata(dev);
+	lx_keyb_reset(kbd);
+	return 0;
+}
+
+static struct platform_driver lx_kbd = {
+	.driver = {
+		.name	= "lx-kbd",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= lx_keyb_probe,
+	.remove	= __devexit_p(lx_keyb_remove),
+	.resume	= lx_keyb_resume,
+};
+
+static int __init lx_keyb_init(void)
+{
+	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec Electronics\n");
+	return platform_driver_register(&lx_kbd);
+}
+
+static void __exit lx_keyb_exit(void)
+{
+	platform_driver_unregister(&lx_kbd);
+}
+
+module_init(lx_keyb_init);
+module_exit(lx_keyb_exit);
+
+MODULE_AUTHOR("Ben Dooks, Simtec Electronics <ben@simtec.co.uk>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LX Keyboard Driver");
-- 
1.6.0.6

Re: [PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Dmitry Torokhov]

Hi Russell,

On Sat, Apr 18, 2009 at 03:20:50PM +0100, Russell King wrote:
> This adds support for the keyboard support on the LX platform.
> We aren't directly connected to the keyboard itself, but the
> keyboard is managed by PCON via the system I2C bus.
> 

Very nice driver. I prefer input devices having a private copy of keymap
but I suppose there is just such device in the system anyways...

>  
> +config KEYBOARD_LX
> +	tristate "LX Keyboard support"
> +	depends on MACH_NETBOOKPRO && INPUT && INPUT_KEYBOARD

Dependancy on INPUT_KEYBOARD is not needed, strictly speaking.
INPUT_KEYBOARD is only [menu]config syntactic sugar, there is no meat
behind the option.

> +
> +	/*
> +	 * Some keys have additional operations after we have
> +	 * decoded them.  We don't report these "special" keys
> +	 * to the input layer.
> +	 */
> +	switch (key) {
> +	case KEY_BRIGHTNESSUP:
> +	case KEY_BRIGHTNESSDOWN:
> +		input_event(kbd, EV_PWR, key, !up);

Umm... some userspace apps might still want to get these. Strictly
speaking KEY_* codes belong to EV_KEY namespace.

> +		break;
> +
> +	case KEY_SUSPEND:
> +		input_event(kbd, EV_PWR, key, !up);
> +#if defined(CONFIG_APM_EMULATION) && !defined(CONFIG_INPUT_APMPOWER)
> +		if (!up)
> +			apm_queue_event(APM_SYS_SUSPEND);
> +#endif
> +		break;
> +
> +	default:
> +		input_report_key(kbd, key, !up);
> +		break;
> +	}
> +}
> +
> +static void lx_keyb_reset(struct input_dev *kbd)
> +{
> +	unsigned char *map;
> +	int key;
> +
> +	for (key = 0; key < LX_NUMKEYCODES; key++)
> +		if (keystate[key] & KP_DOWN) {
> +			map = normal_map;
> +			if (keystate[key] & KP_FN)
> +				map = fn_map;
> +			else if (keystate[key] & KP_NUMLCK)
> +				map = numlck_map;
> +			keystate[key] = KP_NONE;
> +			input_report_key(kbd, map[key], 0);
> +		}

Why don't you just send 'up' event for all known keys? Input core will
filter out the ones that have not been 'down'ed... Better yet, don't do
anything here, I should finish adding forced release at suspend/resume
to the input core shortly.

> +
> +	num_lck = 0;
> +}
> +
> +static void lx_keyb_write(void *data, unsigned int addr, unsigned char val)
> +{
> +	struct input_dev *kbd = data;

Blank line between variables and the code is always appreciated ;)

> +	pcon_kbd_ack();
> +	lx_keyb_process_key(kbd, val);
> +}
> +
> +static struct lx_i2cslave_watcher lx_keyb_watcher = {
> +	.start	= PHST_KEYBOARD_START,
> +	.size	= PHST_KEYBOARD_SIZE,
> +	.write	= lx_keyb_write,
> +};
> +
> +static int lx_keyb_probe(struct platform_device *dev)
> +{
> +	struct input_dev *kbd;
> +	int i, ret;
> +
> +	kbd = input_allocate_device();
> +	if (!kbd)
> +		return -ENOMEM;
> +
> +	kbd->name        = "LX Keyboard";
> +	kbd->phys        = "pcon/input0";
> +	kbd->dev.parent  = &dev->dev;
> +	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
> +			   BIT_MASK(EV_PWR);
> +	kbd->keycode     = normal_map;
> +	kbd->keycodesize = sizeof(unsigned char);
> +	kbd->keycodemax  = ARRAY_SIZE(normal_map);
> +

So we have 3 keymaps but only one can be changed form userspace... I
suppose it is OK given how special the other 2 keymaps are. Or we could
provide an alternative {get,set}keycode methods.

> +	for (i = 0; i < ARRAY_SIZE(normal_map); i++) {
> +		if (normal_map[i])
> +			__set_bit(normal_map[i], kbd->keybit);
> +	}
> +	for (i = 0; i < ARRAY_SIZE(numlck_map); i++) {
> +		if (numlck_map[i])
> +			__set_bit(numlck_map[i], kbd->keybit);
> +	}
> +	for (i = 0; i < ARRAY_SIZE(fn_map); i++) {
> +		if (fn_map[i])
> +			__set_bit(fn_map[i], kbd->keybit);
> +	}
> +
> +	ret = input_register_device(kbd);
> +	if (ret == 0) {
> +		platform_set_drvdata(dev, kbd);
> +		lx_i2cslave_addwatcher(&lx_keyb_watcher, kbd);
> +	} else {
> +		input_free_device(kbd);
> +	}
> +	return ret;
> +}
> +
> +static int __devexit lx_keyb_remove(struct platform_device *dev)
> +{
> +	struct input_dev *kbd = platform_get_drvdata(dev);
> +	platform_set_drvdata(dev, NULL);
> +	lx_i2cslave_delwatcher(&lx_keyb_watcher, kbd);
> +	input_unregister_device(kbd);
> +	return 0;
> +}
> +
> +static int lx_keyb_resume(struct platform_device *dev)
> +{
> +	struct input_dev *kbd = platform_get_drvdata(dev);
> +	lx_keyb_reset(kbd);
> +	return 0;
> +}
> +
> +static struct platform_driver lx_kbd = {
> +	.driver = {
> +		.name	= "lx-kbd",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= lx_keyb_probe,
> +	.remove	= __devexit_p(lx_keyb_remove),
> +	.resume	= lx_keyb_resume,
> +};
> +
> +static int __init lx_keyb_init(void)
> +{
> +	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec Electronics\n");

Is this needed? Input core will already add printk when input device is
added and our boot is already chatty enough...

Thanks!

-- 
Dmitry

Re: [PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Russell King - ARM Linux]

On Sat, Apr 18, 2009 at 04:10:28PM -0700, Dmitry Torokhov wrote:
> Hi Russell,
> 
> On Sat, Apr 18, 2009 at 03:20:50PM +0100, Russell King wrote:
> > +static int __init lx_keyb_init(void)
> > +{
> > +	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec Electronics\n");
> 
> Is this needed? Input core will already add printk when input device is
> added and our boot is already chatty enough...

Simtec themselves would need to remove that line.

Re: [PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Dmitry Torokhov]

On Monday 20 April 2009 01:25:13 Russell King - ARM Linux wrote:
> On Sat, Apr 18, 2009 at 04:10:28PM -0700, Dmitry Torokhov wrote:
> > Hi Russell,
> >
> > On Sat, Apr 18, 2009 at 03:20:50PM +0100, Russell King wrote:
> > > +static int __init lx_keyb_init(void)
> > > +{
> > > +	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec
> > > Electronics\n");
> >
> > Is this needed? Input core will already add printk when input device is
> > added and our boot is already chatty enough...
>
> Simtec themselves would need to remove that line.

Umm, why? We not removing copyright notice from the file, just the
message...

-- 
Dmitry

Re: [PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Russell King - ARM Linux]

On Mon, Apr 20, 2009 at 02:04:12AM -0700, Dmitry Torokhov wrote:
> On Monday 20 April 2009 01:25:13 Russell King - ARM Linux wrote:
> > On Sat, Apr 18, 2009 at 04:10:28PM -0700, Dmitry Torokhov wrote:
> > > Hi Russell,
> > >
> > > On Sat, Apr 18, 2009 at 03:20:50PM +0100, Russell King wrote:
> > > > +static int __init lx_keyb_init(void)
> > > > +{
> > > > +	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec
> > > > Electronics\n");
> > >
> > > Is this needed? Input core will already add printk when input device is
> > > added and our boot is already chatty enough...
> >
> > Simtec themselves would need to remove that line.
> 
> Umm, why? We not removing copyright notice from the file, just the
> message...

Because it's polite to at least ask them.  I'm just the third party in
this (although I've already done substantial cleaning up of this code.)

They've agreed to remove the line, so it's now gone.

Re: [PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Russell King - ARM Linux]

On Sat, Apr 18, 2009 at 04:10:28PM -0700, Dmitry Torokhov wrote:
> > +config KEYBOARD_LX
> > +	tristate "LX Keyboard support"
> > +	depends on MACH_NETBOOKPRO && INPUT && INPUT_KEYBOARD
> 
> Dependancy on INPUT_KEYBOARD is not needed, strictly speaking.
> INPUT_KEYBOARD is only [menu]config syntactic sugar, there is no meat
> behind the option.

Gone.

> > +
> > +	/*
> > +	 * Some keys have additional operations after we have
> > +	 * decoded them.  We don't report these "special" keys
> > +	 * to the input layer.
> > +	 */
> > +	switch (key) {
> > +	case KEY_BRIGHTNESSUP:
> > +	case KEY_BRIGHTNESSDOWN:
> > +		input_event(kbd, EV_PWR, key, !up);
> 
> Umm... some userspace apps might still want to get these. Strictly
> speaking KEY_* codes belong to EV_KEY namespace.

Maybe, if someone wants it I guess they can modify the driver.

> > +static void lx_keyb_reset(struct input_dev *kbd)
> > +{
> > +	unsigned char *map;
> > +	int key;
> > +
> > +	for (key = 0; key < LX_NUMKEYCODES; key++)
> > +		if (keystate[key] & KP_DOWN) {
> > +			map = normal_map;
> > +			if (keystate[key] & KP_FN)
> > +				map = fn_map;
> > +			else if (keystate[key] & KP_NUMLCK)
> > +				map = numlck_map;
> > +			keystate[key] = KP_NONE;
> > +			input_report_key(kbd, map[key], 0);
> > +		}
> 
> Why don't you just send 'up' event for all known keys? Input core will
> filter out the ones that have not been 'down'ed... Better yet, don't do
> anything here, I should finish adding forced release at suspend/resume
> to the input core shortly.

I'll leave it as is until you've decided what to do with the input core.

> > +static void lx_keyb_write(void *data, unsigned int addr, unsigned char val)
> > +{
> > +	struct input_dev *kbd = data;
> 
> Blank line between variables and the code is always appreciated ;)

Added.

> > +	kbd->name        = "LX Keyboard";
> > +	kbd->phys        = "pcon/input0";
> > +	kbd->dev.parent  = &dev->dev;
> > +	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
> > +			   BIT_MASK(EV_PWR);
> > +	kbd->keycode     = normal_map;
> > +	kbd->keycodesize = sizeof(unsigned char);
> > +	kbd->keycodemax  = ARRAY_SIZE(normal_map);
> > +
> 
> So we have 3 keymaps but only one can be changed form userspace... I
> suppose it is OK given how special the other 2 keymaps are. Or we could
> provide an alternative {get,set}keycode methods.

Yes, not sure what to do about those.  It is a rather specialised
keyboard in itself, so the standard maps wouldn't work with it.

The other issue here is that changing what's published may break the
existing userspace (and since I didn't take a copy of the userspace
build for this platform before the server was taken down, that could
present something of a problem resulting in my use of the device being
crippled.)  Let's leave it as is and if someone does want to change it
later...

> > +static int __init lx_keyb_init(void)
> > +{
> > +	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec Electronics\n");
> 
> Is this needed? Input core will already add printk when input device is
> added and our boot is already chatty enough...

As mentioned in the other mail, now removed.

Re: [PATCH 3/5 UPDATED] [ARM] pxa: lx: add keyboard driver

[Russell King - ARM Linux]

This adds support for the keyboard support on the LX platform.
We aren't directly connected to the keyboard itself, but the
keyboard is managed by PCON via the system I2C bus.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: <linux-input@vger.kernel.org>
---
 drivers/input/keyboard/Kconfig  |    9 +
 drivers/input/keyboard/Makefile |    1 +
 drivers/input/keyboard/lx-kbd.c |  347 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 357 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/lx-kbd.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..996ff38 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -289,6 +289,15 @@ config KEYBOARD_AAED2000
 	  To compile this driver as a module, choose M here: the
 	  module will be called aaed2000_kbd.
 
+config KEYBOARD_LX
+	tristate "LX Keyboard support"
+	depends on MACH_NETBOOKPRO && INPUT
+	help
+	  Say Y here if you want to use the LX keyboard integrated in
+	  your system.
+
+	  If you have a LX platform, you should say Y.
+
 config KEYBOARD_GPIO
 	tristate "GPIO Buttons"
 	depends on GENERIC_GPIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..c475f79 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_KEYBOARD_OMAP)		+= omap-keypad.o
 obj-$(CONFIG_KEYBOARD_PXA27x)		+= pxa27x_keypad.o
 obj-$(CONFIG_KEYBOARD_PXA930_ROTARY)	+= pxa930_rotary.o
 obj-$(CONFIG_KEYBOARD_AAED2000)		+= aaed2000_kbd.o
+obj-$(CONFIG_KEYBOARD_LX)		+= lx-kbd.o
 obj-$(CONFIG_KEYBOARD_GPIO)		+= gpio_keys.o
 obj-$(CONFIG_KEYBOARD_HP6XX)		+= jornada680_kbd.o
 obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
diff --git a/drivers/input/keyboard/lx-kbd.c b/drivers/input/keyboard/lx-kbd.c
new file mode 100644
index 0000000..2fa000b
--- /dev/null
+++ b/drivers/input/keyboard/lx-kbd.c
@@ -0,0 +1,347 @@
+/*
+ * drivers/input/keyboard/lx-kbd.c
+ *
+ * Copyright (c) 2004 Simtec Electronics
+ * Written by Ben Dooks
+ *
+ * http://armlinux.simtec.co.uk/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/apm-emulation.h>
+
+#include <mach/lx-pcon.h>
+
+#define LX_NUMKEYCODES (0x80)
+
+static unsigned char normal_map[LX_NUMKEYCODES] = {
+	[0x5b]  = KEY_ESC,
+	[0x5c]  = KEY_1,
+	[0x5d]  = KEY_2,
+	[0x47]  = KEY_3,
+	[0x36]  = KEY_4,
+	[0x37]  = KEY_5,
+	[0x5e]  = KEY_6,
+	[0x2d]  = KEY_7,
+	[0x1d]  = KEY_8,
+	[0x1e]  = KEY_9,
+	[0x11]  = KEY_0,
+	[0x0f]  = KEY_MINUS,
+	[0x0d]  = KEY_BACKSPACE,  /* delete or backspace? */
+
+	[0x02]  = KEY_TAB,
+	[0x4f]  = KEY_Q,
+	[0x53]  = KEY_W,
+	[0x46]  = KEY_E,
+	[0x35]  = KEY_R,
+	[0x38]  = KEY_T,
+	[0x03]  = KEY_Y,
+	[0x2c]  = KEY_U,
+	[0x1c]  = KEY_I,
+	[0x1f]  = KEY_O,
+	[0x12]  = KEY_P,
+	[0x10]  = KEY_EQUAL,
+	[0x00]  = KEY_ENTER,
+
+	[0x3f]  = KEY_BACKSLASH,   /* ? correct */
+	[0x50]  = KEY_A,
+	[0x52]  = KEY_S,
+	[0x45]  = KEY_D,
+	[0x42]  = KEY_F,
+	[0x39]  = KEY_G,
+	[0x61]  = KEY_H,
+	[0x2b]  = KEY_J,
+	[0x1b]  = KEY_K,
+	[0x20]  = KEY_L,
+	[0x13]  = KEY_SEMICOLON,
+	[0x29]  = KEY_APOSTROPHE,
+
+	[0x07]  = KEY_LEFTSHIFT,
+	[0x51]  = KEY_Z,
+	[0x54]  = KEY_X,
+	[0x44]  = KEY_C,
+	[0x43]  = KEY_V,
+	[0x3a]  = KEY_B,
+	[0x06]  = KEY_N,
+	[0x2a]  = KEY_M,
+	[0x28]  = KEY_COMMA,
+	[0x5f]  = KEY_DOT,
+	[0x60]  = KEY_UP,
+	[0x15]  = KEY_RIGHTSHIFT,
+
+	[0x23]  = KEY_LEFTCTRL,
+	[0x41]  = KEY_LEFTALT,
+	[0x31]  = KEY_LEFTMETA,
+	[0x34]  = KEY_SPACE,
+	[0x4d]  = KEY_SLASH,
+	[0x04]  = KEY_LEFT,
+	[0x05]  = KEY_DOWN,
+	[0x01]  = KEY_RIGHT,
+};
+
+/*
+ * Numlock mappings.  This table defines only the special keys
+ * which are affected by the numlock mode.  Other keys are mapped
+ * by the above table.
+ */
+static unsigned char numlck_map[LX_NUMKEYCODES] = {
+	[0x2d]      = KEY_KP7,
+	[0x1d]      = KEY_KP8,
+	[0x1e]      = KEY_KP9,
+	[0x11]      = KEY_KPSLASH,
+
+	[0x2c]      = KEY_KP4,
+	[0x1c]      = KEY_KP5,
+	[0x1f]      = KEY_KP6,
+	[0x12]      = KEY_KPASTERISK,
+
+	[0x2b]      = KEY_KP1,
+	[0x1b]      = KEY_KP2,
+	[0x20]      = KEY_KP3,
+	[0x13]      = KEY_KPMINUS,
+
+	[0x2a]      = KEY_KP0,
+	[0x28]      = KEY_KPDOT,
+	[0x5f]      = KEY_KPPLUS,
+};
+
+/*
+ * mappings for function key - note *ONLY* keys that produce a unique
+ * keycode should be here - others should be handled in userspace as
+ * keymap modifiers , fn is the left meta modifier
+ */
+static unsigned char fn_map[LX_NUMKEYCODES] = {
+	[0x02]	= KEY_CAPSLOCK,
+	[0x41]	= KEY_UNKNOWN,
+	[0x4d]	= KEY_F1,
+	[0x04]	= KEY_HOME,
+	[0x60]	= KEY_PAGEUP,
+	[0x05]	= KEY_PAGEDOWN,
+	[0x01]	= KEY_END,
+	[0x28]	= KEY_BRIGHTNESSDOWN,
+	[0x5f]	= KEY_BRIGHTNESSUP,
+	[0x5b]	= KEY_SUSPEND,
+};
+
+/*
+ * Record the state of key presses, so that we can do the right thing
+ * after the FN key is released.
+ */
+#define KP_NONE		(0)
+#define KP_DOWN		(1 << 0)
+#define KP_FN		(1 << 1)
+#define KP_NUMLCK	(1 << 2)
+
+static unsigned char keystate[LX_NUMKEYCODES];
+
+static int num_lck;
+
+#define key_fn		(keystate[0x31] & KP_DOWN)
+#define key_shift	(keystate[0x15] & KP_DOWN)
+#define key_alt		(keystate[0x41] & KP_DOWN)
+
+static void lx_keyb_process_key(struct input_dev *kbd, unsigned int key)
+{
+	unsigned char *map = normal_map;
+	unsigned int up;
+
+	up = key & 0x80;
+	key &= 0x7f;
+
+	if (key >= LX_NUMKEYCODES)
+		return;
+
+	/*
+	 * Use the Fn map if the function key is pressed and this
+	 * is a key press, or the key is currently in "pressed Fn"
+	 * state.  We do not want to map a press without Fn and a
+	 * subsequent release of the same key with Fn to different
+	 * keycodes.
+	 *
+	 * Ditto for the numlock state.
+	 */
+	if (fn_map[key] && ((key_fn && !up) || keystate[key] & KP_FN)) {
+		keystate[key] |= KP_FN;
+		map = fn_map;
+	} else if (numlck_map[key] &&
+		   ((num_lck && !up) || keystate[key] & KP_NUMLCK)) {
+		keystate[key] |= KP_NUMLCK;
+		map = numlck_map;
+	}
+
+	/*
+	 * The KP_DOWN bit always indicates whether a key is
+	 * pressed or released.  Releases always clear all
+	 * state about the key.
+	 */
+	if (up)
+		keystate[key] = KP_NONE;
+	else
+		keystate[key] |= KP_DOWN;
+
+	/*
+	 * Now handle the special right-shift + alt combination
+	 * for numlock.
+	 */
+	if (key == 0x41 && key_shift && key_alt) /* alt key */
+		num_lck ^= 1;
+
+	/*
+	 * Map the keycode from the LX keyboard code to the input
+	 * subsystem keycode, using the appropriate mapping table.
+	 */
+	key = map[key];
+
+	/*
+	 * Some keys have additional operations after we have
+	 * decoded them.  We don't report these "special" keys
+	 * to the input layer.
+	 */
+	switch (key) {
+	case KEY_BRIGHTNESSUP:
+	case KEY_BRIGHTNESSDOWN:
+		input_event(kbd, EV_PWR, key, !up);
+		break;
+
+	case KEY_SUSPEND:
+		input_event(kbd, EV_PWR, key, !up);
+#if defined(CONFIG_APM_EMULATION) && !defined(CONFIG_INPUT_APMPOWER)
+		if (!up)
+			apm_queue_event(APM_SYS_SUSPEND);
+#endif
+		break;
+
+	default:
+		input_report_key(kbd, key, !up);
+		break;
+	}
+}
+
+static void lx_keyb_reset(struct input_dev *kbd)
+{
+	unsigned char *map;
+	int key;
+
+	for (key = 0; key < LX_NUMKEYCODES; key++)
+		if (keystate[key] & KP_DOWN) {
+			map = normal_map;
+			if (keystate[key] & KP_FN)
+				map = fn_map;
+			else if (keystate[key] & KP_NUMLCK)
+				map = numlck_map;
+			keystate[key] = KP_NONE;
+			input_report_key(kbd, map[key], 0);
+		}
+
+	num_lck = 0;
+}
+
+static void lx_keyb_write(void *data, unsigned int addr, unsigned char val)
+{
+	struct input_dev *kbd = data;
+
+	pcon_kbd_ack();
+	lx_keyb_process_key(kbd, val);
+}
+
+static struct lx_i2cslave_watcher lx_keyb_watcher = {
+	.start	= PHST_KEYBOARD_START,
+	.size	= PHST_KEYBOARD_SIZE,
+	.write	= lx_keyb_write,
+};
+
+static int lx_keyb_probe(struct platform_device *dev)
+{
+	struct input_dev *kbd;
+	int i, ret;
+
+	kbd = input_allocate_device();
+	if (!kbd)
+		return -ENOMEM;
+
+	kbd->name        = "LX Keyboard";
+	kbd->phys        = "pcon/input0";
+	kbd->dev.parent  = &dev->dev;
+	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
+			   BIT_MASK(EV_PWR);
+	kbd->keycode     = normal_map;
+	kbd->keycodesize = sizeof(unsigned char);
+	kbd->keycodemax  = ARRAY_SIZE(normal_map);
+
+	for (i = 0; i < ARRAY_SIZE(normal_map); i++) {
+		if (normal_map[i])
+			__set_bit(normal_map[i], kbd->keybit);
+	}
+	for (i = 0; i < ARRAY_SIZE(numlck_map); i++) {
+		if (numlck_map[i])
+			__set_bit(numlck_map[i], kbd->keybit);
+	}
+	for (i = 0; i < ARRAY_SIZE(fn_map); i++) {
+		if (fn_map[i])
+			__set_bit(fn_map[i], kbd->keybit);
+	}
+
+	ret = input_register_device(kbd);
+	if (ret == 0) {
+		platform_set_drvdata(dev, kbd);
+		lx_i2cslave_addwatcher(&lx_keyb_watcher, kbd);
+	} else {
+		input_free_device(kbd);
+	}
+	return ret;
+}
+
+static int __devexit lx_keyb_remove(struct platform_device *dev)
+{
+	struct input_dev *kbd = platform_get_drvdata(dev);
+
+	platform_set_drvdata(dev, NULL);
+	lx_i2cslave_delwatcher(&lx_keyb_watcher, kbd);
+	input_unregister_device(kbd);
+	return 0;
+}
+
+static int lx_keyb_resume(struct platform_device *dev)
+{
+	struct input_dev *kbd = platform_get_drvdata(dev);
+
+	lx_keyb_reset(kbd);
+	return 0;
+}
+
+static struct platform_driver lx_kbd = {
+	.driver = {
+		.name	= "lx-kbd",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= lx_keyb_probe,
+	.remove	= __devexit_p(lx_keyb_remove),
+	.resume	= lx_keyb_resume,
+};
+
+static int __init lx_keyb_init(void)
+{
+	return platform_driver_register(&lx_kbd);
+}
+
+static void __exit lx_keyb_exit(void)
+{
+	platform_driver_unregister(&lx_kbd);
+}
+
+module_init(lx_keyb_init);
+module_exit(lx_keyb_exit);
+
+MODULE_AUTHOR("Ben Dooks, Simtec Electronics <ben@simtec.co.uk>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LX Keyboard Driver");
-- 
1.6.0.6

Re: [PATCH 3/5 UPDATED] [ARM] pxa: lx: add keyboard driver

[Dmitry Torokhov]

Hi Russell,

On Mon, Apr 20, 2009 at 12:13:47PM +0100, Russell King - ARM Linux wrote:
> +
> +static unsigned char normal_map[LX_NUMKEYCODES] = {

Please chanege this to unsigned short since we _do_ allow changing this
part of keymap from userspace and possible KEY_* values extend beyond
unsigned char range.

> +static unsigned char numlck_map[LX_NUMKEYCODES] = {
> +static unsigned char fn_map[LX_NUMKEYCODES] = {

These 2 will have to be unisgned short as well, unless you want to adjust
lx_keyb_process_key() to cope with different keymap sizes...

> +	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
> +			   BIT_MASK(EV_PWR);
> +	kbd->keycode     = normal_map;
> +	kbd->keycodesize = sizeof(unsigned char);

And this should be "sizeof(normal_map[0]);" then.

Otherwise you may add:

	Acked-by: Dmitry Torokhov <dtor@mail.ru>

I assume the driver will be merged with the rest of LX changes through
your tree, is that right?

-- 
Dmitry

Re: [PATCH 3/5 UPDATED] [ARM] pxa: lx: add keyboard driver

[Russell King - ARM Linux]

> Otherwise you may add:
> 
> 	Acked-by: Dmitry Torokhov <dtor@mail.ru>
> 
> I assume the driver will be merged with the rest of LX changes through
> your tree, is that right?

Yes.

Final version below, I'm assuming you have no further comments so added
your acked by as above.

Subject: [PATCH] [ARM] pxa: lx: add keyboard driver

This adds support for the keyboard support on the LX platform.
We aren't directly connected to the keyboard itself, but the
keyboard is managed by PCON via the system I2C bus.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: <linux-input@vger.kernel.org>
---
 drivers/input/keyboard/Kconfig  |    9 +
 drivers/input/keyboard/Makefile |    1 +
 drivers/input/keyboard/lx-kbd.c |  347 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 357 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/lx-kbd.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..996ff38 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -289,6 +289,15 @@ config KEYBOARD_AAED2000
 	  To compile this driver as a module, choose M here: the
 	  module will be called aaed2000_kbd.
 
+config KEYBOARD_LX
+	tristate "LX Keyboard support"
+	depends on MACH_NETBOOKPRO && INPUT
+	help
+	  Say Y here if you want to use the LX keyboard integrated in
+	  your system.
+
+	  If you have a LX platform, you should say Y.
+
 config KEYBOARD_GPIO
 	tristate "GPIO Buttons"
 	depends on GENERIC_GPIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..c475f79 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_KEYBOARD_OMAP)		+= omap-keypad.o
 obj-$(CONFIG_KEYBOARD_PXA27x)		+= pxa27x_keypad.o
 obj-$(CONFIG_KEYBOARD_PXA930_ROTARY)	+= pxa930_rotary.o
 obj-$(CONFIG_KEYBOARD_AAED2000)		+= aaed2000_kbd.o
+obj-$(CONFIG_KEYBOARD_LX)		+= lx-kbd.o
 obj-$(CONFIG_KEYBOARD_GPIO)		+= gpio_keys.o
 obj-$(CONFIG_KEYBOARD_HP6XX)		+= jornada680_kbd.o
 obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
diff --git a/drivers/input/keyboard/lx-kbd.c b/drivers/input/keyboard/lx-kbd.c
new file mode 100644
index 0000000..98986cb
--- /dev/null
+++ b/drivers/input/keyboard/lx-kbd.c
@@ -0,0 +1,347 @@
+/*
+ * drivers/input/keyboard/lx-kbd.c
+ *
+ * Copyright (c) 2004 Simtec Electronics
+ * Written by Ben Dooks
+ *
+ * http://armlinux.simtec.co.uk/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/apm-emulation.h>
+
+#include <mach/lx-pcon.h>
+
+#define LX_NUMKEYCODES (0x80)
+
+static unsigned short normal_map[LX_NUMKEYCODES] = {
+	[0x5b]  = KEY_ESC,
+	[0x5c]  = KEY_1,
+	[0x5d]  = KEY_2,
+	[0x47]  = KEY_3,
+	[0x36]  = KEY_4,
+	[0x37]  = KEY_5,
+	[0x5e]  = KEY_6,
+	[0x2d]  = KEY_7,
+	[0x1d]  = KEY_8,
+	[0x1e]  = KEY_9,
+	[0x11]  = KEY_0,
+	[0x0f]  = KEY_MINUS,
+	[0x0d]  = KEY_BACKSPACE,  /* delete or backspace? */
+
+	[0x02]  = KEY_TAB,
+	[0x4f]  = KEY_Q,
+	[0x53]  = KEY_W,
+	[0x46]  = KEY_E,
+	[0x35]  = KEY_R,
+	[0x38]  = KEY_T,
+	[0x03]  = KEY_Y,
+	[0x2c]  = KEY_U,
+	[0x1c]  = KEY_I,
+	[0x1f]  = KEY_O,
+	[0x12]  = KEY_P,
+	[0x10]  = KEY_EQUAL,
+	[0x00]  = KEY_ENTER,
+
+	[0x3f]  = KEY_BACKSLASH,   /* ? correct */
+	[0x50]  = KEY_A,
+	[0x52]  = KEY_S,
+	[0x45]  = KEY_D,
+	[0x42]  = KEY_F,
+	[0x39]  = KEY_G,
+	[0x61]  = KEY_H,
+	[0x2b]  = KEY_J,
+	[0x1b]  = KEY_K,
+	[0x20]  = KEY_L,
+	[0x13]  = KEY_SEMICOLON,
+	[0x29]  = KEY_APOSTROPHE,
+
+	[0x07]  = KEY_LEFTSHIFT,
+	[0x51]  = KEY_Z,
+	[0x54]  = KEY_X,
+	[0x44]  = KEY_C,
+	[0x43]  = KEY_V,
+	[0x3a]  = KEY_B,
+	[0x06]  = KEY_N,
+	[0x2a]  = KEY_M,
+	[0x28]  = KEY_COMMA,
+	[0x5f]  = KEY_DOT,
+	[0x60]  = KEY_UP,
+	[0x15]  = KEY_RIGHTSHIFT,
+
+	[0x23]  = KEY_LEFTCTRL,
+	[0x41]  = KEY_LEFTALT,
+	[0x31]  = KEY_LEFTMETA,
+	[0x34]  = KEY_SPACE,
+	[0x4d]  = KEY_SLASH,
+	[0x04]  = KEY_LEFT,
+	[0x05]  = KEY_DOWN,
+	[0x01]  = KEY_RIGHT,
+};
+
+/*
+ * Numlock mappings.  This table defines only the special keys
+ * which are affected by the numlock mode.  Other keys are mapped
+ * by the above table.
+ */
+static unsigned short numlck_map[LX_NUMKEYCODES] = {
+	[0x2d]      = KEY_KP7,
+	[0x1d]      = KEY_KP8,
+	[0x1e]      = KEY_KP9,
+	[0x11]      = KEY_KPSLASH,
+
+	[0x2c]      = KEY_KP4,
+	[0x1c]      = KEY_KP5,
+	[0x1f]      = KEY_KP6,
+	[0x12]      = KEY_KPASTERISK,
+
+	[0x2b]      = KEY_KP1,
+	[0x1b]      = KEY_KP2,
+	[0x20]      = KEY_KP3,
+	[0x13]      = KEY_KPMINUS,
+
+	[0x2a]      = KEY_KP0,
+	[0x28]      = KEY_KPDOT,
+	[0x5f]      = KEY_KPPLUS,
+};
+
+/*
+ * mappings for function key - note *ONLY* keys that produce a unique
+ * keycode should be here - others should be handled in userspace as
+ * keymap modifiers , fn is the left meta modifier
+ */
+static unsigned short fn_map[LX_NUMKEYCODES] = {
+	[0x02]	= KEY_CAPSLOCK,
+	[0x41]	= KEY_UNKNOWN,
+	[0x4d]	= KEY_F1,
+	[0x04]	= KEY_HOME,
+	[0x60]	= KEY_PAGEUP,
+	[0x05]	= KEY_PAGEDOWN,
+	[0x01]	= KEY_END,
+	[0x28]	= KEY_BRIGHTNESSDOWN,
+	[0x5f]	= KEY_BRIGHTNESSUP,
+	[0x5b]	= KEY_SUSPEND,
+};
+
+/*
+ * Record the state of key presses, so that we can do the right thing
+ * after the FN key is released.
+ */
+#define KP_NONE		(0)
+#define KP_DOWN		(1 << 0)
+#define KP_FN		(1 << 1)
+#define KP_NUMLCK	(1 << 2)
+
+static unsigned char keystate[LX_NUMKEYCODES];
+
+static int num_lck;
+
+#define key_fn		(keystate[0x31] & KP_DOWN)
+#define key_shift	(keystate[0x15] & KP_DOWN)
+#define key_alt		(keystate[0x41] & KP_DOWN)
+
+static void lx_keyb_process_key(struct input_dev *kbd, unsigned int key)
+{
+	unsigned short *map = normal_map;
+	unsigned int up;
+
+	up = key & 0x80;
+	key &= 0x7f;
+
+	if (key >= LX_NUMKEYCODES)
+		return;
+
+	/*
+	 * Use the Fn map if the function key is pressed and this
+	 * is a key press, or the key is currently in "pressed Fn"
+	 * state.  We do not want to map a press without Fn and a
+	 * subsequent release of the same key with Fn to different
+	 * keycodes.
+	 *
+	 * Ditto for the numlock state.
+	 */
+	if (fn_map[key] && ((key_fn && !up) || keystate[key] & KP_FN)) {
+		keystate[key] |= KP_FN;
+		map = fn_map;
+	} else if (numlck_map[key] &&
+		   ((num_lck && !up) || keystate[key] & KP_NUMLCK)) {
+		keystate[key] |= KP_NUMLCK;
+		map = numlck_map;
+	}
+
+	/*
+	 * The KP_DOWN bit always indicates whether a key is
+	 * pressed or released.  Releases always clear all
+	 * state about the key.
+	 */
+	if (up)
+		keystate[key] = KP_NONE;
+	else
+		keystate[key] |= KP_DOWN;
+
+	/*
+	 * Now handle the special right-shift + alt combination
+	 * for numlock.
+	 */
+	if (key == 0x41 && key_shift && key_alt) /* alt key */
+		num_lck ^= 1;
+
+	/*
+	 * Map the keycode from the LX keyboard code to the input
+	 * subsystem keycode, using the appropriate mapping table.
+	 */
+	key = map[key];
+
+	/*
+	 * Some keys have additional operations after we have
+	 * decoded them.  We don't report these "special" keys
+	 * to the input layer.
+	 */
+	switch (key) {
+	case KEY_BRIGHTNESSUP:
+	case KEY_BRIGHTNESSDOWN:
+		input_event(kbd, EV_PWR, key, !up);
+		break;
+
+	case KEY_SUSPEND:
+		input_event(kbd, EV_PWR, key, !up);
+#if defined(CONFIG_APM_EMULATION) && !defined(CONFIG_INPUT_APMPOWER)
+		if (!up)
+			apm_queue_event(APM_SYS_SUSPEND);
+#endif
+		break;
+
+	default:
+		input_report_key(kbd, key, !up);
+		break;
+	}
+}
+
+static void lx_keyb_reset(struct input_dev *kbd)
+{
+	unsigned short *map;
+	int key;
+
+	for (key = 0; key < LX_NUMKEYCODES; key++)
+		if (keystate[key] & KP_DOWN) {
+			map = normal_map;
+			if (keystate[key] & KP_FN)
+				map = fn_map;
+			else if (keystate[key] & KP_NUMLCK)
+				map = numlck_map;
+			keystate[key] = KP_NONE;
+			input_report_key(kbd, map[key], 0);
+		}
+
+	num_lck = 0;
+}
+
+static void lx_keyb_write(void *data, unsigned int addr, unsigned char val)
+{
+	struct input_dev *kbd = data;
+
+	pcon_kbd_ack();
+	lx_keyb_process_key(kbd, val);
+}
+
+static struct lx_i2cslave_watcher lx_keyb_watcher = {
+	.start	= PHST_KEYBOARD_START,
+	.size	= PHST_KEYBOARD_SIZE,
+	.write	= lx_keyb_write,
+};
+
+static int lx_keyb_probe(struct platform_device *dev)
+{
+	struct input_dev *kbd;
+	int i, ret;
+
+	kbd = input_allocate_device();
+	if (!kbd)
+		return -ENOMEM;
+
+	kbd->name        = "LX Keyboard";
+	kbd->phys        = "pcon/input0";
+	kbd->dev.parent  = &dev->dev;
+	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
+			   BIT_MASK(EV_PWR);
+	kbd->keycode     = normal_map;
+	kbd->keycodesize = sizeof(normal_map[0]);
+	kbd->keycodemax  = ARRAY_SIZE(normal_map);
+
+	for (i = 0; i < ARRAY_SIZE(normal_map); i++) {
+		if (normal_map[i])
+			__set_bit(normal_map[i], kbd->keybit);
+	}
+	for (i = 0; i < ARRAY_SIZE(numlck_map); i++) {
+		if (numlck_map[i])
+			__set_bit(numlck_map[i], kbd->keybit);
+	}
+	for (i = 0; i < ARRAY_SIZE(fn_map); i++) {
+		if (fn_map[i])
+			__set_bit(fn_map[i], kbd->keybit);
+	}
+
+	ret = input_register_device(kbd);
+	if (ret == 0) {
+		platform_set_drvdata(dev, kbd);
+		lx_i2cslave_addwatcher(&lx_keyb_watcher, kbd);
+	} else {
+		input_free_device(kbd);
+	}
+	return ret;
+}
+
+static int __devexit lx_keyb_remove(struct platform_device *dev)
+{
+	struct input_dev *kbd = platform_get_drvdata(dev);
+
+	platform_set_drvdata(dev, NULL);
+	lx_i2cslave_delwatcher(&lx_keyb_watcher, kbd);
+	input_unregister_device(kbd);
+	return 0;
+}
+
+static int lx_keyb_resume(struct platform_device *dev)
+{
+	struct input_dev *kbd = platform_get_drvdata(dev);
+
+	lx_keyb_reset(kbd);
+	return 0;
+}
+
+static struct platform_driver lx_kbd = {
+	.driver = {
+		.name	= "lx-kbd",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= lx_keyb_probe,
+	.remove	= __devexit_p(lx_keyb_remove),
+	.resume	= lx_keyb_resume,
+};
+
+static int __init lx_keyb_init(void)
+{
+	return platform_driver_register(&lx_kbd);
+}
+
+static void __exit lx_keyb_exit(void)
+{
+	platform_driver_unregister(&lx_kbd);
+}
+
+module_init(lx_keyb_init);
+module_exit(lx_keyb_exit);
+
+MODULE_AUTHOR("Ben Dooks, Simtec Electronics <ben@simtec.co.uk>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LX Keyboard Driver");
-- 
1.6.0.6

Re: [PATCH 3/5] [ARM] pxa: lx: add keyboard driver

[Dmitry Torokhov]

On Mon, Apr 20, 2009 at 12:06:40PM +0100, Russell King - ARM Linux wrote:
> On Sat, Apr 18, 2009 at 04:10:28PM -0700, Dmitry Torokhov wrote:
> > > +config KEYBOARD_LX
> > > +	tristate "LX Keyboard support"
> > > +	depends on MACH_NETBOOKPRO && INPUT && INPUT_KEYBOARD
> > 
> > Dependancy on INPUT_KEYBOARD is not needed, strictly speaking.
> > INPUT_KEYBOARD is only [menu]config syntactic sugar, there is no meat
> > behind the option.
> 
> Gone.
> 
> > > +
> > > +	/*
> > > +	 * Some keys have additional operations after we have
> > > +	 * decoded them.  We don't report these "special" keys
> > > +	 * to the input layer.
> > > +	 */
> > > +	switch (key) {
> > > +	case KEY_BRIGHTNESSUP:
> > > +	case KEY_BRIGHTNESSDOWN:
> > > +		input_event(kbd, EV_PWR, key, !up);
> > 
> > Umm... some userspace apps might still want to get these. Strictly
> > speaking KEY_* codes belong to EV_KEY namespace.
> 
> Maybe, if someone wants it I guess they can modify the driver.
> 

OK.

> > > +static void lx_keyb_reset(struct input_dev *kbd)
> > > +{
> > > +	unsigned char *map;
> > > +	int key;
> > > +
> > > +	for (key = 0; key < LX_NUMKEYCODES; key++)
> > > +		if (keystate[key] & KP_DOWN) {
> > > +			map = normal_map;
> > > +			if (keystate[key] & KP_FN)
> > > +				map = fn_map;
> > > +			else if (keystate[key] & KP_NUMLCK)
> > > +				map = numlck_map;
> > > +			keystate[key] = KP_NONE;
> > > +			input_report_key(kbd, map[key], 0);
> > > +		}
> > 
> > Why don't you just send 'up' event for all known keys? Input core will
> > filter out the ones that have not been 'down'ed... Better yet, don't do
> > anything here, I should finish adding forced release at suspend/resume
> > to the input core shortly.
> 
> I'll leave it as is until you've decided what to do with the input core.
>

Fair enough.

> > > +static void lx_keyb_write(void *data, unsigned int addr, unsigned char val)
> > > +{
> > > +	struct input_dev *kbd = data;
> > 
> > Blank line between variables and the code is always appreciated ;)
> 
> Added.
> 
> > > +	kbd->name        = "LX Keyboard";
> > > +	kbd->phys        = "pcon/input0";
> > > +	kbd->dev.parent  = &dev->dev;
> > > +	kbd->evbit[0]    = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
> > > +			   BIT_MASK(EV_PWR);
> > > +	kbd->keycode     = normal_map;
> > > +	kbd->keycodesize = sizeof(unsigned char);
> > > +	kbd->keycodemax  = ARRAY_SIZE(normal_map);
> > > +
> > 
> > So we have 3 keymaps but only one can be changed form userspace... I
> > suppose it is OK given how special the other 2 keymaps are. Or we could
> > provide an alternative {get,set}keycode methods.
> 
> Yes, not sure what to do about those.  It is a rather specialised
> keyboard in itself, so the standard maps wouldn't work with it.
> 
> The other issue here is that changing what's published may break the
> existing userspace (and since I didn't take a copy of the userspace
> build for this platform before the server was taken down, that could
> present something of a problem resulting in my use of the device being
> crippled.)  Let's leave it as is and if someone does want to change it
> later...
> 

I did not mean we would change anything as far as default setup is
concerned and is visible from userspace; it would be all internal
adjustment that would also allow changing numlck_map and fn_map mapping
in addition to changing normal_map. I will just put it on my TODO
list...

> > > +static int __init lx_keyb_init(void)
> > > +{
> > > +	printk(KERN_INFO "LX Keyboard Driver, (c) 2004 Simtec Electronics\n");
> > 
> > Is this needed? Input core will already add printk when input device is
> > added and our boot is already chatty enough...
> 
> As mentioned in the other mail, now removed.

Thanks.

-- 
Dmitry