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 <[email protected]>
---
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
+
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;
schedule_delayed_work(&ts->work, TS_POLL_PERIOD);
}
@@ -195,7 +204,14 @@
{
struct tsc2007 *ts =
container_of(to_delayed_work(work), struct tsc2007, work);
- if (unlikely(!ts->get_pendown_state() && ts->pendown)) {
+
+ /* either we have the pendown callback, and use it, otherwise
+ * we look at the pendown variable
+ */
+ if ((ts->get_pendown_state && unlikely(!ts->get_pendown_state()) &&
+ ts->penstate != PEN_STATE_UP) ||
+ (!ts->get_pendown_state &&
+ unlikely(ts->penstate == PEN_STATE_UP))) {
struct input_dev *input = ts->input;
dev_dbg(&ts->client->dev, "UP\n");
@@ -204,7 +220,7 @@
input_report_abs(input, ABS_PRESSURE, 0);
input_sync(input);
- ts->pendown = 0;
+ ts->penstate = PEN_STATE_UP;
enable_irq(ts->irq);
} else {
/* pen is still down, continue with the measurement */
@@ -219,8 +235,9 @@
{
struct tsc2007 *ts = handle;
- if (likely(ts->get_pendown_state())) {
+ if (!ts->get_pendown_state || likely(ts->get_pendown_state())) {
disable_irq_nosync(ts->irq);
+ ts->penstate = PEN_STATE_IRQ;
schedule_delayed_work(&ts->work, 0);
}
@@ -264,7 +281,8 @@
ts->get_pendown_state = pdata->get_pendown_state;
ts->clear_penirq = pdata->clear_penirq;
- pdata->init_platform_hw();
+ if (pdata->init_platform_hw)
+ pdata->init_platform_hw();
snprintf(ts->phys, sizeof(ts->phys),
"%s/input0", dev_name(&client->dev));
On Tue, 23 Jun 2009 13:54:54 +0200
Richard R__jfors <[email protected]> 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.
>
Again, I don't understand the reason for the change from the above
description.
Is there some driver in the tree which does not implement
->get_pendown_state()? If so, it will oops, won't it? Which driver is
that?
Or is there some other driver which you're developing which does not
implement ->get_pendown_state()?
If the latter, why should the problem be solved in this way, rather
than implementing an empty ->get_pendown_state() within that driver?
etc. More details, please.
On 6/25/09 11:23 PM, Andrew Morton wrote:
> On Tue, 23 Jun 2009 13:54:54 +0200
> Richard R__jfors<[email protected]> 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.
>>
>
> Again, I don't understand the reason for the change from the above
> description.
>
> Is there some driver in the tree which does not implement
> ->get_pendown_state()? If so, it will oops, won't it? Which driver is
> that?
>
> Or is there some other driver which you're developing which does not
> implement ->get_pendown_state()?
>
> If the latter, why should the problem be solved in this way, rather
> than implementing an empty ->get_pendown_state() within that driver?
On the board I'm currently writing drivers for we don't have any chance
of getting the pendown state, we have to trust the touch controller,
(which is not very accurate in all cases).
So we can not implement a dummy function, because there is no dummy
default value to return. In that case the function must also do I2C
calls to the touch controller, which is the responsibility of this driver.
--Richard
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 <[email protected]>
> ---
> 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