On Mon, 2010-02-22 at 19:38 +0100, ext Dmitry Torokhov wrote:
> On Mon, Feb 22, 2010 at 03:19:36PM +0200, Jari Vanhala wrote:
> > On Mon, 2010-02-22 at 07:40 +0100, ext Dmitry Torokhov wrote:
> > > > +struct vibra_info {
> > > > + struct device *dev;
> > > > + struct input_dev *input_dev;
> > > > +
> > > > + struct workqueue_struct *workqueue;
> > >
> > > Why do we need a separate workqueue? Can't keventd serve us?
> >
> > There were problems with too long delays once in a while, own workqueue
> > made things much better. Also, you can change priority if needed.
> >
>
> OK, in this case you shoudl consider starting it only when input device
> is opened and shutting it down when it is closed.
Hum.. Delayed workqueue creating/destroying makes driver much more
complex. I create separate patch for it.
> > > > +static int vibra_play(struct input_dev *dev, void *data,
> > > > + struct ff_effect *effect)
> > > > +{
> > > > + struct vibra_info *info = data;
> > > > +
> > > > + if (!info->workqueue)
> > > > + return 0;
> > >
> > > How can workqueue be missig here?
> >
> > It's missing when we remove driver. ff-memless wants to stop vibra (if
> > it was running) and destroys info. And also we really don't want to
> > start worker anymore.
>
> I would expect the code to make sure workqueue is present while there
> are any outstanding playback requests. Otherwise you need more locking
> (what stops workqueue from being deleted right after you checked it?)
I think locking is kind of overkill. Vibra_play is run in atomic-context
and.. but I can add spinlock to protect work.
++Jam
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
