On 08/06/12 23:52, Kevin Hilman wrote:

Igor Grinberg [off-list ref] writes:

quoted

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 <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Igor Grinberg <redacted>
---
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.

quoted

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 <redacted>
Tested-by: Kevin Hilman <redacted>

Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.

Thanks!

[...]


-- 
Regards,
Igor.