Re: [PATCH] Revert "ARM: OMAP3530evm: set pendown_state and debounce time for ads7846"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/06/12 23:52, Kevin Hilman wrote:
> Igor Grinberg <grinberg@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>> Signed-off-by: Igor Grinberg <grinberg@xxxxxxxxxxxxxx>
>> ---
>> 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@xxxxxx>
> Tested-by: Kevin Hilman <khilman@xxxxxx>
> 
> Tested on 3430/n900, 3530/Overo, 3730/Overo STORM, 3730/BB-xM.

Thanks!

[...]


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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux