On Mon, Oct 15, 2007 at 11:48:17AM -0400, Dmitry Torokhov wrote:
> Hi Bryan,
>
> On 10/15/07, Bryan Wu <[EMAIL PROTECTED]> wrote:
> > +
> > +static int ad7142_thread(void *nothing)
> > +{
> > + do {
> > + wait_for_completion(&ad7142_completion);
> > + ad7142_decode();
> > + enable_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX);
> > + } while (!kthread_should_stop());
> > +
>
> No, this is not going to work well:
> - you at least need to reinitialize the completion before enabling
> IRQ, otherwise you will spin in a very tight loop
> - if noone would touch the joystick ad7142_clsoe would() block
> infinitely because noone would signal the completion and
> ad7142_thread() would never stop.
>
> Completion is just not a good abstraction here... Please use work
> abstraction and possibly a separate workqueue.
>
Bryan, I'm very interested in the technical advantage of using a completion
here.
In my _not-experienced_ opinion, I remember completions was created mainly for
"create_task, wait till task got finished, go on" case. Why using it in a
different context while workqueues was created for a similar situation to
ad7142 one (non-irq context bottom-half) ?
Regards,
--
Ahmed S. Darwish
HomePage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
