[PATCH 4/6] [TSC2301] Fix keyboard "geiger" noise

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This patch fixes the "geiger" sound that was generated when
touching the touchscreen and pressing a key at the same time.

The most serious error was the use of the keyb_int interrupt
line as a shortcut to see if any keys were pressed.

With the tsc2301 it is essential that the keypad status is read
soon after the interrupt arrive - otherwise you will occasionally
read the keyboard status while the keys are still being de-bounced
(ie nothing).

Signed-off-by: Klaus Pedersen <klaus.k.pedersen@xxxxxxxxx>
---
 arch/arm/mach-omap2/board-n800.c    |    7 --
 drivers/input/keyboard/tsc2301_kp.c |  128 +++++++----------------------------
 include/linux/spi/tsc2301.h         |    1 -
 3 files changed, 25 insertions(+), 111 deletions(-)

diff --git a/arch/arm/mach-omap2/board-n800.c b/arch/arm/mach-omap2/board-n800.c
index 41550cd..4ded021 100644
--- a/arch/arm/mach-omap2/board-n800.c
+++ b/arch/arm/mach-omap2/board-n800.c
@@ -209,12 +209,6 @@ static struct omap_board_config_kernel n800_config[] __initdata = {
 	{ OMAP_TAG_MMC,				&n800_mmc_config },
 };
 
-
-static int n800_get_keyb_irq_state(struct device *dev)
-{
-	return !omap_get_gpio_datain(N800_KEYB_IRQ_GPIO);
-}
-
 static struct tsc2301_platform_data tsc2301_config = {
 	.reset_gpio	= 118,
 	.dav_gpio	= 103,
@@ -237,7 +231,6 @@ static struct tsc2301_platform_data tsc2301_config = {
 		-1,		/* Event for bit 15 */
 	},
 	.kp_rep 	= 0,
-	.get_keyb_irq_state = n800_get_keyb_irq_state,
 };
 
 static void tsc2301_dev_init(void)
diff --git a/drivers/input/keyboard/tsc2301_kp.c b/drivers/input/keyboard/tsc2301_kp.c
index ac4cbae..473c0f2 100644
--- a/drivers/input/keyboard/tsc2301_kp.c
+++ b/drivers/input/keyboard/tsc2301_kp.c
@@ -49,7 +49,7 @@
 
 #define TSC2301_DEBOUNCE_TIME		TSC2301_DEBOUNCE_TIME_20MS
 
-#define TSC2301_POLL_TIME		30
+#define TSC2301_RELEASE_TIMEOUT		50
 
 struct tsc2301_kp {
 	struct input_dev	*idev;
@@ -64,14 +64,12 @@ struct tsc2301_kp {
 
 	struct spi_transfer	read_xfer[4];
 	struct spi_message	read_msg;
-	u16			state;
+
 	u16			data;
 	u16			mask;
 
 	int			irq;
 	s16			keymap[16];
-
-	int			(*get_keyb_irq_state)(struct device *dev);
 };
 
 static inline int tsc2301_kp_disabled(struct tsc2301 *tsc)
@@ -79,24 +77,6 @@ static inline int tsc2301_kp_disabled(struct tsc2301 *tsc)
 	return tsc->kp->disable_depth != 0;
 }
 
-static inline void tsc2301_kp_set_keypressed_state(struct tsc2301 *tsc)
-{
-	struct tsc2301_kp *kp = tsc->kp;
-
-	/* kp->state is updated only if we don't have the callback */
-	if (kp->get_keyb_irq_state != NULL) {
-		if (kp->get_keyb_irq_state(&tsc->spi->dev))
-			kp->state = 1 << 15;
-		else
-			kp->state = 0;
-	}
-}
-
-static inline int tsc2301_kp_key_is_pressed(struct tsc2301 *tsc)
-{
-	return tsc->kp->state & (1 << 15);
-}
-
 static void tsc2301_kp_send_key_events(struct tsc2301 *tsc,
 				       u16 prev_state,
 				       u16 new_state)
@@ -125,20 +105,6 @@ static void tsc2301_kp_send_key_events(struct tsc2301 *tsc,
 	input_sync(kp->idev);
 }
 
-static inline void tsc2301_kp_release_all_keys(struct tsc2301 *tsc)
-{
-	struct tsc2301_kp *kp = tsc->kp;
-	unsigned long flags;
-	int keys_pressed;
-
-	spin_lock_irqsave(&kp->lock, flags);
-	keys_pressed = kp->keys_pressed;
-	kp->keys_pressed = 0;
-	spin_unlock_irqrestore(&kp->lock, flags);
-	if (keys_pressed)
-		tsc2301_kp_send_key_events(tsc, keys_pressed, 0);
-}
-
 static inline void _filter_out(struct tsc2301 *tsc, u16 prev_state,
 			       u16 *new_state, int row1, int row2, u8 rect_pat)
 {
@@ -183,16 +149,13 @@ static void tsc2301_filter_ghost_keys(struct tsc2301 *tsc, u16 prev_state,
 static void tsc2301_kp_timer(unsigned long arg)
 {
 	struct tsc2301 *tsc = (void *) arg;
-	int r;
+	struct tsc2301_kp *kp = tsc->kp;
+	unsigned long flags;
 
-	/* This needs to be done early enough, since reading the key data
-	 * register clears the IRQ line, which may be used to determine
-	 * the key pressed state.
-	 */
-	tsc2301_kp_set_keypressed_state(tsc);
-	r = spi_async(tsc->spi, &tsc->kp->read_msg);
-	if (unlikely(r < 0 && printk_ratelimit()))
-		dev_err(&tsc->spi->dev, "kp: spi_async() failed");
+	tsc2301_kp_send_key_events(tsc, kp->keys_pressed, 0);
+	spin_lock_irqsave(&kp->lock, flags);
+	kp->keys_pressed = 0;
+	spin_unlock_irqrestore(&kp->lock, flags);
 }
 
 static void tsc2301_kp_rx(void *arg)
@@ -200,30 +163,16 @@ static void tsc2301_kp_rx(void *arg)
 	struct tsc2301 *tsc = arg;
 	struct tsc2301_kp *kp = tsc->kp;
 	unsigned long flags;
-	int key_pressed;
 	u16 kp_data;
 
 	kp_data = kp->data;
-	key_pressed = tsc2301_kp_key_is_pressed(tsc);
-	dev_dbg(&tsc->spi->dev, "KP data %04x (%s)\n",
-		kp_data, key_pressed ? "pressed" : "not pressed");
-	if (!key_pressed)
-		kp_data = 0;
-	else
-		tsc2301_filter_ghost_keys(tsc, kp->keys_pressed, &kp_data);
+	dev_dbg(&tsc->spi->dev, "KP data %04x\n", kp_data);
+
+	tsc2301_filter_ghost_keys(tsc, kp->keys_pressed, &kp_data);
 	tsc2301_kp_send_key_events(tsc, kp->keys_pressed, kp_data);
-	kp->keys_pressed = kp_data;
 	spin_lock_irqsave(&kp->lock, flags);
-	if (likely(!tsc2301_kp_disabled(tsc))) {
-		if (likely(key_pressed))
-			mod_timer(&kp->timer,
-				 jiffies + msecs_to_jiffies(TSC2301_POLL_TIME));
-		else {
-			kp->pending = 0;
-			enable_irq(kp->irq);
-		}
-	} else
-		kp->pending = 0;
+	kp->keys_pressed = kp_data;
+	kp->pending = 0;
 	spin_unlock_irqrestore(&kp->lock, flags);
 }
 
@@ -232,17 +181,20 @@ static irqreturn_t tsc2301_kp_irq_handler(int irq, void *dev_id)
 	struct tsc2301 *tsc = dev_id;
 	struct tsc2301_kp *kp = tsc->kp;
 	unsigned long flags;
+	int r;
 
 	spin_lock_irqsave(&kp->lock, flags);
-	BUG_ON(kp->pending);
 	if (tsc2301_kp_disabled(tsc)) {
 		spin_unlock_irqrestore(&kp->lock, flags);
 		return IRQ_HANDLED;
 	}
 	kp->pending = 1;
-	disable_irq_nosync(irq);
 	spin_unlock_irqrestore(&kp->lock, flags);
-	tsc2301_kp_timer((unsigned long) tsc);
+	mod_timer(&kp->timer,
+		 jiffies + msecs_to_jiffies(TSC2301_RELEASE_TIMEOUT));
+	r = spi_async(tsc->spi, &tsc->kp->read_msg);
+	if (r)
+		dev_err(&tsc->spi->dev, "kp: spi_async() failed");
 	return IRQ_HANDLED;
 }
 
@@ -255,7 +207,6 @@ static void tsc2301_kp_start_scan(struct tsc2301 *tsc)
 static void tsc2301_kp_stop_scan(struct tsc2301 *tsc)
 {
 	tsc2301_write_reg(tsc, TSC2301_REG_KEY, 1 << 14);
-	tsc2301_write_reg(tsc, TSC2301_REG_KPMASK, 0xffff);
 }
 
 /* Must be called with the mutex held */
@@ -270,18 +221,11 @@ static void tsc2301_kp_enable(struct tsc2301 *tsc)
 		spin_unlock_irqrestore(&kp->lock, flags);
 		return;
 	}
-	if (kp->keys_pressed)
-		kp->pending = 1;
 	spin_unlock_irqrestore(&kp->lock, flags);
 
 	set_irq_type(kp->irq, IRQT_FALLING);
 	tsc2301_kp_start_scan(tsc);
-	if (kp->pending)
-		/* continue an interrupted polling */
-		mod_timer(&kp->timer,
-			  jiffies + msecs_to_jiffies(TSC2301_POLL_TIME));
-	else
-		enable_irq(kp->irq);
+	enable_irq(kp->irq);
 }
 
 /* Must be called with the mutex held */
@@ -295,21 +239,18 @@ static int tsc2301_kp_disable(struct tsc2301 *tsc, int release_keys)
 		spin_unlock_irqrestore(&kp->lock, flags);
 		goto out;
 	}
-	if (!kp->pending)
-		disable_irq_nosync(kp->irq);
+	disable_irq_nosync(kp->irq);
+	set_irq_type(kp->irq, IRQT_NOEDGE);
+	spin_unlock_irqrestore(&kp->lock, flags);
 
 	while (kp->pending) {
-		spin_unlock_irqrestore(&kp->lock, flags);
 		msleep(1);
-		spin_lock_irqsave(&kp->lock, flags);
 	}
-	spin_unlock_irqrestore(&kp->lock, flags);
 
 	tsc2301_kp_stop_scan(tsc);
-	set_irq_type(kp->irq, IRQT_NOEDGE);
 out:
-	if (release_keys)
-		tsc2301_kp_release_all_keys(tsc);
+	if (!release_keys)
+		del_timer(&kp->timer); /* let timeout release keys */
 
 	return 0;
 }
@@ -379,7 +320,6 @@ static DEVICE_ATTR(disable_kp, 0664, tsc2301_kp_disable_show,
 		   tsc2301_kp_disable_store);
 
 static const u16 tsc2301_kp_read_data = 0x8000 | TSC2301_REG_KPDATA;
-static const u16 tsc2301_kp_read_state = 0x8000 | TSC2301_REG_KEY;
 
 static void tsc2301_kp_setup_spi_xfer(struct tsc2301 *tsc)
 {
@@ -389,22 +329,6 @@ static void tsc2301_kp_setup_spi_xfer(struct tsc2301 *tsc)
 
 	spi_message_init(&kp->read_msg);
 
-	if (kp->get_keyb_irq_state == NULL) {
-		/* No platform specific way for determining the keypress
-		 * state, so we'll need an extra status register read.
-		 */
-		x->tx_buf = &tsc2301_kp_read_state;
-		x->len = 2;
-		spi_message_add_tail(x, m);
-		x++;
-
-		x->rx_buf = &kp->state;
-		x->len = 2;
-		x->cs_change = 1;
-		spi_message_add_tail(x, m);
-		x++;
-	}
-
 	x->tx_buf = &tsc2301_kp_read_data;
 	x->len = 2;
 	spi_message_add_tail(x, m);
@@ -465,8 +389,6 @@ int __devinit tsc2301_kp_init(struct tsc2301 *tsc,
 	kp->timer.data = (unsigned long) tsc;
 	kp->timer.function = tsc2301_kp_timer;
 
-	kp->get_keyb_irq_state = pdata->get_keyb_irq_state;
-
 	idev = input_allocate_device();
 	if (idev == NULL) {
 		r = -ENOMEM;
diff --git a/include/linux/spi/tsc2301.h b/include/linux/spi/tsc2301.h
index a29e947..7d03e76 100644
--- a/include/linux/spi/tsc2301.h
+++ b/include/linux/spi/tsc2301.h
@@ -53,7 +53,6 @@ struct tsc2301_platform_data {
 	void (* codec_cleanup)(struct device *tsc2301_dev);
 	int	(*enable_clock)(struct device *dev);
 	void	(*disable_clock)(struct device *dev);
-	int	(*get_keyb_irq_state)(struct device *dev);
 
 	const struct tsc2301_mixer_gpio {
 		const char	*name;
-- 
1.5.3.3

-
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux