On Tue, Nov 25, 2008 at 06:15:16PM +0800, Tick Chen wrote: > > (Sending to openmoko-kernel ML) > > Hi Andy, > I created a porting patch for the delay event > scheme of ts driver for stable-tracking branch. > Basically it's the same as that one of > http://git.openmoko.org/?p=kernel.git;a=commit;h=abe8f448547d1bd69ac2963e07e2657f27b79691
Hello Tick.
> +#define TOUCH_RELEASE_TIMEOUT (HZ >> 4)
I did read your patch. Nice comments.
In the way interrupts and timer are handled now the logic is hard to follow.
I am testing a filter (that might find a place with the median filter
because the data it needs is identical) that allows us to reject
erratic clicks (using standard deviation).
If we introduce a FIFO and states for the TS it is easier to add more
logic -- like the one that avoids jitter. I'm working in this patch and I
added the jitter detection.
I'm too sleepy to submit now but I'm sending it here... I'll submit it
later formally.
Best regards.
cleanup-adc-s3c2410_ts.patch
From: Nelson Castillo <[EMAIL PROTECTED]
I'll submit this patch later!
I'm too sleepy now...
Cleanup:
- Factor the ADC conversion.
- Improve the logic of the timer function.
- Include a FIFO that gives us more flexibility in the interrupt handlers.
- Integrate the jitter detection.
Signed-off-by: Nelson Castillo <[EMAIL PROTECTED]>
---
drivers/input/touchscreen/s3c2410_ts.c | 223 +++++++++++++++++++++++++-------
1 files changed, 171 insertions(+), 52 deletions(-)
diff --git a/drivers/input/touchscreen/s3c2410_ts.c
b/drivers/input/touchscreen/s3c2410_ts.c
index 8e580a8..90d31ef 100644
--- a/drivers/input/touchscreen/s3c2410_ts.c
+++ b/drivers/input/touchscreen/s3c2410_ts.c
@@ -52,6 +52,7 @@
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
+#include <linux/kfifo.h>
#include <asm/io.h>
#include <asm/irq.h>
@@ -101,7 +102,8 @@ struct s3c2410ts {
struct ts_filter *tsf[MAX_TS_FILTER_CHAIN];
int coords[2]; /* just X and Y for us */
int is_down;
- int need_to_send_first_touch;
+ int state;
+ struct kfifo *event_fifo;
};
static struct s3c2410ts ts;
@@ -117,69 +119,155 @@ static inline void s3c2410_ts_connect(void)
s3c2410_gpio_cfgpin(S3C2410_GPG15, S3C2410_GPG15_nYPON);
}
+static void s3c2410_ts_start_adc_conversion(void)
+{
+ writel(S3C2410_ADCTSC_PULL_UP_DISABLE | AUTOPST,
+ base_addr + S3C2410_ADCTSC);
+ writel(readl(base_addr + S3C2410_ADCCON) | S3C2410_ADCCON_ENABLE_START,
+ base_addr + S3C2410_ADCCON);
+}
+
+/*
+ * Just send the input events.
+ */
+
enum ts_input_event {IE_DOWN = 0, IE_UP, IE_UPDATE};
-static void ts_input_report(int event)
+static void ts_input_report(int event, int coords[])
{
+#ifdef CONFIG_TOUCHSCREEN_S3C2410_DEBUG
+ static char *s[] = {"down", "up", "update"};
+ struct timeval tv;
+
+ do_gettimeofday(&tv);
+#endif
+
if (event == IE_DOWN || event == IE_UPDATE) {
- input_report_abs(ts.dev, ABS_X, ts.coords[0]);
- input_report_abs(ts.dev, ABS_Y, ts.coords[1]);
+ input_report_abs(ts.dev, ABS_X, coords[0]);
+ input_report_abs(ts.dev, ABS_Y, coords[1]);
input_report_key(ts.dev, BTN_TOUCH, 1);
input_report_abs(ts.dev, ABS_PRESSURE, 1);
+
+#ifdef CONFIG_TOUCHSCREEN_S3C2410_DEBUG
+ printk(DEBUG_LVL "T:%06d %6s (X:%03d, Y:%03d)\n",
+ (int)tv.tv_usec, s[event], coords[0], coords[1]);
+#endif
} else {
input_report_key(ts.dev, BTN_TOUCH, 0);
input_report_abs(ts.dev, ABS_PRESSURE, 0);
- }
-
- input_sync(ts.dev);
#ifdef CONFIG_TOUCHSCREEN_S3C2410_DEBUG
- {
- static char *s[] = {"down", "up", "update"};
- struct timeval tv;
- do_gettimeofday(&tv);
- printk(DEBUG_LVL "T:%06d %6s (X:%03d, Y:%03d)\n",
- (int)tv.tv_usec, s[event], ts.coords[0], ts.coords[1]);
- }
+ printk(DEBUG_LVL "T:%06d %6s\n",
+ (int)tv.tv_usec, s[event]);
#endif
+ }
+
+ input_sync(ts.dev);
}
-static void touch_timer_fire(unsigned long data)
+
+/*
+ * Manage state of the touchscreen and send events.
+ */
+
+#define TS_RELEASE_TIMEOUT (HZ >> 4) /* ~ 60 milliseconds */
+#define TS_EVENT_TIMER_INTERVAL (HZ >> 6) /* ~ 15 milliseconds */
+#define N_NOOP_CALLS (TS_RELEASE_TIMEOUT / TS_EVENT_TIMER_INTERVAL)
+
+#define TS_EVENT_FIFO_SIZE (2 << 6) /* must be a power of 2 */
+
+static void event_send_timer_f(unsigned long data);
+
+static struct timer_list event_send_timer =
+ TIMER_INITIALIZER(event_send_timer_f, 0, 0);
+
+#define TS_STATE_STANDBY 0 /* initial state */
+#define TS_STATE_PRESSED_PENDING 1
+#define TS_STATE_PRESSED 2
+#define TS_STATE_RELEASE_PENDING 3
+#define TS_STATE_RELEASE 4
+
+static void event_send_timer_f(unsigned long data)
{
- if (ts.tsf[0])
- (ts.tsf[0]->api->scale)(ts.tsf[0], &ts.coords[0]);
+ static unsigned long running;
+ static int noop_counter;
+ int event_type;
+
+ if (unlikely(test_and_set_bit(0, &running))) {
+ mod_timer(&event_send_timer,
+ jiffies + TS_EVENT_TIMER_INTERVAL + 1);
+ return;
+ }
+
+ while (__kfifo_get(ts.event_fifo, (unsigned char *)&event_type,
+ sizeof(int))) {
+ int buf[2];
+
+ switch(event_type) {
+ case 'D':
+ if (ts.state == TS_STATE_RELEASE_PENDING)
+ ts.state = TS_STATE_PRESSED;
+ else
+ ts.state = TS_STATE_PRESSED_PENDING;
+ break;
+
+ case 'U':
+ ts.state = TS_STATE_RELEASE_PENDING;
+ break;
+
+ case 'P':
+ if (ts.is_down) /* stylus_action needs a conversion */
+ s3c2410_ts_start_adc_conversion();
+
+ if (unlikely(__kfifo_get(ts.event_fifo,
+ (unsigned char *)buf,
+ sizeof(int) * 2)
+ != sizeof(int) * 2))
+ goto ts_exit_error;
- if (ts.need_to_send_first_touch) {
- ts.need_to_send_first_touch = 0;
- ts_input_report(IE_DOWN);
- if (!ts.is_down) { /* Do we need this? I think so. */
- ts_input_report(IE_UPDATE);
- ts_input_report(IE_UP);
+ if (ts.state == TS_STATE_PRESSED_PENDING)
+ ts_input_report(IE_DOWN, buf);
+ else
+ ts_input_report(IE_UPDATE, buf);
+
+ ts.state = TS_STATE_PRESSED;
+
+ break;
+
+ default:
+ goto ts_exit_error;
}
- } else if (ts.is_down) {
- ts_input_report(IE_UPDATE);
- } else {
- ts_input_report(IE_UP);
+
+ noop_counter = 0;
}
- if (ts.is_down) {
- writel(S3C2410_ADCTSC_PULL_UP_DISABLE | AUTOPST,
- base_addr+S3C2410_ADCTSC);
- writel(readl(base_addr+S3C2410_ADCCON) |
- S3C2410_ADCCON_ENABLE_START, base_addr+S3C2410_ADCCON);
+ /* We delay the up event for a while to avoid jitter */
+
+ if (noop_counter++ >= N_NOOP_CALLS) {
+ noop_counter = 0;
+ if (ts.state == TS_STATE_RELEASE_PENDING)
+ ts_input_report(IE_UP, NULL);
+ ts.state = TS_STATE_STANDBY;
} else {
- if (ts.tsf[0])
- (ts.tsf[0]->api->clear)(ts.tsf[0]);
- writel(WAIT4INT(0), base_addr+S3C2410_ADCTSC);
+ mod_timer(&event_send_timer, jiffies + TS_EVENT_TIMER_INTERVAL);
}
+
+ clear_bit(0, &running);
+
+ return;
+
+ts_exit_error: /* should not happen unless we have a bug */
+ printk(KERN_ERR __FILE__ ": event_send_timer_f failed\n");
}
-static struct timer_list touch_timer =
- TIMER_INITIALIZER(touch_timer_fire, 0, 0);
+/*
+ * Manage interrupts.
+ */
static irqreturn_t stylus_updown(int irq, void *dev_id)
{
unsigned long data0;
unsigned long data1;
+ int event_type;
data0 = readl(base_addr+S3C2410_ADCDAT0);
data1 = readl(base_addr+S3C2410_ADCDAT1);
@@ -187,12 +275,17 @@ static irqreturn_t stylus_updown(int irq, void *dev_id)
ts.is_down = (!(data0 & S3C2410_ADCDAT0_UPDOWN)) &&
(!(data1 & S3C2410_ADCDAT0_UPDOWN));
+ event_type = ts.is_down ? 'D' : 'U';
+
+ if (unlikely(__kfifo_put(ts.event_fifo, (unsigned char *)&event_type,
+ sizeof(int)) != sizeof(int))) /* should not happen */
+ printk(KERN_ERR __FILE__": stylus_updown lost event!\n");
+
if (ts.is_down) {
- ts.need_to_send_first_touch = 1;
- writel(S3C2410_ADCTSC_PULL_UP_DISABLE | AUTOPST,
- base_addr+S3C2410_ADCTSC);
- writel(readl(base_addr+S3C2410_ADCCON) |
- S3C2410_ADCCON_ENABLE_START, base_addr+S3C2410_ADCCON);
+ s3c2410_ts_start_adc_conversion();
+ mod_timer(&event_send_timer, jiffies + 1);
+ } else {
+ writel(WAIT4INT(0), base_addr+S3C2410_ADCTSC);
}
return IRQ_HANDLED;
@@ -200,6 +293,8 @@ static irqreturn_t stylus_updown(int irq, void *dev_id)
static irqreturn_t stylus_action(int irq, void *dev_id)
{
+ int buf[3];
+
/* grab the ADC results */
ts.coords[0] = readl(base_addr + S3C2410_ADCDAT0) &
S3C2410_ADCDAT0_XPDATA_MASK;
@@ -217,15 +312,28 @@ static irqreturn_t stylus_action(int irq, void *dev_id)
* no real sample came out of processing yet,
* get another raw result to feed it
*/
- writel(S3C2410_ADCTSC_PULL_UP_DISABLE | AUTOPST,
- base_addr + S3C2410_ADCTSC);
- writel(readl(base_addr + S3C2410_ADCCON) | S3C2410_ADCCON_ENABLE_START,
- base_addr + S3C2410_ADCCON);
+
+ s3c2410_ts_start_adc_conversion();
+
return IRQ_HANDLED;
real_sample:
- mod_timer(&touch_timer, jiffies + 1);
+
+ if (ts.tsf[0])
+ (ts.tsf[0]->api->scale)(ts.tsf[0], &ts.coords[0]);
+
+ buf[0] = 'P';
+ buf[1] = ts.coords[0];
+ buf[2] = ts.coords[1];
+
+ if (unlikely(__kfifo_put(ts.event_fifo, (unsigned char *)buf,
+ sizeof(int) * 3) != sizeof(int) * 3))
+ /* should not happen */
+ printk(KERN_ERR __FILE__": stylus_action lost event!\n");
+
writel(WAIT4INT(1), base_addr + S3C2410_ADCTSC);
+ //mod_timer(&touch_schedule_adc_timer, jiffies + 1);
+ mod_timer(&event_send_timer, jiffies + 1);
return IRQ_HANDLED;
}
@@ -348,22 +456,31 @@ static int __init s3c2410ts_probe(struct platform_device
*pdev)
goto bail4;
}
+ ts.event_fifo = kfifo_alloc(TS_EVENT_FIFO_SIZE, GFP_KERNEL, NULL);
+
+ if (IS_ERR(ts.event_fifo)) {
+ ret = -EIO;
+ goto bail5;
+ }
+
dev_info(&pdev->dev, "successfully loaded\n");
/* All went ok, so register to the input system */
rc = input_register_device(ts.dev);
if (rc) {
- free_irq(IRQ_TC, ts.dev);
- free_irq(IRQ_ADC, ts.dev);
- clk_disable(adc_clock);
- iounmap(base_addr);
ret = -EIO;
- goto bail5;
+ goto bail6;
}
return 0;
+bail6:
+ kfifo_free(ts.event_fifo);
bail5:
+ free_irq(IRQ_TC, ts.dev);
+ free_irq(IRQ_ADC, ts.dev);
+ clk_disable(adc_clock);
+ iounmap(base_addr);
disable_irq(IRQ_TC);
bail4:
disable_irq(IRQ_ADC);
@@ -396,6 +513,8 @@ static int s3c2410ts_remove(struct platform_device *pdev)
ts_filter_destroy_chain(ts.tsf);
+ kfifo_free(ts.event_fifo);
+
return 0;
}
signature.asc
Description: Digital signature
