On Thu, Sep 4, 2008 at 6:00 PM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> On Thu, Sep 04, 2008 at 05:21:05PM +0800, Eric Miao wrote:
>> The GPIO connected to ADS7846 nPENIRQ signal is usually used to get
>> the pendown state as well. Introduce a .gpio_pendown, and use this
>> to decide the pendown state if .get_pendown_state is NULL.
>
>> +static int get_pendown_state(struct ads7846 *ts)
>> +{
>> + if (ts->get_pendown_state)
>> + return ts->get_pendown_state();
>> +
>> + if (ts->gpio_pendown != -1)
>> + return !gpio_get_value(ts->gpio_pendown);
>> +
>> + return 0;
>> +}
>> +
>
>
>> + int gpio_pendown; /* the GPIO usually connected to PENIRQ
>> + * and used to decide pendown state
>> + * (-1) to use (*get_pendown_state) */
>> +
>
> The comment doesn't match the code. get_pendown_state() is used if it's
> non-NULL, otherwise, the GPIO if it's not -1.
>
> I'm getting a little worried about these tests for valid gpio numbers in
> drivers - some check for non-zero, this one checks for not -1. I get the
> feeling that this is storing up problems for later.
>
> Maybe using gpio_is_valid() would be a good idea?
>
Indeed, I'll fix the other SPI GPIO CS patch as well.
> But... a related question: do we need to do the check here? Surely either
> a pendown function is going to be supplied, or if not we're going to use
> a GPIO, in which case the GPIO better be valid. Maybe something to check
> at driver initialisation time only?
>
There is a warning If none is provided, and probe() will fail.
So actually get_pendown_state() will never reach the last
"return 0" statement.
--
Cheers
- eric
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
