* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-26 19:30 ` Kevin Hilman
0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-26 19:30 UTC (permalink / raw)
To: linux-arm-kernel
+ Zumeng Chen
Igor Grinberg <grinberg@compulab.co.il> writes:
> Hi Kevin,
>
> I've just noticed that the patch has been modified by Arnd in a way
> that of course will trigger GPIO use without being requested.
> I'm sorry, I was not available by that time Arnd changed the patch.
Your right, your original patch isn't the problem. I found the root
cause.
The real problem is actually introduced by the merge of your patch from
the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
ads7846) from the arm-soc/boards branch.
However, looking closer at the one from Zumeng, that one is clearly not
right. It unconditionally adds a *board-specific* ->get_pendown_state
function to the pdata that is common to *all* boards. That's just wrong
and has the side-effect of making ->get_pendown_state() wrong on every
board except the OMAP3EVM. Oops.
So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
GPIO working on non OMAP3EVM boards, we also need something like this as
well.
Igor, Zumeng, could you try this out on your boards anc confirm if it's
working? I currently don't have a board setup with a touchscreen in my
board farm.
Kevin
>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Thu, 26 Jul 2012 12:15:38 -0700
Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
on all boards
commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
time for ads7846) introduced a common ->get_pendown_state() function
into the generic code, but that function was board-specific for the
OMAP3EVM.
Fix this up to work for all boards that pass in a pendown GPIO.
Cc: Zumeng Chen <zumeng.chen@windriver.com>
Cc: Igor Grinberg <grinberg@compulab.co.il>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
arch/arm/mach-omap2/board-omap3evm.c | 1 +
arch/arm/mach-omap2/common-board-devices.c | 4 +++-
arch/arm/mach-omap2/common-board-devices.h | 1 -
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index ef230a0..0d362e9 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -58,6 +58,7 @@
#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 0fee95f..06f176f 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
* of pdata->get_pendown_state, but we have done this. So set
* get_pendown_state to avoid twice gpio requesting.
*/
+static int omap3_gpio_pendown;
static int omap3_get_pendown_state(void)
{
- return !gpio_get_value(OMAP3_EVM_TS_GPIO);
+ return !gpio_get_value(omap3_gpio_pendown);
}
static struct ads7846_platform_data ads7846_config = {
@@ -74,6 +75,7 @@ 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;
+ omap3_gpio_pendown = gpio_pendown;
err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
if (err) {
pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
index 4c4ef6a..a0b4a428 100644
--- a/arch/arm/mach-omap2/common-board-devices.h
+++ b/arch/arm/mach-omap2/common-board-devices.h
@@ -4,7 +4,6 @@
#include "twl-common.h"
#define NAND_BLOCK_SIZE SZ_128K
-#define OMAP3_EVM_TS_GPIO 175
struct mtd_partition;
struct ads7846_platform_data;
--
1.7.9.2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-26 19:30 ` Kevin Hilman
@ 2012-07-26 21:19 ` zumeng.chen
-1 siblings, 0 replies; 42+ messages in thread
From: zumeng.chen @ 2012-07-26 21:19 UTC (permalink / raw)
To: Kevin Hilman
Cc: Igor Grinberg, Tony Lindgren, linux-omap, linux-arm-kernel,
Arnd Bergmann, Zumeng Chen
On 2012年07月27日 03:30, Kevin Hilman wrote:
> + Zumeng Chen
>
> Igor Grinberg<grinberg@compulab.co.il> writes:
>
>> Hi Kevin,
>>
>> I've just noticed that the patch has been modified by Arnd in a way
>> that of course will trigger GPIO use without being requested.
>> I'm sorry, I was not available by that time Arnd changed the patch.
> Your right, your original patch isn't the problem. I found the root
> cause.
>
> The real problem is actually introduced by the merge of your patch from
> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
> ads7846) from the arm-soc/boards branch.
>
> However, looking closer at the one from Zumeng, that one is clearly not
> right. It unconditionally adds a *board-specific* ->get_pendown_state
> function to the pdata that is common to *all* boards. That's just wrong
> and has the side-effect of making ->get_pendown_state() wrong on every
> board except the OMAP3EVM. Oops.
>
> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
> GPIO working on non OMAP3EVM boards, we also need something like this as
> well.
Definitely, thanks Kevin.
> Igor, Zumeng, could you try this out on your boards anc confirm if it's
> working? I currently don't have a board setup with a touchscreen in my
> board farm.
Acked
Zumeng
> Kevin
>
> From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman<khilman@ti.com>
> Date: Thu, 26 Jul 2012 12:15:38 -0700
> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
> on all boards
>
> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
> time for ads7846) introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM.
>
> Fix this up to work for all boards that pass in a pendown GPIO.
>
> Cc: Zumeng Chen<zumeng.chen@windriver.com>
> Cc: Igor Grinberg<grinberg@compulab.co.il>
> Signed-off-by: Kevin Hilman<khilman@ti.com>
> ---
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 4 +++-
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 0fee95f..06f176f 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> * of pdata->get_pendown_state, but we have done this. So set
> * get_pendown_state to avoid twice gpio requesting.
> */
> +static int omap3_gpio_pendown;
> static int omap3_get_pendown_state(void)
> {
> - return !gpio_get_value(OMAP3_EVM_TS_GPIO);
> + return !gpio_get_value(omap3_gpio_pendown);
> }
>
> static struct ads7846_platform_data ads7846_config = {
> @@ -74,6 +75,7 @@ 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;
>
> + omap3_gpio_pendown = gpio_pendown;
> err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
> if (err) {
> pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-26 21:19 ` zumeng.chen
0 siblings, 0 replies; 42+ messages in thread
From: zumeng.chen @ 2012-07-26 21:19 UTC (permalink / raw)
To: linux-arm-kernel
On 2012?07?27? 03:30, Kevin Hilman wrote:
> + Zumeng Chen
>
> Igor Grinberg<grinberg@compulab.co.il> writes:
>
>> Hi Kevin,
>>
>> I've just noticed that the patch has been modified by Arnd in a way
>> that of course will trigger GPIO use without being requested.
>> I'm sorry, I was not available by that time Arnd changed the patch.
> Your right, your original patch isn't the problem. I found the root
> cause.
>
> The real problem is actually introduced by the merge of your patch from
> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
> ads7846) from the arm-soc/boards branch.
>
> However, looking closer at the one from Zumeng, that one is clearly not
> right. It unconditionally adds a *board-specific* ->get_pendown_state
> function to the pdata that is common to *all* boards. That's just wrong
> and has the side-effect of making ->get_pendown_state() wrong on every
> board except the OMAP3EVM. Oops.
>
> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
> GPIO working on non OMAP3EVM boards, we also need something like this as
> well.
Definitely, thanks Kevin.
> Igor, Zumeng, could you try this out on your boards anc confirm if it's
> working? I currently don't have a board setup with a touchscreen in my
> board farm.
Acked
Zumeng
> Kevin
>
> From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman<khilman@ti.com>
> Date: Thu, 26 Jul 2012 12:15:38 -0700
> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
> on all boards
>
> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
> time for ads7846) introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM.
>
> Fix this up to work for all boards that pass in a pendown GPIO.
>
> Cc: Zumeng Chen<zumeng.chen@windriver.com>
> Cc: Igor Grinberg<grinberg@compulab.co.il>
> Signed-off-by: Kevin Hilman<khilman@ti.com>
> ---
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 4 +++-
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 0fee95f..06f176f 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> * of pdata->get_pendown_state, but we have done this. So set
> * get_pendown_state to avoid twice gpio requesting.
> */
> +static int omap3_gpio_pendown;
> static int omap3_get_pendown_state(void)
> {
> - return !gpio_get_value(OMAP3_EVM_TS_GPIO);
> + return !gpio_get_value(omap3_gpio_pendown);
> }
>
> static struct ads7846_platform_data ads7846_config = {
> @@ -74,6 +75,7 @@ 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;
>
> + omap3_gpio_pendown = gpio_pendown;
> err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
> if (err) {
> pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-26 21:19 ` zumeng.chen
@ 2012-07-26 21:58 ` Kevin Hilman
-1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-26 21:58 UTC (permalink / raw)
To: zumeng.chen
Cc: Igor Grinberg, Tony Lindgren, linux-omap, linux-arm-kernel,
Arnd Bergmann, Zumeng Chen
"zumeng.chen" <zchen@windriver.com> writes:
> On 2012年07月27日 03:30, Kevin Hilman wrote:
>> + Zumeng Chen
>>
>> Igor Grinberg<grinberg@compulab.co.il> writes:
>>
>>> Hi Kevin,
>>>
>>> I've just noticed that the patch has been modified by Arnd in a way
>>> that of course will trigger GPIO use without being requested.
>>> I'm sorry, I was not available by that time Arnd changed the patch.
>> Your right, your original patch isn't the problem. I found the root
>> cause.
>>
>> The real problem is actually introduced by the merge of your patch from
>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>> ads7846) from the arm-soc/boards branch.
>>
>> However, looking closer at the one from Zumeng, that one is clearly not
>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>> function to the pdata that is common to *all* boards. That's just wrong
>> and has the side-effect of making ->get_pendown_state() wrong on every
>> board except the OMAP3EVM. Oops.
>>
>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>> GPIO working on non OMAP3EVM boards, we also need something like this as
>> well.
> Definitely, thanks Kevin.
>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>> working? I currently don't have a board setup with a touchscreen in my
>> board farm.
> Acked
Did you test this on your board? If so, could you respond with a
Tested-by tag? Thanks.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-26 21:58 ` Kevin Hilman
0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-26 21:58 UTC (permalink / raw)
To: linux-arm-kernel
"zumeng.chen" <zchen@windriver.com> writes:
> On 2012?07?27? 03:30, Kevin Hilman wrote:
>> + Zumeng Chen
>>
>> Igor Grinberg<grinberg@compulab.co.il> writes:
>>
>>> Hi Kevin,
>>>
>>> I've just noticed that the patch has been modified by Arnd in a way
>>> that of course will trigger GPIO use without being requested.
>>> I'm sorry, I was not available by that time Arnd changed the patch.
>> Your right, your original patch isn't the problem. I found the root
>> cause.
>>
>> The real problem is actually introduced by the merge of your patch from
>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>> ads7846) from the arm-soc/boards branch.
>>
>> However, looking closer at the one from Zumeng, that one is clearly not
>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>> function to the pdata that is common to *all* boards. That's just wrong
>> and has the side-effect of making ->get_pendown_state() wrong on every
>> board except the OMAP3EVM. Oops.
>>
>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>> GPIO working on non OMAP3EVM boards, we also need something like this as
>> well.
> Definitely, thanks Kevin.
>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>> working? I currently don't have a board setup with a touchscreen in my
>> board farm.
> Acked
Did you test this on your board? If so, could you respond with a
Tested-by tag? Thanks.
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-26 21:58 ` Kevin Hilman
@ 2012-07-26 22:40 ` zumeng.chen
-1 siblings, 0 replies; 42+ messages in thread
From: zumeng.chen @ 2012-07-26 22:40 UTC (permalink / raw)
To: Kevin Hilman
Cc: Igor Grinberg, Tony Lindgren, linux-omap, linux-arm-kernel,
Arnd Bergmann, Zumeng Chen
On 2012年07月27日 05:58, Kevin Hilman wrote:
> "zumeng.chen"<zchen@windriver.com> writes:
>
>> On 2012年07月27日 03:30, Kevin Hilman wrote:
>>> + Zumeng Chen
>>>
>>> Igor Grinberg<grinberg@compulab.co.il> writes:
>>>
>>>> Hi Kevin,
>>>>
>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>> that of course will trigger GPIO use without being requested.
>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>> Your right, your original patch isn't the problem. I found the root
>>> cause.
>>>
>>> The real problem is actually introduced by the merge of your patch from
>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>> ads7846) from the arm-soc/boards branch.
>>>
>>> However, looking closer at the one from Zumeng, that one is clearly not
>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>> function to the pdata that is common to *all* boards. That's just wrong
>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>> board except the OMAP3EVM. Oops.
>>>
>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>> well.
>> Definitely, thanks Kevin.
>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>> working? I currently don't have a board setup with a touchscreen in my
>>> board farm.
>> Acked
> Did you test this on your board?
No, I just read/compiled it.
> If so, could you respond with a
> Tested-by tag? Thanks
NP, Kevin, I'll test it after being office, and
it's about ten o'clock this morning.
Regards,
Zumeng
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-26 22:40 ` zumeng.chen
0 siblings, 0 replies; 42+ messages in thread
From: zumeng.chen @ 2012-07-26 22:40 UTC (permalink / raw)
To: linux-arm-kernel
On 2012?07?27? 05:58, Kevin Hilman wrote:
> "zumeng.chen"<zchen@windriver.com> writes:
>
>> On 2012?07?27? 03:30, Kevin Hilman wrote:
>>> + Zumeng Chen
>>>
>>> Igor Grinberg<grinberg@compulab.co.il> writes:
>>>
>>>> Hi Kevin,
>>>>
>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>> that of course will trigger GPIO use without being requested.
>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>> Your right, your original patch isn't the problem. I found the root
>>> cause.
>>>
>>> The real problem is actually introduced by the merge of your patch from
>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>> ads7846) from the arm-soc/boards branch.
>>>
>>> However, looking closer at the one from Zumeng, that one is clearly not
>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>> function to the pdata that is common to *all* boards. That's just wrong
>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>> board except the OMAP3EVM. Oops.
>>>
>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>> well.
>> Definitely, thanks Kevin.
>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>> working? I currently don't have a board setup with a touchscreen in my
>>> board farm.
>> Acked
> Did you test this on your board?
No, I just read/compiled it.
> If so, could you respond with a
> Tested-by tag? Thanks
NP, Kevin, I'll test it after being office, and
it's about ten o'clock this morning.
Regards,
Zumeng
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-26 21:58 ` Kevin Hilman
@ 2012-07-27 5:02 ` Zumeng Chen
-1 siblings, 0 replies; 42+ messages in thread
From: Zumeng Chen @ 2012-07-27 5:02 UTC (permalink / raw)
To: Kevin Hilman
Cc: zumeng.chen, Igor Grinberg, Tony Lindgren, linux-omap,
linux-arm-kernel, Arnd Bergmann
于 2012年07月27日 05:58, Kevin Hilman 写道:
> "zumeng.chen"<zchen@windriver.com> writes:
>
>> On 2012年07月27日 03:30, Kevin Hilman wrote:
>>> + Zumeng Chen
>>>
>>> Igor Grinberg<grinberg@compulab.co.il> writes:
>>>
>>>> Hi Kevin,
>>>>
>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>> that of course will trigger GPIO use without being requested.
>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>> Your right, your original patch isn't the problem. I found the root
>>> cause.
>>>
>>> The real problem is actually introduced by the merge of your patch from
>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>> ads7846) from the arm-soc/boards branch.
>>>
>>> However, looking closer at the one from Zumeng, that one is clearly not
>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>> function to the pdata that is common to *all* boards. That's just wrong
>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>> board except the OMAP3EVM. Oops.
>>>
>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>> well.
>> Definitely, thanks Kevin.
>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>> working? I currently don't have a board setup with a touchscreen in my
>>> board farm.
>> Acked
> Did you test this on your board? If so, could you respond with a
> Tested-by tag? Thanks.
Hi Kevin,
Your patch self has no problem, feel free to add
"Tested-by: Zumeng Chen <zumeng.chen@windriver.com>
But obviously, the current ads7846 doesn't work, I have found
one reason, another reason is about spi-dma update, I'm trying
to fix it.
Regards,
Zumeng
>
> Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-27 5:02 ` Zumeng Chen
0 siblings, 0 replies; 42+ messages in thread
From: Zumeng Chen @ 2012-07-27 5:02 UTC (permalink / raw)
To: linux-arm-kernel
? 2012?07?27? 05:58, Kevin Hilman ??:
> "zumeng.chen"<zchen@windriver.com> writes:
>
>> On 2012?07?27? 03:30, Kevin Hilman wrote:
>>> + Zumeng Chen
>>>
>>> Igor Grinberg<grinberg@compulab.co.il> writes:
>>>
>>>> Hi Kevin,
>>>>
>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>> that of course will trigger GPIO use without being requested.
>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>> Your right, your original patch isn't the problem. I found the root
>>> cause.
>>>
>>> The real problem is actually introduced by the merge of your patch from
>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>> ads7846) from the arm-soc/boards branch.
>>>
>>> However, looking closer at the one from Zumeng, that one is clearly not
>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>> function to the pdata that is common to *all* boards. That's just wrong
>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>> board except the OMAP3EVM. Oops.
>>>
>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>> well.
>> Definitely, thanks Kevin.
>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>> working? I currently don't have a board setup with a touchscreen in my
>>> board farm.
>> Acked
> Did you test this on your board? If so, could you respond with a
> Tested-by tag? Thanks.
Hi Kevin,
Your patch self has no problem, feel free to add
"Tested-by: Zumeng Chen <zumeng.chen@windriver.com>
But obviously, the current ads7846 doesn't work, I have found
one reason, another reason is about spi-dma update, I'm trying
to fix it.
Regards,
Zumeng
>
> Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-26 19:30 ` Kevin Hilman
@ 2012-07-27 15:30 ` Igor Grinberg
-1 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-07-27 15:30 UTC (permalink / raw)
To: Kevin Hilman
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Arnd Bergmann, Zumeng Chen
On 07/26/12 22:30, Kevin Hilman wrote:
> + Zumeng Chen
>
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> Hi Kevin,
>>
>> I've just noticed that the patch has been modified by Arnd in a way
>> that of course will trigger GPIO use without being requested.
>> I'm sorry, I was not available by that time Arnd changed the patch.
>
> Your right, your original patch isn't the problem. I found the root
> cause.
>
> The real problem is actually introduced by the merge of your patch from
> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
> ads7846) from the arm-soc/boards branch.
>
> However, looking closer at the one from Zumeng, that one is clearly not
> right. It unconditionally adds a *board-specific* ->get_pendown_state
> function to the pdata that is common to *all* boards. That's just wrong
> and has the side-effect of making ->get_pendown_state() wrong on every
> board except the OMAP3EVM. Oops.
Argh... that should not be applied in first place...
I double sorry, I was not available when that conflict was resolved...
The right resolution would be just to revert the 16aced80f6
(ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
but who cares now...
>
> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
> GPIO working on non OMAP3EVM boards, we also need something like this as
> well.
>
> Igor, Zumeng, could you try this out on your boards anc confirm if it's
> working? I currently don't have a board setup with a touchscreen in my
> board farm.
Since you have already dig into this, which branch would be good for testing?
>
> Kevin
>
>>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 26 Jul 2012 12:15:38 -0700
> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
> on all boards
>
> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
> time for ads7846) introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM.
>
> Fix this up to work for all boards that pass in a pendown GPIO.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 4 +++-
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 0fee95f..06f176f 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> * of pdata->get_pendown_state, but we have done this. So set
> * get_pendown_state to avoid twice gpio requesting.
> */
> +static int omap3_gpio_pendown;
> static int omap3_get_pendown_state(void)
> {
> - return !gpio_get_value(OMAP3_EVM_TS_GPIO);
> + return !gpio_get_value(omap3_gpio_pendown);
> }
>
> static struct ads7846_platform_data ads7846_config = {
> @@ -74,6 +75,7 @@ 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;
>
> + omap3_gpio_pendown = gpio_pendown;
> err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
> if (err) {
> pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-27 15:30 ` Igor Grinberg
0 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-07-27 15:30 UTC (permalink / raw)
To: linux-arm-kernel
On 07/26/12 22:30, Kevin Hilman wrote:
> + Zumeng Chen
>
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> Hi Kevin,
>>
>> I've just noticed that the patch has been modified by Arnd in a way
>> that of course will trigger GPIO use without being requested.
>> I'm sorry, I was not available by that time Arnd changed the patch.
>
> Your right, your original patch isn't the problem. I found the root
> cause.
>
> The real problem is actually introduced by the merge of your patch from
> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
> ads7846) from the arm-soc/boards branch.
>
> However, looking closer at the one from Zumeng, that one is clearly not
> right. It unconditionally adds a *board-specific* ->get_pendown_state
> function to the pdata that is common to *all* boards. That's just wrong
> and has the side-effect of making ->get_pendown_state() wrong on every
> board except the OMAP3EVM. Oops.
Argh... that should not be applied in first place...
I double sorry, I was not available when that conflict was resolved...
The right resolution would be just to revert the 16aced80f6
(ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
but who cares now...
>
> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
> GPIO working on non OMAP3EVM boards, we also need something like this as
> well.
>
> Igor, Zumeng, could you try this out on your boards anc confirm if it's
> working? I currently don't have a board setup with a touchscreen in my
> board farm.
Since you have already dig into this, which branch would be good for testing?
>
> Kevin
>
>>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 26 Jul 2012 12:15:38 -0700
> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
> on all boards
>
> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
> time for ads7846) introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM.
>
> Fix this up to work for all boards that pass in a pendown GPIO.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 4 +++-
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 0fee95f..06f176f 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> * of pdata->get_pendown_state, but we have done this. So set
> * get_pendown_state to avoid twice gpio requesting.
> */
> +static int omap3_gpio_pendown;
> static int omap3_get_pendown_state(void)
> {
> - return !gpio_get_value(OMAP3_EVM_TS_GPIO);
> + return !gpio_get_value(omap3_gpio_pendown);
> }
>
> static struct ads7846_platform_data ads7846_config = {
> @@ -74,6 +75,7 @@ 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;
>
> + omap3_gpio_pendown = gpio_pendown;
> err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
> if (err) {
> pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-27 15:30 ` Igor Grinberg
@ 2012-07-27 17:46 ` Kevin Hilman
-1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-27 17:46 UTC (permalink / raw)
To: Igor Grinberg
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Arnd Bergmann, Zumeng Chen
Igor Grinberg <grinberg@compulab.co.il> writes:
> On 07/26/12 22:30, Kevin Hilman wrote:
>> + Zumeng Chen
>>
>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>
>>> Hi Kevin,
>>>
>>> I've just noticed that the patch has been modified by Arnd in a way
>>> that of course will trigger GPIO use without being requested.
>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>
>> Your right, your original patch isn't the problem. I found the root
>> cause.
>>
>> The real problem is actually introduced by the merge of your patch from
>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>> ads7846) from the arm-soc/boards branch.
>>
>> However, looking closer at the one from Zumeng, that one is clearly not
>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>> function to the pdata that is common to *all* boards. That's just wrong
>> and has the side-effect of making ->get_pendown_state() wrong on every
>> board except the OMAP3EVM. Oops.
>
> Argh... that should not be applied in first place...
Agreed, but it's already in mainline.
> I double sorry, I was not available when that conflict was resolved...
> The right resolution would be just to revert the 16aced80f6
> (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
> but who cares now...
>
>>
>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>> GPIO working on non OMAP3EVM boards, we also need something like this as
>> well.
>>
>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>> working? I currently don't have a board setup with a touchscreen in my
>> board farm.
>
> Since you have already dig into this, which branch would be good for testing?
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.6/fixes/ads7846
I just pushed this out, so it might take a bit to propagate to all the
mirrors.
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-27 17:46 ` Kevin Hilman
0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-27 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Igor Grinberg <grinberg@compulab.co.il> writes:
> On 07/26/12 22:30, Kevin Hilman wrote:
>> + Zumeng Chen
>>
>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>
>>> Hi Kevin,
>>>
>>> I've just noticed that the patch has been modified by Arnd in a way
>>> that of course will trigger GPIO use without being requested.
>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>
>> Your right, your original patch isn't the problem. I found the root
>> cause.
>>
>> The real problem is actually introduced by the merge of your patch from
>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>> ads7846) from the arm-soc/boards branch.
>>
>> However, looking closer at the one from Zumeng, that one is clearly not
>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>> function to the pdata that is common to *all* boards. That's just wrong
>> and has the side-effect of making ->get_pendown_state() wrong on every
>> board except the OMAP3EVM. Oops.
>
> Argh... that should not be applied in first place...
Agreed, but it's already in mainline.
> I double sorry, I was not available when that conflict was resolved...
> The right resolution would be just to revert the 16aced80f6
> (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
> but who cares now...
>
>>
>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>> GPIO working on non OMAP3EVM boards, we also need something like this as
>> well.
>>
>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>> working? I currently don't have a board setup with a touchscreen in my
>> board farm.
>
> Since you have already dig into this, which branch would be good for testing?
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.6/fixes/ads7846
I just pushed this out, so it might take a bit to propagate to all the
mirrors.
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-27 17:46 ` Kevin Hilman
@ 2012-07-27 19:04 ` Igor Grinberg
-1 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-07-27 19:04 UTC (permalink / raw)
To: Kevin Hilman
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Arnd Bergmann, Zumeng Chen
On 07/27/12 20:46, Kevin Hilman wrote:
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> On 07/26/12 22:30, Kevin Hilman wrote:
>>> + Zumeng Chen
>>>
>>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>>
>>>> Hi Kevin,
>>>>
>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>> that of course will trigger GPIO use without being requested.
>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>>
>>> Your right, your original patch isn't the problem. I found the root
>>> cause.
>>>
>>> The real problem is actually introduced by the merge of your patch from
>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>> ads7846) from the arm-soc/boards branch.
>>>
>>> However, looking closer at the one from Zumeng, that one is clearly not
>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>> function to the pdata that is common to *all* boards. That's just wrong
>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>> board except the OMAP3EVM. Oops.
>>
>> Argh... that should not be applied in first place...
>
> Agreed, but it's already in mainline.
>
>> I double sorry, I was not available when that conflict was resolved...
>> The right resolution would be just to revert the 16aced80f6
>> (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
>> but who cares now...
>
>
>>
>>>
>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>> well.
>>>
>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>> working? I currently don't have a board setup with a touchscreen in my
>>> board farm.
>>
>> Since you have already dig into this, which branch would be good for testing?
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.6/fixes/ads7846
>
> I just pushed this out, so it might take a bit to propagate to all the
> mirrors.
Thanks, Kevin, I've just sent the tested-by.
I would also like to see only one patch instead of current two patches,
can you please, amend them into one, write a correct message and resend?
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-27 19:04 ` Igor Grinberg
0 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-07-27 19:04 UTC (permalink / raw)
To: linux-arm-kernel
On 07/27/12 20:46, Kevin Hilman wrote:
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> On 07/26/12 22:30, Kevin Hilman wrote:
>>> + Zumeng Chen
>>>
>>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>>
>>>> Hi Kevin,
>>>>
>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>> that of course will trigger GPIO use without being requested.
>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>>
>>> Your right, your original patch isn't the problem. I found the root
>>> cause.
>>>
>>> The real problem is actually introduced by the merge of your patch from
>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>> ads7846) from the arm-soc/boards branch.
>>>
>>> However, looking closer at the one from Zumeng, that one is clearly not
>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>> function to the pdata that is common to *all* boards. That's just wrong
>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>> board except the OMAP3EVM. Oops.
>>
>> Argh... that should not be applied in first place...
>
> Agreed, but it's already in mainline.
>
>> I double sorry, I was not available when that conflict was resolved...
>> The right resolution would be just to revert the 16aced80f6
>> (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
>> but who cares now...
>
>
>>
>>>
>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>> well.
>>>
>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>> working? I currently don't have a board setup with a touchscreen in my
>>> board farm.
>>
>> Since you have already dig into this, which branch would be good for testing?
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.6/fixes/ads7846
>
> I just pushed this out, so it might take a bit to propagate to all the
> mirrors.
Thanks, Kevin, I've just sent the tested-by.
I would also like to see only one patch instead of current two patches,
can you please, amend them into one, write a correct message and resend?
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-27 19:04 ` Igor Grinberg
@ 2012-07-28 0:28 ` Kevin Hilman
-1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-28 0:28 UTC (permalink / raw)
To: Igor Grinberg
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Arnd Bergmann, Zumeng Chen
Igor Grinberg <grinberg@compulab.co.il> writes:
> On 07/27/12 20:46, Kevin Hilman wrote:
>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>
>>> On 07/26/12 22:30, Kevin Hilman wrote:
>>>> + Zumeng Chen
>>>>
>>>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>>>
>>>>> Hi Kevin,
>>>>>
>>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>>> that of course will trigger GPIO use without being requested.
>>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>>>
>>>> Your right, your original patch isn't the problem. I found the root
>>>> cause.
>>>>
>>>> The real problem is actually introduced by the merge of your patch from
>>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>>> ads7846) from the arm-soc/boards branch.
>>>>
>>>> However, looking closer at the one from Zumeng, that one is clearly not
>>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>>> function to the pdata that is common to *all* boards. That's just wrong
>>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>>> board except the OMAP3EVM. Oops.
>>>
>>> Argh... that should not be applied in first place...
>>
>> Agreed, but it's already in mainline.
>>
>>> I double sorry, I was not available when that conflict was resolved...
>>> The right resolution would be just to revert the 16aced80f6
>>> (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
>>> but who cares now...
>>
>>
>>>
>>>>
>>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>>> well.
>>>>
>>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>>> working? I currently don't have a board setup with a touchscreen in my
>>>> board farm.
>>>
>>> Since you have already dig into this, which branch would be good for testing?
>>
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.6/fixes/ads7846
>>
>> I just pushed this out, so it might take a bit to propagate to all the
>> mirrors.
>
> Thanks, Kevin, I've just sent the tested-by.
> I would also like to see only one patch instead of current two patches,
> can you please, amend them into one, write a correct message and resend?
Yeah, the changelog in my first patch isn't quite right anymore since
your patch isn't the one that caused the problems.
You're more familiar with this code than me, so feel free to recombine
them if you like, and I'll queue it up for v3.6-rc.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-28 0:28 ` Kevin Hilman
0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-28 0:28 UTC (permalink / raw)
To: linux-arm-kernel
Igor Grinberg <grinberg@compulab.co.il> writes:
> On 07/27/12 20:46, Kevin Hilman wrote:
>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>
>>> On 07/26/12 22:30, Kevin Hilman wrote:
>>>> + Zumeng Chen
>>>>
>>>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>>>
>>>>> Hi Kevin,
>>>>>
>>>>> I've just noticed that the patch has been modified by Arnd in a way
>>>>> that of course will trigger GPIO use without being requested.
>>>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>>>
>>>> Your right, your original patch isn't the problem. I found the root
>>>> cause.
>>>>
>>>> The real problem is actually introduced by the merge of your patch from
>>>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>>>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>>>> ads7846) from the arm-soc/boards branch.
>>>>
>>>> However, looking closer at the one from Zumeng, that one is clearly not
>>>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>>>> function to the pdata that is common to *all* boards. That's just wrong
>>>> and has the side-effect of making ->get_pendown_state() wrong on every
>>>> board except the OMAP3EVM. Oops.
>>>
>>> Argh... that should not be applied in first place...
>>
>> Agreed, but it's already in mainline.
>>
>>> I double sorry, I was not available when that conflict was resolved...
>>> The right resolution would be just to revert the 16aced80f6
>>> (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846),
>>> but who cares now...
>>
>>
>>>
>>>>
>>>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>>>> GPIO working on non OMAP3EVM boards, we also need something like this as
>>>> well.
>>>>
>>>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>>>> working? I currently don't have a board setup with a touchscreen in my
>>>> board farm.
>>>
>>> Since you have already dig into this, which branch would be good for testing?
>>
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.6/fixes/ads7846
>>
>> I just pushed this out, so it might take a bit to propagate to all the
>> mirrors.
>
> Thanks, Kevin, I've just sent the tested-by.
> I would also like to see only one patch instead of current two patches,
> can you please, amend them into one, write a correct message and resend?
Yeah, the changelog in my first patch isn't quite right anymore since
your patch isn't the one that caused the problems.
You're more familiar with this code than me, so feel free to recombine
them if you like, and I'll queue it up for v3.6-rc.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-07-28 0:28 ` Kevin Hilman
@ 2012-08-06 20:22 ` Igor Grinberg
-1 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-08-06 20:22 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-omap, linux-arm-kernel, Igor Grinberg, Zumeng Chen, Arnd Bergmann
1) The above commit introduced a common ->get_pendown_state() function
into the generic code, but that function was board-specific for the
OMAP3EVM and thus broke most other boards using this code.
2) The above commit was mis-merged introducing another bug which
prevents the ads7846 driver probe function to succeed.
The omap_ads7846_init() function frees the pendown GPIO in case there is
no ->get_pendown_state() function set by the caller (board specific
code), so it can be requested later by the ads7846 driver.
The above commit add a common ->get_pendown_state() function without
removing the gpio_free() call and thus once the ads7846 driver tries
to use the pendown GPIO, it crashes as the pendown GPIO has not been
requested.
3) The above commit introduces NO new functionality as
get_pendown_state() function is already implemented in a suitable way by
the ads7846 driver and the debounce time handling has already been
fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
Conflicts:
arch/arm/mach-omap2/common-board-devices.c
Solved by taking the working version prior to the above commit.
Cc: Zumeng Chen <zumeng.chen@windriver.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
Kevin, sorry for the late reply.
How about the above commit message and the below patch?
The patch applies cleanly to Tony's master branch (6 Aug 2012)
or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
after resetting two top most commits.
This patch has been tested on both above branches on cm-t3730.
Any other board tested will be greately appreciated.
Also, if after all said in the commit message, you still don't
want to revert the original patch, feel free to post your thoughts.
arch/arm/mach-omap2/board-omap3evm.c | 1 +
arch/arm/mach-omap2/common-board-devices.c | 11 -----------
arch/arm/mach-omap2/common-board-devices.h | 1 -
3 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index ef230a0..0d362e9 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -58,6 +58,7 @@
#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 1473474..c187586 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
.turbo_mode = 0,
};
-/*
- * ADS7846 driver maybe request a gpio according to the value
- * of pdata->get_pendown_state, but we have done this. So set
- * get_pendown_state to avoid twice gpio requesting.
- */
-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,
.y_max = 0x0fff,
@@ -55,7 +45,6 @@ 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 = {
diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
index 4c4ef6a..a0b4a428 100644
--- a/arch/arm/mach-omap2/common-board-devices.h
+++ b/arch/arm/mach-omap2/common-board-devices.h
@@ -4,7 +4,6 @@
#include "twl-common.h"
#define NAND_BLOCK_SIZE SZ_128K
-#define OMAP3_EVM_TS_GPIO 175
struct mtd_partition;
struct ads7846_platform_data;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-06 20:22 ` Igor Grinberg
0 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-08-06 20:22 UTC (permalink / raw)
To: linux-arm-kernel
1) The above commit introduced a common ->get_pendown_state() function
into the generic code, but that function was board-specific for the
OMAP3EVM and thus broke most other boards using this code.
2) The above commit was mis-merged introducing another bug which
prevents the ads7846 driver probe function to succeed.
The omap_ads7846_init() function frees the pendown GPIO in case there is
no ->get_pendown_state() function set by the caller (board specific
code), so it can be requested later by the ads7846 driver.
The above commit add a common ->get_pendown_state() function without
removing the gpio_free() call and thus once the ads7846 driver tries
to use the pendown GPIO, it crashes as the pendown GPIO has not been
requested.
3) The above commit introduces NO new functionality as
get_pendown_state() function is already implemented in a suitable way by
the ads7846 driver and the debounce time handling has already been
fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
Conflicts:
arch/arm/mach-omap2/common-board-devices.c
Solved by taking the working version prior to the above commit.
Cc: Zumeng Chen <zumeng.chen@windriver.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
Kevin, sorry for the late reply.
How about the above commit message and the below patch?
The patch applies cleanly to Tony's master branch (6 Aug 2012)
or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
after resetting two top most commits.
This patch has been tested on both above branches on cm-t3730.
Any other board tested will be greately appreciated.
Also, if after all said in the commit message, you still don't
want to revert the original patch, feel free to post your thoughts.
arch/arm/mach-omap2/board-omap3evm.c | 1 +
arch/arm/mach-omap2/common-board-devices.c | 11 -----------
arch/arm/mach-omap2/common-board-devices.h | 1 -
3 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index ef230a0..0d362e9 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -58,6 +58,7 @@
#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 1473474..c187586 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
.turbo_mode = 0,
};
-/*
- * ADS7846 driver maybe request a gpio according to the value
- * of pdata->get_pendown_state, but we have done this. So set
- * get_pendown_state to avoid twice gpio requesting.
- */
-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,
.y_max = 0x0fff,
@@ -55,7 +45,6 @@ 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 = {
diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
index 4c4ef6a..a0b4a428 100644
--- a/arch/arm/mach-omap2/common-board-devices.h
+++ b/arch/arm/mach-omap2/common-board-devices.h
@@ -4,7 +4,6 @@
#include "twl-common.h"
#define NAND_BLOCK_SIZE SZ_128K
-#define OMAP3_EVM_TS_GPIO 175
struct mtd_partition;
struct ads7846_platform_data;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-08-06 20:22 ` Igor Grinberg
@ 2012-08-06 20:52 ` Kevin Hilman
-1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-08-06 20:52 UTC (permalink / raw)
To: Igor Grinberg
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Zumeng Chen, Arnd Bergmann
Igor Grinberg <grinberg@compulab.co.il> writes:
> 1) The above commit introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM and thus broke most other boards using this code.
>
> 2) The above commit was mis-merged introducing another bug which
> prevents the ads7846 driver probe function to succeed.
> The omap_ads7846_init() function frees the pendown GPIO in case there is
> no ->get_pendown_state() function set by the caller (board specific
> code), so it can be requested later by the ads7846 driver.
> The above commit add a common ->get_pendown_state() function without
> removing the gpio_free() call and thus once the ads7846 driver tries
> to use the pendown GPIO, it crashes as the pendown GPIO has not been
> requested.
>
> 3) The above commit introduces NO new functionality as
> get_pendown_state() function is already implemented in a suitable way by
> the ads7846 driver and the debounce time handling has already been
> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
>
> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>
> Conflicts:
> arch/arm/mach-omap2/common-board-devices.c
>
> Solved by taking the working version prior to the above commit.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> Kevin, sorry for the late reply.
> How about the above commit message and the below patch?
>
> The patch applies cleanly to Tony's master branch (6 Aug 2012)
> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
> after resetting two top most commits.
Now that v3.6-rc1 is out, it should probalby be applied on top of -rc1.
I've taken care of that and tested as well.
> This patch has been tested on both above branches on cm-t3730.
> Any other board tested will be greately appreciated.
>
> Also, if after all said in the commit message, you still don't
> want to revert the original patch, feel free to post your thoughts.
After all the digging I did, I agree that the revert is the best
solution.
While it's a slightly different problem/bug, I think the gpio_free() in
common-board-devices.c should still be unconditonal since the
gpio_request() is now unconditional. That can be a separate patch though.
Acked-by: Kevin Hilman <khilman@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.
The Overo boards were the ones that were crashing due to this bug since
the pendown GPIO was the only one used in the bank.
Tony, can you queue this up for v3.6-rc? I have a version in my
for_3.6/fixes/ads7846-2 branch with the tags above applied, based on v3.6-rc1.
Thanks
Kevin
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 1473474..c187586 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> .turbo_mode = 0,
> };
>
> -/*
> - * ADS7846 driver maybe request a gpio according to the value
> - * of pdata->get_pendown_state, but we have done this. So set
> - * get_pendown_state to avoid twice gpio requesting.
> - */
> -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,
> .y_max = 0x0fff,
> @@ -55,7 +45,6 @@ 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 = {
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-06 20:52 ` Kevin Hilman
0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-08-06 20:52 UTC (permalink / raw)
To: linux-arm-kernel
Igor Grinberg <grinberg@compulab.co.il> writes:
> 1) The above commit introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM and thus broke most other boards using this code.
>
> 2) The above commit was mis-merged introducing another bug which
> prevents the ads7846 driver probe function to succeed.
> The omap_ads7846_init() function frees the pendown GPIO in case there is
> no ->get_pendown_state() function set by the caller (board specific
> code), so it can be requested later by the ads7846 driver.
> The above commit add a common ->get_pendown_state() function without
> removing the gpio_free() call and thus once the ads7846 driver tries
> to use the pendown GPIO, it crashes as the pendown GPIO has not been
> requested.
>
> 3) The above commit introduces NO new functionality as
> get_pendown_state() function is already implemented in a suitable way by
> the ads7846 driver and the debounce time handling has already been
> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
>
> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>
> Conflicts:
> arch/arm/mach-omap2/common-board-devices.c
>
> Solved by taking the working version prior to the above commit.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> Kevin, sorry for the late reply.
> How about the above commit message and the below patch?
>
> The patch applies cleanly to Tony's master branch (6 Aug 2012)
> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
> after resetting two top most commits.
Now that v3.6-rc1 is out, it should probalby be applied on top of -rc1.
I've taken care of that and tested as well.
> This patch has been tested on both above branches on cm-t3730.
> Any other board tested will be greately appreciated.
>
> Also, if after all said in the commit message, you still don't
> want to revert the original patch, feel free to post your thoughts.
After all the digging I did, I agree that the revert is the best
solution.
While it's a slightly different problem/bug, I think the gpio_free() in
common-board-devices.c should still be unconditonal since the
gpio_request() is now unconditional. That can be a separate patch though.
Acked-by: Kevin Hilman <khilman@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.
The Overo boards were the ones that were crashing due to this bug since
the pendown GPIO was the only one used in the bank.
Tony, can you queue this up for v3.6-rc? I have a version in my
for_3.6/fixes/ads7846-2 branch with the tags above applied, based on v3.6-rc1.
Thanks
Kevin
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 1473474..c187586 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> .turbo_mode = 0,
> };
>
> -/*
> - * ADS7846 driver maybe request a gpio according to the value
> - * of pdata->get_pendown_state, but we have done this. So set
> - * get_pendown_state to avoid twice gpio requesting.
> - */
> -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,
> .y_max = 0x0fff,
> @@ -55,7 +45,6 @@ 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 = {
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-08-06 20:52 ` Kevin Hilman
@ 2012-08-07 1:44 ` Zumeng Chen
-1 siblings, 0 replies; 42+ messages in thread
From: Zumeng Chen @ 2012-08-07 1:44 UTC (permalink / raw)
To: Kevin Hilman
Cc: Igor Grinberg, Tony Lindgren, linux-omap, linux-arm-kernel,
Arnd Bergmann
于 2012年08月07日 04:52, Kevin Hilman 写道:
> Igor Grinberg<grinberg@compulab.co.il> writes:
>
>> 1) The above commit introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM and thus broke most other boards using this code.
>>
>> 2) The above commit was mis-merged introducing another bug which
>> prevents the ads7846 driver probe function to succeed.
>> The omap_ads7846_init() function frees the pendown GPIO in case there is
>> no ->get_pendown_state() function set by the caller (board specific
>> code), so it can be requested later by the ads7846 driver.
>> The above commit add a common ->get_pendown_state() function without
>> removing the gpio_free() call and thus once the ads7846 driver tries
>> to use the pendown GPIO, it crashes as the pendown GPIO has not been
>> requested.
>>
>> 3) The above commit introduces NO new functionality as
>> get_pendown_state() function is already implemented in a suitable way by
>> the ads7846 driver and the debounce time handling has already been
>> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
>>
>> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>>
>> Conflicts:
>> arch/arm/mach-omap2/common-board-devices.c
>>
>> Solved by taking the working version prior to the above commit.
>>
>> Cc: Zumeng Chen<zumeng.chen@windriver.com>
>> Cc: Arnd Bergmann<arnd@arndb.de>
>> Signed-off-by: Igor Grinberg<grinberg@compulab.co.il>
>> ---
>> Kevin, sorry for the late reply.
>> How about the above commit message and the below patch?
>>
>> The patch applies cleanly to Tony's master branch (6 Aug 2012)
>> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
>> after resetting two top most commits.
> Now that v3.6-rc1 is out, it should probalby be applied on top of -rc1.
> I've taken care of that and tested as well.
>
>> This patch has been tested on both above branches on cm-t3730.
>> Any other board tested will be greately appreciated.
>>
>> Also, if after all said in the commit message, you still don't
>> want to revert the original patch, feel free to post your thoughts.
> After all the digging I did, I agree that the revert is the best
> solution.
>
> While it's a slightly different problem/bug, I think the gpio_free() in
> common-board-devices.c should still be unconditonal since the
> gpio_request() is now unconditional. That can be a separate patch though.
Absolutely right, acked this.
Regards,
Zumeng
>
> Acked-by: Kevin Hilman<khilman@ti.com>
> Tested-by: Kevin Hilman<khilman@ti.com>
>
> Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.
>
> The Overo boards were the ones that were crashing due to this bug since
> the pendown GPIO was the only one used in the bank.
>
> Tony, can you queue this up for v3.6-rc? I have a version in my
> for_3.6/fixes/ads7846-2 branch with the tags above applied, based on v3.6-rc1.
>
> Thanks
>
> Kevin
>
>> arch/arm/mach-omap2/board-omap3evm.c | 1 +
>> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
>> arch/arm/mach-omap2/common-board-devices.h | 1 -
>> 3 files changed, 1 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
>> index ef230a0..0d362e9 100644
>> --- a/arch/arm/mach-omap2/board-omap3evm.c
>> +++ b/arch/arm/mach-omap2/board-omap3evm.c
>> @@ -58,6 +58,7 @@
>> #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 1473474..c187586 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.c
>> +++ b/arch/arm/mach-omap2/common-board-devices.c
>> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
>> .turbo_mode = 0,
>> };
>>
>> -/*
>> - * ADS7846 driver maybe request a gpio according to the value
>> - * of pdata->get_pendown_state, but we have done this. So set
>> - * get_pendown_state to avoid twice gpio requesting.
>> - */
>> -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,
>> .y_max = 0x0fff,
>> @@ -55,7 +45,6 @@ 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 = {
>> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
>> index 4c4ef6a..a0b4a428 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.h
>> +++ b/arch/arm/mach-omap2/common-board-devices.h
>> @@ -4,7 +4,6 @@
>> #include "twl-common.h"
>>
>> #define NAND_BLOCK_SIZE SZ_128K
>> -#define OMAP3_EVM_TS_GPIO 175
>>
>> struct mtd_partition;
>> struct ads7846_platform_data;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-07 1:44 ` Zumeng Chen
0 siblings, 0 replies; 42+ messages in thread
From: Zumeng Chen @ 2012-08-07 1:44 UTC (permalink / raw)
To: linux-arm-kernel
? 2012?08?07? 04:52, Kevin Hilman ??:
> Igor Grinberg<grinberg@compulab.co.il> writes:
>
>> 1) The above commit introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM and thus broke most other boards using this code.
>>
>> 2) The above commit was mis-merged introducing another bug which
>> prevents the ads7846 driver probe function to succeed.
>> The omap_ads7846_init() function frees the pendown GPIO in case there is
>> no ->get_pendown_state() function set by the caller (board specific
>> code), so it can be requested later by the ads7846 driver.
>> The above commit add a common ->get_pendown_state() function without
>> removing the gpio_free() call and thus once the ads7846 driver tries
>> to use the pendown GPIO, it crashes as the pendown GPIO has not been
>> requested.
>>
>> 3) The above commit introduces NO new functionality as
>> get_pendown_state() function is already implemented in a suitable way by
>> the ads7846 driver and the debounce time handling has already been
>> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
>>
>> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>>
>> Conflicts:
>> arch/arm/mach-omap2/common-board-devices.c
>>
>> Solved by taking the working version prior to the above commit.
>>
>> Cc: Zumeng Chen<zumeng.chen@windriver.com>
>> Cc: Arnd Bergmann<arnd@arndb.de>
>> Signed-off-by: Igor Grinberg<grinberg@compulab.co.il>
>> ---
>> Kevin, sorry for the late reply.
>> How about the above commit message and the below patch?
>>
>> The patch applies cleanly to Tony's master branch (6 Aug 2012)
>> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
>> after resetting two top most commits.
> Now that v3.6-rc1 is out, it should probalby be applied on top of -rc1.
> I've taken care of that and tested as well.
>
>> This patch has been tested on both above branches on cm-t3730.
>> Any other board tested will be greately appreciated.
>>
>> Also, if after all said in the commit message, you still don't
>> want to revert the original patch, feel free to post your thoughts.
> After all the digging I did, I agree that the revert is the best
> solution.
>
> While it's a slightly different problem/bug, I think the gpio_free() in
> common-board-devices.c should still be unconditonal since the
> gpio_request() is now unconditional. That can be a separate patch though.
Absolutely right, acked this.
Regards,
Zumeng
>
> Acked-by: Kevin Hilman<khilman@ti.com>
> Tested-by: Kevin Hilman<khilman@ti.com>
>
> Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.
>
> The Overo boards were the ones that were crashing due to this bug since
> the pendown GPIO was the only one used in the bank.
>
> Tony, can you queue this up for v3.6-rc? I have a version in my
> for_3.6/fixes/ads7846-2 branch with the tags above applied, based on v3.6-rc1.
>
> Thanks
>
> Kevin
>
>> arch/arm/mach-omap2/board-omap3evm.c | 1 +
>> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
>> arch/arm/mach-omap2/common-board-devices.h | 1 -
>> 3 files changed, 1 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
>> index ef230a0..0d362e9 100644
>> --- a/arch/arm/mach-omap2/board-omap3evm.c
>> +++ b/arch/arm/mach-omap2/board-omap3evm.c
>> @@ -58,6 +58,7 @@
>> #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 1473474..c187586 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.c
>> +++ b/arch/arm/mach-omap2/common-board-devices.c
>> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
>> .turbo_mode = 0,
>> };
>>
>> -/*
>> - * ADS7846 driver maybe request a gpio according to the value
>> - * of pdata->get_pendown_state, but we have done this. So set
>> - * get_pendown_state to avoid twice gpio requesting.
>> - */
>> -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,
>> .y_max = 0x0fff,
>> @@ -55,7 +45,6 @@ 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 = {
>> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
>> index 4c4ef6a..a0b4a428 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.h
>> +++ b/arch/arm/mach-omap2/common-board-devices.h
>> @@ -4,7 +4,6 @@
>> #include "twl-common.h"
>>
>> #define NAND_BLOCK_SIZE SZ_128K
>> -#define OMAP3_EVM_TS_GPIO 175
>>
>> struct mtd_partition;
>> struct ads7846_platform_data;
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-08-06 20:52 ` Kevin Hilman
@ 2012-08-07 5:58 ` Igor Grinberg
-1 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-08-07 5:58 UTC (permalink / raw)
To: Kevin Hilman
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Zumeng Chen, Arnd Bergmann
On 08/06/12 23:52, Kevin Hilman wrote:
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> 1) The above commit introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM and thus broke most other boards using this code.
>>
>> 2) The above commit was mis-merged introducing another bug which
>> prevents the ads7846 driver probe function to succeed.
>> The omap_ads7846_init() function frees the pendown GPIO in case there is
>> no ->get_pendown_state() function set by the caller (board specific
>> code), so it can be requested later by the ads7846 driver.
>> The above commit add a common ->get_pendown_state() function without
>> removing the gpio_free() call and thus once the ads7846 driver tries
>> to use the pendown GPIO, it crashes as the pendown GPIO has not been
>> requested.
>>
>> 3) The above commit introduces NO new functionality as
>> get_pendown_state() function is already implemented in a suitable way by
>> the ads7846 driver and the debounce time handling has already been
>> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
>>
>> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>>
>> Conflicts:
>> arch/arm/mach-omap2/common-board-devices.c
>>
>> Solved by taking the working version prior to the above commit.
>>
>> Cc: Zumeng Chen <zumeng.chen@windriver.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>> Kevin, sorry for the late reply.
>> How about the above commit message and the below patch?
>>
>> The patch applies cleanly to Tony's master branch (6 Aug 2012)
>> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
>> after resetting two top most commits.
>
> Now that v3.6-rc1 is out, it should probalby be applied on top of -rc1.
> I've taken care of that and tested as well.
>
>> This patch has been tested on both above branches on cm-t3730.
>> Any other board tested will be greately appreciated.
>>
>> Also, if after all said in the commit message, you still don't
>> want to revert the original patch, feel free to post your thoughts.
>
> After all the digging I did, I agree that the revert is the best
> solution.
>
> While it's a slightly different problem/bug, I think the gpio_free() in
> common-board-devices.c should still be unconditonal since the
> gpio_request() is now unconditional. That can be a separate patch though.
Well, the logic behind the conditional gpio_free() is:
if a board specific code provides a board specific get_pendown_state()
function (which is different from the one in the ads7846 driver),
that board will not need to re-request the pendown GPIO (duplicate code).
If we free the pendown GPIO unconditionally, then the board specific code
will have to re-request it.
So, I think, no - it should be conditional.
>
> Acked-by: Kevin Hilman <khilman@ti.com>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.
Thanks!
[...]
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-07 5:58 ` Igor Grinberg
0 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-08-07 5:58 UTC (permalink / raw)
To: linux-arm-kernel
On 08/06/12 23:52, Kevin Hilman wrote:
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> 1) The above commit introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM and thus broke most other boards using this code.
>>
>> 2) The above commit was mis-merged introducing another bug which
>> prevents the ads7846 driver probe function to succeed.
>> The omap_ads7846_init() function frees the pendown GPIO in case there is
>> no ->get_pendown_state() function set by the caller (board specific
>> code), so it can be requested later by the ads7846 driver.
>> The above commit add a common ->get_pendown_state() function without
>> removing the gpio_free() call and thus once the ads7846 driver tries
>> to use the pendown GPIO, it crashes as the pendown GPIO has not been
>> requested.
>>
>> 3) The above commit introduces NO new functionality as
>> get_pendown_state() function is already implemented in a suitable way by
>> the ads7846 driver and the debounce time handling has already been
>> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
>>
>> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>>
>> Conflicts:
>> arch/arm/mach-omap2/common-board-devices.c
>>
>> Solved by taking the working version prior to the above commit.
>>
>> Cc: Zumeng Chen <zumeng.chen@windriver.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>> Kevin, sorry for the late reply.
>> How about the above commit message and the below patch?
>>
>> The patch applies cleanly to Tony's master branch (6 Aug 2012)
>> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
>> after resetting two top most commits.
>
> Now that v3.6-rc1 is out, it should probalby be applied on top of -rc1.
> I've taken care of that and tested as well.
>
>> This patch has been tested on both above branches on cm-t3730.
>> Any other board tested will be greately appreciated.
>>
>> Also, if after all said in the commit message, you still don't
>> want to revert the original patch, feel free to post your thoughts.
>
> After all the digging I did, I agree that the revert is the best
> solution.
>
> While it's a slightly different problem/bug, I think the gpio_free() in
> common-board-devices.c should still be unconditonal since the
> gpio_request() is now unconditional. That can be a separate patch though.
Well, the logic behind the conditional gpio_free() is:
if a board specific code provides a board specific get_pendown_state()
function (which is different from the one in the ads7846 driver),
that board will not need to re-request the pendown GPIO (duplicate code).
If we free the pendown GPIO unconditionally, then the board specific code
will have to re-request it.
So, I think, no - it should be conditional.
>
> Acked-by: Kevin Hilman <khilman@ti.com>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.
Thanks!
[...]
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-08-06 20:52 ` Kevin Hilman
@ 2012-08-07 10:58 ` Tony Lindgren
-1 siblings, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2012-08-07 10:58 UTC (permalink / raw)
To: Kevin Hilman
Cc: Igor Grinberg, linux-omap, linux-arm-kernel, Zumeng Chen, Arnd Bergmann
* Kevin Hilman <khilman@ti.com> [120806 13:52]:
>
> The Overo boards were the ones that were crashing due to this bug since
> the pendown GPIO was the only one used in the bank.
>
> Tony, can you queue this up for v3.6-rc? I have a version in my
> for_3.6/fixes/ads7846-2 branch with the tags above applied, based on v3.6-rc1.
Yes thanks pulling into fixes.
Tony
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-07 10:58 ` Tony Lindgren
0 siblings, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2012-08-07 10:58 UTC (permalink / raw)
To: linux-arm-kernel
* Kevin Hilman <khilman@ti.com> [120806 13:52]:
>
> The Overo boards were the ones that were crashing due to this bug since
> the pendown GPIO was the only one used in the bank.
>
> Tony, can you queue this up for v3.6-rc? I have a version in my
> for_3.6/fixes/ads7846-2 branch with the tags above applied, based on v3.6-rc1.
Yes thanks pulling into fixes.
Tony
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-08-06 20:22 ` Igor Grinberg
@ 2012-08-07 1:42 ` Zumeng Chen
-1 siblings, 0 replies; 42+ messages in thread
From: Zumeng Chen @ 2012-08-07 1:42 UTC (permalink / raw)
To: Igor Grinberg
Cc: Tony Lindgren, Kevin Hilman, linux-omap, linux-arm-kernel, Arnd Bergmann
于 2012年08月07日 04:22, Igor Grinberg 写道:
> 1) The above commit introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM and thus broke most other boards using this code.
>
> 2) The above commit was mis-merged introducing another bug which
> prevents the ads7846 driver probe function to succeed.
> The omap_ads7846_init() function frees the pendown GPIO in case there is
> no ->get_pendown_state() function set by the caller (board specific
> code), so it can be requested later by the ads7846 driver.
> The above commit add a common ->get_pendown_state() function without
> removing the gpio_free() call and thus once the ads7846 driver tries
> to use the pendown GPIO, it crashes as the pendown GPIO has not been
> requested.
>
> 3) The above commit introduces NO new functionality as
> get_pendown_state() function is already implemented in a suitable way by
> the ads7846 driver and the debounce time handling has already been
> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
Igor,
I suspect these two patches(this and 97ee9f01) works well, and there
is something new introduced in ads7846.c, I'll try to look at that new
stuff in this weekend.
Please refer to in-line comments:
Regards,
Zumeng
>
> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>
> Conflicts:
> arch/arm/mach-omap2/common-board-devices.c
>
> Solved by taking the working version prior to the above commit.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> Kevin, sorry for the late reply.
> How about the above commit message and the below patch?
>
> The patch applies cleanly to Tony's master branch (6 Aug 2012)
> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
> after resetting two top most commits.
>
> This patch has been tested on both above branches on cm-t3730.
> Any other board tested will be greately appreciated.
>
> Also, if after all said in the commit message, you still don't
> want to revert the original patch, feel free to post your thoughts.
>
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 1473474..c187586 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> .turbo_mode = 0,
> };
>
> -/*
> - * ADS7846 driver maybe request a gpio according to the value
> - * of pdata->get_pendown_state, but we have done this. So set
> - * get_pendown_state to avoid twice gpio requesting.
> - */
> -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,
> .y_max = 0x0fff,
> @@ -55,7 +45,6 @@ static struct ads7846_platform_data ads7846_config = {
> .debounce_rep = 1,
> .gpio_pendown = -EINVAL,
> .keep_vref_on = 1,
> - .get_pendown_state = &omap3_get_pendown_state,
You remove this, then please look at the following codes:
drivers/input/touchscreen/ads7846.c
969 if (pdata->get_pendown_state) {
970 ts->get_pendown_state = pdata->get_pendown_state;
971 } else if (gpio_is_valid(pdata->gpio_pendown)) {
972
973 err = gpio_request_one(pdata->gpio_pendown, GPIOF_IN,
^^^, the driver will want to request again, then it should
be error if i'm right.
974 "ads7846_pendown");
975 if (err) {
976 dev_err(&spi->dev,
977 "failed to request/setup pendown GPIO%d: %d\n",
978 pdata->gpio_pendown, err);
979 return err;
980 }
981
982 ts->gpio_pendown = pdata->gpio_pendown;
983
984 } else {
Regards,
Zumeng
> };
>
> static struct spi_board_info ads7846_spi_board_info __initdata = {
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-07 1:42 ` Zumeng Chen
0 siblings, 0 replies; 42+ messages in thread
From: Zumeng Chen @ 2012-08-07 1:42 UTC (permalink / raw)
To: linux-arm-kernel
? 2012?08?07? 04:22, Igor Grinberg ??:
> 1) The above commit introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM and thus broke most other boards using this code.
>
> 2) The above commit was mis-merged introducing another bug which
> prevents the ads7846 driver probe function to succeed.
> The omap_ads7846_init() function frees the pendown GPIO in case there is
> no ->get_pendown_state() function set by the caller (board specific
> code), so it can be requested later by the ads7846 driver.
> The above commit add a common ->get_pendown_state() function without
> removing the gpio_free() call and thus once the ads7846 driver tries
> to use the pendown GPIO, it crashes as the pendown GPIO has not been
> requested.
>
> 3) The above commit introduces NO new functionality as
> get_pendown_state() function is already implemented in a suitable way by
> the ads7846 driver and the debounce time handling has already been
> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
Igor,
I suspect these two patches(this and 97ee9f01) works well, and there
is something new introduced in ads7846.c, I'll try to look at that new
stuff in this weekend.
Please refer to in-line comments:
Regards,
Zumeng
>
> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>
> Conflicts:
> arch/arm/mach-omap2/common-board-devices.c
>
> Solved by taking the working version prior to the above commit.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> Kevin, sorry for the late reply.
> How about the above commit message and the below patch?
>
> The patch applies cleanly to Tony's master branch (6 Aug 2012)
> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
> after resetting two top most commits.
>
> This patch has been tested on both above branches on cm-t3730.
> Any other board tested will be greately appreciated.
>
> Also, if after all said in the commit message, you still don't
> want to revert the original patch, feel free to post your thoughts.
>
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 1473474..c187586 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> .turbo_mode = 0,
> };
>
> -/*
> - * ADS7846 driver maybe request a gpio according to the value
> - * of pdata->get_pendown_state, but we have done this. So set
> - * get_pendown_state to avoid twice gpio requesting.
> - */
> -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,
> .y_max = 0x0fff,
> @@ -55,7 +45,6 @@ static struct ads7846_platform_data ads7846_config = {
> .debounce_rep = 1,
> .gpio_pendown = -EINVAL,
> .keep_vref_on = 1,
> - .get_pendown_state = &omap3_get_pendown_state,
You remove this, then please look at the following codes:
drivers/input/touchscreen/ads7846.c
969 if (pdata->get_pendown_state) {
970 ts->get_pendown_state = pdata->get_pendown_state;
971 } else if (gpio_is_valid(pdata->gpio_pendown)) {
972
973 err = gpio_request_one(pdata->gpio_pendown, GPIOF_IN,
^^^, the driver will want to request again, then it should
be error if i'm right.
974 "ads7846_pendown");
975 if (err) {
976 dev_err(&spi->dev,
977 "failed to request/setup pendown GPIO%d: %d\n",
978 pdata->gpio_pendown, err);
979 return err;
980 }
981
982 ts->gpio_pendown = pdata->gpio_pendown;
983
984 } else {
Regards,
Zumeng
> };
>
> static struct spi_board_info ads7846_spi_board_info __initdata = {
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
2012-08-07 1:42 ` Zumeng Chen
@ 2012-08-07 6:05 ` Igor Grinberg
-1 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-08-07 6:05 UTC (permalink / raw)
To: Zumeng Chen
Cc: Tony Lindgren, Kevin Hilman, linux-omap, Arnd Bergmann, linux-arm-kernel
On 08/07/12 04:42, Zumeng Chen wrote:
> 于 2012年08月07日 04:22, Igor Grinberg 写道:
>> 1) The above commit introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM and thus broke most other boards using this code.
>>
>> 2) The above commit was mis-merged introducing another bug which
>> prevents the ads7846 driver probe function to succeed.
>> The omap_ads7846_init() function frees the pendown GPIO in case there is
>> no ->get_pendown_state() function set by the caller (board specific
>> code), so it can be requested later by the ads7846 driver.
>> The above commit add a common ->get_pendown_state() function without
>> removing the gpio_free() call and thus once the ads7846 driver tries
>> to use the pendown GPIO, it crashes as the pendown GPIO has not been
>> requested.
>>
>> 3) The above commit introduces NO new functionality as
>> get_pendown_state() function is already implemented in a suitable way by
>> the ads7846 driver and the debounce time handling has already been
>> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
> Igor,
>
> I suspect these two patches(this and 97ee9f01) works well, and there
> is something new introduced in ads7846.c, I'll try to look at that new
> stuff in this weekend.
>
> Please refer to in-line comments:
>
> Regards,
> Zumeng
>>
>> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>>
>> Conflicts:
>> arch/arm/mach-omap2/common-board-devices.c
>>
>> Solved by taking the working version prior to the above commit.
>>
>> Cc: Zumeng Chen <zumeng.chen@windriver.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>> Kevin, sorry for the late reply.
>> How about the above commit message and the below patch?
>>
>> The patch applies cleanly to Tony's master branch (6 Aug 2012)
>> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
>> after resetting two top most commits.
>>
>> This patch has been tested on both above branches on cm-t3730.
>> Any other board tested will be greately appreciated.
>>
>> Also, if after all said in the commit message, you still don't
>> want to revert the original patch, feel free to post your thoughts.
>>
>> arch/arm/mach-omap2/board-omap3evm.c | 1 +
>> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
>> arch/arm/mach-omap2/common-board-devices.h | 1 -
>> 3 files changed, 1 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
>> index ef230a0..0d362e9 100644
>> --- a/arch/arm/mach-omap2/board-omap3evm.c
>> +++ b/arch/arm/mach-omap2/board-omap3evm.c
>> @@ -58,6 +58,7 @@
>> #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 1473474..c187586 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.c
>> +++ b/arch/arm/mach-omap2/common-board-devices.c
>> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
>> .turbo_mode = 0,
>> };
>>
>> -/*
>> - * ADS7846 driver maybe request a gpio according to the value
>> - * of pdata->get_pendown_state, but we have done this. So set
>> - * get_pendown_state to avoid twice gpio requesting.
>> - */
>> -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,
>> .y_max = 0x0fff,
>> @@ -55,7 +45,6 @@ static struct ads7846_platform_data ads7846_config = {
>> .debounce_rep = 1,
>> .gpio_pendown = -EINVAL,
>> .keep_vref_on = 1,
>> - .get_pendown_state = &omap3_get_pendown_state,
> You remove this, then please look at the following codes:
>
> drivers/input/touchscreen/ads7846.c
>
> 969 if (pdata->get_pendown_state) {
> 970 ts->get_pendown_state = pdata->get_pendown_state;
> 971 } else if (gpio_is_valid(pdata->gpio_pendown)) {
> 972
> 973 err = gpio_request_one(pdata->gpio_pendown, GPIOF_IN,
> ^^^, the driver will want to request again, then it should
> be error if i'm right.
This patch removes the get_pendown_state function pointer from
the ads7846_platform_data and gpio_free() fires as there is no
get_pendown_state and therefore there will be no problem to request
it again by the ads7846 driver.
So, no there will be no error and if you still doubt, please test.
>
> 974 "ads7846_pendown");
> 975 if (err) {
> 976 dev_err(&spi->dev,
> 977 "failed to request/setup pendown GPIO%d: %d\n",
> 978 pdata->gpio_pendown, err);
> 979 return err;
> 980 }
> 981
> 982 ts->gpio_pendown = pdata->gpio_pendown;
> 983
> 984 } else {
[...]
--
Regards,
Igor.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 42+ messages in thread
* [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"
@ 2012-08-07 6:05 ` Igor Grinberg
0 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-08-07 6:05 UTC (permalink / raw)
To: linux-arm-kernel
On 08/07/12 04:42, Zumeng Chen wrote:
> ? 2012?08?07? 04:22, Igor Grinberg ??:
>> 1) The above commit introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM and thus broke most other boards using this code.
>>
>> 2) The above commit was mis-merged introducing another bug which
>> prevents the ads7846 driver probe function to succeed.
>> The omap_ads7846_init() function frees the pendown GPIO in case there is
>> no ->get_pendown_state() function set by the caller (board specific
>> code), so it can be requested later by the ads7846 driver.
>> The above commit add a common ->get_pendown_state() function without
>> removing the gpio_free() call and thus once the ads7846 driver tries
>> to use the pendown GPIO, it crashes as the pendown GPIO has not been
>> requested.
>>
>> 3) The above commit introduces NO new functionality as
>> get_pendown_state() function is already implemented in a suitable way by
>> the ads7846 driver and the debounce time handling has already been
>> fixed by commit 97ee9f01 (ARM: OMAP: fix the ads7846 init code).
> Igor,
>
> I suspect these two patches(this and 97ee9f01) works well, and there
> is something new introduced in ads7846.c, I'll try to look at that new
> stuff in this weekend.
>
> Please refer to in-line comments:
>
> Regards,
> Zumeng
>>
>> This reverts commit 16aced80f6739beb2a6ff7b6f96c83ba80d331e8.
>>
>> Conflicts:
>> arch/arm/mach-omap2/common-board-devices.c
>>
>> Solved by taking the working version prior to the above commit.
>>
>> Cc: Zumeng Chen <zumeng.chen@windriver.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>> Kevin, sorry for the late reply.
>> How about the above commit message and the below patch?
>>
>> The patch applies cleanly to Tony's master branch (6 Aug 2012)
>> or Kevin's kevin-omap-pm/for_3.6/fixes/ads7846
>> after resetting two top most commits.
>>
>> This patch has been tested on both above branches on cm-t3730.
>> Any other board tested will be greately appreciated.
>>
>> Also, if after all said in the commit message, you still don't
>> want to revert the original patch, feel free to post your thoughts.
>>
>> arch/arm/mach-omap2/board-omap3evm.c | 1 +
>> arch/arm/mach-omap2/common-board-devices.c | 11 -----------
>> arch/arm/mach-omap2/common-board-devices.h | 1 -
>> 3 files changed, 1 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
>> index ef230a0..0d362e9 100644
>> --- a/arch/arm/mach-omap2/board-omap3evm.c
>> +++ b/arch/arm/mach-omap2/board-omap3evm.c
>> @@ -58,6 +58,7 @@
>> #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 1473474..c187586 100644
>> --- a/arch/arm/mach-omap2/common-board-devices.c
>> +++ b/arch/arm/mach-omap2/common-board-devices.c
>> @@ -35,16 +35,6 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
>> .turbo_mode = 0,
>> };
>>
>> -/*
>> - * ADS7846 driver maybe request a gpio according to the value
>> - * of pdata->get_pendown_state, but we have done this. So set
>> - * get_pendown_state to avoid twice gpio requesting.
>> - */
>> -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,
>> .y_max = 0x0fff,
>> @@ -55,7 +45,6 @@ static struct ads7846_platform_data ads7846_config = {
>> .debounce_rep = 1,
>> .gpio_pendown = -EINVAL,
>> .keep_vref_on = 1,
>> - .get_pendown_state = &omap3_get_pendown_state,
> You remove this, then please look at the following codes:
>
> drivers/input/touchscreen/ads7846.c
>
> 969 if (pdata->get_pendown_state) {
> 970 ts->get_pendown_state = pdata->get_pendown_state;
> 971 } else if (gpio_is_valid(pdata->gpio_pendown)) {
> 972
> 973 err = gpio_request_one(pdata->gpio_pendown, GPIOF_IN,
> ^^^, the driver will want to request again, then it should
> be error if i'm right.
This patch removes the get_pendown_state function pointer from
the ads7846_platform_data and gpio_free() fires as there is no
get_pendown_state and therefore there will be no problem to request
it again by the ads7846 driver.
So, no there will be no error and if you still doubt, please test.
>
> 974 "ads7846_pendown");
> 975 if (err) {
> 976 dev_err(&spi->dev,
> 977 "failed to request/setup pendown GPIO%d: %d\n",
> 978 pdata->gpio_pendown, err);
> 979 return err;
> 980 }
> 981
> 982 ts->gpio_pendown = pdata->gpio_pendown;
> 983
> 984 } else {
[...]
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-26 19:30 ` Kevin Hilman
@ 2012-07-27 18:58 ` Igor Grinberg
-1 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-07-27 18:58 UTC (permalink / raw)
To: Kevin Hilman
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Arnd Bergmann, Zumeng Chen
On 07/26/12 22:30, Kevin Hilman wrote:
> + Zumeng Chen
>
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> Hi Kevin,
>>
>> I've just noticed that the patch has been modified by Arnd in a way
>> that of course will trigger GPIO use without being requested.
>> I'm sorry, I was not available by that time Arnd changed the patch.
>
> Your right, your original patch isn't the problem. I found the root
> cause.
>
> The real problem is actually introduced by the merge of your patch from
> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
> ads7846) from the arm-soc/boards branch.
>
> However, looking closer at the one from Zumeng, that one is clearly not
> right. It unconditionally adds a *board-specific* ->get_pendown_state
> function to the pdata that is common to *all* boards. That's just wrong
> and has the side-effect of making ->get_pendown_state() wrong on every
> board except the OMAP3EVM. Oops.
>
> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
> GPIO working on non OMAP3EVM boards, we also need something like this as
> well.
>
> Igor, Zumeng, could you try this out on your boards anc confirm if it's
> working? I currently don't have a board setup with a touchscreen in my
> board farm.
>
> Kevin
>
>>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 26 Jul 2012 12:15:38 -0700
> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
> on all boards
>
> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
> time for ads7846) introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM.
>
> Fix this up to work for all boards that pass in a pendown GPIO.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
Tested-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 4 +++-
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 0fee95f..06f176f 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> * of pdata->get_pendown_state, but we have done this. So set
> * get_pendown_state to avoid twice gpio requesting.
> */
> +static int omap3_gpio_pendown;
> static int omap3_get_pendown_state(void)
> {
> - return !gpio_get_value(OMAP3_EVM_TS_GPIO);
> + return !gpio_get_value(omap3_gpio_pendown);
> }
>
> static struct ads7846_platform_data ads7846_config = {
> @@ -74,6 +75,7 @@ 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;
>
> + omap3_gpio_pendown = gpio_pendown;
> err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
> if (err) {
> pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-27 18:58 ` Igor Grinberg
0 siblings, 0 replies; 42+ messages in thread
From: Igor Grinberg @ 2012-07-27 18:58 UTC (permalink / raw)
To: linux-arm-kernel
On 07/26/12 22:30, Kevin Hilman wrote:
> + Zumeng Chen
>
> Igor Grinberg <grinberg@compulab.co.il> writes:
>
>> Hi Kevin,
>>
>> I've just noticed that the patch has been modified by Arnd in a way
>> that of course will trigger GPIO use without being requested.
>> I'm sorry, I was not available by that time Arnd changed the patch.
>
> Your right, your original patch isn't the problem. I found the root
> cause.
>
> The real problem is actually introduced by the merge of your patch from
> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
> ads7846) from the arm-soc/boards branch.
>
> However, looking closer at the one from Zumeng, that one is clearly not
> right. It unconditionally adds a *board-specific* ->get_pendown_state
> function to the pdata that is common to *all* boards. That's just wrong
> and has the side-effect of making ->get_pendown_state() wrong on every
> board except the OMAP3EVM. Oops.
>
> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
> GPIO working on non OMAP3EVM boards, we also need something like this as
> well.
>
> Igor, Zumeng, could you try this out on your boards anc confirm if it's
> working? I currently don't have a board setup with a touchscreen in my
> board farm.
>
> Kevin
>
>>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 26 Jul 2012 12:15:38 -0700
> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
> on all boards
>
> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
> time for ads7846) introduced a common ->get_pendown_state() function
> into the generic code, but that function was board-specific for the
> OMAP3EVM.
>
> Fix this up to work for all boards that pass in a pendown GPIO.
>
> Cc: Zumeng Chen <zumeng.chen@windriver.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
Tested-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> arch/arm/mach-omap2/board-omap3evm.c | 1 +
> arch/arm/mach-omap2/common-board-devices.c | 4 +++-
> arch/arm/mach-omap2/common-board-devices.h | 1 -
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index ef230a0..0d362e9 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -58,6 +58,7 @@
> #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 0fee95f..06f176f 100644
> --- a/arch/arm/mach-omap2/common-board-devices.c
> +++ b/arch/arm/mach-omap2/common-board-devices.c
> @@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> * of pdata->get_pendown_state, but we have done this. So set
> * get_pendown_state to avoid twice gpio requesting.
> */
> +static int omap3_gpio_pendown;
> static int omap3_get_pendown_state(void)
> {
> - return !gpio_get_value(OMAP3_EVM_TS_GPIO);
> + return !gpio_get_value(omap3_gpio_pendown);
> }
>
> static struct ads7846_platform_data ads7846_config = {
> @@ -74,6 +75,7 @@ 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;
>
> + omap3_gpio_pendown = gpio_pendown;
> err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown");
> if (err) {
> pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err);
> diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h
> index 4c4ef6a..a0b4a428 100644
> --- a/arch/arm/mach-omap2/common-board-devices.h
> +++ b/arch/arm/mach-omap2/common-board-devices.h
> @@ -4,7 +4,6 @@
> #include "twl-common.h"
>
> #define NAND_BLOCK_SIZE SZ_128K
> -#define OMAP3_EVM_TS_GPIO 175
>
> struct mtd_partition;
> struct ads7846_platform_data;
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
2012-07-27 18:58 ` Igor Grinberg
@ 2012-07-27 19:05 ` Kevin Hilman
-1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-27 19:05 UTC (permalink / raw)
To: Igor Grinberg
Cc: Tony Lindgren, linux-omap, linux-arm-kernel, Arnd Bergmann, Zumeng Chen
Igor Grinberg <grinberg@compulab.co.il> writes:
> On 07/26/12 22:30, Kevin Hilman wrote:
>> + Zumeng Chen
>>
>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>
>>> Hi Kevin,
>>>
>>> I've just noticed that the patch has been modified by Arnd in a way
>>> that of course will trigger GPIO use without being requested.
>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>
>> Your right, your original patch isn't the problem. I found the root
>> cause.
>>
>> The real problem is actually introduced by the merge of your patch from
>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>> ads7846) from the arm-soc/boards branch.
>>
>> However, looking closer at the one from Zumeng, that one is clearly not
>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>> function to the pdata that is common to *all* boards. That's just wrong
>> and has the side-effect of making ->get_pendown_state() wrong on every
>> board except the OMAP3EVM. Oops.
>>
>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>> GPIO working on non OMAP3EVM boards, we also need something like this as
>> well.
>>
>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>> working? I currently don't have a board setup with a touchscreen in my
>> board farm.
>>
>> Kevin
>>
>>>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
>> From: Kevin Hilman <khilman@ti.com>
>> Date: Thu, 26 Jul 2012 12:15:38 -0700
>> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
>> on all boards
>>
>> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
>> time for ads7846) introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM.
>>
>> Fix this up to work for all boards that pass in a pendown GPIO.
>>
>> Cc: Zumeng Chen <zumeng.chen@windriver.com>
>> Cc: Igor Grinberg <grinberg@compulab.co.il>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>
> Tested-by: Igor Grinberg <grinberg@compulab.co.il>
Thanks to both of you for testing, I've updated the branch to include
the tags, and will submit during the v3.6-rc cycle.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
@ 2012-07-27 19:05 ` Kevin Hilman
0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2012-07-27 19:05 UTC (permalink / raw)
To: linux-arm-kernel
Igor Grinberg <grinberg@compulab.co.il> writes:
> On 07/26/12 22:30, Kevin Hilman wrote:
>> + Zumeng Chen
>>
>> Igor Grinberg <grinberg@compulab.co.il> writes:
>>
>>> Hi Kevin,
>>>
>>> I've just noticed that the patch has been modified by Arnd in a way
>>> that of course will trigger GPIO use without being requested.
>>> I'm sorry, I was not available by that time Arnd changed the patch.
>>
>> Your right, your original patch isn't the problem. I found the root
>> cause.
>>
>> The real problem is actually introduced by the merge of your patch from
>> the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
>> 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
>> ads7846) from the arm-soc/boards branch.
>>
>> However, looking closer at the one from Zumeng, that one is clearly not
>> right. It unconditionally adds a *board-specific* ->get_pendown_state
>> function to the pdata that is common to *all* boards. That's just wrong
>> and has the side-effect of making ->get_pendown_state() wrong on every
>> board except the OMAP3EVM. Oops.
>>
>> So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
>> GPIO working on non OMAP3EVM boards, we also need something like this as
>> well.
>>
>> Igor, Zumeng, could you try this out on your boards anc confirm if it's
>> working? I currently don't have a board setup with a touchscreen in my
>> board farm.
>>
>> Kevin
>>
>>>From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
>> From: Kevin Hilman <khilman@ti.com>
>> Date: Thu, 26 Jul 2012 12:15:38 -0700
>> Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix ->get_pendown_state() to work
>> on all boards
>>
>> commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
>> time for ads7846) introduced a common ->get_pendown_state() function
>> into the generic code, but that function was board-specific for the
>> OMAP3EVM.
>>
>> Fix this up to work for all boards that pass in a pendown GPIO.
>>
>> Cc: Zumeng Chen <zumeng.chen@windriver.com>
>> Cc: Igor Grinberg <grinberg@compulab.co.il>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>
> Tested-by: Igor Grinberg <grinberg@compulab.co.il>
Thanks to both of you for testing, I've updated the branch to include
the tags, and will submit during the v3.6-rc cycle.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 42+ messages in thread