Re: [PATCH 1/1] driver,usb: Fix a warning in uhci-hcd driver

Li, Zhen-Hua (USL-China)
2013-05-06T01:43:58Z

On 04/29/2013 02:55 AM, Alan Stern wrote:
> On Sun, 28 Apr 2013, ZhenHua wrote:
>
>>>>> In fact, the patch is so easy that I am including it below. Please
>>>>> test this (without either of your patches) to see if it works.
>>>>>
>>>>> Alan Stern
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Index: usb-3.9/drivers/usb/host/uhci-hub.c
>>>>> ===================================================================
>>>>> --- usb-3.9.orig/drivers/usb/host/uhci-hub.c
>>>>> +++ usb-3.9/drivers/usb/host/uhci-hub.c
>>>>> @@ -225,7 +225,8 @@ static int uhci_hub_status_data(struct u
>>>>> /* auto-stop if nothing connected for 1 second */
>>>>> if (any_ports_active(uhci))
>>>>> uhci->rh_state = UHCI_RH_RUNNING;
>>>>> - else if (time_after_eq(jiffies, uhci->auto_stop_time))
>>>>> + else if (time_after_eq(jiffies, uhci->auto_stop_time) &&
>>>>> + !uhci->wait_for_hp)
>>>>> suspend_rh(uhci, UHCI_RH_AUTO_STOPPED);
>>>>> break;
>>>>>
>>>>>
>>>> I have tested the UHCI_RH_RUNNING_NODEVS case yeasterday, and it works.
>>>> But the function suspend_rh is also called in other places, so I think
>>>> it only fixes
>>>> the warning when auto stop is called, but not fix the warning when
>>>> uhci's bus_suspend
>>>> is called, it will come out again.
>>> Have you tried this? I expect the warning will not occur when the
>>> bus_suspend routine is called, because then there will be a 1-ms delay,
>>> not just a 400-us delay.
>> I tested this, and the warning is gone. Is this patch committed ?
>> I need to paste the link to suse bugzilla.
Not joking. I tested both of them , uhci->wait_for_hp and no_auto_stop ,
for the UHCI_RH_RUNNING_NODEVS
case. And as uhci->wait_for_hp is 1 on my system, so they got the same
result.

> You must be joking. I wrote that patch while composing the email
> message to you, and nobody except you has tested it.
>
> Since you confirm that it works, I will submit it. But new patches
> like this won't be accepted until after the upcoming merge window
> closes, which won't be for more than two weeks.

>
>>>> And if you add uhci->wait_for_hp check in the UHCI_RH_RUNNING_NODEVS case,
>>>> all hp uhci devices will not auto stop, not only the virtual devices. I
>>>> guess it may waste resource.
>>> If you want, you can add a new flag specifically for virtual
>>> controllers. But it shouldn't matter -- as long as your kernels are
>>> built with CONFIG_PM_RUNTIME enabled, there won't be any significant
>>> waste of resources.
>>>
>>> Alan Stern
>>>
>> I think we can check the product id to determine whether a device is
>> virtual.
>> Do you know if there is another way to check this?
> I don't know anything at all about your virtual UHCI controllers, other
> than what you have told me. In particular, I don't know what product
> IDs are used by HP's virtual and non-virtual controllers. Maybe
> somebody else at HP can tell you.
>
> Alan Stern
>
> .
So I can only check the product IDs.

Thanks for helping me on this bug.

Regards
Zhen-Hua