Skip to contentSkip to conversation

input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.

PATCH Superseded Alberto Panizzo 2 participants 4 messages Jan 30, 2010 – Feb 1, 2010

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v6 1/1 input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.

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.

---

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
drivers/input/keyboard/Kconfig +9 -0
index 02c836e..78e6cf5 100644
@@ -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
drivers/input/keyboard/Makefile +1 -0
index 78654ef..b3f8567 100644
@@ -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
drivers/input/keyboard/imx_keypad.c new file +605 -0

605 lines not on this page · raw message

Signature · 1 line
1.6.3.3
maintainer
#1 lore raw

PATCH v6 1/1 input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.

Hi Alberto,

Review

Quoted 10 lines · Alberto Panizzo

Changes in v6:

(quoted diff) +0 -5
-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.

Signature · 10 lines
Dmitry


Input: imx-keypad - assorted fixes

Signed-off-by: Dmitry Torokhov <[email protected]>
---

 drivers/input/keyboard/imx_keypad.c |  153 ++++++++++++++++-------------------
 1 files changed, 71 insertions(+), 82 deletions(-)
drivers/input/keyboard/imx_keypad.c +71 -82
index 6bdea71..2ee5b79 100644
@@ -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;
 }
 
#2 lore raw

PATCH v6 1/1 input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.

Quoted 16 lines · Dmitry Torokhov

Hi Alberto,

Review

Quoted 6 lines · Alberto Panizzo

Changes in v6:

(quoted diff) +0 -4
-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

Quoted 1 line

close functions. And if we handle an interrupt out there, leave all

interrupts

Quoted 3 lines

masked, and wait for another open to re enable they.
-Remove unnecessary "free events" mechanism (tested with evtest).

Quoted 8 lines

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.

Signature · 1 line
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 :)

Signature · 1 line
Alberto!
maintainer
#3 lore raw

PATCH v6 1/1 input: IMX: add imx-keypad driver to support the Keypad Port present in the imx application processors family.

Hi Alberto,

Quoted 21 lines · Alberto Panizzo
Quoted 20 lines · Dmitry Torokhov

Hi Alberto,

Review

Quoted 6 lines · Alberto Panizzo

Changes in v6:

(quoted diff) +0 -4
-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

Quoted 1 line

close functions. And if we handle an interrupt out there, leave all

interrupts

Quoted 3 lines

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.

Quoted 7 lines
Quoted 4 lines

Thank you.

Signature · 1 line
Dmitry 

For me it is ok. Your patch looks good and I tested your changes and all goes well.

Great, thanks a lot.

Quoted 6 lines

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.

Signature · 1 line
Dmitry