> > I wonder if it makes sense to merge both patches under the name of "fix > > gpio-handling" or similar. Not sure, though... > > I'd rather not do that, because this patch fixes the request/free problem > and the second is changing the functionality (e.g. configures the gpio as input) Ack. > > >> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c > >> index 14ea54b..ce5baee 100644 > >> --- a/drivers/input/touchscreen/ads7846.c > >> +++ b/drivers/input/touchscreen/ads7846.c > >> @@ -952,6 +952,7 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 > >> > >> if (pdata->get_pendown_state) { > >> ts->get_pendown_state = pdata->get_pendown_state; > >> + ts->gpio_pendown = -EINVAL; > >> return 0; > >> } > > Will probably work, but maybe it is better to reorganize the code to > > just have one success-exit-point. That would be mean adding an else > > branch to this if-block. > > This is something that can be done, though I fear the code readability > will suffer. Is it worth? I thought it to be more readable to have one-entry-one-OK-exit. But actually I don't mind that much. > > >> > >> @@ -1353,7 +1354,7 @@ static int __devinit ads7846_probe(struct spi_device *spi) > >> err_put_regulator: > >> regulator_put(ts->reg); > >> err_free_gpio: > >> - if (ts->gpio_pendown != -1) > >> + if (gpio_is_valid(ts->gpio_pendown)) > > You could do the same in the remove-path. > > You mean, _should_... ;) Yes, I meant that :) -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature