<<snip>>
>> > diff --git a/drivers/input/touchscreen/ads7846.c
> b/drivers/input/touchscreen/ads7846.c
>> > index 14ea54b..036f245 100644
>> > --- a/drivers/input/touchscreen/ads7846.c
>> > +++ b/drivers/input/touchscreen/ads7846.c
>> > @@ -1221,6 +1221,7 @@ static int __devinit ads7846_probe(struct
> spi_device *spi)
>> > ts->input = input_dev;
>> > ts->vref_mv = pdata->vref_mv;
>> > ts->swap_xy = pdata->swap_xy;
>> > + ts->gpio_pendown = -1;
>>
>> Going through the code, I see that ts->gpio_pendown is being filled
>> in ads7846_setup_pendown() which in turn is called by the
>> ads7846_probe().
>>
>> Actually in ads7846_dev_init() [board-3430sdp.c], the gpio_pendown
>> pin has been requested already. The problem is with the following
>> line in ads7846_probe():
>> if (pdata->get_pendown_state) {
>> ts->get_pendown_state = pdata->get_pendown_state;
>> return 0;
>> }
>>
>> Instead it should have been
>> if (pdata->get_pendown_state( )) {
>> ts->get_pendown_state = pdata->get_pendown_state;
>> return 0;
>> }
>>
>
> This is not correct. This section of the code is simply assigning
> the function pointer that platform data passed down to the
> local structure. So all you need to check is if the platform data
> pointer is valid or not.
>
> So that section of the code is correct - you should not call
> pdata->get_pendown_state(). It is unnecessary during probe.
Okay.
>
>
>> Also, filling ts->gpio_pendown = -1 should not be done in the
>> driver file.
>> Instead pdata should either send a valid GPIO pin number or an -1
>> if it is not applicable to the board and this has to be done
>> in the board file.
>
> I'm not very familiar with this driver, but it looks to me like
> there is no hard and fast rule that a gpio be passed down from
> platform files.
>
> The platform file could just as well pass down a way to get
> the pendown state directly (which is what board-3430sdp.c does).
>
>> Also in board-3430sdp.c file, I observed that pdata->gpio_pendown
>> is not filled at all. But the ads7846_setup_pendown() in the
>> driver relies on pdata->gpio_pendown. This also needs to be fixed.
>
> ads7846_setup_pendown calls the get_pendown_state function pointer
> if the platform provided one, or requests the gpio to use if
> the platform provided one. The platform could provide either one,
> and doesn't necessarily have to provide both (I'm not sure).
>
> ads7846_setup_pendown doesn't populate the ts->gpio_pendown at all,
> if there was a get_pendown_state pointer passed. So fixing the
> platform files will not be enough. This is the bug that Sourav's
> patch appears to fix.
Okay.
--
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
