[34-stable regression] USB delay init quirk causes device events loss

[34-stable regression] USB delay init quirk causes device events loss

[Jiri Slaby]

Hi,

this commit:
commit 93362a875fc69881ae69299efaf19a55a1f57db0
Author: Phil Dibowitz <phil@ipom.com>
Date:   Thu Jul 22 00:05:01 2010 +0200

    USB delay init quirk for logitech Harmony 700-series devices

causes a lot of pain for users who loose events from their input devices:
https://bugzilla.novell.com/show_bug.cgi?id=638200

opensuse 11.3 uses 2.6.34 stable and that's how the patch got there.
Reverting it makes it work again.

The problem is that the patch shuffles with usb_detect_quirks which used
to disable autosuspend by default. Apparently it no longer does.

regards,
-- 
js
suse labs

Re: [34-stable regression] USB delay init quirk causes device events loss

[Alan Stern]

On Mon, 13 Sep 2010, Jiri Slaby wrote:

> Hi,
> 
> this commit:
> commit 93362a875fc69881ae69299efaf19a55a1f57db0
> Author: Phil Dibowitz <phil@ipom.com>
> Date:   Thu Jul 22 00:05:01 2010 +0200
> 
>     USB delay init quirk for logitech Harmony 700-series devices
> 
> causes a lot of pain for users who loose events from their input devices:
> https://bugzilla.novell.com/show_bug.cgi?id=638200
> 
> opensuse 11.3 uses 2.6.34 stable and that's how the patch got there.
> Reverting it makes it work again.
> 
> The problem is that the patch shuffles with usb_detect_quirks which used
> to disable autosuspend by default. Apparently it no longer does.

I don't see how that patch could have the effect you claim.  
usb_detect_quirks still calls usb_disable_autosuspend.  Can you do a 
little more debugging to find out why it isn't working as expected?  
Reverting the commit is not a good solution.

Alan Stern

Re: [34-stable regression] USB delay init quirk causes device events loss

[Jiri Slaby]

On 09/13/2010 04:16 PM, Alan Stern wrote:
> On Mon, 13 Sep 2010, Jiri Slaby wrote:
>> The problem is that the patch shuffles with usb_detect_quirks which used
>> to disable autosuspend by default. Apparently it no longer does.
> 
> I don't see how that patch could have the effect you claim.  
> usb_detect_quirks still calls usb_disable_autosuspend.  Can you do a 
> little more debugging to find out why it isn't working as expected?  
> Reverting the commit is not a good solution.

Ok, I could, but I have a near-zero knowledge about usb core stuff. What
I see is that usb_detect_quirks moved from usb_new_device to
hub_port_connect_change. Might that be that there are broken devices
which doesn't generate state changes properly?

In other words, what I could do is to add some printks into
hub_port_connect_change if that's called at all. If you need some
thorough debugging printks, please send me a patch to test instead.

thanks,
-- 
js
suse labs

Re: [34-stable regression] USB delay init quirk causes device events loss

[Alan Stern]

On Mon, 13 Sep 2010, Jiri Slaby wrote:

> On 09/13/2010 04:16 PM, Alan Stern wrote:
> > On Mon, 13 Sep 2010, Jiri Slaby wrote:
> >> The problem is that the patch shuffles with usb_detect_quirks which used
> >> to disable autosuspend by default. Apparently it no longer does.
> > 
> > I don't see how that patch could have the effect you claim.  
> > usb_detect_quirks still calls usb_disable_autosuspend.  Can you do a 
> > little more debugging to find out why it isn't working as expected?  
> > Reverting the commit is not a good solution.
> 
> Ok, I could, but I have a near-zero knowledge about usb core stuff. What
> I see is that usb_detect_quirks moved from usb_new_device to
> hub_port_connect_change. Might that be that there are broken devices
> which doesn't generate state changes properly?

No, that's not possible.  That state change comes from the device's 
parent hub; without it we wouldn't know that the device was plugged 
in at all.

> In other words, what I could do is to add some printks into
> hub_port_connect_change if that's called at all. If you need some
> thorough debugging printks, please send me a patch to test instead.

Hmm.  I'd prefer to explain how it's all supposed to work and let you
figure out where best to look.  Or try to debug it myself.  Does this
happen with other sorts of USB devices as well, or just wacom?

Alan Stern

Re: [34-stable regression] USB delay init quirk causes device events loss

[Jiri Slaby]

On 09/13/2010 04:48 PM, Alan Stern wrote:
> On Mon, 13 Sep 2010, Jiri Slaby wrote:
>> In other words, what I could do is to add some printks into
>> hub_port_connect_change if that's called at all. If you need some
>> thorough debugging printks, please send me a patch to test instead.
> 
> Hmm.  I'd prefer to explain how it's all supposed to work and let you
> figure out where best to look.  Or try to debug it myself.  Does this
> happen with other sorts of USB devices as well, or just wacom?

With some sort of devices (I cannot reproduce myself). It's unrelated to
wacom -- people with wacom were unable to even start X with this kernel.
If you look at the bugreport, there are several examples:
ID 046d:c517 Logitech, Inc. LX710 Cordless Desktop Laser defunct
Microsoft USB keyboard defunct x Logitech mice OK
046a:0021 Cherry GmbH keyboard defunct
logitech wireless laptop mouse defunct
A$Tech wireless laptop mouse defunct
TypeMatrix USB 2030 (ID : 1e54:2030) defunct

thanks,
-- 
js
suse labs

Re: [34-stable regression] USB delay init quirk causes device events loss

[Alan Stern]

On Mon, 13 Sep 2010, Jiri Slaby wrote:

> On 09/13/2010 04:48 PM, Alan Stern wrote:
> > On Mon, 13 Sep 2010, Jiri Slaby wrote:
> >> In other words, what I could do is to add some printks into
> >> hub_port_connect_change if that's called at all. If you need some
> >> thorough debugging printks, please send me a patch to test instead.
> > 
> > Hmm.  I'd prefer to explain how it's all supposed to work and let you
> > figure out where best to look.  Or try to debug it myself.  Does this
> > happen with other sorts of USB devices as well, or just wacom?
> 
> With some sort of devices (I cannot reproduce myself). It's unrelated to
> wacom -- people with wacom were unable to even start X with this kernel.
> If you look at the bugreport, there are several examples:
> ID 046d:c517 Logitech, Inc. LX710 Cordless Desktop Laser defunct
> Microsoft USB keyboard defunct x Logitech mice OK
> 046a:0021 Cherry GmbH keyboard defunct
> logitech wireless laptop mouse defunct
> A$Tech wireless laptop mouse defunct
> TypeMatrix USB 2030 (ID : 1e54:2030) defunct

Okay, I see the problem.  By moving usb_detect_quirks earlier, we end 
up calling usb_disable_autosuspend too soon -- before the 
pm_runtime_enable call in usb_new_device.  In 2.6.35 this doesn't 
matter because the implementation of usb_autosuspend_device has 
changed.

So yes, in the end it looks like the best course is to revert this
patch from 2.6.34.stable.  This is unfortunate but I don't see any way
around it without making changes that aren't present in the current
kernel. For example, the pm_runtime_set_active and pm_runtime_enable
calls could also be moved from usb_new_device into
hub_port_connect_change.

Alan Stern

Re: [34-stable regression] USB delay init quirk causes device events loss

[Greg KH]

On Mon, Sep 13, 2010 at 11:37:49AM -0400, Alan Stern wrote:
> On Mon, 13 Sep 2010, Jiri Slaby wrote:
> 
> > On 09/13/2010 04:48 PM, Alan Stern wrote:
> > > On Mon, 13 Sep 2010, Jiri Slaby wrote:
> > >> In other words, what I could do is to add some printks into
> > >> hub_port_connect_change if that's called at all. If you need some
> > >> thorough debugging printks, please send me a patch to test instead.
> > > 
> > > Hmm.  I'd prefer to explain how it's all supposed to work and let you
> > > figure out where best to look.  Or try to debug it myself.  Does this
> > > happen with other sorts of USB devices as well, or just wacom?
> > 
> > With some sort of devices (I cannot reproduce myself). It's unrelated to
> > wacom -- people with wacom were unable to even start X with this kernel.
> > If you look at the bugreport, there are several examples:
> > ID 046d:c517 Logitech, Inc. LX710 Cordless Desktop Laser defunct
> > Microsoft USB keyboard defunct x Logitech mice OK
> > 046a:0021 Cherry GmbH keyboard defunct
> > logitech wireless laptop mouse defunct
> > A$Tech wireless laptop mouse defunct
> > TypeMatrix USB 2030 (ID : 1e54:2030) defunct
> 
> Okay, I see the problem.  By moving usb_detect_quirks earlier, we end 
> up calling usb_disable_autosuspend too soon -- before the 
> pm_runtime_enable call in usb_new_device.  In 2.6.35 this doesn't 
> matter because the implementation of usb_autosuspend_device has 
> changed.
> 
> So yes, in the end it looks like the best course is to revert this
> patch from 2.6.34.stable.  This is unfortunate but I don't see any way
> around it without making changes that aren't present in the current
> kernel. For example, the pm_runtime_set_active and pm_runtime_enable
> calls could also be moved from usb_new_device into
> hub_port_connect_change.

Ok, I'll go revert that from the .34-stable tree and do a new release,
as it's not nice to have a broken tree out there.

thanks,

greg k-h

Re: [34-stable regression] USB delay init quirk causes device events loss

[Phil Dibowitz]

[-- Attachment #1: Type: text/plain, Size: 2633 bytes --]

On 09/13/2010 05:56 PM, Greg KH wrote:
> On Mon, Sep 13, 2010 at 11:37:49AM -0400, Alan Stern wrote:
>> On Mon, 13 Sep 2010, Jiri Slaby wrote:
>>
>>> On 09/13/2010 04:48 PM, Alan Stern wrote:
>>>> On Mon, 13 Sep 2010, Jiri Slaby wrote:
>>>>> In other words, what I could do is to add some printks into
>>>>> hub_port_connect_change if that's called at all. If you need some
>>>>> thorough debugging printks, please send me a patch to test instead.
>>>>
>>>> Hmm.  I'd prefer to explain how it's all supposed to work and let you
>>>> figure out where best to look.  Or try to debug it myself.  Does this
>>>> happen with other sorts of USB devices as well, or just wacom?
>>>
>>> With some sort of devices (I cannot reproduce myself). It's unrelated to
>>> wacom -- people with wacom were unable to even start X with this kernel.
>>> If you look at the bugreport, there are several examples:
>>> ID 046d:c517 Logitech, Inc. LX710 Cordless Desktop Laser defunct
>>> Microsoft USB keyboard defunct x Logitech mice OK
>>> 046a:0021 Cherry GmbH keyboard defunct
>>> logitech wireless laptop mouse defunct
>>> A$Tech wireless laptop mouse defunct
>>> TypeMatrix USB 2030 (ID : 1e54:2030) defunct
>>
>> Okay, I see the problem.  By moving usb_detect_quirks earlier, we end 
>> up calling usb_disable_autosuspend too soon -- before the 
>> pm_runtime_enable call in usb_new_device.  In 2.6.35 this doesn't 
>> matter because the implementation of usb_autosuspend_device has 
>> changed.
>>
>> So yes, in the end it looks like the best course is to revert this
>> patch from 2.6.34.stable.  This is unfortunate but I don't see any way
>> around it without making changes that aren't present in the current
>> kernel. For example, the pm_runtime_set_active and pm_runtime_enable
>> calls could also be moved from usb_new_device into
>> hub_port_connect_change.
> 
> Ok, I'll go revert that from the .34-stable tree and do a new release,
> as it's not nice to have a broken tree out there.

Sorry to chime in late on the conversation on my patch.

Thanks for figuring this out Alan. I think this is reasonable. However, I
think this also went into .32-stable and should probably be reverted there
as well.

I'll advise my users to make sure they have at least .35.

-- 
Phil Dibowitz                             phil@ipom.com
Open Source software and tech docs        Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"Be who you are and say what you feel, because those who mind don't matter
 and those who matter don't mind."
 - Dr. Seuss



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [34-stable regression] USB delay init quirk causes device events loss

[Alan Stern]

On Mon, 13 Sep 2010, Phil Dibowitz wrote:

> Thanks for figuring this out Alan. I think this is reasonable. However, I
> think this also went into .32-stable and should probably be reverted there
> as well.

I think kernels earlier than .34 will be okay.  The changeover to the
runtime-PM framework occurred during the .33-.35 timeframe, and so .34
was in an intermediate state.

Jiri, as a workaround for people using the current 3.6.34.stable, you
can ask them to do:

	echo on >/sys/bus/usb/devices/.../power/level

with the "..." filled in appropriately.

> I'll advise my users to make sure they have at least .35.

That probably isn't necessary.

Alan Stern

Re: [34-stable regression] USB delay init quirk causes device events loss

[Jiri Slaby]

On 09/13/2010 06:24 PM, Alan Stern wrote:
> Jiri, as a workaround for people using the current 3.6.34.stable, you
> can ask them to do:
> 
> 	echo on >/sys/bus/usb/devices/.../power/level
> 
> with the "..." filled in appropriately.

Yeah, the people are using that already. (Actually, they attached a udev
script to do that automatically.) Or usbcore.autosuspend=-1 is an easier
workaround (but disables autosuspend completely, indeed).

thanks,
-- 
js
suse labs

Re: [34-stable regression] USB delay init quirk causes device events loss

[Greg KH]

On Mon, Sep 13, 2010 at 07:03:07PM +0200, Jiri Slaby wrote:
> On 09/13/2010 06:24 PM, Alan Stern wrote:
> > Jiri, as a workaround for people using the current 3.6.34.stable, you
> > can ask them to do:
> > 
> > 	echo on >/sys/bus/usb/devices/.../power/level
> > 
> > with the "..." filled in appropriately.
> 
> Yeah, the people are using that already. (Actually, they attached a udev
> script to do that automatically.) Or usbcore.autosuspend=-1 is an easier
> workaround (but disables autosuspend completely, indeed).

I've now released 2.6.34.7 with this patch reverted so all should be
good now.

thanks,

greg k-h

Re: [34-stable regression] USB delay init quirk causes device events loss

[Oliver Neukum]

Am Montag, 13. September 2010, 17:37:49 schrieb Alan Stern:
> Okay, I see the problem.  By moving usb_detect_quirks earlier, we end 
> up calling usb_disable_autosuspend too soon -- before the 
> pm_runtime_enable call in usb_new_device.  In 2.6.35 this doesn't 
> matter because the implementation of usb_autosuspend_device has 
> changed.
> 
> So yes, in the end it looks like the best course is to revert this
> patch from 2.6.34.stable.  This is unfortunate but I don't see any way
> around it without making changes that aren't present in the current
> kernel. For example, the pm_runtime_set_active and pm_runtime_enable
> calls could also be moved from usb_new_device into
> hub_port_connect_change.

Calling usb_detect_quirks() twice?

	Regards
		Oliver

Re: [34-stable regression] USB delay init quirk causes device events loss

[Alan Stern]

On Tue, 14 Sep 2010, Oliver Neukum wrote:

> Am Montag, 13. September 2010, 17:37:49 schrieb Alan Stern:
> > Okay, I see the problem.  By moving usb_detect_quirks earlier, we end 
> > up calling usb_disable_autosuspend too soon -- before the 
> > pm_runtime_enable call in usb_new_device.  In 2.6.35 this doesn't 
> > matter because the implementation of usb_autosuspend_device has 
> > changed.
> > 
> > So yes, in the end it looks like the best course is to revert this
> > patch from 2.6.34.stable.  This is unfortunate but I don't see any way
> > around it without making changes that aren't present in the current
> > kernel. For example, the pm_runtime_set_active and pm_runtime_enable
> > calls could also be moved from usb_new_device into
> > hub_port_connect_change.
> 
> Calling usb_detect_quirks() twice?

The issue is moot now since Greg has already done the reversion.

But no, usb_detect_quirks should still be called only once.  The idea 
was to make sure this happens _after_ pm_runtime_set_active and 
pm_runtime_enable, by moving those two calls earlier.

Alan Stern