Re: [PATCH 2/2] tsc2007: make platform callbacks optional

From: Dmitry Torokhov
Date: Tue Jul 14 2009 - 00:18:35 EST


Hi Richard,

On Tue, Jun 23, 2009 at 01:54:54PM +0200, Richard Röjfors wrote:
> The platform callbacks are only called if supplied. Makes the driver
> to fallback on only pressure calculation to decide when the pen is up.
>
> Signed-off-by: Richard Röjfors <richard.rojfors.ext@xxxxxxxxxxxxxxx>
> ---
> Index: linux-2.6.30/drivers/input/touchscreen/tsc2007.c
> ===================================================================
> --- linux-2.6.30/drivers/input/touchscreen/tsc2007.c (revision 943)
> +++ linux-2.6.30/drivers/input/touchscreen/tsc2007.c (revision 945)
> @@ -59,6 +59,10 @@
> #define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
> #define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
>
> +#define PEN_STATE_UP 0x00
> +#define PEN_STATE_DOWN 0x01
> +#define PEN_STATE_IRQ 0x01

Why the last 2 are the same?

> +
> struct ts_event {
> u16 x;
> u16 y;
> @@ -76,7 +80,7 @@
> u16 model;
> u16 x_plate_ohms;
>
> - unsigned pendown;
> + unsigned penstate;
> int irq;
>
> int (*get_pendown_state)(void);
> @@ -149,15 +153,18 @@
> *
> * The only safe way to check for the pen up condition is in the
> * work function by reading the pen signal state (it's a GPIO and IRQ).
> + *
> + * But sadly we don't always have the possibility to use such callback
> + * in that case rely on the pressure anyway
> */
> if (rt) {
> struct input_dev *input = ts->input;
>
> - if (!ts->pendown) {
> + if (ts->penstate != PEN_STATE_DOWN) {
> dev_dbg(&ts->client->dev, "DOWN\n");
>
> input_report_key(input, BTN_TOUCH, 1);
> - ts->pendown = 1;
> + ts->penstate = PEN_STATE_DOWN;
> }
>
> input_report_abs(input, ABS_X, x);
> @@ -168,7 +175,9 @@
>
> dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n",
> x, y, rt);
> - }
> + } else if (!ts->get_pendown_state)
> + /* no callback to check pendown state, use pressure */
> + ts->penstate = PEN_STATE_UP;
>

Since we are not going to re-check pen state why don't we report "pen
up" event here right away and forego rescheduling the work?

> schedule_delayed_work(&ts->work, TS_POLL_PERIOD);
> }

Thanks.

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/