On Mon, Oct 15, 2007 at 11:48:17AM -0400, Dmitry Torokhov wrote:

Hi Bryan,

On 10/15/07, Bryan Wu [off-list ref] wrote:

quoted

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