Messages in this thread | | | Date | Thu, 16 Oct 2008 16:32:40 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 1/1] input: AD7879 Touchscreen driver |
| |
On Fri, 17 Oct 2008 01:57:16 +0800 Bryan Wu <cooloney@kernel.org> wrote:
a little thing..
> +static void ad7879_disable(struct ad7879 *ts) > +{ > + unsigned long flags; > + > + if (ts->disabled) > + return; > + > + spin_lock_irqsave(&ts->lock, flags); > + ts->disabled = 1; > + disable_irq(ts->bus->irq); > + spin_unlock_irqrestore(&ts->lock, flags); > + > + cancel_work_sync(&ts->work); > + > + if (del_timer_sync(&ts->timer)) > + ad7879_ts_event_release(ts); > + > + /* we know the chip's in lowpower mode since we always > + * leave it that way after every request > + */ > +} > + > +static void ad7879_enable(struct ad7879 *ts) > +{ > + unsigned long flags; > + > + if (!ts->disabled) > + return; > + > + spin_lock_irqsave(&ts->lock, flags); > + ts->disabled = 0; > + enable_irq(ts->bus->irq); > + spin_unlock_irqrestore(&ts->lock, flags); > +}
It would looks less racy if ts->disabled was tested while the lock was held.
Also, it would be more grammatically pleasing if ad7879_destruct() was called ad7879_destroy(), but the current spelling will still compile and run :)
| |