On Fri, 5 Nov 2010 14:54:37 -0700 Russ Gorby <[email protected]> wrote:
> > Signed-off-by: Russ Gorby <[email protected]> Working back through the backlog.. > @@ -623,6 +627,14 @@ static int ifx_spi_write(struct tty_struct *tty, > const unsigned char *buf, int srdy_value; > > dev_dbg(&ifx_dev->spi_dev->dev, "%s called", __func__); > + if (test_bit(IFX_SPI_STATE_SUSPEND, &ifx_dev->flags)) { > + /* > + * FIXME: > + * some modems have been observed to die (requiring > reset) > + * if MRDY/SRDY handshake occurs w/o clocking data > + */ > + return -EIO; > + } So what stops the suspend occurring between here after your test and the mrdy change - seems you are only narrowing the window ? > + * ifx_spi_retry_timer_fn - Timer function for ifx_spi_complete() > + * @data: Device pointer passed by ifx_spi_complete(). > + * > + * The delay timer has expired so reschedule the tasklet > + */ > +static void ifx_spi_retry_timer_fn(unsigned long data) > +{ > + struct ifx_spi_device *ifx_dev = (struct ifx_spi_device > *)data; + > + tasklet_schedule(&ifx_dev->io_work_tasklet); > +} What is this change about - it isn't explained anywhere > + > +/** > * ifx_spi_complete - SPI transfer completed > * @ctx: our SPI device > * > @@ -1030,6 +1055,9 @@ complete_exit: > } > > clear_bit(IFX_SPI_STATE_IO_IN_PROGRESS, &(ifx_dev->flags)); > + clear_bit(IFX_SPI_STATE_IO_RETRY, &(ifx_dev->flags)); > + del_timer(&ifx_dev->spi_retry_timer); And if the timer handler is running in parallel on another CPU isn't this going to cause confusion ? > + if (!test_and_set_bit(IFX_SPI_STATE_IO_RETRY, > &ifx_dev->flags)) > + setup_timer(&ifx_dev->spi_retry_timer, > ifx_spi_retry_timer_fn, > + (unsigned long)ifx_dev); > + expires = jiffies + IFX_RETRY_TIMEOUT; > + mod_timer(&ifx_dev->spi_retry_timer, expires); What stops this racing the timer delete - it seems we can set the flag, clear it again, delete the timer then re-add it and end up in a right mess ? This is then attached to a pile of SPI driver changes which are also undocumented and relate to a driver which isn't actually going to get used in the normal course of things as the other SPI driver will claim its ID *and* supports the other ports too. Alan _______________________________________________ MeeGo-kernel mailing list [email protected] http://lists.meego.com/listinfo/meego-kernel
