[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Sourav Poddar]

The ads7846 driver requests a gpio to detect pendown events,
but does not configure its direction. Configure this gpio
as an input after requesting it.

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
changes between v2 and v1: Added return error support in v2. 
 
drivers/input/touchscreen/ads7846.c |    7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 036f245..f30768b 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
 		return err;
 	}
 
+	err = gpio_direction_input(pdata->gpio_pendown);
+	if (err) {
+		dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
+			pdata->gpio_pendown);
+		return err;
+        }
+
 	ts->gpio_pendown = pdata->gpio_pendown;
 
 	return 0;
-- 
1.7.0.4

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Felipe Balbi]

On Thu, Feb 03, 2011 at 03:18:50PM +0530, Sourav Poddar wrote:
> The ads7846 driver requests a gpio to detect pendown events,
> but does not configure its direction. Configure this gpio
> as an input after requesting it.
> 
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> changes between v2 and v1: Added return error support in v2. 
>  
> drivers/input/touchscreen/ads7846.c |    7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index 036f245..f30768b 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
>  		return err;
>  	}
>  
> +	err = gpio_direction_input(pdata->gpio_pendown);
> +	if (err) {
> +		dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
> +			pdata->gpio_pendown);
> +		return err;

and now you miss a gpio_free().

-- 
balbi

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Varadarajan, Charulatha]

Sourav,

On Thu, Feb 3, 2011 at 15:28, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Feb 03, 2011 at 03:18:50PM +0530, Sourav Poddar wrote:
>> The ads7846 driver requests a gpio to detect pendown events,
>> but does not configure its direction. Configure this gpio
>> as an input after requesting it.
>>
>> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
>> ---
>> changes between v2 and v1: Added return error support in v2.
>>
>> drivers/input/touchscreen/ads7846.c | ? ?7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
>> index 036f245..f30768b 100644
>> --- a/drivers/input/touchscreen/ads7846.c
>> +++ b/drivers/input/touchscreen/ads7846.c
>> @@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
>> ? ? ? ? ? ? ? return err;
>> ? ? ? }
>>
>> + ? ? err = gpio_direction_input(pdata->gpio_pendown);
>> + ? ? if (err) {
>> + ? ? ? ? ? ? dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
>> + ? ? ? ? ? ? ? ? ? ? pdata->gpio_pendown);
>> + ? ? ? ? ? ? return err;
>
> and now you miss a gpio_free().

My question to your previous version is still unanswered.
Would it be relevant to add gpio_set_debounce() for this
gpio?

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Wolfram Sang]

On Thu, Feb 03, 2011 at 11:58:45AM +0200, Felipe Balbi wrote:
> On Thu, Feb 03, 2011 at 03:18:50PM +0530, Sourav Poddar wrote:
> > The ads7846 driver requests a gpio to detect pendown events,
> > but does not configure its direction. Configure this gpio
> > as an input after requesting it.
> > 
> > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> > ---
> > changes between v2 and v1: Added return error support in v2. 
> >  
> > drivers/input/touchscreen/ads7846.c |    7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> > index 036f245..f30768b 100644
> > --- a/drivers/input/touchscreen/ads7846.c
> > +++ b/drivers/input/touchscreen/ads7846.c
> > @@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
> >  		return err;
> >  	}
> >  
> > +	err = gpio_direction_input(pdata->gpio_pendown);
> > +	if (err) {
> > +		dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
> > +			pdata->gpio_pendown);
> > +		return err;
> 
> and now you miss a gpio_free().

Better use gpio_request_one()?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110203/1d4304b4/attachment.sig>

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Felipe Balbi]

On Thu, Feb 03, 2011 at 11:10:33AM +0100, Wolfram Sang wrote:
> On Thu, Feb 03, 2011 at 11:58:45AM +0200, Felipe Balbi wrote:
> > On Thu, Feb 03, 2011 at 03:18:50PM +0530, Sourav Poddar wrote:
> > > The ads7846 driver requests a gpio to detect pendown events,
> > > but does not configure its direction. Configure this gpio
> > > as an input after requesting it.
> > > 
> > > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> > > ---
> > > changes between v2 and v1: Added return error support in v2. 
> > >  
> > > drivers/input/touchscreen/ads7846.c |    7 +++++++
> > > 1 files changed, 7 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> > > index 036f245..f30768b 100644
> > > --- a/drivers/input/touchscreen/ads7846.c
> > > +++ b/drivers/input/touchscreen/ads7846.c
> > > @@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
> > >  		return err;
> > >  	}
> > >  
> > > +	err = gpio_direction_input(pdata->gpio_pendown);
> > > +	if (err) {
> > > +		dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
> > > +			pdata->gpio_pendown);
> > > +		return err;
> > 
> > and now you miss a gpio_free().
> 
> Better use gpio_request_one()?

Why not ?!? I'm not sure if $SUBJECT is the best for that though.

-- 
balbi

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Poddar, Sourav]

On Thu, Feb 3, 2011 at 3:40 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Thu, Feb 03, 2011 at 11:58:45AM +0200, Felipe Balbi wrote:
>> On Thu, Feb 03, 2011 at 03:18:50PM +0530, Sourav Poddar wrote:
>> > The ads7846 driver requests a gpio to detect pendown events,
>> > but does not configure its direction. Configure this gpio
>> > as an input after requesting it.
>> >
>> > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
>> > ---
>> > changes between v2 and v1: Added return error support in v2.
>> >
>> > drivers/input/touchscreen/ads7846.c | ? ?7 +++++++
>> > 1 files changed, 7 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
>> > index 036f245..f30768b 100644
>> > --- a/drivers/input/touchscreen/ads7846.c
>> > +++ b/drivers/input/touchscreen/ads7846.c
>> > @@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
>> > ? ? ? ? ? ? return err;
>> > ? ? }
>> >
>> > + ? err = gpio_direction_input(pdata->gpio_pendown);
>> > + ? if (err) {
>> > + ? ? ? ? ? dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
>> > + ? ? ? ? ? ? ? ? ? pdata->gpio_pendown);
>> > + ? ? ? ? ? return err;
>>
>> and now you miss a gpio_free().
>
> Better use gpio_request_one()?

Totally agreed,using gpio_request_one seems to be the better option.
Will post a patch for it.

[PATCHv2 2/2] Input: ads7846 - configure pendown gpio as input

[Poddar, Sourav]

On Thu, Feb 3, 2011 at 3:34 PM, Varadarajan, Charulatha <charu@ti.com> wrote:
> Sourav,
>
> On Thu, Feb 3, 2011 at 15:28, Felipe Balbi <balbi@ti.com> wrote:
>> On Thu, Feb 03, 2011 at 03:18:50PM +0530, Sourav Poddar wrote:
>>> The ads7846 driver requests a gpio to detect pendown events,
>>> but does not configure its direction. Configure this gpio
>>> as an input after requesting it.
>>>
>>> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
>>> ---
>>> changes between v2 and v1: Added return error support in v2.
>>>
>>> drivers/input/touchscreen/ads7846.c | ? ?7 +++++++
>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
>>> index 036f245..f30768b 100644
>>> --- a/drivers/input/touchscreen/ads7846.c
>>> +++ b/drivers/input/touchscreen/ads7846.c
>>> @@ -962,6 +962,13 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784
>>> ? ? ? ? ? ? ? return err;
>>> ? ? ? }
>>>
>>> + ? ? err = gpio_direction_input(pdata->gpio_pendown);
>>> + ? ? if (err) {
>>> + ? ? ? ? ? ? dev_err(&spi->dev, "failed to configure pendown GPIO input direction %d\n",
>>> + ? ? ? ? ? ? ? ? ? ? pdata->gpio_pendown);
>>> + ? ? ? ? ? ? return err;
>>
>> and now you miss a gpio_free().
>
> My question to your previous version is still unanswered.
> Would it be relevant to add gpio_set_debounce() for this
> gpio?
>
 I was also wondering that.I too think that just a request and then an
 configuring direction would do.But,I am not so sure about this one.