linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6] input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.
@ 2010-01-30 11:53 Alberto Panizzo
  2010-01-31  9:08 ` Dmitry Torokhov
  0 siblings, 1 reply; 4+ messages in thread
From: Alberto Panizzo @ 2010-01-30 11:53 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Uwe Kleine-König, Lothar Waßmann, H Hartley Sweeten,
	Sascha linux-arm, linux-input, linux-arm-kernel

The IMX family of Application Processors is shipped with a Keypad Port
supported now by this driver.

The peripheral can control up to an 8x8 matrix key pad where all the scanning
procedure is done via software.

The hardware provide two interrupts: one for a key pressed (KDI) and one for
all key releases (KRI). There is also a simple circuit for glitch reduction
(said for synchronization) made by two series of 3 D-latches clocked by the
keypad-clock that stabilize the interrupts sources.
KDI and KRI are fired only if the respective conditions are maintained for at
last 4 keypad-clock cycle.

Since those circuits are poor for a correct debounce process (the keypad-clock
frequency is 32K and bounces longer than 94us are not masked) the driver,
when an interrupt arrive, sample the matrix with a period of 10ms until
for IMX_KEYPAD_SCANS_FOR_STABILITY (3) times the matrix result stable.
After reached the stability proper events are fired comparing the new and
previous stable state.
If some keys are maintained pressed, the driver continue to probe the matrix
with a greater period (60ms) to catch possible multiple key pressures without
overloading the cpu.
This process ends when all keys are released.

This driver is tested to build in kernel or as a module and follow the
specification of Freescale Application processors:
i.MX25 i.MX27 i.MX31 i.MX35 i.MX51 especially tested on i.MX31.

Signed-off-by: Alberto Panizzo <maramaopercheseimorto@gmail.com>
---

Changes in v4:
-Better error handling.
-Coding style fixes.
-Fix parameters of clk_get.

Changes in v5:
-Rework of debouncing process. Now it is the driver that decide when the
 matrix is stable to check for events.

Changes in v6:
-MXC to IMX pattern change (apart of Kconfig dependencies)
-Comment style fixes
-Greater check delay in debouncing process (10). 
-Improve the usage of keypad->exit_flag: now it is false only between open and 
 close functions. And if we handle an interrupt out there, leave all interrupts
 masked, and wait for another open to re enable they.
-Remove unnecessary "free events" mechanism (tested with evtest).

 drivers/input/keyboard/Kconfig      |    9 +
 drivers/input/keyboard/Makefile     |    1 +
 drivers/input/keyboard/imx_keypad.c |  605 +++++++++++++++++++++++++++++++++++
 3 files changed, 615 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/imx_keypad.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 02c836e..78e6cf5 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -292,6 +292,15 @@ config KEYBOARD_MAX7359
 	  To compile this driver as a module, choose M here: the
 	  module will be called max7359_keypad.
 
+config KEYBOARD_IMX
+	tristate "IMX keypad support"
+	depends on ARCH_MXC
+	help
+	  Enable support for IMX keypad port.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called imx_keypad.
+
 config KEYBOARD_NEWTON
 	tristate "Newton keyboard"
 	select SERIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 78654ef..b3f8567 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_KEYBOARD_LOCOMO)		+= locomokbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
+obj-$(CONFIG_KEYBOARD_IMX)		+= imx_keypad.o
 obj-$(CONFIG_KEYBOARD_NEWTON)		+= newtonkbd.o
 obj-$(CONFIG_KEYBOARD_OMAP)		+= omap-keypad.o
 obj-$(CONFIG_KEYBOARD_OPENCORES)	+= opencores-kbd.o
diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
new file mode 100644
index 0000000..6bdea71
--- /dev/null
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -0,0 +1,605 @@
+/*
+ * Driver for the IMX keypad port.
+ * Copyright (C) 2009 Alberto Panizzo <maramaopercheseimorto@gmail.com>
+ *
+ * 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.
+ *
+ * <<Power management needs to be implemented>>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/timer.h>
+
+/*
+ * Keypad Controller registers (halfword)
+ */
+#define KPCR		0x00 /* Keypad Control Register */
+
+#define KPSR		0x02 /* Keypad Status Register */
+#define KBD_STAT_KPKD	(0x1 << 0) /* Key Press Interrupt Status bit (w1c) */
+#define KBD_STAT_KPKR	(0x1 << 1) /* Key Release Interrupt Status bit (w1c) */
+#define KBD_STAT_KDSC	(0x1 << 2) /* Key Depress Synch Chain Status bit (w1c)*/
+#define KBD_STAT_KRSS	(0x1 << 3) /* Key Release Synch Status bit (w1c)*/
+#define KBD_STAT_KDIE	(0x1 << 8) /* Key Depress Interrupt Enable Status bit */
+#define KBD_STAT_KRIE	(0x1 << 9) /* Key Release Interrupt Enable */
+#define KBD_STAT_KPPEN	(0x1 << 10) /* Keypad Clock Enable */
+
+#define KDDR		0x04 /* Keypad Data Direction Register */
+#define KPDR		0x06 /* Keypad Data Register */
+
+#define MAX_MATRIX_KEY_ROWS	8
+#define MAX_MATRIX_KEY_COLS	8
+#define MATRIX_ROW_SHIFT	3
+
+#define MAX_MATRIX_KEY_NUM	(MAX_MATRIX_KEY_ROWS * MAX_MATRIX_KEY_COLS)
+
+struct imx_keypad {
+
+	struct clk *clk;
+	struct input_dev *input_dev;
+	void __iomem *mmio_base;
+
+	int			irq;
+	struct timer_list	check_matrix_timer;
+
+	/*
+	 * The matrix is stable only if no changes are detected after
+	 * IMX_KEYPAD_SCANS_FOR_STABILITY scans
+	 */
+#define IMX_KEYPAD_SCANS_FOR_STABILITY 3
+	int			stable_count;
+
+	/* If true the driver is shutting down */
+	bool			exit_flag;
+
+	/* Masks for enabled rows/cols */
+	unsigned short		rows_en_mask;
+	unsigned short		cols_en_mask;
+
+	unsigned short		keycodes[MAX_MATRIX_KEY_NUM];
+
+	/*
+	 * Matrix states:
+	 * -stable: achieved after a complete debounce process.
+	 * -unstable: used in the debouncing process.
+	 */
+	unsigned short		matrix_stable_state[MAX_MATRIX_KEY_COLS];
+	unsigned short		matrix_unstable_state[MAX_MATRIX_KEY_COLS];
+};
+
+/* Scan the matrix and return the new state in *matrix_volatile_state. */
+static void imx_keypad_scan_matrix(struct imx_keypad *keypad,
+				  unsigned short *matrix_volatile_state)
+{
+	int col;
+	unsigned short reg_val;
+
+	for (col = 0; col < MAX_MATRIX_KEY_COLS; col++) {
+		if ((keypad->cols_en_mask & (1 << col)) == 0)
+			continue;
+		/*
+		 * Discharge keypad capacitance:
+		 * 2. write 1s on column data.
+		 * 3. configure columns as totem-pole to discharge capacitance.
+		 * 4. configure columns as open-drain.
+		 */
+		reg_val = readw(keypad->mmio_base + KPDR);
+		reg_val |= 0xff00;
+		writew(reg_val, keypad->mmio_base + KPDR);
+
+		reg_val = readw(keypad->mmio_base + KPCR);
+		reg_val &= ~((keypad->cols_en_mask & 0xff) << 8);
+		writew(reg_val, keypad->mmio_base + KPCR);
+
+		udelay(2);
+
+		reg_val = readw(keypad->mmio_base + KPCR);
+		reg_val |= (keypad->cols_en_mask & 0xff) << 8;
+		writew(reg_val, keypad->mmio_base + KPCR);
+
+		/*
+		 * 5. Write a single column to 0, others to 1.
+		 * 6. Sample row inputs and save data.
+		 * 7. Repeat steps 2 - 6 for remaining columns.
+		 */
+		reg_val = readw(keypad->mmio_base + KPDR);
+		reg_val &= ~(1 << (8 + col));
+		writew(reg_val, keypad->mmio_base + KPDR);
+
+		/*
+		 * Delay added to avoid propagating the 0 from column to row
+		 * when scanning.
+		 */
+		udelay(5);
+
+		/*
+		 * 1s in matrix_volatile_state[col] means key pressures
+		 * throw data from non enabled rows.
+		 */
+		reg_val = readw(keypad->mmio_base + KPDR);
+		matrix_volatile_state[col] = (~reg_val) & keypad->rows_en_mask;
+	}
+
+	/*
+	 * Return in standby mode:
+	 * 9. write 0s to columns
+	 */
+	reg_val = readw(keypad->mmio_base + KPDR);
+	reg_val &= 0x00ff;
+	writew(reg_val, keypad->mmio_base + KPDR);
+}
+
+/*
+ * Compare the new matrix state (volatile) with the stable one stored in
+ * keypad->matrix_stable_state and fire events if changes are detected.
+ */
+static void imx_keypad_fire_events(struct imx_keypad *keypad,
+				   unsigned short *matrix_volatile_state)
+{
+	struct input_dev *input_dev = keypad->input_dev;
+	int row, col;
+
+	for (col = 0; col < MAX_MATRIX_KEY_COLS; col++) {
+		unsigned short bits_changed;
+		int code;
+
+		if ((keypad->cols_en_mask & (1 << col)) == 0)
+			continue; /* Column not enabled */
+
+		bits_changed = keypad->matrix_stable_state[col] ^
+						matrix_volatile_state[col];
+
+		if (bits_changed == 0)
+			continue; /* Column not contain changes */
+
+		for (row = 0; row < MAX_MATRIX_KEY_ROWS; row++) {
+			if ((keypad->rows_en_mask & (1 << row)) == 0)
+				continue; /* Row not enabled */
+			if ((bits_changed & (1 << row)) == 0)
+				continue; /* Row not contain changes */
+
+			code = MATRIX_SCAN_CODE(row, col, MATRIX_ROW_SHIFT);
+			input_event(input_dev, EV_MSC, MSC_SCAN, code);
+			input_report_key(input_dev, keypad->keycodes[code],
+				       matrix_volatile_state[col] & (1 << row));
+			dev_dbg(&input_dev->dev, "Event code: %d, val: %d",
+					keypad->keycodes[code],
+				       matrix_volatile_state[col] & (1 << row));
+		}
+	}
+	input_sync(input_dev);
+}
+
+/*
+ * imx_keypad_check_for_events is the timer handler.
+ * It is executed in a non interruptible area of the kernel (Soft interrupt)
+ */
+static void imx_keypad_check_for_events(unsigned long data)
+{
+	struct imx_keypad *keypad = (struct imx_keypad *) data;
+	struct input_dev *input_dev = keypad->input_dev;
+	unsigned short matrix_volatile_state[MAX_MATRIX_KEY_COLS];
+	unsigned short reg_val;
+	bool state_changed, is_zero_matrix;
+	int i;
+
+	memset(matrix_volatile_state, 0, sizeof(matrix_volatile_state));
+
+	/* If the driver is shutting down, exit now.*/
+	if (keypad->exit_flag) {
+		dev_dbg(&input_dev->dev, "%s: exiting.\n", __func__);
+		return;
+	}
+
+	imx_keypad_scan_matrix(keypad, matrix_volatile_state);
+
+	state_changed = false;
+	for (i = 0; (i < MAX_MATRIX_KEY_COLS) && !state_changed; i++) {
+		if ((keypad->cols_en_mask & (1 << i)) == 0)
+			continue;
+
+		if (keypad->matrix_unstable_state[i] ^
+						matrix_volatile_state[i])
+			state_changed = true;
+	}
+
+	/*
+	 * If the matrix state is changed from the previous scan
+	 *   (Re)Begin the debouncing process, saving the new state in
+	 *    keypad->matrix_unstable_state.
+	 * else
+	 *   Increase the count of number of scans with a stable state.
+	 */
+	if (state_changed) {
+		memcpy(keypad->matrix_unstable_state, matrix_volatile_state,
+						sizeof(matrix_volatile_state));
+		keypad->stable_count = 0;
+	} else
+		keypad->stable_count++;
+
+	/*
+	 * If the matrix is not as stable as we want reschedule a matrix scan
+	 * near in the future.
+	 */
+	if (keypad->stable_count < IMX_KEYPAD_SCANS_FOR_STABILITY) {
+		mod_timer(&keypad->check_matrix_timer,
+			  jiffies + msecs_to_jiffies(10));
+		return;
+	}
+
+	/*
+	 * If the matrix is stable as we need, fire the events and save the new
+	 * stable state.
+	 * Note, if the matrix is more stable (keypad->stable_count >
+	 * IMX_KEYPAD_SCANS_FOR_STABILITY)all events are already fired.We are in
+	 * the loop of multiple key pressure detection waiting for a change.
+	 */
+	if (keypad->stable_count == IMX_KEYPAD_SCANS_FOR_STABILITY) {
+		imx_keypad_fire_events(keypad, matrix_volatile_state);
+
+		memcpy(keypad->matrix_stable_state, matrix_volatile_state,
+						sizeof(matrix_volatile_state));
+	}
+
+	is_zero_matrix = true;
+	for (i = 0; (i < MAX_MATRIX_KEY_COLS) && is_zero_matrix; i++)
+		if (matrix_volatile_state[i] != 0)
+			is_zero_matrix = false;
+
+
+	if (is_zero_matrix) {
+		/*
+		 * No keys are still pressed.
+		 * Enable only the KDI interrupt for future key pressures.
+		 * (clear the KDI status bit and his sync chain before)
+		 */
+		reg_val = readw(keypad->mmio_base + KPSR);
+		reg_val |= KBD_STAT_KPKD | KBD_STAT_KDSC;
+		writew(reg_val, keypad->mmio_base + KPSR);
+
+		reg_val = readw(keypad->mmio_base + KPSR);
+		reg_val |= KBD_STAT_KDIE;
+		reg_val &= ~KBD_STAT_KRIE;
+		writew(reg_val, keypad->mmio_base + KPSR);
+	} else {
+		/*
+		 * Still there are keys pressed. Schedule a rescan for multiple
+		 * key pressure detection and enable the KRI interrupt for fast
+		 * reaction to an all key release event.
+		 */
+		mod_timer(&keypad->check_matrix_timer,
+						jiffies + msecs_to_jiffies(60));
+
+		reg_val = readw(keypad->mmio_base + KPSR);
+		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
+		writew(reg_val, keypad->mmio_base + KPSR);
+
+		reg_val = readw(keypad->mmio_base + KPSR);
+		reg_val |= KBD_STAT_KRIE;
+		reg_val &= ~KBD_STAT_KDIE;
+		writew(reg_val, keypad->mmio_base + KPSR);
+	}
+}
+
+static irqreturn_t imx_keypad_irq_handler(int irq, void *dev_id)
+{
+	struct imx_keypad *keypad = dev_id;
+	unsigned short reg_val;
+
+	reg_val = readw(keypad->mmio_base + KPSR);
+
+	/* Disable every keypad interrupt */
+	reg_val &= ~(KBD_STAT_KRIE | KBD_STAT_KDIE);
+	/* Clear interrupts status bits */
+	reg_val |= KBD_STAT_KPKR | KBD_STAT_KPKD;
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	/* If the driver is shutting down, leave all interrupts disabled.*/
+	if (keypad->exit_flag)
+		return IRQ_HANDLED;
+
+	/* The matrix is supposed to be changed */
+	keypad->stable_count = 0;
+
+	/* Schedule the scanning procedure near in the future */
+	mod_timer(&keypad->check_matrix_timer, jiffies + msecs_to_jiffies(2));
+
+	return IRQ_HANDLED;
+}
+
+static void imx_keypad_config(struct imx_keypad *keypad)
+{
+	unsigned short reg_val;
+
+	/*
+	 * Include enabled rows in interrupt generation (KPCR[7:0])
+	 * Configure keypad columns as open-drain (KPCR[15:8])
+	 */
+	reg_val = readw(keypad->mmio_base + KPCR);
+	reg_val |= keypad->rows_en_mask & 0xff;		/* rows */
+	reg_val |= (keypad->cols_en_mask & 0xff) << 8;	/* cols */
+	writew(reg_val, keypad->mmio_base + KPCR);
+
+	/* Write 0's to KPDR[15:8] (Colums)*/
+	reg_val = readw(keypad->mmio_base + KPDR);
+	reg_val &= 0x00ff;
+	writew(reg_val, keypad->mmio_base + KPDR);
+
+	/* Configure columns as output, rows as input (KDDR[15:0]) */
+	writew(0xff00, keypad->mmio_base + KDDR);
+
+	/*
+	 * Clear Key Depress and Key Release status bit.
+	 * Clear both synchronizer chain.
+	 */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val |= KBD_STAT_KPKR | KBD_STAT_KPKD |
+		   KBD_STAT_KDSC | KBD_STAT_KRSS;
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	/* Enable KDI and disable KRI (avoid false release events). */
+	reg_val |= KBD_STAT_KDIE;
+	reg_val &= ~KBD_STAT_KRIE;
+	writew(reg_val, keypad->mmio_base + KPSR);
+}
+
+static void imx_keypad_inhibit(struct imx_keypad *keypad)
+{
+	unsigned short reg_val;
+
+	/* Inhibit KDI and KRI interrupts. */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val &= ~(KBD_STAT_KRIE | KBD_STAT_KDIE);
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	/* Colums as open drain and disable all rows */
+	writew(0xff00, keypad->mmio_base + KPCR);
+}
+
+static int imx_keypad_open(struct input_dev *dev)
+{
+	struct imx_keypad *keypad = input_get_drvdata(dev);
+
+	dev_dbg(&dev->dev, ">%s\n", __func__);
+
+	/* We became active from now */
+	keypad->exit_flag = false;
+	/* Init Keypad timer */
+	init_timer(&keypad->check_matrix_timer);
+	keypad->check_matrix_timer.function = imx_keypad_check_for_events;
+	keypad->check_matrix_timer.data = (unsigned long) keypad;
+
+	/* Enable the kpp clock */
+	clk_enable(keypad->clk);
+	imx_keypad_config(keypad);
+
+	/* Sanity control, not all the rows must be actived now. */
+	if ((readw(keypad->mmio_base + KPDR) & keypad->rows_en_mask) == 0) {
+		dev_err(&dev->dev,
+		"too much keys pressed for now, control pins initialisation\n");
+		goto open_err;
+	}
+
+	return 0;
+
+open_err:
+	keypad->exit_flag = true;
+	del_timer_sync(&keypad->check_matrix_timer);
+	imx_keypad_inhibit(keypad);
+	return -EIO;
+}
+
+static void imx_keypad_close(struct input_dev *dev)
+{
+	struct imx_keypad *keypad = input_get_drvdata(dev);
+
+	dev_dbg(&dev->dev, ">%s\n", __func__);
+
+	/* Make sure no checks are pending (avoid races).*/
+	keypad->exit_flag = true;
+	del_timer_sync(&keypad->check_matrix_timer);
+
+	imx_keypad_inhibit(keypad);
+
+	/* Disable clock unit */
+	clk_disable(keypad->clk);
+}
+
+static int __devinit imx_keypad_probe(struct platform_device *pdev)
+{
+	const struct matrix_keymap_data *keymap_data = pdev->dev.platform_data;
+	struct imx_keypad *keypad;
+	struct input_dev *input_dev;
+	struct resource *res;
+	int irq, error, i;
+
+	if (keymap_data == NULL) {
+		dev_err(&pdev->dev, "no keymap defined\n");
+		return -EINVAL;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq defined in platform data\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "no I/O memory defined in platform data\n");
+		return -EINVAL;
+	}
+
+	res = request_mem_region(res->start, resource_size(res), pdev->name);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "failed to request I/O memory\n");
+		return -EBUSY;
+	}
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		dev_err(&pdev->dev, "failed to allocate the input device\n");
+		error = -ENOMEM;
+		goto failed_rel_mem;
+	}
+
+	keypad = kzalloc(sizeof(struct imx_keypad), GFP_KERNEL);
+	if (!keypad) {
+		dev_err(&pdev->dev, "not enough memory for driver data\n");
+		error = -ENOMEM;
+		goto failed_free_input;
+	}
+
+	keypad->input_dev = input_dev;
+	keypad->irq = irq;
+	keypad->stable_count = 0;
+
+	keypad->mmio_base = ioremap(res->start, resource_size(res));
+	if (keypad->mmio_base == NULL) {
+		dev_err(&pdev->dev, "failed to remap I/O memory\n");
+		error = -ENOMEM;
+		goto failed_free_priv;
+	}
+
+	keypad->clk = clk_get(&pdev->dev, "kpp");
+	if (IS_ERR(keypad->clk)) {
+		dev_err(&pdev->dev, "failed to get keypad clock\n");
+		error = PTR_ERR(keypad->clk);
+		goto failed_unmap;
+	}
+
+	/* Search for rows and cols enabled */
+	for (i = 0; i < keymap_data->keymap_size; i++) {
+		keypad->rows_en_mask |= 1 << KEY_ROW(keymap_data->keymap[i]);
+		keypad->cols_en_mask |= 1 << KEY_COL(keymap_data->keymap[i]);
+	}
+
+	if (keypad->rows_en_mask > ((1 << MAX_MATRIX_KEY_ROWS) - 1) ||
+	   keypad->cols_en_mask > ((1 << MAX_MATRIX_KEY_COLS) - 1)) {
+		dev_err(&pdev->dev, "invalid key data (too rows or colums)\n");
+		error = -EINVAL;
+		goto failed_clock_put;
+	}
+	dev_dbg(&pdev->dev, "enabled rows mask: %x\n", keypad->rows_en_mask);
+	dev_dbg(&pdev->dev, "enabled cols mask: %x\n", keypad->cols_en_mask);
+
+	/* Init the Input device */
+	input_dev->name = pdev->name;
+	input_dev->id.bustype = BUS_HOST;
+	input_dev->dev.parent = &pdev->dev;
+	input_dev->open = imx_keypad_open;
+	input_dev->close = imx_keypad_close;
+	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	input_dev->keycode = keypad->keycodes;
+	input_dev->keycodesize = sizeof(keypad->keycodes[0]);
+	input_dev->keycodemax = ARRAY_SIZE(keypad->keycodes);
+
+	matrix_keypad_build_keymap(keymap_data, MATRIX_ROW_SHIFT,
+				keypad->keycodes, input_dev->keybit);
+
+	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+	input_set_drvdata(input_dev, keypad);
+
+	/* Enable the interrupt handler. */
+	/* If there are spurious interrupts the handler will mask them all. */
+	keypad->exit_flag = true;
+	error = request_irq(irq, imx_keypad_irq_handler, IRQF_DISABLED,
+			pdev->name, keypad);
+	if (error) {
+		dev_err(&pdev->dev, "failed to request IRQ\n");
+		goto failed_clock_put;
+	}
+
+	/* Register the input device */
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&pdev->dev, "failed to register input device\n");
+		goto failed_free_irq;
+	}
+
+	platform_set_drvdata(pdev, keypad);
+	device_init_wakeup(&pdev->dev, 1);
+
+	dev_info(&pdev->dev, "device probed\n");
+
+	return 0;
+
+failed_free_irq:
+	free_irq(irq, pdev);
+failed_clock_put:
+	clk_put(keypad->clk);
+failed_unmap:
+	iounmap(keypad->mmio_base);
+failed_free_priv:
+	kfree(keypad);
+failed_free_input:
+	input_free_device(input_dev);
+failed_rel_mem:
+	release_mem_region(res->start, resource_size(res));
+	return error;
+}
+
+static int __devexit imx_keypad_remove(struct platform_device *pdev)
+{
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+	struct resource *res;
+
+	dev_dbg(&pdev->dev, ">%s\n", __func__);
+
+	platform_set_drvdata(pdev, NULL);
+
+	input_unregister_device(keypad->input_dev);
+
+	free_irq(keypad->irq, keypad);
+	clk_put(keypad->clk);
+
+	iounmap(keypad->mmio_base);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(res->start, resource_size(res));
+
+	kfree(keypad);
+
+	dev_info(&pdev->dev, "device removed\n");
+
+	return 0;
+}
+
+static struct platform_driver imx_keypad_driver = {
+	.driver		= {
+		.name	= "imx-keypad",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= imx_keypad_probe,
+	.remove		= __devexit_p(imx_keypad_remove),
+};
+
+static int __init imx_keypad_init(void)
+{
+	return platform_driver_register(&imx_keypad_driver);
+}
+
+static void __exit imx_keypad_exit(void)
+{
+	platform_driver_unregister(&imx_keypad_driver);
+}
+
+module_init(imx_keypad_init);
+module_exit(imx_keypad_exit);
+
+MODULE_AUTHOR("Alberto Panizzo <maramaopercheseimorto@gmail.com>");
+MODULE_DESCRIPTION("IMX Keypad Port Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:imx-keypad");
-- 
1.6.3.3




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v6] input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.
  2010-01-30 11:53 [PATCH v6] input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family Alberto Panizzo
@ 2010-01-31  9:08 ` Dmitry Torokhov
  2010-01-31 13:42   ` Alberto Panizzo
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2010-01-31  9:08 UTC (permalink / raw)
  To: Alberto Panizzo
  Cc: Uwe Kleine-König, Lothar Waßmann, H Hartley Sweeten,
	Sascha linux-arm, linux-input, linux-arm-kernel

Hi Alberto,

On Sat, Jan 30, 2010 at 12:53:27PM +0100, Alberto Panizzo wrote:
> 
> Changes in v6:
> -MXC to IMX pattern change (apart of Kconfig dependencies)
> -Comment style fixes
> -Greater check delay in debouncing process (10). 
> -Improve the usage of keypad->exit_flag: now it is false only between open and 
>  close functions. And if we handle an interrupt out there, leave all interrupts
>  masked, and wait for another open to re enable they.
> -Remove unnecessary "free events" mechanism (tested with evtest).
> 

I took the liberty of making some changes to the driver, could you
please give the patch below a try and if I did not mess it up I will
fold it into your v6 version and apply to next.

Thank you.

-- 
Dmitry


Input: imx-keypad - assorted fixes

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

 drivers/input/keyboard/imx_keypad.c |  153 ++++++++++++++++-------------------
 1 files changed, 71 insertions(+), 82 deletions(-)


diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 6bdea71..2ee5b79 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -62,8 +62,7 @@ struct imx_keypad {
 #define IMX_KEYPAD_SCANS_FOR_STABILITY 3
 	int			stable_count;
 
-	/* If true the driver is shutting down */
-	bool			exit_flag;
+	bool			enabled;
 
 	/* Masks for enabled rows/cols */
 	unsigned short		rows_en_mask;
@@ -157,27 +156,27 @@ static void imx_keypad_fire_events(struct imx_keypad *keypad,
 		int code;
 
 		if ((keypad->cols_en_mask & (1 << col)) == 0)
-			continue; /* Column not enabled */
+			continue; /* Column is not enabled */
 
 		bits_changed = keypad->matrix_stable_state[col] ^
 						matrix_volatile_state[col];
 
 		if (bits_changed == 0)
-			continue; /* Column not contain changes */
+			continue; /* Column does not contain changes */
 
 		for (row = 0; row < MAX_MATRIX_KEY_ROWS; row++) {
 			if ((keypad->rows_en_mask & (1 << row)) == 0)
-				continue; /* Row not enabled */
+				continue; /* Row is not enabled */
 			if ((bits_changed & (1 << row)) == 0)
-				continue; /* Row not contain changes */
+				continue; /* Row does not contain changes */
 
 			code = MATRIX_SCAN_CODE(row, col, MATRIX_ROW_SHIFT);
 			input_event(input_dev, EV_MSC, MSC_SCAN, code);
 			input_report_key(input_dev, keypad->keycodes[code],
-				       matrix_volatile_state[col] & (1 << row));
+				matrix_volatile_state[col] & (1 << row));
 			dev_dbg(&input_dev->dev, "Event code: %d, val: %d",
-					keypad->keycodes[code],
-				       matrix_volatile_state[col] & (1 << row));
+				keypad->keycodes[code],
+				matrix_volatile_state[col] & (1 << row));
 		}
 	}
 	input_sync(input_dev);
@@ -185,12 +184,10 @@ static void imx_keypad_fire_events(struct imx_keypad *keypad,
 
 /*
  * imx_keypad_check_for_events is the timer handler.
- * It is executed in a non interruptible area of the kernel (Soft interrupt)
  */
 static void imx_keypad_check_for_events(unsigned long data)
 {
 	struct imx_keypad *keypad = (struct imx_keypad *) data;
-	struct input_dev *input_dev = keypad->input_dev;
 	unsigned short matrix_volatile_state[MAX_MATRIX_KEY_COLS];
 	unsigned short reg_val;
 	bool state_changed, is_zero_matrix;
@@ -198,22 +195,17 @@ static void imx_keypad_check_for_events(unsigned long data)
 
 	memset(matrix_volatile_state, 0, sizeof(matrix_volatile_state));
 
-	/* If the driver is shutting down, exit now.*/
-	if (keypad->exit_flag) {
-		dev_dbg(&input_dev->dev, "%s: exiting.\n", __func__);
-		return;
-	}
-
 	imx_keypad_scan_matrix(keypad, matrix_volatile_state);
 
 	state_changed = false;
-	for (i = 0; (i < MAX_MATRIX_KEY_COLS) && !state_changed; i++) {
+	for (i = 0; i < MAX_MATRIX_KEY_COLS; i++) {
 		if ((keypad->cols_en_mask & (1 << i)) == 0)
 			continue;
 
-		if (keypad->matrix_unstable_state[i] ^
-						matrix_volatile_state[i])
+		if (keypad->matrix_unstable_state[i] ^ matrix_volatile_state[i]) {
 			state_changed = true;
+			break;
+		}
 	}
 
 	/*
@@ -225,14 +217,14 @@ static void imx_keypad_check_for_events(unsigned long data)
 	 */
 	if (state_changed) {
 		memcpy(keypad->matrix_unstable_state, matrix_volatile_state,
-						sizeof(matrix_volatile_state));
+			sizeof(matrix_volatile_state));
 		keypad->stable_count = 0;
 	} else
 		keypad->stable_count++;
 
 	/*
-	 * If the matrix is not as stable as we want reschedule a matrix scan
-	 * near in the future.
+	 * If the matrix is not as stable as we want reschedule scan
+	 * in the near future.
 	 */
 	if (keypad->stable_count < IMX_KEYPAD_SCANS_FOR_STABILITY) {
 		mod_timer(&keypad->check_matrix_timer,
@@ -241,30 +233,32 @@ static void imx_keypad_check_for_events(unsigned long data)
 	}
 
 	/*
-	 * If the matrix is stable as we need, fire the events and save the new
-	 * stable state.
-	 * Note, if the matrix is more stable (keypad->stable_count >
-	 * IMX_KEYPAD_SCANS_FOR_STABILITY)all events are already fired.We are in
-	 * the loop of multiple key pressure detection waiting for a change.
+	 * If the matrix state is stable, fire the events and save the new
+	 * stable state. Note, if the matrix is kept stable for longer
+	 * (keypad->stable_count > IMX_KEYPAD_SCANS_FOR_STABILITY) all
+	 * events have already been generated.
 	 */
 	if (keypad->stable_count == IMX_KEYPAD_SCANS_FOR_STABILITY) {
 		imx_keypad_fire_events(keypad, matrix_volatile_state);
 
 		memcpy(keypad->matrix_stable_state, matrix_volatile_state,
-						sizeof(matrix_volatile_state));
+			sizeof(matrix_volatile_state));
 	}
 
 	is_zero_matrix = true;
-	for (i = 0; (i < MAX_MATRIX_KEY_COLS) && is_zero_matrix; i++)
-		if (matrix_volatile_state[i] != 0)
+	for (i = 0; i < MAX_MATRIX_KEY_COLS; i++) {
+		if (matrix_volatile_state[i] != 0) {
 			is_zero_matrix = false;
+			break;
+		}
+	}
 
 
 	if (is_zero_matrix) {
 		/*
-		 * No keys are still pressed.
-		 * Enable only the KDI interrupt for future key pressures.
-		 * (clear the KDI status bit and his sync chain before)
+		 * All keys have been released. Enable only the KDI
+		 * interrupt for future key presses (clear the KDI
+		 * status bit and its sync chain before that).
 		 */
 		reg_val = readw(keypad->mmio_base + KPSR);
 		reg_val |= KBD_STAT_KPKD | KBD_STAT_KDSC;
@@ -276,12 +270,13 @@ static void imx_keypad_check_for_events(unsigned long data)
 		writew(reg_val, keypad->mmio_base + KPSR);
 	} else {
 		/*
-		 * Still there are keys pressed. Schedule a rescan for multiple
-		 * key pressure detection and enable the KRI interrupt for fast
-		 * reaction to an all key release event.
+		 * Some keys are still pressed. Schedule a rescan in
+		 * attempt to detect multiple key presses and enable
+		 * the KRI interrupt to react quickly to key release
+		 * event.
 		 */
 		mod_timer(&keypad->check_matrix_timer,
-						jiffies + msecs_to_jiffies(60));
+			  jiffies + msecs_to_jiffies(60));
 
 		reg_val = readw(keypad->mmio_base + KPSR);
 		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
@@ -301,21 +296,20 @@ static irqreturn_t imx_keypad_irq_handler(int irq, void *dev_id)
 
 	reg_val = readw(keypad->mmio_base + KPSR);
 
-	/* Disable every keypad interrupt */
+	/* Disable both interrupt types */
 	reg_val &= ~(KBD_STAT_KRIE | KBD_STAT_KDIE);
 	/* Clear interrupts status bits */
 	reg_val |= KBD_STAT_KPKR | KBD_STAT_KPKD;
 	writew(reg_val, keypad->mmio_base + KPSR);
 
-	/* If the driver is shutting down, leave all interrupts disabled.*/
-	if (keypad->exit_flag)
-		return IRQ_HANDLED;
-
-	/* The matrix is supposed to be changed */
-	keypad->stable_count = 0;
+	if (keypad->enabled) {
+		/* The matrix is supposed to be changed */
+		keypad->stable_count = 0;
 
-	/* Schedule the scanning procedure near in the future */
-	mod_timer(&keypad->check_matrix_timer, jiffies + msecs_to_jiffies(2));
+		/* Schedule the scanning procedure near in the future */
+		mod_timer(&keypad->check_matrix_timer,
+			  jiffies + msecs_to_jiffies(2));
+	}
 
 	return IRQ_HANDLED;
 }
@@ -333,7 +327,7 @@ static void imx_keypad_config(struct imx_keypad *keypad)
 	reg_val |= (keypad->cols_en_mask & 0xff) << 8;	/* cols */
 	writew(reg_val, keypad->mmio_base + KPCR);
 
-	/* Write 0's to KPDR[15:8] (Colums)*/
+	/* Write 0's to KPDR[15:8] (Colums) */
 	reg_val = readw(keypad->mmio_base + KPDR);
 	reg_val &= 0x00ff;
 	writew(reg_val, keypad->mmio_base + KPDR);
@@ -369,6 +363,23 @@ static void imx_keypad_inhibit(struct imx_keypad *keypad)
 	writew(0xff00, keypad->mmio_base + KPCR);
 }
 
+static void imx_keypad_close(struct input_dev *dev)
+{
+	struct imx_keypad *keypad = input_get_drvdata(dev);
+
+	dev_dbg(&dev->dev, ">%s\n", __func__);
+
+	/* Mark keypad as being inactive */
+	keypad->enabled = false;
+	synchronize_irq(keypad->irq);
+	del_timer_sync(&keypad->check_matrix_timer);
+
+	imx_keypad_inhibit(keypad);
+
+	/* Disable clock unit */
+	clk_disable(keypad->clk);
+}
+
 static int imx_keypad_open(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);
@@ -376,11 +387,7 @@ static int imx_keypad_open(struct input_dev *dev)
 	dev_dbg(&dev->dev, ">%s\n", __func__);
 
 	/* We became active from now */
-	keypad->exit_flag = false;
-	/* Init Keypad timer */
-	init_timer(&keypad->check_matrix_timer);
-	keypad->check_matrix_timer.function = imx_keypad_check_for_events;
-	keypad->check_matrix_timer.data = (unsigned long) keypad;
+	keypad->enabled = true;
 
 	/* Enable the kpp clock */
 	clk_enable(keypad->clk);
@@ -389,35 +396,17 @@ static int imx_keypad_open(struct input_dev *dev)
 	/* Sanity control, not all the rows must be actived now. */
 	if ((readw(keypad->mmio_base + KPDR) & keypad->rows_en_mask) == 0) {
 		dev_err(&dev->dev,
-		"too much keys pressed for now, control pins initialisation\n");
+			"too many keys pressed, control pins initialisation\n");
 		goto open_err;
 	}
 
 	return 0;
 
 open_err:
-	keypad->exit_flag = true;
-	del_timer_sync(&keypad->check_matrix_timer);
-	imx_keypad_inhibit(keypad);
+	imx_keypad_close(dev);
 	return -EIO;
 }
 
-static void imx_keypad_close(struct input_dev *dev)
-{
-	struct imx_keypad *keypad = input_get_drvdata(dev);
-
-	dev_dbg(&dev->dev, ">%s\n", __func__);
-
-	/* Make sure no checks are pending (avoid races).*/
-	keypad->exit_flag = true;
-	del_timer_sync(&keypad->check_matrix_timer);
-
-	imx_keypad_inhibit(keypad);
-
-	/* Disable clock unit */
-	clk_disable(keypad->clk);
-}
-
 static int __devinit imx_keypad_probe(struct platform_device *pdev)
 {
 	const struct matrix_keymap_data *keymap_data = pdev->dev.platform_data;
@@ -467,6 +456,9 @@ static int __devinit imx_keypad_probe(struct platform_device *pdev)
 	keypad->irq = irq;
 	keypad->stable_count = 0;
 
+	setup_timer(&keypad->check_matrix_timer,
+		    imx_keypad_check_for_events, (unsigned long) keypad);
+
 	keypad->mmio_base = ioremap(res->start, resource_size(res));
 	if (keypad->mmio_base == NULL) {
 		dev_err(&pdev->dev, "failed to remap I/O memory\n");
@@ -489,7 +481,8 @@ static int __devinit imx_keypad_probe(struct platform_device *pdev)
 
 	if (keypad->rows_en_mask > ((1 << MAX_MATRIX_KEY_ROWS) - 1) ||
 	   keypad->cols_en_mask > ((1 << MAX_MATRIX_KEY_COLS) - 1)) {
-		dev_err(&pdev->dev, "invalid key data (too rows or colums)\n");
+		dev_err(&pdev->dev,
+			"invalid key data (too many rows or colums)\n");
 		error = -EINVAL;
 		goto failed_clock_put;
 	}
@@ -513,11 +506,11 @@ static int __devinit imx_keypad_probe(struct platform_device *pdev)
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
 	input_set_drvdata(input_dev, keypad);
 
-	/* Enable the interrupt handler. */
-	/* If there are spurious interrupts the handler will mask them all. */
-	keypad->exit_flag = true;
+	/* Ensure that the keypad will stay dormant until opened */
+	imx_keypad_inhibit(keypad);
+
 	error = request_irq(irq, imx_keypad_irq_handler, IRQF_DISABLED,
-			pdev->name, keypad);
+			    pdev->name, keypad);
 	if (error) {
 		dev_err(&pdev->dev, "failed to request IRQ\n");
 		goto failed_clock_put;
@@ -533,8 +526,6 @@ static int __devinit imx_keypad_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, keypad);
 	device_init_wakeup(&pdev->dev, 1);
 
-	dev_info(&pdev->dev, "device probed\n");
-
 	return 0;
 
 failed_free_irq:
@@ -572,8 +563,6 @@ static int __devexit imx_keypad_remove(struct platform_device *pdev)
 
 	kfree(keypad);
 
-	dev_info(&pdev->dev, "device removed\n");
-
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v6] input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.
  2010-01-31  9:08 ` Dmitry Torokhov
@ 2010-01-31 13:42   ` Alberto Panizzo
  2010-02-01  2:01     ` Dmitry Torokhov
  0 siblings, 1 reply; 4+ messages in thread
From: Alberto Panizzo @ 2010-01-31 13:42 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Uwe Kleine-König, Lothar Waßmann, H Hartley Sweeten,
	Sascha linux-arm, linux-input, linux-arm-kernel

On dom, 2010-01-31 at 01:08 -0800, Dmitry Torokhov wrote:
> Hi Alberto,
> 
> On Sat, Jan 30, 2010 at 12:53:27PM +0100, Alberto Panizzo wrote:
> > 
> > Changes in v6:
> > -MXC to IMX pattern change (apart of Kconfig dependencies)
> > -Comment style fixes
> > -Greater check delay in debouncing process (10). 
> > -Improve the usage of keypad->exit_flag: now it is false only
> between open and 
> >  close functions. And if we handle an interrupt out there, leave all
> interrupts
> >  masked, and wait for another open to re enable they.
> > -Remove unnecessary "free events" mechanism (tested with evtest).
> > 
> 
> I took the liberty of making some changes to the driver, could you
> please give the patch below a try and if I did not mess it up I will
> fold it into your v6 version and apply to next.
> 
> Thank you.
> 
> -- 
> Dmitry 

For me it is ok. Your patch looks good and I tested your changes and all
goes well. I have to work on my English and other changes make the 
source code more readable.
Dmitry, put also your Signed-off line in v6 patch after merge if you
feel to wont this. (I've already thought to ask you this before this
patch fix) You help me a lot on building this version and an Acked-by
maybe is not fair about your work.

Thank You Dmirty :)


-- 
Alberto!


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v6] input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.
  2010-01-31 13:42   ` Alberto Panizzo
@ 2010-02-01  2:01     ` Dmitry Torokhov
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Torokhov @ 2010-02-01  2:01 UTC (permalink / raw)
  To: Alberto Panizzo
  Cc: Uwe Kleine-König, Lothar Waßmann, H Hartley Sweeten,
	Sascha linux-arm, linux-input, linux-arm-kernel

Hi Alberto,

On Sun, Jan 31, 2010 at 02:42:31PM +0100, Alberto Panizzo wrote:
> On dom, 2010-01-31 at 01:08 -0800, Dmitry Torokhov wrote:
> > Hi Alberto,
> > 
> > On Sat, Jan 30, 2010 at 12:53:27PM +0100, Alberto Panizzo wrote:
> > > 
> > > Changes in v6:
> > > -MXC to IMX pattern change (apart of Kconfig dependencies)
> > > -Comment style fixes
> > > -Greater check delay in debouncing process (10). 
> > > -Improve the usage of keypad->exit_flag: now it is false only
> > between open and 
> > >  close functions. And if we handle an interrupt out there, leave all
> > interrupts
> > >  masked, and wait for another open to re enable they.
> > > -Remove unnecessary "free events" mechanism (tested with evtest).
> > > 
> > 
> > I took the liberty of making some changes to the driver, could you
> > please give the patch below a try and if I did not mess it up I will
> > fold it into your v6 version and apply to next.
> > 
> > Thank you.
> > 
> > -- 
> > Dmitry 
> 
> For me it is ok. Your patch looks good and I tested your changes and all
> goes well.

Great, thanks a lot.

> I have to work on my English and other changes make the 
> source code more readable.
> Dmitry, put also your Signed-off line in v6 patch after merge if you
> feel to wont this. (I've already thought to ask you this before this
> patch fix) You help me a lot on building this version and an Acked-by
> maybe is not fair about your work.

Yes, I will fold this patch into your original v6 patch and will add my
sign-off. BTW, adding sign-off does not imply in any way that a person
had more or less involvement in the development of a driver/feature;
this is just to track the path of patricular code reaching mainline.
I am adding my sign-offs to all patches that are applied to my tree as
do all the other maintainers, the authorship of the patch is conveyed
via git's 'author' field.

OK, so the driver is applied to the 'next' branch of my tree and should
be in mainline when 2.6.34 merge window opens.

Thank you.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-02-01  2:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-30 11:53 [PATCH v6] input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family Alberto Panizzo
2010-01-31  9:08 ` Dmitry Torokhov
2010-01-31 13:42   ` Alberto Panizzo
2010-02-01  2:01     ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).