[PATCH] Input: Make ADS7846 independent on regulator

[PATCH] Input: Make ADS7846 independent on regulator

[Marek Vasut]

In case regulator was not found, ADS7846 failed to probe.

This fixes a problem on some Sharp Zaurus devices, where there is no regulator
present and yet the touchscreen is used.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/input/touchscreen/ads7846.c |   50 +++++++++++++++++++++++------------
 1 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index a9fdf55..dafaef6 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -791,7 +791,10 @@ static void ads7846_disable(struct ads7846 *ts)
 		}
 	}
 
-	regulator_disable(ts->reg);
+#ifdef	CONFIG_REGULATOR
+	if (ts->reg)
+		regulator_disable(ts->reg);
+#endif
 
 	/* we know the chip's in lowpower mode since we always
 	 * leave it that way after every request
@@ -804,7 +807,10 @@ static void ads7846_enable(struct ads7846 *ts)
 	if (!ts->disabled)
 		return;
 
-	regulator_enable(ts->reg);
+#ifdef	CONFIG_REGULATOR
+	if (ts->reg)
+		regulator_enable(ts->reg);
+#endif
 
 	ts->disabled = 0;
 	ts->irq_disabled = 0;
@@ -1161,18 +1167,21 @@ static int __devinit ads7846_probe(struct spi_device *spi)
 
 	ts->last_msg = m;
 
+#ifdef	CONFIG_REGULATOR
 	ts->reg = regulator_get(&spi->dev, "vcc");
-	if (IS_ERR(ts->reg)) {
-		err = PTR_ERR(ts->reg);
-		dev_err(&spi->dev, "unable to get regulator: %d\n", err);
-		goto err_free_gpio;
-	}
-
-	err = regulator_enable(ts->reg);
-	if (err) {
-		dev_err(&spi->dev, "unable to enable regulator: %d\n", err);
-		goto err_put_regulator;
+	if (IS_ERR(ts->reg))
+		ts->reg = NULL;
+	else {
+		err = regulator_enable(ts->reg);
+		if (err) {
+			dev_err(&spi->dev, "unable to enable regulator: %d\n",
+				err);
+			goto err_put_regulator;
+		}
 	}
+#else
+	ts->reg = NULL;
+#endif
 
 	if (request_irq(spi->irq, ads7846_irq, IRQF_TRIGGER_FALLING,
 			spi->dev.driver->name, ts)) {
@@ -1218,10 +1227,13 @@ static int __devinit ads7846_probe(struct spi_device *spi)
  err_free_irq:
 	free_irq(spi->irq, ts);
  err_disable_regulator:
-	regulator_disable(ts->reg);
+#ifdef	CONFIG_REGULATOR
+	if (ts->reg)
+		regulator_disable(ts->reg);
  err_put_regulator:
-	regulator_put(ts->reg);
- err_free_gpio:
+	if (ts->reg)
+		regulator_put(ts->reg);
+#endif
 	if (ts->gpio_pendown != -1)
 		gpio_free(ts->gpio_pendown);
  err_cleanup_filter:
@@ -1251,8 +1263,12 @@ static int __devexit ads7846_remove(struct spi_device *spi)
 	/* suspend left the IRQ disabled */
 	enable_irq(ts->spi->irq);
 
-	regulator_disable(ts->reg);
-	regulator_put(ts->reg);
+#ifdef	CONFIG_REGULATOR
+	if (ts->reg) {
+		regulator_disable(ts->reg);
+		regulator_put(ts->reg);
+	}
+#endif
 
 	if (ts->gpio_pendown != -1)
 		gpio_free(ts->gpio_pendown);
-- 
1.7.1

Re: [PATCH] Input: Make ADS7846 independent on regulator

[Dmitry Torokhov]

Hi Marek,

On Sat, Jul 31, 2010 at 07:45:08AM +0200, Marek Vasut wrote:
> In case regulator was not found, ADS7846 failed to probe.
> 
> This fixes a problem on some Sharp Zaurus devices, where there is no regulator
> present and yet the touchscreen is used.
> 

Too many ifdefs to my taste... Can we have ads7846_regulator_enable()
and friends stubbed out in case !CONFIG_REGULATOR?

-- 
Dmitry

Re: [PATCH] Input: Make ADS7846 independent on regulator

[Marek Vasut]

Dne So 31. července 2010 08:33:29 Dmitry Torokhov napsal(a):
> Hi Marek,
> 
> On Sat, Jul 31, 2010 at 07:45:08AM +0200, Marek Vasut wrote:
> > In case regulator was not found, ADS7846 failed to probe.
> > 
> > This fixes a problem on some Sharp Zaurus devices, where there is no
> > regulator present and yet the touchscreen is used.
> 
> Too many ifdefs to my taste... Can we have ads7846_regulator_enable()
> and friends stubbed out in case !CONFIG_REGULATOR?

Hi,

I believe you should ask the regulator guys about these. We can certainly just 
remove the ifdefs (I was thinking about it, because it poluted the code quality 
quite a bit). Just removing the ifdefs shouldn't cause any harm.

Cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input: Make ADS7846 independent on regulator

[Linus Walleij]

2010/7/31 Marek Vasut <marek.vasut@gmail.com>:

> This fixes a problem on some Sharp Zaurus devices, where there is no regulator
> present and yet the touchscreen is used.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  drivers/input/touchscreen/ads7846.c |   50 +++++++++++++++++++++++------------
>  1 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index a9fdf55..dafaef6 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -791,7 +791,10 @@ static void ads7846_disable(struct ads7846 *ts)
>                }
>        }
>
> -       regulator_disable(ts->reg);
> +#ifdef CONFIG_REGULATOR

Skip the #ifdef, if the regulators are configured out, there will be
a stub providing NULL for all get_regulator() calls anyway.

> +       if (ts->reg)
> +               regulator_disable(ts->reg);
> +#endif
>
>        /* we know the chip's in lowpower mode since we always
>         * leave it that way after every request
> @@ -804,7 +807,10 @@ static void ads7846_enable(struct ads7846 *ts)
>        if (!ts->disabled)
>                return;
>
> -       regulator_enable(ts->reg);
> +#ifdef CONFIG_REGULATOR

Skip the #ifdef.

> +       if (ts->reg)
> +               regulator_enable(ts->reg);
> +#endif
>
>        ts->disabled = 0;
>        ts->irq_disabled = 0;
> @@ -1161,18 +1167,21 @@ static int __devinit ads7846_probe(struct spi_device *spi)
>
>        ts->last_msg = m;
>
> +#ifdef CONFIG_REGULATOR

Skip the #ifdef.

>        ts->reg = regulator_get(&spi->dev, "vcc");
> -       if (IS_ERR(ts->reg)) {
> -               err = PTR_ERR(ts->reg);
> -               dev_err(&spi->dev, "unable to get regulator: %d\n", err);
> -               goto err_free_gpio;
> -       }
> -
> -       err = regulator_enable(ts->reg);
> -       if (err) {
> -               dev_err(&spi->dev, "unable to enable regulator: %d\n", err);
> -               goto err_put_regulator;
> +       if (IS_ERR(ts->reg))
> +               ts->reg = NULL;
> +       else {
> +               err = regulator_enable(ts->reg);
> +               if (err) {
> +                       dev_err(&spi->dev, "unable to enable regulator: %d\n",
> +                               err);
> +                       goto err_put_regulator;
> +               }
>        }
> +#else
> +       ts->reg = NULL;
> +#endif

Delete the #else/#endif clause entirely.

>
>        if (request_irq(spi->irq, ads7846_irq, IRQF_TRIGGER_FALLING,
>                        spi->dev.driver->name, ts)) {
> @@ -1218,10 +1227,13 @@ static int __devinit ads7846_probe(struct spi_device *spi)
>  err_free_irq:
>        free_irq(spi->irq, ts);
>  err_disable_regulator:
> -       regulator_disable(ts->reg);
> +#ifdef CONFIG_REGULATOR

Skip the #ifdef.

> +       if (ts->reg)
> +               regulator_disable(ts->reg);
>  err_put_regulator:
> -       regulator_put(ts->reg);
> - err_free_gpio:
> +       if (ts->reg)
> +               regulator_put(ts->reg);
> +#endif
>        if (ts->gpio_pendown != -1)
>                gpio_free(ts->gpio_pendown);
>  err_cleanup_filter:
> @@ -1251,8 +1263,12 @@ static int __devexit ads7846_remove(struct spi_device *spi)
>        /* suspend left the IRQ disabled */
>        enable_irq(ts->spi->irq);
>
> -       regulator_disable(ts->reg);
> -       regulator_put(ts->reg);
> +#ifdef CONFIG_REGULATOR

Skip the #ifdef.

> +       if (ts->reg) {
> +               regulator_disable(ts->reg);
> +               regulator_put(ts->reg);
> +       }
> +#endif
>
>        if (ts->gpio_pendown != -1)
>                gpio_free(ts->gpio_pendown);
> --
> 1.7.1

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input: Make ADS7846 independent on regulator

[Mark Brown]

On Wed, Oct 06, 2010 at 07:16:05PM +0200, Linus Walleij wrote:
> 2010/7/31 Marek Vasut <marek.vasut@gmail.com>:

> > +#ifdef CONFIG_REGULATOR

> Skip the #ifdef, if the regulators are configured out, there will be
> a stub providing NULL for all get_regulator() calls anyway.

While ifdefs on CONFIG_REGULATOR should indeed be skipped I'd not expect
the stubs to be returning NULL for anything (but then as discussed
elsewhere this whole approach of doing everything in the drivers is a
poor one).