Hi Bruno On Sat, May 5, 2012 at 12:58 PM, Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx> wrote: [snip] >> The nicest solution probably is releasing the semaphore during >> hid_hw_start(). That way every driver is prepared to receive data. >> However, it's an ugly solution because we release the lock not in the >> same function as we acquire it. We also need to catch cases where >> probe() actually did never call hid_hw_start(). > > On probe side: > > Maybe instead of doing it within hid_hw_start() it could also be done > in hid_connect() in combination of a flag on hdev->claimed. Could you elaborate this? I don't understand what you mean. > On remove side: > > Let the semaphore be taken in hid_disconnect() (instead of hid_hw_stop(), > yet again in combination with flag on hdev->claimed. > > > This way probe and remove could WARN_ON() case where hdev->claim does > not show the flag as expected (that is, assuming probe() and remove() > can't be interlaced for the same device) > > Otherwise it would require one more lock or doing some of the probe > initialization in a work queue (but that second option would make race > conditions much more of a problem)... My suggestion is to call up(&hdev->driver_lock) in hid_hw_start() if some quirk is set. hid_device_probe() can then check whether the semaphore is locked before calling up(). This way we would release the lock before initializing a device. However, this seems ugly. Your idea with the work queue seems much cleaner. However, there is currently no way to make sure the work is executed _after_ hid_device_probe() releases the mutex. We could, however, introduce some "hid_wait_until_initialized()" function that just tries to take the semaphore and make the work wait until the hid_device_probe finishes. Maybe Jiri can comment here. But I think picolcd is broken and not the semaphore. If I have some time I will try to send a fix for that. I do not have the hardware, though, so I cannot test it. > Thanks, > Bruno Cheers David -- 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
