Messages in this thread |  | | Date | Mon, 7 Apr 2008 16:16:05 -0400 | From | Dmitry Torokhov <> | Subject | Re: [PATCH 1/1] Input/Touchscreen Driver: add support AD7877 touchscreen driver |
| |
Hi Bryan, Michael,
On Thu, Feb 14, 2008 at 05:17:28PM +0800, Bryan Wu wrote: > From: Michael Hennerich <michael.hennerich@analog.com> > > [try #3] Changlog (Add feedback from Dmitry Torokhov): > - Change handling of spi_sync / spi_async return value handling > - Remove depreciated dev->power.power_state > - Fix error return path in ad7877_probe > - delete pending kernel timer > - Some minor cleanup (indention, use dev_err etc.)
Sorry for the long silence... I have a couple of comments at the moment but I am sure i will have more ;)
> + > + status = spi_sync(spi, &req->msg); > + > + if (status == 0) > + status = req->msg.status; > + > + kfree(req); > + return status ? status : req->sample;
Use after free here.
> + > + ts->irq_disabled = 1; > + disable_irq(spi->irq);
I am a bit uneasy here... do we need to wait for an async spi completion here before proceeding? Overall I have some concerns about the irq/spi/removal/sysfs iteractions, I will need some more time to look through the driver.
> + status = spi_sync(spi, &req->msg); > + ts->irq_disabled = 0; > + enable_irq(spi->irq); > +
-- Dmitry
|  |