Re: [PATCH] input/touch: Introduce the LPC32xx touchscreen controller driver

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Aug 13, 2010 at 12:00:33PM -0700, Kevin Wells wrote:
> >>
> >> This patch set introduces support for the LPC32xx touchscreen
> >> controller driver. The LPC32xx touchscreen controller supports
> >> automated event detection and X/Y data conversion for resistive
> >> touchscreens.
> >>
> >
> > Overall looks very nice, a few comments below.
> >
> 
> Thanks for helping review this driver. I'll update and repost once
> the fixes and changes are finalized.
> 
> >> +
> >> +     if (input_register_device(tsc->dev)) {
> >> +             dev_err(&pdev->dev, "failed registering input device\n");
> >> +             goto err_stop_clk;
> >
> > retval is garbage here, you need to do:
> >
> >        error = input_register_device();
> >        if (error) {
> >                ...
> >                goto err_stop_clk;
> >        }
> >
> > I must say that I do not like mixing devm_* with the standard error path
> > unwinding, if we can rely on devm_xxx() calls to take care of everything
> > we should revert to standard error unsinding practice for everythng.
> >
> 
> Would it be preferable to just use standard resource functions and error
> path unwinding (with fixes in _remove too) similar to the other drivers in
> ./drivers/input?

Yes, that what I was trying to say. If only I didn't make so many
typos...

-- 
Dmitry
--
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