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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 13 Jul 2009 21:17:59 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Richard Röjfors 
	<richard.rojfors.ext@...ean-labs.com>
Cc:	linux-input@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	kwangwoo.lee@...il.com,
	Thierry Reding <thierry.reding@...onic-design.de>,
	Trilok Soni <soni.trilok@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 2/2] tsc2007: make platform callbacks optional

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@...ean-labs.com>
> ---
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/