[PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO

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.

So that is not my original patch that is triggering the issue,
but it does not meter already...
And currently, I think that the proper solution would be
to remove the gpio_free() call as your patch does, but.......

Tony, please pay attention to which branch it gets applied....
It should only apply to the branches that have the patch modified
by Arnd and not to those with unmodified patch
(e.g. fixes-non-critical should not receive the patch).

Kevin, thanks for the patch.

On 07/23/12 15:53, Igor Grinberg wrote:

On 07/12/12 02:18, Kevin Hilman wrote:

quoted

commit 97ee9f01d6 (ARM: OMAP: fix the ads7846 init code) mistakenly
frees the pen-down GPIO even though it will be used by the ads7846
driver.

But the driver requests the GPIO in ads7846_setup_pendown() method...

quoted

Freeing a GPIO means that the GPIO bank containing that GPIO can be
runtime suspended if its the last/only GPIO being used in that bank.
If the GPIO bank is runtime suspended, any accesses to that bank will
cause faults.

Unless, it is requested again, or am I missing something?

quoted

Because the current code frees the GPIO, the ads7846 driver probe will
fault when it requests its IRQ line.

But it requests that GPIO... in ads7846_setup_pendown()
prior to requesting the IRQ...

quoted

Because the IRQ is a GPIO line,
the request IRQ will trickle down into the OMAP GPIO layer:
gpio_irq_type() --> _set_gpio_triggering() which can fault if the
bank has been runtime suspended.

This is exctly what happens on Overo platforms (3530 Water, 3730 Overo
FireSTORM) since this is the only GPIO used in the bank.

To fix, don't free the GPIO at all since it is always in use.

Cc: Igor Grinberg <redacted>
Signed-off-by: Kevin Hilman <redacted>

Acked-by: Igor Grinberg <redacted>

quoted

---
Tony, this applies on top of your current fixes-non-critical branch and
should probably go in to v3.5-rc since the patch which introduced the
problem did as well.

No, this patch is not good for fixes-non-critical,
as fixes-non-critical has unmodified version and
will result in double requesting.
It should be applied to master or derivatives that
have the modified patch version.

quoted

 arch/arm/mach-omap2/common-board-devices.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c
index c187586..1ae6fd6 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c

@@ -84,9 +84,6 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce,
 		ads7846_config.gpio_pendown = gpio_pendown;
 	}
 
-	if (!board_pdata || (board_pdata && !board_pdata->get_pendown_state))
-		gpio_free(gpio_pendown);
-
 	spi_register_board_info(&ads7846_spi_board_info, 1);
 }
 #else

-- 
Regards,
Igor.