Re: [PATCH 1/1] Input/Touchscreen Driver: add support AD7877touchscreen driver

On Wed, Apr 9, 2008 at 1:13 AM, Hennerich, Michael
<Michael.Hennerich@xxxxxxxxxx> wrote:


-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx]
Sent: Montag, 7. April 2008 22:16
To: Bryan Wu
Cc: linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Michael
Hennerich
Subject: Re: [PATCH 1/1] Input/Touchscreen Driver: add support
AD7877touchscreen driver

Hi Bryan, Michael,

On Thu, Feb 14, 2008 at 05:17:28PM +0800, Bryan Wu wrote:
From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>

[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.

Yeah this is definitely broken.


+
+ 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.


I think you are right - let me come up with a patch.

Thanks and best regards,
Michael



Hi Dmitry,

Sorry for no updates, I forget to send out that patch, although
Michael fixed the driver as your review.

So need I send out one updated patch including Michael's fixing or
just send out his incremental patch？

Thanks
-Bryan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: bcm43xx regression in 2.6.24 (with patch)
    ... > On Sun, 24 Feb 2008, Michael Buesch wrote: ... Isn't the resolution Michael is suggesting is, "use the different driver"? ... And the patch must be considered to do The Right Thing. ... bcm43xx driver is still there. ...
    (Linux-Kernel)
  • Re: [PATCH] uio: User IRQ Mode
    ... In this mode the user space driver ... is responsible for acknowledging and re-enabling the interrupt. ... This can easily be done without your patch. ...
    (Linux-Kernel)
  • Re: [PATCH] ne.c fix for hibernate and rmmod oops fix
    ... This patch is ready to be merged. ... that was not the case before, because the probe is invasive. ... legacy driver. ... const unsigned char *buf, const int start_page); ...
    (Linux-Kernel)
  • Re: [PATCH] Blackfin: blackfin on-chip SPI controller driver
    ... patch, rather than initial-plus-cleanups. ... Please put this in Kconfig up with the other SPI controller drivers, ... relevant points in the driver. ... place to reverse any DMA mappings ... ...
    (Linux-Kernel)
  • Re: [git patches] libata updates
    ... Bug fixes, and a new driver. ... (fixed in the attached patch). ...     else ... This is incorrect if slave/master devices use different PIO modes ...
    (Linux-Kernel)