lkml.org 
[lkml]   [2006]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 03/12] input: new force feedback interface
    On 6/6/06, Anssi Hannula <anssi.hannula@gmail.com> wrote:
    > Dmitry Torokhov wrote:
    > > On Monday 05 June 2006 17:11, Anssi Hannula wrote:
    > >
    > >>Dmitry Torokhov wrote:
    > >>
    > >>>On 5/30/06, Anssi Hannula <anssi.hannula@gmail.com> wrote:
    > >>>>--- linux-2.6.17-rc4-git12.orig/drivers/input/input.c 2006-05-27
    > >>>>02:28:57.000000000 +0300
    > >>>>+++ linux-2.6.17-rc4-git12/drivers/input/input.c 2006-05-27
    > >>>>02:38:35.000000000 +0300
    > >>>>@@ -733,6 +733,17 @@ static void input_dev_release(struct cla
    > >>>> {
    > >>>> struct input_dev *dev = to_input_dev(class_dev);
    > >>>>
    > >>>>+ if (dev->ff) {
    > >>>>+ struct ff_device *ff = dev->ff;
    > >>>>+ clear_bit(EV_FF, dev->evbit);
    > >>>>+ mutex_lock(&dev->ff_lock);
    > >>>>+ del_timer_sync(&ff->timer);
    > >>>
    > >>>
    > >>>This is too late. We need to stop timer when device gets unregistered.
    > >>
    > >>And what if driver has called input_allocate_device(),
    > >>input_ff_allocate(), input_ff_register(), but then decides to abort and
    > >>calls input_dev_release()? input_unregister_device() would not get
    > >>called at all.
    > >>
    > >
    > >
    > > Right, but if device was never registered there is no device node so noone
    > > could start the timer and deleting it is a noop. Hmm, I think even better
    > > place would be to stop ff timer when device is closed (i.e. when last user
    > > closes file handle).
    > >
    >
    > Hmm... actually, they are stopped in flush(), and IIRC that is always
    > called before deleting input_dev.
    >

    flush is called when you close one file handle. If there are more than
    one process opened event device you only want to stop timer when they
    all closed ther handles, not when the first one did.

    > >
    > >>>Clearing FF bits is pointless here as device is about to disappear;
    > >>>locking is also not needed because we are guaranteed to be the last
    > >>>user of the device structure.
    > >>
    > >>True, if that guarantee really exists.
    > >>
    > > Yes, this is guaranteed.
    > >
    >
    > So, now you guarantee it, but it isn't really so? ;)
    >

    Hmm, I thought I could guarantee it, but not yet ;)

    > When we remove locking, timer_del, clear_bit, all that is left is
    > kfree() and I guess that has to still be run in the input_dev_release().
    >

    Yes.

    --
    Dmitry
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-06-06 14:48    [W:2.964 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site