Skip to contentSkip to conversation

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

PATCH Superseded Alberto Panizzo 2 participants 4 messages Jan 27, 2010 – Jan 28, 2010

Subsystems (5)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 input: MXC: add mxc-keypad driver to support the Keypad Port present in the mxc application processors family.

The MXC 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.

Those simple synchronization circuits are used also for multiple key pressures:
between a KDI and a KRI the driver reset the sync circuit and re-enable the KDI interrupt so after 3 keypad-clock cycle another KDI is fired making possible to repeat the matrix scan operation.

This algorithm is done through the interrupt management code and delayed by a proper (and longer) debounce interval controlled by the platform initialization. If a key is pressed for a lot of time, the driver relaxes the interrupt re-enabling procedure to not over load the cpu in a long time keypad interaction.

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 v2:
-This driver completely apply to matrix_keypad interface. -Rearranged remove procedure.
-Automated hardware configuration, based on matrix_keypad_data. -Removed useless macro.

Changes in v3:
-Make use of timer API for interrupt management instead of threaded irq. -Little fixes.

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

 arch/arm/plat-mxc/include/mach/mxc_keypad.h |   18 +
 drivers/input/keyboard/Kconfig              |    9 +
 drivers/input/keyboard/Makefile             |    1 +
 drivers/input/keyboard/mxc_keypad.c         |  511 +++++++++++++++++++++++++++
 4 files changed, 539 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-mxc/include/mach/mxc_keypad.h
 create mode 100644 drivers/input/keyboard/mxc_keypad.c
arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +18 -0
new file mode 100644
index 0000000..c59cab2
@@ -0,0 +1,18 @@
+#ifndef __MACH_MXC_KEYPAD_H
+#define __MACH_MXC_KEYPAD_H
+
+#include <linux/input/matrix_keypad.h>
+
+#define MAX_MATRIX_KEY_ROWS	8
+#define MAX_MATRIX_KEY_COLS	8
+#define MATRIX_ROW_SHIFT	3
+
+struct mxc_keypad_platform_data {
+
+	const struct matrix_keymap_data *keymap_data;
+
+	/* key debounce interval */
+	unsigned int	debounce_ms;
+};
+
+#endif /* __MACH_MXC_KEYPAD_H */
drivers/input/keyboard/Kconfig +9 -0
index 02c836e..2abdab3 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_MXC
+	tristate "MXC keypad support"
+	depends on ARCH_MXC
+	help
+	  Enable support for MXC keypad port.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mxc_keypad.
+
 config KEYBOARD_NEWTON
 	tristate "Newton keyboard"
 	select SERIO
drivers/input/keyboard/Makefile +1 -0
index 78654ef..0530b78 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_MXC)		+= mxc_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/mxc_keypad.c new file +511 -0

511 lines not on this page · raw message

Signature · 1 line
1.6.3.3
maintainer
#1 lore raw

PATCH v4 1/1 input: MXC: add mxc-keypad driver to support the Keypad Port present in the mxc application processors family.

Hi Alberto,

Quoted 18 lines · Alberto Panizzo

The MXC 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.

Those simple synchronization circuits are used also for multiple key pressures:
between a KDI and a KRI the driver reset the sync circuit and re-enable the KDI interrupt so after 3 keypad-clock cycle another KDI is fired making possible to repeat the matrix scan operation.

Quoted 5 lines

This algorithm is done through the interrupt management code and delayed by a proper (and longer) debounce interval controlled by the platform initialization. If a key is pressed for a lot of time, the driver relaxes the interrupt re-enabling procedure to not over load the cpu in a long time keypad interaction.

I was looking at the debounce logic and I am not quite sure about it. Normally you have 2 ways for dealing with jitter:

1. You let interrupts to come in and reschedule the scan until they stop arriving. Then to tak ethe stable reading.

2. You inhibit interrupt, take a reading and schedule another reading in the future. If they match you decide that reading is stable otherwise you schedule another reading.

In your case you seem to be simply postponing the reading but this does not guarantee that the reading is stable.

I also do not think that yopu need 2 timers - you can easily requeue currently running timer.

BTW, you need to pay close attention to the races between re-enabling intterrupts (form the timer context) and inhibiting interrupts when you close the device. Currently there is a race - if you close the device while scan is scheduled the timer will re-enable them again. You disable all rows so I am not sure if it is possible for interrupts to be raised again at this point, but if it is, then you porbbaly need a spinlock there.

Quoted 12 lines
drivers/input/keyboard/mxc_keypad.c new file +12 -0
+
+static void mxc_keypad_relax_timer_handler(unsigned long data)
+{
+	struct mxc_keypad *keypad = (struct mxc_keypad *) data;
+	unsigned short reg_val;
+
+	/* 10. Clear KPKD and KPKR status bits
+	 *     Set the KPKR sync chain and clear the KPKD sync chain */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val |= KBD_STAT_KPKD | KBD_STAT_KPKR |
+		   KBD_STAT_KDSC | KBD_STAT_KRSS;
+	writew(reg_val, keypad->mmio_base + KPSR);

drivers/input/keyboard/mxc_keypad.c line 182

Quoted 10 lines
drivers/input/keyboard/mxc_keypad.c new file +10 -0
+
+	/* Re enable interrupts and clear sync reset bits.
+	 * Next KDI is used for detect multiple pressures. */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val &= ~(KBD_STAT_KDSC | KBD_STAT_KRSS);
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	reg_val |= KBD_STAT_KDIE | KBD_STAT_KRIE;
+	if (keypad->irq_type == MXC_IRQ_KRI)
+		reg_val &= ~KBD_STAT_KRIE;

So we are keeping the press interrupt always... As far as I understand this will cause us to effectively poll the matrix as long as at least one key is pressed. Why do we even need to bother with release interrupt then?

Thanks.

Signature · 1 line
Dmitry
#2 lore raw

PATCH v4 1/1 input: MXC: add mxc-keypad driver to support the Keypad Port present in the mxc application processors family.

Hi Dmitry,

Quoted 33 lines · Dmitry Torokhov

Hi Alberto,

Quoted 18 lines · Alberto Panizzo

The MXC 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.

Those simple synchronization circuits are used also for multiple key pressures:
between a KDI and a KRI the driver reset the sync circuit and re-enable the KDI interrupt so after 3 keypad-clock cycle another KDI is fired making possible to repeat the matrix scan operation.

Quoted 5 lines

This algorithm is done through the interrupt management code and delayed by a proper (and longer) debounce interval controlled by the platform initialization. If a key is pressed for a lot of time, the driver relaxes the interrupt re-enabling procedure to not over load the cpu in a long time keypad interaction.

I was looking at the debounce logic and I am not quite sure about it. Normally you have 2 ways for dealing with jitter:

1. You let interrupts to come in and reschedule the scan until they stop arriving. Then to tak ethe stable reading.

Quoted 6 lines

2. You inhibit interrupt, take a reading and schedule another reading in the future. If they match you decide that reading is stable otherwise you schedule another reading.

In your case you seem to be simply postponing the reading but this does not guarantee that the reading is stable.

Yes, because of the glitch suppression circuit, I suppose that when an interrupt arrive, it is a key pressure for sure.
Then I assume that the matrix will be stable after a proper debounce time (test look fine with 20 ms).

1 should be a more accurate way, I can study an implementation.

Quoted 3 lines

I also do not think that yopu need 2 timers - you can easily requeue currently running timer.

The first version I made was with one timer: if for too many repeating interrupts the matrix state do not change, the scanning procedure was scheduled with a summed delay.

It resulted in a degradation of responsiveness and more key pressure losing. It is a better choice to let the scanning procedure near the interrupt.

Changing the timer handler over the time? Would be acceptable?

Quoted 8 lines

BTW, you need to pay close attention to the races between re-enabling intterrupts (form the timer context) and inhibiting interrupts when you close the device. Currently there is a race - if you close the device while scan is scheduled the timer will re-enable them again. You disable all rows so I am not sure if it is possible for interrupts to be raised again at this point, but if it is, then you porbbaly need a spinlock there.

This is a true race condition that I've not caught!

Quoted 29 lines
Quoted 13 lines
drivers/input/keyboard/mxc_keypad.c new file +12 -0
+
+static void mxc_keypad_relax_timer_handler(unsigned long data)
+{
+	struct mxc_keypad *keypad = (struct mxc_keypad *) data;
+	unsigned short reg_val;
+
+	/* 10. Clear KPKD and KPKR status bits
+	 *     Set the KPKR sync chain and clear the KPKD sync chain */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val |= KBD_STAT_KPKD | KBD_STAT_KPKR |
+		   KBD_STAT_KDSC | KBD_STAT_KRSS;
+	writew(reg_val, keypad->mmio_base + KPSR);

drivers/input/keyboard/mxc_keypad.c line 182

Quoted 10 lines
drivers/input/keyboard/mxc_keypad.c new file +10 -0
+
+	/* Re enable interrupts and clear sync reset bits.
+	 * Next KDI is used for detect multiple pressures. */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val &= ~(KBD_STAT_KDSC | KBD_STAT_KRSS);
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	reg_val |= KBD_STAT_KDIE | KBD_STAT_KRIE;
+	if (keypad->irq_type == MXC_IRQ_KRI)
+		reg_val &= ~KBD_STAT_KRIE;

So we are keeping the press interrupt always... As far as I understand this will cause us to effectively poll the matrix as long as at least one key is pressed. Why do we even need to bother with release interrupt then?

Quoted 1 line

Thanks.

I always keep the press interrupt because of matrix-rescan in a long time pressure of a key, to find multiple key pressures. Resetting the press interrupt synchronization chain will reset also the interrupt status bit, and if the press condition hold for the next 3*keypad_clock_periods, it is fired another press interrupt.

I can think at an architecture that after a key pressure event continually reschedule a matrix scan in the future to look for changes in the matrix until all keys are released.. but need a consistent analysis of timings.

maintainer
#3 lore raw

PATCH v4 1/1 input: MXC: add mxc-keypad driver to support the Keypad Port present in the mxc application processors family.

Quoted 54 lines · Alberto Panizzo

Hi Dmitry,

Quoted 33 lines · Dmitry Torokhov

Hi Alberto,

Quoted 18 lines · Alberto Panizzo

The MXC 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.

Those simple synchronization circuits are used also for multiple key pressures:
between a KDI and a KRI the driver reset the sync circuit and re-enable the KDI interrupt so after 3 keypad-clock cycle another KDI is fired making possible to repeat the matrix scan operation.

Quoted 5 lines

This algorithm is done through the interrupt management code and delayed by a proper (and longer) debounce interval controlled by the platform initialization. If a key is pressed for a lot of time, the driver relaxes the interrupt re-enabling procedure to not over load the cpu in a long time keypad interaction.

I was looking at the debounce logic and I am not quite sure about it. Normally you have 2 ways for dealing with jitter:

1. You let interrupts to come in and reschedule the scan until they stop arriving. Then to tak ethe stable reading.

Quoted 6 lines

2. You inhibit interrupt, take a reading and schedule another reading in the future. If they match you decide that reading is stable otherwise you schedule another reading.

In your case you seem to be simply postponing the reading but this does not guarantee that the reading is stable.

Yes, because of the glitch suppression circuit, I suppose that when an interrupt arrive, it is a key pressure for sure.
Then I assume that the matrix will be stable after a proper debounce time (test look fine with 20 ms).

1 should be a more accurate way, I can study an implementation.

Quoted 3 lines

I also do not think that yopu need 2 timers - you can easily requeue currently running timer.

Quoted 7 lines

The first version I made was with one timer: if for too many repeating interrupts the matrix state do not change, the scanning procedure was scheduled with a summed delay.

It resulted in a degradation of responsiveness and more key pressure losing. It is a better choice to let the scanning procedure near the interrupt.

Hmm, that is unexpected result. I am pretty sure if was implementation deficiency and you can achieve the same performance with one timer and unified timer function.

Quoted 2 lines

Changing the timer handler over the time? Would be acceptable?

No, it is a bad taste.

Signature · 1 line
Dmitry