Hi,
This is input subsystem, add Dmitry and linux-input.
On 06/11/12 17:00, Zumeng Chen wrote:
If we don't set proper debouce time for ads7846, then there are
flooded interrupt counters of ads7846 responding to one time
touch on screen, so the driver couldn't work well.
And since most OMAP3 series boards pass NULL pointer of board_pdata
to omap_ads7846_init, so it's more proper to set it in driver level
after having gpio_request done.
What about other non-OMAP platforms?
NULL pointer for board_pdata, only means that the default pdata is used.
Please, see the common-board-devices.c file more closely.
quoted hunk
This patch has been validated on 3530evm.
Signed-off-by: Zumeng Chen <redacted>
Signed-off-by: Syed Mohammed Khasim <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/input/touchscreen/ads7846.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index f02028e..a82a5fb 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -61,6 +61,7 @@
/* this driver doesn't aim at the peak continuous sample rate */
#define SAMPLE_BITS (8 /*cmd*/ + 16 /*sample*/ + 2 /* before, after */)
+#define DEBOUNCE_TIME 310 /* About 10 ms */
I think hard coding this value is wrong.
Can't it be derived from the pdata->debounce_* fields?
quoted hunk
struct ts_event {
/*@@ -980,6 +981,7 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
}
ts->gpio_pendown = pdata->gpio_pendown;
+ gpio_set_debounce(pdata->gpio_pendown, DEBOUNCE_TIME);
} else {
dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
--
Regards,
Igor.
2012/6/11 Igor Grinberg [off-list ref]
Hi,
This is input subsystem, add Dmitry and linux-input.
On 06/11/12 17:00, Zumeng Chen wrote:
quoted
If we don't set proper debouce time for ads7846, then there are
flooded interrupt counters of ads7846 responding to one time
touch on screen, so the driver couldn't work well.
And since most OMAP3 series boards pass NULL pointer of board_pdata
to omap_ads7846_init, so it's more proper to set it in driver level
after having gpio_request done.
What about other non-OMAP platforms?
Good point, I thought it should be the same situation, and I have no other
boards
to validate it :) Then I'll fall back on my original patch to bracket them
with OMAP3EVM
NULL pointer for board_pdata, only means that the default pdata is used.
Please, see the common-board-devices.c file more closely.
Yes, I just went through again, two points:
1 ) get_pendown_state is not set for OMAP3 boards, and the state
will be get by gpio_get_value(gpio-omap.c)
The second path will be available in the ads7846_setup_pendown
if (pdata->get_pendown_state) {
ts->get_pendown_state = pdata->get_pendown_state;
} else if (gpio_is_valid(pdata->gpio_pendown)) {
2 ) All omap3 boards set gpio_pendown for pdata.
So it had better we set_debounce in driver level after
gpio_request_one
having done
I'll remove DEBOUNCE_TIME in V2 and change into like following:
+#ifdef CONFIG_MACH_OMAP3EVM
+ /* 310 means 10 microsecond for omap3 */
+ gpio_set_debounce(pdata->gpio_pendown, 310);
+#endif
quoted
This patch has been validated on 3530evm.
Signed-off-by: Zumeng Chen <redacted>
Signed-off-by: Syed Mohammed Khasim <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/input/touchscreen/ads7846.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c
b/drivers/input/touchscreen/ads7846.c
quoted
index f02028e..a82a5fb 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -61,6 +61,7 @@
/* this driver doesn't aim at the peak continuous sample rate */
#define SAMPLE_BITS (8 /*cmd*/ + 16 /*sample*/ + 2 /* before,
after */)
quoted
+#define DEBOUNCE_TIME 310 /* About 10 ms */
I think hard coding this value is wrong.
Yes, me too.
Can't it be derived from the pdata->debounce_* fields?
Yes, I agreed this way, and to be honest, my first choice
is to find if there is a member for debounce, but no. And
there is no more sense to derive it from debounce_max,
although which is the right value for us.
So I have to use the hardcode here.
Regards,
Zumeng
quoted
struct ts_event {
/*@@ -980,6 +981,7 @@ static int __devinit ads7846_setup_pendown(struct
spi_device *spi, struct ads784
quoted
}
ts->gpio_pendown = pdata->gpio_pendown;
+ gpio_set_debounce(pdata->gpio_pendown, DEBOUNCE_TIME);
} else {
dev_err(&spi->dev, "no get_pendown_state nor
gpio_pendown?\n");
--
Regards,
Igor.
On 06/12/12 05:49, Zumeng Chen wrote:
2012/6/11 Igor Grinberg <grinberg@compulab.co.il <mailto:grinberg@compulab.co.il>>
Hi,
This is input subsystem, add Dmitry and linux-input.
On 06/11/12 17:00, Zumeng Chen wrote:
> If we don't set proper debouce time for ads7846, then there are
> flooded interrupt counters of ads7846 responding to one time
> touch on screen, so the driver couldn't work well.
>
> And since most OMAP3 series boards pass NULL pointer of board_pdata
> to omap_ads7846_init, so it's more proper to set it in driver level
> after having gpio_request done.
What about other non-OMAP platforms?
Good point, I thought it should be the same situation, and I have no other boards
to validate it :) Then I'll fall back on my original patch to bracket them with OMAP3EVM
This isn't a good solution either...
NULL pointer for board_pdata, only means that the default pdata is used.
Please, see the common-board-devices.c file more closely.
Yes, I just went through again, two points:
1 ) get_pendown_state is not set for OMAP3 boards, and the state
You can supply your own pdata and provide the get_pendown_state() callback.
will be get by gpio_get_value(gpio-omap.c)
The second path will be available in the ads7846_setup_pendown
if (pdata->get_pendown_state) {
ts->get_pendown_state = pdata->get_pendown_state;
} else if (gpio_is_valid(pdata->gpio_pendown)) {
2 ) All omap3 boards set gpio_pendown for pdata.
So it had better we set_debounce in driver level after gpio_request_one
having done
I'll remove DEBOUNCE_TIME in V2 and change into like following:
+#ifdef CONFIG_MACH_OMAP3EVM
+ /* 310 means 10 microsecond for omap3 */
+ gpio_set_debounce(pdata->gpio_pendown, 310);
+#endif
I don't think this kind of fix is acceptable...
>
> This patch has been validated on 3530evm.
>
> Signed-off-by: Zumeng Chen <zumeng.chen@gmail.com <mailto:zumeng.chen@gmail.com>>
> Signed-off-by: Syed Mohammed Khasim <khasim@ti.com <mailto:khasim@ti.com>>
> Signed-off-by: Tony Lindgren <tony@atomide.com <mailto:tony@atomide.com>>
> ---
> drivers/input/touchscreen/ads7846.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index f02028e..a82a5fb 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -61,6 +61,7 @@
>
> /* this driver doesn't aim at the peak continuous sample rate */
> #define SAMPLE_BITS (8 /*cmd*/ + 16 /*sample*/ + 2 /* before, after */)
> +#define DEBOUNCE_TIME 310 /* About 10 ms */
I think hard coding this value is wrong.
Yes, me too.
Can't it be derived from the pdata->debounce_* fields?
Yes, I agreed this way, and to be honest, my first choice
is to find if there is a member for debounce, but no. And
there is no more sense to derive it from debounce_max,
although which is the right value for us.
It makes sense to me... Why do you think there is no sense?
--
Regards,
Igor.