On Tue, May 25, 2010 at 03:18:13PM -0700, Kevin Hilman wrote:
> Mike Frysinger <vapier.adi@xxxxxxxxx> writes:
>
> > On Tue, May 25, 2010 at 17:46, Kevin Hilman wrote:
> >> After digging into the driver core and realizing that it seemed to
> >> have sane error handling itself, I took a closer look at
> >> ads7846_probe() and discovered it doesn't actually return an error
> >> code for certain failure cases! That was the root cause.
> >
> > that is crappy
> >
> >> Subject: [PATCH] input: touchscreen: ads7846: return error on probe failure
> >
> > i'd refer to the specific probe issue rather than just "probe". maybe:
> > input: touchscreen: ads7846: return error on regulator_get() failure
>
> Thanks for the review, here's one with updated subject and ack added.
>
> Kevin
>
> From 8ce49a91341d8713f870d2a931969f227a82b8ad Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> Date: Tue, 25 May 2010 14:38:04 -0700
> Subject: [PATCH] input: touchscreen: ads7846: return error on regulator_get() failure
>
> In probe(), if regulator_get() failed, an error code was not being
> returned causing the driver to be successfully bound, even though
> probe failed. This in turn caused the suspend, resume and remove
> methods to be registered and accessed via the SPI core. Since these
> functions all access private driver data using pointers that had been
> freed during the failed probe, this would lead to unpredictable
> behavior.
>
> This patch ensures that probe() returns an error code in this failure
> case so the driver is not bound.
>
> Found using lockdep and noticing the lock used in the suspend/resum
> path pointed to a bogus lock due to the freed memory.
>
> Signed-off-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> Acked-by: Mike Frysinger <vapier@xxxxxxxxxx>
OK, this makes much better sense. Will aplly, thank you Kevin.
> ---
> drivers/input/touchscreen/ads7846.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index 532279c..634f6f6 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -1163,8 +1163,8 @@ static int __devinit ads7846_probe(struct spi_device *spi)
>
> ts->reg = regulator_get(&spi->dev, "vcc");
> if (IS_ERR(ts->reg)) {
> - dev_err(&spi->dev, "unable to get regulator: %ld\n",
> - PTR_ERR(ts->reg));
> + err = PTR_ERR(ts->reg);
> + dev_err(&spi->dev, "unable to get regulator: %ld\n", err);
> goto err_free_gpio;
> }
>
> --
> 1.7.0.2
>
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
