On 2012年06月14日 14:59, Zumeng Chen wrote:

于 2012年06月14日 14:31, Hiremath, Vaibhav 写道:

quoted

On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote:

quoted

于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:

quoted

On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:

quoted

From: Zumeng Chen<redacted>

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.

This patch has been validated on 3530evm.

Signed-off-by: Zumeng Chen<redacted>
Signed-off-by: Syed Mohammed Khasim<redacted>
---
  drivers/input/touchscreen/ads7846.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index f02028e..459ff29 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c

@@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
  		}

  		ts->gpio_pendown = pdata->gpio_pendown;
+#ifdef CONFIG_ARCH_OMAP3
+		/* 310 means about 10 microsecond for omap3 */
+		gpio_set_debounce(pdata->gpio_pendown, 310);
+#endif

Zumeng,

With my sign-off you are changing the original code, that too
without my sign-off and ack.
I wouldn't mind you to submit patches from my tree, but the expectation is
if you are changing the original code, it should be under your sign-off.

Thanks, good to learn. I'll remove in next time.

quoted

Coming to the patch, #ifdef in driver is not recommended, and this 10msec
delay is specific to OMAP GPIO and driver should not be aware of this,
that's where you will find the original patch handling it in board file.

According to the git blame of the board-omap3evm.c I think
96974a24 did a good thing to all the related codes for omap3
boards. So I think we can call board and driver as BSP level :-)

If #ifdef in driver can save many codes, I guess it's deserved.

No, not really.

In the same commit, the debounce time is already handled as an argument to 
the function omap_ads7846_init(), and that’s the way it should be.

That means you'd like to implement the same get_pendown_state for every
omap3 board? Currently, board_pdata is NULL.

And actually, the reason why I agree 96974a24 is that get_pendown_state
for all omap3 boards is the common chip level gpio operations. so I think
we should set debounce for them in one common point.

But since Igor and you don't like them, I have created and tested the
attachment
patch, and I'd like Mike to check if convenience too
But obviously there are more codes than mine before :-)

Tony & Mike,

How about your comments on this issue? Do you think the following is
acceptable VS my last email _attachment_.

+#ifdef CONFIG_ARCH_OMAP3
+		/* 310 means about 10 microsecond for omap3 */
+		gpio_set_debounce(pdata->gpio_pendown, 310);
+#endif


Regards,
Zumeng

Regards,
Zumeng

quoted

So no need for #ifdefs in driver...

Thanks,
Vaibhav