* [PATCH 0/4] s3c24xx_ts driver fixes
@ 2010-02-18 16:32 Vasily Khoruzhick
2010-02-18 16:32 ` [PATCH 1/4] Add resources description for s3c24xx ts driver Vasily Khoruzhick
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Vasily Khoruzhick @ 2010-02-18 16:32 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: ben-linux, linux-input
Following series fix some issues in s3c24xx_ts driver and
related s3c24xx_adc driver.
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/4] Add resources description for s3c24xx ts driver 2010-02-18 16:32 [PATCH 0/4] s3c24xx_ts driver fixes Vasily Khoruzhick @ 2010-02-18 16:32 ` Vasily Khoruzhick 2010-02-18 16:32 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Vasily Khoruzhick ` (3 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-18 16:32 UTC (permalink / raw) To: linux-arm-kernel; +Cc: ben-linux, linux-input, Vasily Khoruzhick Without resources description driver fails during initialization. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- arch/arm/plat-s3c24xx/devs.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/arch/arm/plat-s3c24xx/devs.c b/arch/arm/plat-s3c24xx/devs.c index 7f686a3..83c1507 100644 --- a/arch/arm/plat-s3c24xx/devs.c +++ b/arch/arm/plat-s3c24xx/devs.c @@ -185,9 +185,27 @@ void __init s3c24xx_fb_set_platdata(struct s3c2410fb_mach_info *pd) } /* Touchscreen */ + +static struct resource s3c_ts_resource[] = { + [0] = { + .start = S3C24XX_PA_ADC, + .end = S3C24XX_PA_ADC + S3C24XX_SZ_ADC - 1, + .flags = IORESOURCE_MEM, + }, + [1] = { + .start = IRQ_TC, + .end = IRQ_TC, + .flags = IORESOURCE_IRQ, + }, + +}; + struct platform_device s3c_device_ts = { .name = "s3c2410-ts", .id = -1, + .dev.parent = &s3c_device_adc.dev, + .num_resources = ARRAY_SIZE(s3c_ts_resource), + .resource = s3c_ts_resource, }; EXPORT_SYMBOL(s3c_device_ts); -- 1.7.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-18 16:32 [PATCH 0/4] s3c24xx_ts driver fixes Vasily Khoruzhick 2010-02-18 16:32 ` [PATCH 1/4] Add resources description for s3c24xx ts driver Vasily Khoruzhick @ 2010-02-18 16:32 ` Vasily Khoruzhick 2010-02-19 9:17 ` Dmitry Torokhov 2010-02-18 16:32 ` [PATCH 3/4] s3c24xx_adc: disable/enable IRQ on suspend/resume Vasily Khoruzhick ` (2 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-18 16:32 UTC (permalink / raw) To: linux-arm-kernel; +Cc: ben-linux, linux-input, Vasily Khoruzhick By default, driver reports touches when it gots (1 << ts.shift) samples, even if stylus is up. This behavior looks wrong to me. Patch makes driver to report touches only when stylus is down. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- drivers/input/touchscreen/s3c2410_ts.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c index 6386b44..e2fe2ed 100644 --- a/drivers/input/touchscreen/s3c2410_ts.c +++ b/drivers/input/touchscreen/s3c2410_ts.c @@ -128,25 +128,25 @@ static void touch_timer_fire(unsigned long data) down = get_down(data0, data1); - if (ts.count == (1 << ts.shift)) { - ts.xp >>= ts.shift; - ts.yp >>= ts.shift; + if (down) { + if (ts.count == (1 << ts.shift)) { + ts.xp >>= ts.shift; + ts.yp >>= ts.shift; - dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", - __func__, ts.xp, ts.yp, ts.count); + dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", + __func__, ts.xp, ts.yp, ts.count); - input_report_abs(ts.input, ABS_X, ts.xp); - input_report_abs(ts.input, ABS_Y, ts.yp); + input_report_abs(ts.input, ABS_X, ts.xp); + input_report_abs(ts.input, ABS_Y, ts.yp); - input_report_key(ts.input, BTN_TOUCH, 1); - input_sync(ts.input); + input_report_key(ts.input, BTN_TOUCH, 1); + input_sync(ts.input); + } ts.xp = 0; ts.yp = 0; ts.count = 0; - } - if (down) { s3c_adc_start(ts.client, 0, 1 << ts.shift); } else { ts.count = 0; -- 1.7.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-18 16:32 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Vasily Khoruzhick @ 2010-02-19 9:17 ` Dmitry Torokhov 2010-02-19 10:02 ` Vasily Khoruzhick 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Torokhov @ 2010-02-19 9:17 UTC (permalink / raw) To: Vasily Khoruzhick; +Cc: linux-arm-kernel, ben-linux, linux-input Hi Vasily, On Thu, Feb 18, 2010 at 06:32:28PM +0200, Vasily Khoruzhick wrote: > By default, driver reports touches when it gots (1 << ts.shift) > samples, even if stylus is up. This behavior looks wrong to me. > Patch makes driver to report touches only when stylus is down. > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > drivers/input/touchscreen/s3c2410_ts.c | 22 +++++++++++----------- > 1 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c > index 6386b44..e2fe2ed 100644 > --- a/drivers/input/touchscreen/s3c2410_ts.c > +++ b/drivers/input/touchscreen/s3c2410_ts.c > @@ -128,25 +128,25 @@ static void touch_timer_fire(unsigned long data) > > down = get_down(data0, data1); > > - if (ts.count == (1 << ts.shift)) { > - ts.xp >>= ts.shift; > - ts.yp >>= ts.shift; > + if (down) { > + if (ts.count == (1 << ts.shift)) { > + ts.xp >>= ts.shift; > + ts.yp >>= ts.shift; > > - dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", > - __func__, ts.xp, ts.yp, ts.count); > + dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", > + __func__, ts.xp, ts.yp, ts.count); > > - input_report_abs(ts.input, ABS_X, ts.xp); > - input_report_abs(ts.input, ABS_Y, ts.yp); > + input_report_abs(ts.input, ABS_X, ts.xp); > + input_report_abs(ts.input, ABS_Y, ts.yp); > > - input_report_key(ts.input, BTN_TOUCH, 1); > - input_sync(ts.input); > + input_report_key(ts.input, BTN_TOUCH, 1); > + input_sync(ts.input); > > + } > ts.xp = 0; > ts.yp = 0; > ts.count = 0; I do not think this is right. Here you reset sampling data regardless of whether you got the right number of samples or not. You should repeat s3c_adc_start only if you got (1 << ts.shift) samples. But I think I see why touchpad may stop reporintg sometimes - if timer fires before you get the needed number of samples nothing will happen as timer is not gettign rearmed. > - } > > - if (down) { > s3c_adc_start(ts.client, 0, 1 << ts.shift); > } else { > ts.count = 0; I also think this branch is missing ts.xp = ts.yp = 0; -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-19 9:17 ` Dmitry Torokhov @ 2010-02-19 10:02 ` Vasily Khoruzhick 2010-02-21 7:35 ` Dmitry Torokhov 0 siblings, 1 reply; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-19 10:02 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-arm-kernel, ben-linux, linux-input [-- Attachment #1.1: Type: Text/Plain, Size: 813 bytes --] Hi Dmitry, В сообщении от 19 февраля 2010 11:17:55 автор Dmitry Torokhov написал: > I do not think this is right. Here you reset sampling data regardless > of whether you got the right number of samples or not. You should repeat > s3c_adc_start only if you got (1 << ts.shift) samples. But I think I see > why touchpad may stop reporintg sometimes - if timer fires before you > get the needed number of samples nothing will happen as timer is not > gettign rearmed. Problem is driver reports "stylus down" event when stylus is already up, anyway I agree that samples should not be discarded. > > } else { > > > > ts.count = 0; > > I also think this branch is missing ts.xp = ts.yp = 0; Ok. Fixed version is in attachment Thanks for review. [-- Attachment #1.2: 0002-s3c24xx_ts-report-touch-only-when-stylus-is-down.patch --] [-- Type: text/x-patch, Size: 1969 bytes --] From 3edbe60fdb0cd2c2d91046557fcd9ad4f8c3b69d Mon Sep 17 00:00:00 2001 From: Vasily Khoruzhick <anarsoul@gmail.com> Date: Fri, 19 Feb 2010 11:48:56 +0200 Subject: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down By default, driver reports touches when it gots (1 << ts.shift) samples, even if stylus is up. This behavior looks wrong to me. Patch makes driver to report touches only when stylus is down. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- drivers/input/touchscreen/s3c2410_ts.c | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c index 6386b44..14afa0e 100644 --- a/drivers/input/touchscreen/s3c2410_ts.c +++ b/drivers/input/touchscreen/s3c2410_ts.c @@ -128,27 +128,29 @@ static void touch_timer_fire(unsigned long data) down = get_down(data0, data1); - if (ts.count == (1 << ts.shift)) { - ts.xp >>= ts.shift; - ts.yp >>= ts.shift; + if (down) { + if (ts.count == (1 << ts.shift)) { + ts.xp >>= ts.shift; + ts.yp >>= ts.shift; - dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", - __func__, ts.xp, ts.yp, ts.count); + dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", + __func__, ts.xp, ts.yp, ts.count); - input_report_abs(ts.input, ABS_X, ts.xp); - input_report_abs(ts.input, ABS_Y, ts.yp); + input_report_abs(ts.input, ABS_X, ts.xp); + input_report_abs(ts.input, ABS_Y, ts.yp); - input_report_key(ts.input, BTN_TOUCH, 1); - input_sync(ts.input); + input_report_key(ts.input, BTN_TOUCH, 1); + input_sync(ts.input); - ts.xp = 0; - ts.yp = 0; - ts.count = 0; - } + ts.xp = 0; + ts.yp = 0; + ts.count = 0; + } - if (down) { s3c_adc_start(ts.client, 0, 1 << ts.shift); } else { + ts.xp = 0; + ts.yp = 0; ts.count = 0; input_report_key(ts.input, BTN_TOUCH, 0); -- 1.7.0 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-19 10:02 ` Vasily Khoruzhick @ 2010-02-21 7:35 ` Dmitry Torokhov 2010-02-21 9:05 ` Vasily Khoruzhick 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Torokhov @ 2010-02-21 7:35 UTC (permalink / raw) To: Vasily Khoruzhick; +Cc: linux-arm-kernel, ben-linux, linux-input On Fri, Feb 19, 2010 at 12:02:10PM +0200, Vasily Khoruzhick wrote: > Hi Dmitry, > > В сообщении от 19 февраля 2010 11:17:55 автор Dmitry Torokhov написал: > > > I do not think this is right. Here you reset sampling data regardless > > of whether you got the right number of samples or not. You should repeat > > s3c_adc_start only if you got (1 << ts.shift) samples. But I think I see > > why touchpad may stop reporintg sometimes - if timer fires before you > > get the needed number of samples nothing will happen as timer is not > > gettign rearmed. > > Problem is driver reports "stylus down" event when stylus is already up, > anyway I agree that samples should not be discarded. > > > > } else { > > > > > > ts.count = 0; > > > > I also think this branch is missing ts.xp = ts.yp = 0; > > Ok. > > Fixed version is in attachment > > Thanks for review. Applied, thank you Vasily. One more thing though - why do we need that timer to report input events and reschedule ADC conversions? It looks like it is not really needed. Could you please give a try to the patch below? Thanks! -- Dmitry Input s3c2410_ts - get rid of timer From: Dmitry Torokhov <dmitry.torokhov@gmail.com> There seem to be no justification for using timer to report input events, this can be done right in the ADC selection callback. Also set proper parent for the input device and use common name prefix throughout the driver. Signed-off-by: Dmitry Torokhov <dtor@mail.ru> --- drivers/input/touchscreen/s3c2410_ts.c | 169 ++++++++++++++------------------ 1 files changed, 75 insertions(+), 94 deletions(-) diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c index 3755a47..60f18c2 100644 --- a/drivers/input/touchscreen/s3c2410_ts.c +++ b/drivers/input/touchscreen/s3c2410_ts.c @@ -38,7 +38,6 @@ #include <plat/adc.h> #include <plat/regs-adc.h> - #include <mach/regs-gpio.h> #include <mach/ts.h> @@ -73,7 +72,7 @@ * @count: The number of samples collected. * @shift: The log2 of the maximum count to read in one go. */ -struct s3c2410ts { +struct s3c2410_ts { struct s3c_adc_client *client; struct device *dev; struct input_dev *input; @@ -86,7 +85,7 @@ struct s3c2410ts { int shift; }; -static struct s3c2410ts ts; +static struct s3c2410_ts ts; /** * s3c2410_ts_connect - configure gpio for s3c2410 systems @@ -95,7 +94,7 @@ static struct s3c2410ts ts; * connected to the device (later systems such as the S3C2440 integrate * these into the device). */ -static inline void s3c2410_ts_connect(void) +static void s3c2410_ts_connect(void) { s3c2410_gpio_cfgpin(S3C2410_GPG(12), S3C2410_GPG12_XMON); s3c2410_gpio_cfgpin(S3C2410_GPG(13), S3C2410_GPG13_nXPON); @@ -104,88 +103,57 @@ static inline void s3c2410_ts_connect(void) } /** - * get_down - return the down state of the pen - * @data0: The data read from ADCDAT0 register. - * @data1: The data read from ADCDAT1 register. - * - * Return non-zero if both readings show that the pen is down. + * s3c2410_ts_check_pen_down - return the down state of the pen */ -static inline bool get_down(unsigned long data0, unsigned long data1) +static bool s3c24xx_ts_check_pen_down(void) { + unsigned long data0 = readl(ts.io + S3C2410_ADCDAT0); + unsigned long data1 = readl(ts.io + S3C2410_ADCDAT1); + /* returns true if both data values show stylus down */ - return (!(data0 & S3C2410_ADCDAT0_UPDOWN) && - !(data1 & S3C2410_ADCDAT0_UPDOWN)); + return !((data0 | data1) & S3C2410_ADCDAT0_UPDOWN); } -static void touch_timer_fire(unsigned long data) +static void s3c24xx_ts_report_state(bool pen_is_down) { - unsigned long data0; - unsigned long data1; - bool down; - - data0 = readl(ts.io + S3C2410_ADCDAT0); - data1 = readl(ts.io + S3C2410_ADCDAT1); - - down = get_down(data0, data1); - - if (down) { - if (ts.count == (1 << ts.shift)) { - ts.xp >>= ts.shift; - ts.yp >>= ts.shift; - - dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", - __func__, ts.xp, ts.yp, ts.count); + if (pen_is_down) { + ts.xp >>= ts.shift; + ts.yp >>= ts.shift; - input_report_abs(ts.input, ABS_X, ts.xp); - input_report_abs(ts.input, ABS_Y, ts.yp); + dev_dbg(ts.dev, "%s: X=%lu, Y=%lu, count=%d\n", + __func__, ts.xp, ts.yp, ts.count); - input_report_key(ts.input, BTN_TOUCH, 1); - input_sync(ts.input); + input_report_abs(ts.input, ABS_X, ts.xp); + input_report_abs(ts.input, ABS_Y, ts.yp); - ts.xp = 0; - ts.yp = 0; - ts.count = 0; - } - - s3c_adc_start(ts.client, 0, 1 << ts.shift); + input_report_key(ts.input, BTN_TOUCH, 1); } else { - ts.xp = 0; - ts.yp = 0; - ts.count = 0; - input_report_key(ts.input, BTN_TOUCH, 0); - input_sync(ts.input); - - writel(WAIT4INT | INT_DOWN, ts.io + S3C2410_ADCTSC); } + + input_sync(ts.input); + } -static DEFINE_TIMER(touch_timer, touch_timer_fire, 0, 0); +static void s3c24xx_ts_schedule_read(void) +{ + ts.xp = ts.yp = 0; + ts.count = 0; + + s3c_adc_start(ts.client, 0, 1 << ts.shift); +} /** - * stylus_irq - touchscreen stylus event interrupt + * s3c24xx_ts_stylus_irq - touchscreen stylus event interrupt * @irq: The interrupt number * @dev_id: The device ID. * * Called when the IRQ_TC is fired for a pen up or down event. */ -static irqreturn_t stylus_irq(int irq, void *dev_id) +static irqreturn_t s3c24xx_ts_stylus_irq(int irq, void *dev_id) { - unsigned long data0; - unsigned long data1; - bool down; - - data0 = readl(ts.io + S3C2410_ADCDAT0); - data1 = readl(ts.io + S3C2410_ADCDAT1); - - down = get_down(data0, data1); - - /* TODO we should never get an interrupt with down set while - * the timer is running, but maybe we ought to verify that the - * timer isn't running anyways. */ - - if (down) - s3c_adc_start(ts.client, 0, 1 << ts.shift); + if (s3c24xx_ts_check_pen_down()) + s3c24xx_ts_schedule_read(); else dev_info(ts.dev, "%s: count=%d\n", __func__, ts.count); @@ -231,12 +199,26 @@ static void s3c24xx_ts_conversion(struct s3c_adc_client *client, */ static void s3c24xx_ts_select(struct s3c_adc_client *client, unsigned select) { + bool pen_is_down; + if (select) { + /* + * Prepare for the next sample. + */ writel(S3C2410_ADCTSC_PULL_UP_DISABLE | AUTOPST, ts.io + S3C2410_ADCTSC); } else { - mod_timer(&touch_timer, jiffies+1); - writel(WAIT4INT | INT_UP, ts.io + S3C2410_ADCTSC); + /* + * Conversion is complete, we have desired number of samples. + */ + pen_is_down = s3c24xx_ts_check_pen_down(); + + s3c24xx_ts_report_state(pen_is_down); + + if (pen_is_down) + s3c24xx_ts_schedule_read(); + else + writel(WAIT4INT | INT_DOWN, ts.io + S3C2410_ADCTSC); } } @@ -247,7 +229,7 @@ static void s3c24xx_ts_select(struct s3c_adc_client *client, unsigned select) * Initialise, find and allocate any resources we need to run and then * register with the ADC and input systems. */ -static int __devinit s3c2410ts_probe(struct platform_device *pdev) +static int __devinit s3c24xx_ts_probe(struct platform_device *pdev) { struct s3c2410_ts_mach_info *info; struct device *dev = &pdev->dev; @@ -256,7 +238,7 @@ static int __devinit s3c2410ts_probe(struct platform_device *pdev) int ret = -EINVAL; /* Initialise input stuff */ - memset(&ts, 0, sizeof(struct s3c2410ts)); + memset(&ts, 0, sizeof(struct s3c2410_ts)); ts.dev = dev; @@ -313,8 +295,6 @@ static int __devinit s3c2410ts_probe(struct platform_device *pdev) if ((info->delay & 0xffff) > 0) writel(info->delay & 0xffff, ts.io + S3C2410_ADCDLY); - writel(WAIT4INT | INT_DOWN, ts.io + S3C2410_ADCTSC); - input_dev = input_allocate_device(); if (!input_dev) { dev_err(dev, "Unable to allocate the input device !!\n"); @@ -333,17 +313,18 @@ static int __devinit s3c2410ts_probe(struct platform_device *pdev) ts.input->id.vendor = 0xDEAD; ts.input->id.product = 0xBEEF; ts.input->id.version = 0x0102; + ts.input->dev.parent = &pdev->dev; ts.shift = info->oversampling_shift; - ret = request_irq(ts.irq_tc, stylus_irq, IRQF_DISABLED, + ret = request_irq(ts.irq_tc, s3c24xx_ts_stylus_irq, IRQF_DISABLED, "s3c2410_ts_pen", ts.input); if (ret) { dev_err(dev, "cannot get TC interrupt\n"); goto err_inputdev; } - dev_info(dev, "driver attached, registering input device\n"); + dev_dbg(dev, "driver attached, registering input device\n"); /* All went ok, so register to the input system */ ret = input_register_device(ts.input); @@ -353,16 +334,17 @@ static int __devinit s3c2410ts_probe(struct platform_device *pdev) goto err_tcirq; } + writel(WAIT4INT | INT_DOWN, ts.io + S3C2410_ADCTSC); + return 0; err_tcirq: free_irq(ts.irq_tc, ts.input); err_inputdev: - input_unregister_device(ts.input); + input_free_device(ts.input); err_iomap: iounmap(ts.io); err_clk: - del_timer_sync(&touch_timer); clk_put(ts.clock); return ret; } @@ -373,10 +355,9 @@ static int __devinit s3c2410ts_probe(struct platform_device *pdev) * * Free up our state ready to be removed. */ -static int __devexit s3c2410ts_remove(struct platform_device *pdev) +static int __devexit s3c24xx_ts_remove(struct platform_device *pdev) { free_irq(ts.irq_tc, ts.input); - del_timer_sync(&touch_timer); clk_disable(ts.clock); clk_put(ts.clock); @@ -388,7 +369,7 @@ static int __devexit s3c2410ts_remove(struct platform_device *pdev) } #ifdef CONFIG_PM -static int s3c2410ts_suspend(struct device *dev) +static int s3c24xx_ts_suspend(struct device *dev) { writel(TSC_SLEEP, ts.io + S3C2410_ADCTSC); disable_irq(ts.irq_tc); @@ -397,7 +378,7 @@ static int s3c2410ts_suspend(struct device *dev) return 0; } -static int s3c2410ts_resume(struct device *dev) +static int s3c24xx_ts_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); struct s3c2410_ts_mach_info *info = pdev->dev.platform_data; @@ -414,44 +395,44 @@ static int s3c2410ts_resume(struct device *dev) return 0; } -static struct dev_pm_ops s3c_ts_pmops = { - .suspend = s3c2410ts_suspend, - .resume = s3c2410ts_resume, +static struct dev_pm_ops s3c24xx_ts_pmops = { + .suspend = s3c24xx_ts_suspend, + .resume = s3c24xx_ts_resume, }; #endif -static struct platform_device_id s3cts_driver_ids[] = { +static struct platform_device_id s3c24xx_ts_driver_ids[] = { { "s3c2410-ts", 0 }, { "s3c2440-ts", 1 }, { } }; -MODULE_DEVICE_TABLE(platform, s3cts_driver_ids); +MODULE_DEVICE_TABLE(platform, s3c24xx_ts_driver_ids); -static struct platform_driver s3c_ts_driver = { +static struct platform_driver s3c24xx_ts_driver = { .driver = { .name = "s3c24xx-ts", .owner = THIS_MODULE, #ifdef CONFIG_PM - .pm = &s3c_ts_pmops, + .pm = &s3c24xx_ts_pmops, #endif }, - .id_table = s3cts_driver_ids, - .probe = s3c2410ts_probe, - .remove = __devexit_p(s3c2410ts_remove), + .id_table = s3c24xx_ts_driver_ids, + .probe = s3c24xx_ts_probe, + .remove = __devexit_p(s3c24xx_ts_remove), }; -static int __init s3c2410ts_init(void) +static int __init s3c24xx_ts_init(void) { - return platform_driver_register(&s3c_ts_driver); + return platform_driver_register(&s3c24xx_ts_driver); } -static void __exit s3c2410ts_exit(void) +static void __exit s3c24xx_ts_exit(void) { - platform_driver_unregister(&s3c_ts_driver); + platform_driver_unregister(&s3c24xx_ts_driver); } -module_init(s3c2410ts_init); -module_exit(s3c2410ts_exit); +module_init(s3c24xx_ts_init); +module_exit(s3c24xx_ts_exit); MODULE_AUTHOR("Arnaud Patard <arnaud.patard@rtp-net.org>, " "Ben Dooks <ben@simtec.co.uk>, " -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-21 7:35 ` Dmitry Torokhov @ 2010-02-21 9:05 ` Vasily Khoruzhick 2010-02-21 9:41 ` Dmitry Torokhov 0 siblings, 1 reply; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-21 9:05 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: ben-linux, linux-arm-kernel, linux-input [-- Attachment #1.1: Type: Text/Plain, Size: 713 bytes --] В сообщении от 21 февраля 2010 09:35:13 автор Dmitry Torokhov написал: > Applied, thank you Vasily. > > One more thing though - why do we need that timer to report input events > and reschedule ADC conversions? It looks like it is not really needed. > Could you please give a try to the patch below? > > Thanks! Timer is used to avoid reporting input events in interrupt context (select callback is called from s3c_adc_try and from s3c_adc_irq), Is it safe to call input_report_abs/input_report_key from interrupt context? Anyway, driver doesn't work for me with this patch (it works as mouse, but it doesn't as touchscreen through tslib) Regards, Vasily [-- Attachment #1.2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-21 9:05 ` Vasily Khoruzhick @ 2010-02-21 9:41 ` Dmitry Torokhov 2010-02-21 10:10 ` Vasily Khoruzhick 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Torokhov @ 2010-02-21 9:41 UTC (permalink / raw) To: Vasily Khoruzhick; +Cc: ben-linux, linux-arm-kernel, linux-input On Sun, Feb 21, 2010 at 11:05:26AM +0200, Vasily Khoruzhick wrote: > В сообщении от 21 февраля 2010 09:35:13 автор Dmitry Torokhov написал: > > > Applied, thank you Vasily. > > > > One more thing though - why do we need that timer to report input events > > and reschedule ADC conversions? It looks like it is not really needed. > > Could you please give a try to the patch below? > > > > Thanks! > > Timer is used to avoid reporting input events in interrupt context (select > callback is called from s3c_adc_try and from s3c_adc_irq), ??? This is quite a novel concept for me... > Is it safe to call > input_report_abs/input_report_key from interrupt context? Yes. > Anyway, driver > doesn't work for me with this patch (it works as mouse, but it doesn't as > touchscreen through tslib) Hmm, does evtest utility report all expected events? -- Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-21 9:41 ` Dmitry Torokhov @ 2010-02-21 10:10 ` Vasily Khoruzhick 2010-02-21 10:28 ` Dmitry Torokhov 0 siblings, 1 reply; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-21 10:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-arm-kernel, ben-linux, linux-input [-- Attachment #1: Type: Text/Plain, Size: 689 bytes --] В сообщении от 21 февраля 2010 11:41:51 автор Dmitry Torokhov написал: > > Timer is used to avoid reporting input events in interrupt context > > (select callback is called from s3c_adc_try and from s3c_adc_irq), > ??? This is quite a novel concept for me... I think so, as I don't see any other reasons to use timer :) > > Anyway, driver > > doesn't work for me with this patch (it works as mouse, but it doesn't as > > touchscreen through tslib) > > Hmm, does evtest utility report all expected events? evtest shows that driver starts event flood on first touch, i.e. it doesn't stop to emit event when stylus is up. Regards Vasily [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-21 10:10 ` Vasily Khoruzhick @ 2010-02-21 10:28 ` Dmitry Torokhov 2010-02-21 10:33 ` Vasily Khoruzhick 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Torokhov @ 2010-02-21 10:28 UTC (permalink / raw) To: Vasily Khoruzhick; +Cc: linux-arm-kernel, ben-linux, linux-input On Sun, Feb 21, 2010 at 12:10:08PM +0200, Vasily Khoruzhick wrote: > В сообщении от 21 февраля 2010 11:41:51 автор Dmitry Torokhov написал: > > > Timer is used to avoid reporting input events in interrupt context > > > (select callback is called from s3c_adc_try and from s3c_adc_irq), > > ??? This is quite a novel concept for me... > > I think so, as I don't see any other reasons to use timer :) > > > > Anyway, driver > > > doesn't work for me with this patch (it works as mouse, but it doesn't as > > > touchscreen through tslib) > > > > Hmm, does evtest utility report all expected events? > > evtest shows that driver starts event flood on first touch, i.e. it doesn't > stop to emit event when stylus is up. > While flooding does it report BTN_TOUCH as up or down? I might have messed s3c24xx_ts_check_pen_down()... -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-21 10:28 ` Dmitry Torokhov @ 2010-02-21 10:33 ` Vasily Khoruzhick 0 siblings, 0 replies; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-21 10:33 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-arm-kernel, ben-linux, linux-input [-- Attachment #1: Type: Text/Plain, Size: 1615 bytes --] В сообщении от 21 февраля 2010 12:28:07 автор Dmitry Torokhov написал: > While flooding does it report BTN_TOUCH as up or down? I might have messed > s3c24xx_ts_check_pen_down()... Nope, it reports coordinates, here's part of evtest output. Regards, Vasily Event: time 1254477272.320125, type 3 (Absolute), code 0 (X), value 515 Event: time 1254477272.320158, type 3 (Absolute), code 1 (Y), value 496 Event: time 1254477272.320179, -------------- Report Sync ------------ Event: time 1254477272.333973, type 3 (Absolute), code 1 (Y), value 497 Event: time 1254477272.334012, -------------- Report Sync ------------ Event: time 1254477272.374775, type 3 (Absolute), code 0 (X), value 516 Event: time 1254477272.374824, -------------- Report Sync ------------ Event: time 1254477272.388559, type 3 (Absolute), code 0 (X), value 515 Event: time 1254477272.388591, type 3 (Absolute), code 1 (Y), value 498 Event: time 1254477272.388612, -------------- Report Sync ------------ Event: time 1254477272.402415, type 3 (Absolute), code 1 (Y), value 497 Event: time 1254477272.402456, -------------- Report Sync ------------ Event: time 1254477272.456858, type 3 (Absolute), code 1 (Y), value 496 Event: time 1254477272.456901, -------------- Report Sync ------------ Event: time 1254477272.470573, type 3 (Absolute), code 1 (Y), value 497 Event: time 1254477272.470615, -------------- Report Sync ------------ Event: time 1254477272.484288, type 3 (Absolute), code 1 (Y), value 498 Event: time 1254477272.484330, -------------- Report Sync ------------ [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] s3c24xx_adc: disable/enable IRQ on suspend/resume 2010-02-18 16:32 [PATCH 0/4] s3c24xx_ts driver fixes Vasily Khoruzhick 2010-02-18 16:32 ` [PATCH 1/4] Add resources description for s3c24xx ts driver Vasily Khoruzhick 2010-02-18 16:32 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Vasily Khoruzhick @ 2010-02-18 16:32 ` Vasily Khoruzhick 2010-02-18 16:32 ` [PATCH 4/4] s3c24xx_ts: re-enable IRQ on resume Vasily Khoruzhick [not found] ` <201002212344.39633.anarsoul@gmail.com> 4 siblings, 0 replies; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-18 16:32 UTC (permalink / raw) To: linux-arm-kernel; +Cc: ben-linux, linux-input, Vasily Khoruzhick IRQ should be disabled on suspend and re-enabled on resume. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- arch/arm/plat-s3c24xx/adc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/arch/arm/plat-s3c24xx/adc.c b/arch/arm/plat-s3c24xx/adc.c index ce47627..284f653 100644 --- a/arch/arm/plat-s3c24xx/adc.c +++ b/arch/arm/plat-s3c24xx/adc.c @@ -388,6 +388,7 @@ static int s3c_adc_suspend(struct platform_device *pdev, pm_message_t state) con |= S3C2410_ADCCON_STDBM; writel(con, adc->regs + S3C2410_ADCCON); + disable_irq(adc->irq); clk_disable(adc->clk); return 0; @@ -398,6 +399,7 @@ static int s3c_adc_resume(struct platform_device *pdev) struct adc_device *adc = platform_get_drvdata(pdev); clk_enable(adc->clk); + enable_irq(adc->irq); writel(adc->prescale | S3C2410_ADCCON_PRSCEN, adc->regs + S3C2410_ADCCON); -- 1.7.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] s3c24xx_ts: re-enable IRQ on resume 2010-02-18 16:32 [PATCH 0/4] s3c24xx_ts driver fixes Vasily Khoruzhick ` (2 preceding siblings ...) 2010-02-18 16:32 ` [PATCH 3/4] s3c24xx_adc: disable/enable IRQ on suspend/resume Vasily Khoruzhick @ 2010-02-18 16:32 ` Vasily Khoruzhick 2010-02-19 9:05 ` Dmitry Torokhov [not found] ` <201002212344.39633.anarsoul@gmail.com> 4 siblings, 1 reply; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-18 16:32 UTC (permalink / raw) To: linux-arm-kernel; +Cc: ben-linux, linux-input, Vasily Khoruzhick IRQ should be enabled on resume, otherwise driver doesn't report events. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- drivers/input/touchscreen/s3c2410_ts.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c index e2fe2ed..6738c26 100644 --- a/drivers/input/touchscreen/s3c2410_ts.c +++ b/drivers/input/touchscreen/s3c2410_ts.c @@ -401,6 +401,7 @@ static int s3c2410ts_resume(struct device *dev) struct s3c2410_ts_mach_info *info = pdev->dev.platform_data; clk_enable(ts.clock); + enable_irq(ts.irq_tc); /* Initialise registers */ if ((info->delay & 0xffff) > 0) -- 1.7.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] s3c24xx_ts: re-enable IRQ on resume 2010-02-18 16:32 ` [PATCH 4/4] s3c24xx_ts: re-enable IRQ on resume Vasily Khoruzhick @ 2010-02-19 9:05 ` Dmitry Torokhov 0 siblings, 0 replies; 17+ messages in thread From: Dmitry Torokhov @ 2010-02-19 9:05 UTC (permalink / raw) To: Vasily Khoruzhick; +Cc: linux-arm-kernel, ben-linux, linux-input On Thu, Feb 18, 2010 at 06:32:30PM +0200, Vasily Khoruzhick wrote: > IRQ should be enabled on resume, otherwise driver doesn't > report events. > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > drivers/input/touchscreen/s3c2410_ts.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c > index e2fe2ed..6738c26 100644 > --- a/drivers/input/touchscreen/s3c2410_ts.c > +++ b/drivers/input/touchscreen/s3c2410_ts.c > @@ -401,6 +401,7 @@ static int s3c2410ts_resume(struct device *dev) > struct s3c2410_ts_mach_info *info = pdev->dev.platform_data; > > clk_enable(ts.clock); > + enable_irq(ts.irq_tc); > > /* Initialise registers */ > if ((info->delay & 0xffff) > 0) Applied to my 'next' branch, thank you Vasily. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <201002212344.39633.anarsoul@gmail.com>]
[parent not found: <20100221225125.GA18604@core.coreip.homeip.net>]
[parent not found: <201002220058.52583.anarsoul@gmail.com>]
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down [not found] ` <201002220058.52583.anarsoul@gmail.com> @ 2010-02-22 7:14 ` Dmitry Torokhov 2010-02-22 8:58 ` Vasily Khoruzhick 2010-02-22 9:03 ` Vasily Khoruzhick 0 siblings, 2 replies; 17+ messages in thread From: Dmitry Torokhov @ 2010-02-22 7:14 UTC (permalink / raw) To: Vasily Khoruzhick; +Cc: arhuaco, linux-arm-kernel, ben-linux, linux-input On Mon, Feb 22, 2010 at 12:58:48AM +0200, Vasily Khoruzhick wrote: > В сообщении от 22 февраля 2010 00:51:25 автор Dmitry Torokhov написал: > > We do put it in WAIT4INT mode rigtht there though so it looks like we > > just need to move that call (well the original doing WAIT4INT | INT_UP) > > before we check for pen state. > > And we need to disable WAIT4INT mode in s3c24xx_ts_schedule_read (actually > according to spec we need to disable it right after we got interrupt). > So is the below sufficient to make it work? Even if it works aI think we need to add socke locking around scheduling reads from irq and callback... -- Dmitry Signed-off-by: Dmitry Torokhov <dtor@mail.ru> --- drivers/input/touchscreen/s3c2410_ts.c | 22 ++++++++++++++++++---- 1 files changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c index 60f18c2..b389648 100644 --- a/drivers/input/touchscreen/s3c2410_ts.c +++ b/drivers/input/touchscreen/s3c2410_ts.c @@ -152,9 +152,12 @@ static void s3c24xx_ts_schedule_read(void) */ static irqreturn_t s3c24xx_ts_stylus_irq(int irq, void *dev_id) { - if (s3c24xx_ts_check_pen_down()) + if (s3c24xx_ts_check_pen_down()) { + /* Reset WAIT4INT state */ + writel(0, ts.io + S3C2410_ADCTSC); + s3c24xx_ts_schedule_read(); - else + } else dev_info(ts.dev, "%s: count=%d\n", __func__, ts.count); return IRQ_HANDLED; @@ -211,14 +214,25 @@ static void s3c24xx_ts_select(struct s3c_adc_client *client, unsigned select) /* * Conversion is complete, we have desired number of samples. */ + + /* + * We need to be in WAIT4INT mode to successfully check for + * pen state. + */ + writel(WAIT4INT | INT_UP, ts.io + S3C2410_ADCTSC); + pen_is_down = s3c24xx_ts_check_pen_down(); s3c24xx_ts_report_state(pen_is_down); - if (pen_is_down) + if (pen_is_down) { + /* Reset WAIT4INT state */ + writel(0, ts.io + S3C2410_ADCTSC); + s3c24xx_ts_schedule_read(); - else + } else { writel(WAIT4INT | INT_DOWN, ts.io + S3C2410_ADCTSC); + } } } -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-22 7:14 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Dmitry Torokhov @ 2010-02-22 8:58 ` Vasily Khoruzhick 2010-02-22 9:03 ` Vasily Khoruzhick 1 sibling, 0 replies; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-22 8:58 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: arhuaco, ben-linux, linux-arm-kernel, linux-input [-- Attachment #1.1: Type: Text/Plain, Size: 302 bytes --] В сообщении от 22 февраля 2010 09:14:03 автор Dmitry Torokhov написал: > So is the below sufficient to make it work? Even if it works aI think we > need to add socke locking around scheduling reads from irq and > callback... Sorry, still doesn't work. Regards Vasily [-- Attachment #1.2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down 2010-02-22 7:14 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Dmitry Torokhov 2010-02-22 8:58 ` Vasily Khoruzhick @ 2010-02-22 9:03 ` Vasily Khoruzhick 1 sibling, 0 replies; 17+ messages in thread From: Vasily Khoruzhick @ 2010-02-22 9:03 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: arhuaco, ben-linux, linux-arm-kernel, linux-input [-- Attachment #1.1: Type: Text/Plain, Size: 573 bytes --] В сообщении от 22 февраля 2010 09:14:03 автор Dmitry Torokhov написал: > + /* Reset WAIT4INT state */ > + writel(0, ts.io + S3C2410_ADCTSC); That's wrong, should be writel(S3C2410_ADCTSC_PULL_UP_DISABLE | AUTOPST, ts.io + S3C2410_ADCTSC), anyway it doesn't work even with this fix > - if (pen_is_down) > + if (pen_is_down) { > + /* Reset WAIT4INT state */ > + writel(0, ts.io + S3C2410_ADCTSC); Same note. Regards Vasily [-- Attachment #1.2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2010-02-22 9:03 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-18 16:32 [PATCH 0/4] s3c24xx_ts driver fixes Vasily Khoruzhick
2010-02-18 16:32 ` [PATCH 1/4] Add resources description for s3c24xx ts driver Vasily Khoruzhick
2010-02-18 16:32 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Vasily Khoruzhick
2010-02-19 9:17 ` Dmitry Torokhov
2010-02-19 10:02 ` Vasily Khoruzhick
2010-02-21 7:35 ` Dmitry Torokhov
2010-02-21 9:05 ` Vasily Khoruzhick
2010-02-21 9:41 ` Dmitry Torokhov
2010-02-21 10:10 ` Vasily Khoruzhick
2010-02-21 10:28 ` Dmitry Torokhov
2010-02-21 10:33 ` Vasily Khoruzhick
2010-02-18 16:32 ` [PATCH 3/4] s3c24xx_adc: disable/enable IRQ on suspend/resume Vasily Khoruzhick
2010-02-18 16:32 ` [PATCH 4/4] s3c24xx_ts: re-enable IRQ on resume Vasily Khoruzhick
2010-02-19 9:05 ` Dmitry Torokhov
[not found] ` <201002212344.39633.anarsoul@gmail.com>
[not found] ` <20100221225125.GA18604@core.coreip.homeip.net>
[not found] ` <201002220058.52583.anarsoul@gmail.com>
2010-02-22 7:14 ` [PATCH 2/4] s3c24xx_ts: report touch only when stylus is down Dmitry Torokhov
2010-02-22 8:58 ` Vasily Khoruzhick
2010-02-22 9:03 ` Vasily Khoruzhick
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).