tsc2301 - correct the handling of the keyb_int interrupt

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

The error was the use of the keyb_int interrupt line as a
shortcut to see if any keys were pressed, that doesn't work.

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
(nothing).

Signed-off-by: Klaus Pedersen <klaus.k.pedersen@nokia.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/mach-omap2/board-n800.c b/arch/arm/mach-omap2/board-n800.c
index f161f8d18ba936280b753715b21ed79172046ef7..878efc11210e067939959248443341e322ee0fe1 100644 (file)
--- 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 ac4cbaecf004fee6d033c52993013758c5d14a47..473c0f2d9a47e08aab4333098d53a2f0b2a1e107 100644 (file)
--- 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 d2adf60ed2b2e84d2eb160d1d337a1c6e1e4be4e..e30d01ddee330dcd49396a7db130d2c3d99127c1 100644 (file)
--- 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;