Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-19 Thread Zumeng Chen
于 2012年06月14日 14:59, Zumeng Chen 写道:
 于 2012年06月14日 14:31, Hiremath, Vaibhav 写道:
 On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote:
 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:
 On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
 From: Zumeng Chenzumeng.c...@windriver.com

 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 Chenzumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasimkha...@ti.com
 ---
   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.
 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 :-)
Hi Mike,

I'll send V3 with the change on the attachment to you for 5/5,
if you happen to get it, it's high appreciated to review it.

Regards,
Zumeng

 Regards,
 Zumeng
 So no need for #ifdefs in driver...

 Thanks,
 Vaibhav

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-16 Thread Marek Vasut
Dear Igor Grinberg,

 On 06/13/12 12:03, Zumeng Chen wrote:
  于 2012年06月13日 15:51, Igor Grinberg 写道:
  On 06/13/12 04:44, Zumeng Chen wrote:
  From: Zumeng Chenzumeng.c...@windriver.com
  
  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 Chenzumeng.c...@windriver.com
  Signed-off-by: Syed Mohammed Khasimkha...@ti.com
  ---
  
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
  
  Unless this concerns many boards/archs/platforms,
  
  Yes, this is the case.
  
  I'd suggest you to implement
  the get_pendown_state() method in the board file.
  
  it seems they are different way between gpio and
  get_pendown_state, and gpio way is used for omap3
  to drive ads7846, so I guess we may have to do like this.
 
 No, this is not (and does not have to be) OMAP wide.
 The decision for this is made on per-board basis.

+1 agreed

  Regards,
  Zumeng
  
  If more users will need this, it can be facilitated in the driver.
  (and of course not with the ugly ifdefs...)
  
} else {

dev_err(spi-dev, no get_pendown_state nor
gpio_pendown?\n);

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-15 Thread zumeng.chen
On 2012年06月14日 14:59, Zumeng Chen wrote:
 于 2012年06月14日 14:31, Hiremath, Vaibhav 写道:
 On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote:
 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:
 On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
 From: Zumeng Chenzumeng.c...@windriver.com

 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 Chenzumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasimkha...@ti.com
 ---
   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.
 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
 So no need for #ifdefs in driver...

 Thanks,
 Vaibhav

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-14 Thread Hiremath, Vaibhav
On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote:
 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:
  On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
  From: Zumeng Chenzumeng.c...@windriver.com
 
  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 Chenzumeng.c...@windriver.com
  Signed-off-by: Syed Mohammed Khasimkha...@ti.com
  ---
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.
  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.
So no need for #ifdefs in driver...

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-14 Thread Igor Grinberg
On 06/14/12 07:46, Zumeng Chen wrote:
 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:
 On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
 From: Zumeng Chenzumeng.c...@windriver.com

 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 Chenzumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasimkha...@ti.com
 ---
   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.
 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, platform/board specific ifdefs in the generic driver code are never
deserved.
How about the attached patch, does it fix the problem for you?


-- 
Regards,
Igor.
diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
index 1706ebc..c187586 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
 	struct spi_board_info *spi_bi = ads7846_spi_board_info;
 	int err;
 
-	if (board_pdata  board_pdata-get_pendown_state) {
-		err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
-		if (err) {
-			pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
-			return;
-		}
-		gpio_export(gpio_pendown, 0);
-
-		if (gpio_debounce)
-			gpio_set_debounce(gpio_pendown, gpio_debounce);
+	err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
+	if (err) {
+		pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
+		return;
 	}
 
+	if (gpio_debounce)
+		gpio_set_debounce(gpio_pendown, gpio_debounce);
+
 	spi_bi-bus_num	= bus_num;
 	spi_bi-irq	= gpio_to_irq(gpio_pendown);
 
 	if (board_pdata) {
 		board_pdata-gpio_pendown = gpio_pendown;
 		spi_bi-platform_data = board_pdata;
+		if (board_pdata-get_pendown_state)
+			gpio_export(gpio_pendown, 0);
 	} else {
 		ads7846_config.gpio_pendown = gpio_pendown;
 	}
 
+	if (!board_pdata || (board_pdata  !board_pdata-get_pendown_state))
+		gpio_free(gpio_pendown);
+
 	spi_register_board_info(ads7846_spi_board_info, 1);
 }
 #else


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-14 Thread Zumeng Chen
于 2012年06月14日 14:31, Hiremath, Vaibhav 写道:
 On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote:
 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:
 On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
 From: Zumeng Chenzumeng.c...@windriver.com

 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 Chenzumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasimkha...@ti.com
 ---
   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.
 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 :-)

Regards,
Zumeng
 So no need for #ifdefs in driver...

 Thanks,
 Vaibhav

diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index 56cce1e..887bd35 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -58,7 +58,6 @@
 #include hsmmc.h
 #include common-board-devices.h
 
-#define OMAP3_EVM_TS_GPIO	175
 #define OMAP3_EVM_EHCI_VBUS	22
 #define OMAP3_EVM_EHCI_SELECT	61
 
diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
index 719f62e..8cf0140 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -34,6 +34,11 @@
 static struct omap2_mcspi_device_config ads7846_mcspi_config = {
 	.turbo_mode	= 0,
 };
+	
+static int omap3_get_pendown_state(void)
+{
+	return !gpio_get_value(OMAP3_EVM_TS_GPIO);
+}
 
 static struct ads7846_platform_data ads7846_config = {
 	.x_max			= 0x0fff,
@@ -45,6 +50,7 @@ static struct ads7846_platform_data ads7846_config = {
 	.debounce_rep		= 1,
 	.gpio_pendown		= -EINVAL,
 	.keep_vref_on		= 1,
+	.get_pendown_state	= omap3_get_pendown_state,
 };
 
 static struct spi_board_info ads7846_spi_board_info __initdata = {
@@ -63,17 +69,14 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
 	struct spi_board_info *spi_bi = ads7846_spi_board_info;
 	int err;
 
-	if (board_pdata  board_pdata-get_pendown_state) {
-		err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
-		if (err) {
-			pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
-			return;
-		}
-		gpio_export(gpio_pendown, 0);
-
-		if (gpio_debounce)
-			gpio_set_debounce(gpio_pendown, gpio_debounce);
+	err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
+	if (err) {
+		pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
+		return;
 	}
+	gpio_export(gpio_pendown, 0);
+	if (gpio_debounce)
+		gpio_set_debounce(gpio_pendown, gpio_debounce);
 
 	spi_bi-bus_num	= bus_num;
 	spi_bi-irq	= gpio_to_irq(gpio_pendown);
diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
index a0b4a428..4c4ef6a 100644
--- 

Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-14 Thread Igor Grinberg
On 06/14/12 10:08, Zumeng Chen wrote:
 于 2012年06月14日 14:44, Igor Grinberg 写道:
 On 06/14/12 07:46, Zumeng Chen wrote:
  于 2012年06月13日 20:18, Hiremath, Vaibhav 写道:
  On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
  From: Zumeng Chenzumeng.c...@windriver.com
 
  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 Chenzumeng.c...@windriver.com
  Signed-off-by: Syed Mohammed Khasimkha...@ti.com
  ---
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.
  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, platform/board specific ifdefs in the generic driver code are never
 deserved.
 How about the attached patch, does it fix the problem for you?
 Sorry Igor, I just read your patch, yes, most are there, and we
 just have to set get_pendown_state as you early said. Really
 *thanks* for your drive :)

No, with my patch, you don't need the get_pendown_state() and
you don't need to change the board-omap3evm.c ADS7846 related code.
So, I'm going to submit my patch properly.

 
 Regards,
 Zumeng
 -- Regards, Igor.
 ads7846.patch


 diff --git a/arch/arm/mach-omap2/common-board-devices.c 
 b/arch/arm/mach-omap2/common-board-devices.c
 index 1706ebc..c187586 100644
 --- a/arch/arm/mach-omap2/common-board-devices.c
 +++ b/arch/arm/mach-omap2/common-board-devices.c
 @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int 
 gpio_pendown, int gpio_debounce,
  struct spi_board_info *spi_bi = ads7846_spi_board_info;
  int err;
  
 -if (board_pdata  board_pdata-get_pendown_state) {
 -err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
 -if (err) {
 -pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
 -return;
 -}
 -gpio_export(gpio_pendown, 0);
 -
 -if (gpio_debounce)
 -gpio_set_debounce(gpio_pendown, gpio_debounce);
 +err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
 +if (err) {
 +pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
 +return;
  }
  
 +if (gpio_debounce)
 +gpio_set_debounce(gpio_pendown, gpio_debounce);
 +
  spi_bi-bus_num = bus_num;
  spi_bi-irq = gpio_to_irq(gpio_pendown);
  
  if (board_pdata) {
  board_pdata-gpio_pendown = gpio_pendown;
  spi_bi-platform_data = board_pdata;
 +if (board_pdata-get_pendown_state)
 +gpio_export(gpio_pendown, 0);
  } else {
  ads7846_config.gpio_pendown = gpio_pendown;
  }
  
 +if (!board_pdata || (board_pdata  !board_pdata-get_pendown_state))
 +gpio_free(gpio_pendown);
 +
  spi_register_board_info(ads7846_spi_board_info, 1);
  }
  #else
 

-- 
Regards,
Igor.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-14 Thread Zumeng Chen

于 2012年06月14日 16:06, Igor Grinberg 写道:

On 06/14/12 10:08, Zumeng Chen wrote:

于 2012年06月14日 14:44, Igor Grinberg 写道:

On 06/14/12 07:46, Zumeng Chen wrote:

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

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

From: Zumeng Chenzumeng.c...@windriver.com

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 Chenzumeng.c...@windriver.com
Signed-off-by: Syed Mohammed Khasimkha...@ti.com
---
   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.

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, platform/board specific ifdefs in the generic driver code are never
deserved.
How about the attached patch, does it fix the problem for you?

Sorry Igor, I just read your patch, yes, most are there, and we
just have to set get_pendown_state as you early said. Really
*thanks* for your drive :)

No, with my patch, you don't need the get_pendown_state() and
you don't need to change the board-omap3evm.c ADS7846 related code.
So, I'm going to submit my patch properly.

Feel free to do that


Regards,
Zumeng

-- Regards, Igor.
ads7846.patch


diff --git a/arch/arm/mach-omap2/common-board-devices.c 
b/arch/arm/mach-omap2/common-board-devices.c
index 1706ebc..c187586 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int 
gpio_pendown, int gpio_debounce,
struct spi_board_info *spi_bi =ads7846_spi_board_info;
int err;

-   if (board_pdata  board_pdata-get_pendown_state) {
-   err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
-   if (err) {
-   pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
-   return;
-   }
-   gpio_export(gpio_pendown, 0);
-
-   if (gpio_debounce)
-   gpio_set_debounce(gpio_pendown, gpio_debounce);
+   err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
+   if (err) {
+   pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
+   return;
}

+   if (gpio_debounce)
+   gpio_set_debounce(gpio_pendown, gpio_debounce);
+
spi_bi-bus_num  = bus_num;
spi_bi-irq  = gpio_to_irq(gpio_pendown);

if (board_pdata) {
board_pdata-gpio_pendown = gpio_pendown;
spi_bi-platform_data = board_pdata;
+   if (board_pdata-get_pendown_state)
+   gpio_export(gpio_pendown, 0);
} else {
ads7846_config.gpio_pendown = gpio_pendown;
}

+   if (!board_pdata || (board_pdata  !board_pdata-get_pendown_state))
+   gpio_free(gpio_pendown);
+
spi_register_board_info(ads7846_spi_board_info, 1);
  }
  #else


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-13 Thread Igor Grinberg
On 06/13/12 04:44, Zumeng Chen wrote:
 From: Zumeng Chen zumeng.c...@windriver.com
 
 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 zumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasim kha...@ti.com
 ---
  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

Unless this concerns many boards/archs/platforms, I'd suggest you to implement
the get_pendown_state() method in the board file.
If more users will need this, it can be facilitated in the driver.
(and of course not with the ugly ifdefs...)

  
   } else {
   dev_err(spi-dev, no get_pendown_state nor gpio_pendown?\n);

-- 
Regards,
Igor.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-13 Thread Zumeng Chen

于 2012年06月13日 15:51, Igor Grinberg 写道:

On 06/13/12 04:44, Zumeng Chen wrote:

From: Zumeng Chenzumeng.c...@windriver.com

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 Chenzumeng.c...@windriver.com
Signed-off-by: Syed Mohammed Khasimkha...@ti.com
---
  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

Unless this concerns many boards/archs/platforms,

Yes, this is the case.

I'd suggest you to implement
the get_pendown_state() method in the board file.

it seems they are different way between gpio and
get_pendown_state, and gpio way is used for omap3
to drive ads7846, so I guess we may have to do like this.

Regards,
Zumeng

If more users will need this, it can be facilitated in the driver.
(and of course not with the ugly ifdefs...)



} else {
dev_err(spi-dev, no get_pendown_state nor gpio_pendown?\n);


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-13 Thread Igor Grinberg
On 06/13/12 12:03, Zumeng Chen wrote:
 于 2012年06月13日 15:51, Igor Grinberg 写道:
 On 06/13/12 04:44, Zumeng Chen wrote:
 From: Zumeng Chenzumeng.c...@windriver.com

 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 Chenzumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasimkha...@ti.com
 ---
   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
 Unless this concerns many boards/archs/platforms,
 Yes, this is the case.
 I'd suggest you to implement
 the get_pendown_state() method in the board file.
 it seems they are different way between gpio and
 get_pendown_state, and gpio way is used for omap3
 to drive ads7846, so I guess we may have to do like this.

No, this is not (and does not have to be) OMAP wide.
The decision for this is made on per-board basis.

 
 Regards,
 Zumeng
 If more users will need this, it can be facilitated in the driver.
 (and of course not with the ugly ifdefs...)


   } else {
   dev_err(spi-dev, no get_pendown_state nor gpio_pendown?\n);
 
 -- 
 To unsubscribe from this list: send the line unsubscribe linux-input in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 
Regards,
Igor.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-13 Thread Hiremath, Vaibhav
On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote:
 From: Zumeng Chen zumeng.c...@windriver.com
 
 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 zumeng.c...@windriver.com
 Signed-off-by: Syed Mohammed Khasim kha...@ti.com
 ---
  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.

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.

Thanks,
Vaibhav
   } else {
   dev_err(spi-dev, no get_pendown_state nor gpio_pendown?\n);
 -- 
 1.7.5.4
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-13 Thread Zumeng Chen

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

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

From: Zumeng Chenzumeng.c...@windriver.com

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 Chenzumeng.c...@windriver.com
Signed-off-by: Syed Mohammed Khasimkha...@ti.com
---
  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.

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.

Regards,
Zumeng

Thanks,
Vaibhav

} else {
dev_err(spi-dev, no get_pendown_state nor gpio_pendown?\n);
--
1.7.5.4




--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level

2012-06-12 Thread Zumeng Chen
From: Zumeng Chen zumeng.c...@windriver.com

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 zumeng.c...@windriver.com
Signed-off-by: Syed Mohammed Khasim kha...@ti.com
---
 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
 
} else {
dev_err(spi-dev, no get_pendown_state nor gpio_pendown?\n);
-- 
1.7.5.4

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html