Decreases the number of I2C transactions transferred by the driver.
During probe we don't need to ask for the coordinates from the controller.
When polling the controller we don't need to power down and enable IRQ
if we are going to poll again.
Signed-off-by: Richard R?jfors <[email protected]>
---
Index: linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c
===================================================================
--- linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1040)
+++ linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1053)
@@ -178,6 +178,12 @@
ts->penstate = PEN_STATE_UP;
}
+static void tsc2007_power_down(struct tsc2007 *tsc)
+{
+ /* power down */
+ tsc2007_xfer(tsc, PWRDOWN);
+}
+
static int tsc2007_read_values(struct tsc2007 *tsc)
{
/* y- still on; turn on only y+ (and ADC) */
@@ -188,11 +194,8 @@
/* turn y+ off, x- on; we'll use formula #1 */
tsc->tc.z1 = tsc2007_xfer(tsc, READ_Z1);
- tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2);
+ tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2 | TSC2007_POWER_OFF_IRQ_EN);
- /* power down */
- tsc2007_xfer(tsc, PWRDOWN);
-
return 0;
}
@@ -217,6 +220,7 @@
input_sync(input);
ts->penstate = PEN_STATE_UP;
+ tsc2007_power_down(ts);
enable_irq(ts->irq);
} else {
/* pen is still down, continue with the measurement */
@@ -305,7 +309,7 @@
input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
- tsc2007_read_values(ts);
+ tsc2007_power_down(ts);
ts->irq = client->irq;
On Fri, Jul 24, 2009 at 06:14:37PM +0200, Richard R?jfors wrote:
> Decreases the number of I2C transactions transferred by the driver.
> During probe we don't need to ask for the coordinates from the controller.
> When polling the controller we don't need to power down and enable IRQ
> if we are going to poll again.
>
> Signed-off-by: Richard R?jfors <[email protected]>
> ---
> Index: linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c
> ===================================================================
> --- linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1040)
> +++ linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1053)
> @@ -178,6 +178,12 @@
> ts->penstate = PEN_STATE_UP;
> }
>
> +static void tsc2007_power_down(struct tsc2007 *tsc)
> +{
> + /* power down */
> + tsc2007_xfer(tsc, PWRDOWN);
> +}
> +
> static int tsc2007_read_values(struct tsc2007 *tsc)
> {
> /* y- still on; turn on only y+ (and ADC) */
> @@ -188,11 +194,8 @@
>
> /* turn y+ off, x- on; we'll use formula #1 */
> tsc->tc.z1 = tsc2007_xfer(tsc, READ_Z1);
> - tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2);
> + tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2 | TSC2007_POWER_OFF_IRQ_EN);
I think this leaves the controller powered on and with with PENIRQ
disabled.
--
Dmitry
On 7/24/09 8:00 PM, Dmitry Torokhov wrote:
> On Fri, Jul 24, 2009 at 06:14:37PM +0200, Richard R?jfors wrote:
>> Decreases the number of I2C transactions transferred by the driver.
>> During probe we don't need to ask for the coordinates from the controller.
>> When polling the controller we don't need to power down and enable IRQ
>> if we are going to poll again.
>>
>> Signed-off-by: Richard R?jfors<[email protected]>
>> ---
>> Index: linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c
>> ===================================================================
>> --- linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1040)
>> +++ linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1053)
>> @@ -178,6 +178,12 @@
>> ts->penstate = PEN_STATE_UP;
>> }
>>
>> +static void tsc2007_power_down(struct tsc2007 *tsc)
>> +{
>> + /* power down */
>> + tsc2007_xfer(tsc, PWRDOWN);
>> +}
>> +
>> static int tsc2007_read_values(struct tsc2007 *tsc)
>> {
>> /* y- still on; turn on only y+ (and ADC) */
>> @@ -188,11 +194,8 @@
>>
>> /* turn y+ off, x- on; we'll use formula #1 */
>> tsc->tc.z1 = tsc2007_xfer(tsc, READ_Z1);
>> - tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2);
>> + tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2 | TSC2007_POWER_OFF_IRQ_EN);
>
> I think this leaves the controller powered on and with with PENIRQ
> disabled.
>
You are right, I think we should leave the patch like below, just get
rid of the unnecessary read during startup.
--Richard
Input: tsc2007: Do not read coordinated when probing driver
From: Richard R?jfors <[email protected]>
Don't read coordinates during probe of the driver, just powering down
the controller and wait for interrupts.
Signed-off-by: Richard R?jfors<[email protected]>
---
Index: linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c
===================================================================
--- linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1040)
+++ linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1056)
@@ -178,6 +178,12 @@
ts->penstate = PEN_STATE_UP;
}
+static void tsc2007_power_down(struct tsc2007 *tsc)
+{
+ /* power down */
+ tsc2007_xfer(tsc, PWRDOWN);
+}
+
static int tsc2007_read_values(struct tsc2007 *tsc)
{
/* y- still on; turn on only y+ (and ADC) */
@@ -190,8 +196,7 @@
tsc->tc.z1 = tsc2007_xfer(tsc, READ_Z1);
tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2);
- /* power down */
- tsc2007_xfer(tsc, PWRDOWN);
+ tsc2007_power_down(tsc);
return 0;
}
@@ -305,7 +310,7 @@
input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
- tsc2007_read_values(ts);
+ tsc2007_power_down(ts);
ts->irq = client->irq;
On Fri, Jul 24, 2009 at 09:37:27PM +0200, Richard R?jfors wrote:
> On 7/24/09 8:00 PM, Dmitry Torokhov wrote:
>> On Fri, Jul 24, 2009 at 06:14:37PM +0200, Richard R?jfors wrote:
>>> Decreases the number of I2C transactions transferred by the driver.
>>> During probe we don't need to ask for the coordinates from the controller.
>>> When polling the controller we don't need to power down and enable IRQ
>>> if we are going to poll again.
>>>
>>> Signed-off-by: Richard R?jfors<[email protected]>
>>> ---
>>> Index: linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c
>>> ===================================================================
>>> --- linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1040)
>>> +++ linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1053)
>>> @@ -178,6 +178,12 @@
>>> ts->penstate = PEN_STATE_UP;
>>> }
>>>
>>> +static void tsc2007_power_down(struct tsc2007 *tsc)
>>> +{
>>> + /* power down */
>>> + tsc2007_xfer(tsc, PWRDOWN);
>>> +}
>>> +
>>> static int tsc2007_read_values(struct tsc2007 *tsc)
>>> {
>>> /* y- still on; turn on only y+ (and ADC) */
>>> @@ -188,11 +194,8 @@
>>>
>>> /* turn y+ off, x- on; we'll use formula #1 */
>>> tsc->tc.z1 = tsc2007_xfer(tsc, READ_Z1);
>>> - tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2);
>>> + tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2 | TSC2007_POWER_OFF_IRQ_EN);
>>
>> I think this leaves the controller powered on and with with PENIRQ
>> disabled.
>>
>
> You are right, I think we should leave the patch like below, just get
> rid of the unnecessary read during startup.
>
I modified the patch a bit before applying. In fact there were a few
changes to all the patces so it would be nice if you could take a look
at the 'next' branch of my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
Also, I di dnot quite like the logic of the patch making
get_pendown_state callback optional; could you please try the patch
below and let me know of it works for you?
Thanks!
--
Dmitry
Input: tsc2007 - make get_pendown_state platform callback optional
In cases when get_pendown_state callback is not available have
the driver to fallback on pressure calculation to determine if
the pen is up.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/touchscreen/tsc2007.c | 170 +++++++++++++++++++----------------
1 files changed, 92 insertions(+), 78 deletions(-)
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index f710af4..ac5e0e8 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -70,7 +70,6 @@ struct tsc2007 {
struct input_dev *input;
char phys[32];
struct delayed_work work;
- struct ts_event tc;
struct i2c_client *client;
@@ -106,51 +105,96 @@ static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
return val;
}
-static void tsc2007_send_event(void *tsc)
+static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
{
- struct tsc2007 *ts = tsc;
- u32 rt;
- u16 x, y, z1, z2;
+ /* y- still on; turn on only y+ (and ADC) */
+ tc->y = tsc2007_xfer(tsc, READ_Y);
+
+ /* turn y- off, x+ on, then leave in lowpower */
+ tc->x = tsc2007_xfer(tsc, READ_X);
+
+ /* turn y+ off, x- on; we'll use formula #1 */
+ tc->z1 = tsc2007_xfer(tsc, READ_Z1);
+ tc->z2 = tsc2007_xfer(tsc, READ_Z2);
- x = ts->tc.x;
- y = ts->tc.y;
- z1 = ts->tc.z1;
- z2 = ts->tc.z2;
+ /* Prepare for next touch reading - power down ADC, enable PENIRQ */
+ tsc2007_xfer(tsc, PWRDOWN);
+}
+
+static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
+{
+ u32 rt = 0;
/* range filtering */
- if (x == MAX_12BIT)
- x = 0;
+ if (tc->x == MAX_12BIT)
+ tc->x = 0;
- if (likely(x && z1)) {
+ if (likely(tc->x && tc->z1)) {
/* compute touch pressure resistance using equation #1 */
- rt = z2;
- rt -= z1;
- rt *= x;
- rt *= ts->x_plate_ohms;
- rt /= z1;
+ rt = tc->z2 - tc->z1;
+ rt *= tc->x;
+ rt *= tsc->x_plate_ohms;
+ rt /= tc->z1;
rt = (rt + 2047) >> 12;
- } else
- rt = 0;
-
- /*
- * Sample found inconsistent by debouncing or pressure is beyond
- * the maximum. Don't report it to user space, repeat at least
- * once more the measurement
- */
- if (rt > MAX_12BIT) {
- dev_dbg(&ts->client->dev, "ignored pressure %d\n", rt);
- return;
}
+ return rt;
+}
+
+static void tsc2007_send_up_event(struct tsc2007 *tsc)
+{
+ struct input_dev *input = tsc->input;
+
+ dev_dbg(&tsc->client->dev, "UP\n");
+
+ input_report_key(input, BTN_TOUCH, 0);
+ input_report_abs(input, ABS_PRESSURE, 0);
+ input_sync(input);
+}
+
+static void tsc2007_work(struct work_struct *work)
+{
+ struct tsc2007 *ts =
+ container_of(to_delayed_work(work), struct tsc2007, work);
+ struct ts_event tc;
+ u32 rt;
+
/*
* NOTE: We can't rely on the pressure to determine the pen down
- * state, even this controller has a pressure sensor. The pressure
- * value can fluctuate for quite a while after lifting the pen and
- * in some cases may not even settle at the expected value.
+ * state, even though this controller has a pressure sensor.
+ * The pressure value can fluctuate for quite a while after
+ * lifting the pen and in some cases may not even settle at the
+ * expected value.
*
* 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).
+ * work function by reading the pen signal state (it's a GPIO
+ * and IRQ). Unfortunately such callback is not always available,
+ * in that case we have rely on the pressure anyway.
*/
+ if (ts->get_pendown_state) {
+ if (unlikely(!ts->get_pendown_state())) {
+ tsc2007_send_up_event(ts);
+ ts->pendown = false;
+ goto out;
+ }
+
+ dev_dbg(&ts->client->dev, "pen is still down\n");
+ }
+
+ tsc2007_read_values(ts, &tc);
+
+ rt = tsc2007_calculate_pressure(ts, &tc);
+ if (rt > MAX_12BIT) {
+ /*
+ * Sample found inconsistent by debouncing or pressure is
+ * beyond the maximum. Don't report it to user space,
+ * repeat at least once more the measurement.
+ */
+ dev_dbg(&ts->client->dev, "ignored pressure %d\n", rt);
+ goto out;
+
+ }
+
if (rt) {
struct input_dev *input = ts->input;
@@ -161,68 +205,38 @@ static void tsc2007_send_event(void *tsc)
ts->pendown = true;
}
- input_report_abs(input, ABS_X, x);
- input_report_abs(input, ABS_Y, y);
+ input_report_abs(input, ABS_X, tc.x);
+ input_report_abs(input, ABS_Y, tc.y);
input_report_abs(input, ABS_PRESSURE, rt);
input_sync(input);
dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n",
- x, y, rt);
- }
-}
-
-static int tsc2007_read_values(struct tsc2007 *tsc)
-{
- /* y- still on; turn on only y+ (and ADC) */
- tsc->tc.y = tsc2007_xfer(tsc, READ_Y);
-
- /* turn y- off, x+ on, then leave in lowpower */
- tsc->tc.x = tsc2007_xfer(tsc, READ_X);
-
- /* turn y+ off, x- on; we'll use formula #1 */
- tsc->tc.z1 = tsc2007_xfer(tsc, READ_Z1);
- tsc->tc.z2 = tsc2007_xfer(tsc, READ_Z2);
-
- /* Prepare for next touch reading - power down ADC, enable PENIRQ */
- tsc2007_xfer(tsc, PWRDOWN);
-
- return 0;
-}
-
-static void tsc2007_work(struct work_struct *work)
-{
- struct tsc2007 *ts =
- container_of(to_delayed_work(work), struct tsc2007, work);
-
- if (unlikely(!ts->get_pendown_state() && ts->pendown)) {
- struct input_dev *input = ts->input;
-
- dev_dbg(&ts->client->dev, "UP\n");
-
- input_report_key(input, BTN_TOUCH, 0);
- input_report_abs(input, ABS_PRESSURE, 0);
- input_sync(input);
+ tc.x, tc.y, rt);
+ } else if (!ts->get_pendown_state && ts->pendown) {
+ /*
+ * We don't have callback to check pendown state, so we
+ * have to assume that since pressure reported is 0 the
+ * pen was lifted up.
+ */
+ tsc2007_send_up_event(ts);
ts->pendown = false;
- enable_irq(ts->irq);
- } else {
- /* pen is still down, continue with the measurement */
- dev_dbg(&ts->client->dev, "pen is still down\n");
-
- tsc2007_read_values(ts);
- tsc2007_send_event(ts);
+ }
+ out:
+ if (ts->pendown)
schedule_delayed_work(&ts->work,
msecs_to_jiffies(TS_POLL_PERIOD));
- }
+ else
+ enable_irq(ts->irq);
}
static irqreturn_t tsc2007_irq(int irq, void *handle)
{
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);
schedule_delayed_work(&ts->work,
msecs_to_jiffies(TS_POLL_DELAY));