Hi, On 02/03/11 13:28, ABRAHAM, KISHON VIJAY wrote: > On Thu, Feb 3, 2011 at 4:42 PM, Igor Grinberg <grinberg@xxxxxxxxxxxxxx> wrote: >> >> On 02/03/11 13:00, Poddar, Sourav wrote: >> >>> On Thu, Feb 3, 2011 at 3:47 PM, Igor Grinberg <grinberg@xxxxxxxxxxxxxx> wrote: >>>> if (pdata->get_pendown_state) { >>>> ts->get_pendown_state = pdata->get_pendown_state; >>>> ts->gpio_pendown = -1; >>>> return 0; >>>> } >>> Yes we can do so .I initialise it at a place where other variables >>> where initialised. >>> >>>>> Also, why don't we use -EINVAL for the invalid gpio number instead of -1 constant? >>>>> >>> I used -1 because conditional check done in probe ads7846_probe function >>> used this value. >>> >>> err_free_gpio: >>> if (ts->gpio_pendown != -1) >>> gpio_free(ts->gpio_pendown); >>> >> Well I understand that and that's why in my proposal I used -1 also, but >> I thought we can make it even better if we switch to -EINVAL >> (though wanted to check if there are any reasonable objections) >> and while you are at this, may be you are willing also to submit a patch for this? > I guess instead of -EINVAL, -EIO should be initialized to > ts->gpio_pendown since that > would be more appropriate for gpio Well, the common practice is to use -EINVAL for gpio _numbers_, I have not seen anyone using -EIO for this. > and as Balbi suggested it would > be better to use > gpio_is_valid() for checking this error condition. Of course gpio_is_valid() should be used for testing the gpio. The -EINVAL in turn should be used for _setting_ the invalid value for that gpio. My suggestion for -EINVAL is related to the initialization: if (pdata->get_pendown_state) { ts->get_pendown_state = pdata->get_pendown_state; ts->gpio_pendown = -EINVAL; return 0; } -- Regards, Igor. -- 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