Hi Roger, On Wed, Apr 30, 2014 at 03:36:27PM +0300, Roger Quadros wrote: > +static int pixcir_stop(struct pixcir_i2c_ts_data *ts) > +{ > + struct device *dev = &ts->client->dev; > + int ret; > + > + /* exit ISR if running, no more report parsing */ > + ts->exiting = true; > + mb(); /* update status before we synchronize irq */ > + > + /* disable ISR from running again */ > + disable_irq(ts->client->irq); > + > + /* wait till running ISR complete */ > + synchronize_irq(ts->client->irq); > + > + /* disable interrupt generation */ > + ret = pixcir_int_enable(ts, 0); > + if (ret) > + dev_err(dev, "Failed to disable interrupt generation\n"); > + > + /* restore IRQ */ > + enable_irq(ts->client->irq); > + > + return ret; Should not this be: pixcir_int_enable(ts, 0); ts->exiting = true; mb(); synchronize_irq(ts->client->irq); Why do we also need to disable/enable irq? By the way, disable_irq() implies synchronize_irq(). Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html