From: "Li, Zhen-Hua" <[email protected]>
There's another patch trying to fix this warning:
"Controller not stopped yet!".
It is : 997ff893603c6455da4c5e26ba1d0f81adfecdfc .
I don't think it is appropriate to avoid auto-stop for all HP uhci
devices. So add one tag for the virtual uhci devices, it is used
to replace "wait_for_hp" in the auto-stop case.
Signed-off-by: Li, Zhen-Hua <[email protected]>
---
drivers/usb/host/uhci-hcd.h | 1 +
drivers/usb/host/uhci-hub.c | 2 +-
drivers/usb/host/uhci-pci.c | 21 +++++++++++++++++++++
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
index 6f986d8..915d5df 100644
--- a/drivers/usb/host/uhci-hcd.h
+++ b/drivers/usb/host/uhci-hcd.h
@@ -425,6 +425,7 @@ struct uhci_hcd {
/* Silicon quirks */
unsigned int oc_low:1; /* OverCurrent bit active low */
unsigned int wait_for_hp:1; /* Wait for HP port reset */
+ unsigned int virtual_device:1; /* For some virtual devices */
unsigned int big_endian_mmio:1; /* Big endian registers */
unsigned int big_endian_desc:1; /* Big endian descriptors */
diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c
index 9189bc9..da00754 100644
--- a/drivers/usb/host/uhci-hub.c
+++ b/drivers/usb/host/uhci-hub.c
@@ -226,7 +226,7 @@ static int uhci_hub_status_data(struct usb_hcd *hcd, char *buf)
if (any_ports_active(uhci))
uhci->rh_state = UHCI_RH_RUNNING;
else if (time_after_eq(jiffies, uhci->auto_stop_time) &&
- !uhci->wait_for_hp)
+ !uhci->virtual_device)
suspend_rh(uhci, UHCI_RH_AUTO_STOPPED);
break;
diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
index c300bd2f7..68e6d92 100644
--- a/drivers/usb/host/uhci-pci.c
+++ b/drivers/usb/host/uhci-pci.c
@@ -110,6 +110,24 @@ static int uhci_pci_global_suspend_mode_is_broken(struct uhci_hcd *uhci)
return 0;
}
+static int uhci_virtual_device_ids[][2] = {
+ {0x103c, 0x3300},
+ {0, 0},
+};
+static int uhci_is_virtual_device(struct uhci_hcd *uhci)
+{
+ int i;
+ struct pci_dev *dev;
+
+ dev = to_pci_dev(uhci_dev(uhci));
+ for (i = 0; uhci_virtual_device_ids[i][0] != 0; i++) {
+ if (dev->vendor == uhci_virtual_device_ids[i][0]
+ && dev->device == uhci_virtual_device_ids[i][1])
+ return 1;
+ }
+ return 0;
+}
+
static int uhci_pci_init(struct usb_hcd *hcd)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
@@ -129,6 +147,9 @@ static int uhci_pci_init(struct usb_hcd *hcd)
if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_HP)
uhci->wait_for_hp = 1;
+ if (uhci_is_virtual_device(uhci))
+ uhci->virtual_device = 1;
+
/* Set up pointers to PCI-specific functions */
uhci->reset_hc = uhci_pci_reset_hc;
uhci->check_and_reset_hc = uhci_pci_check_and_reset_hc;
--
1.7.10.4
On Wed, 26 Jun 2013, Li, Zhen-Hua wrote:
> From: "Li, Zhen-Hua" <[email protected]>
>
> There's another patch trying to fix this warning:
> "Controller not stopped yet!".
> It is : 997ff893603c6455da4c5e26ba1d0f81adfecdfc .
>
> I don't think it is appropriate to avoid auto-stop for all HP uhci
> devices. So add one tag for the virtual uhci devices, it is used
> to replace "wait_for_hp" in the auto-stop case.
Do you have any machines where this makes a difference?
Alan Stern
Hi Alan,
I don't have a machine that this makes action different.
No matter whether it makes different, there is one thing will never change:
We create a patch to FIX a problem, not to avoid a problem.
Only when we can not fix it, we try to avoid it.
Thanks
ZhenHua
On 06/27/2013 03:17 AM, Alan Stern wrote:
> On Wed, 26 Jun 2013, Li, Zhen-Hua wrote:
>
>> From: "Li, Zhen-Hua" <[email protected]>
>>
>> There's another patch trying to fix this warning:
>> "Controller not stopped yet!".
>> It is : 997ff893603c6455da4c5e26ba1d0f81adfecdfc .
>>
>> I don't think it is appropriate to avoid auto-stop for all HP uhci
>> devices. So add one tag for the virtual uhci devices, it is used
>> to replace "wait_for_hp" in the auto-stop case.
> Do you have any machines where this makes a difference?
>
> Alan Stern
>
> .
>
On Thu, 27 Jun 2013, Li, Zhen-Hua (USL-China) wrote:
> Hi Alan,
>
> I don't have a machine that this makes action different.
Then why do you want to apply the patch?
> No matter whether it makes different, there is one thing will never change:
> We create a patch to FIX a problem, not to avoid a problem.
> Only when we can not fix it, we try to avoid it.
But in this case, there is no problem to fix or to avoid, right? After
all, how can there be a problem if no machines are affected?
Alan Stern
There was a problem, the warning "Controller not stopped yet".
And your last patch for this problem does a wrong thing:
It prevents all HP uhci devices from auto-stop, which make HP uhci
devices waste more
power.
This is another new problem.
I think this should be corrected, so I want to apply it.
Thanks
Zhen-Hua
On 06/27/2013 11:52 PM, Alan Stern wrote:
> On Thu, 27 Jun 2013, Li, Zhen-Hua (USL-China) wrote:
>
>> Hi Alan,
>>
>> I don't have a machine that this makes action different.
> Then why do you want to apply the patch?
>
>> No matter whether it makes different, there is one thing will never change:
>> We create a patch to FIX a problem, not to avoid a problem.
>> Only when we can not fix it, we try to avoid it.
> But in this case, there is no problem to fix or to avoid, right? After
> all, how can there be a problem if no machines are affected?
>
> Alan Stern
>
> .
>
On Fri, 28 Jun 2013, Li, Zhen-Hua (USL-China) wrote:
> There was a problem, the warning "Controller not stopped yet".
> And your last patch for this problem does a wrong thing:
> It prevents all HP uhci devices from auto-stop, which make HP uhci
> devices waste more
> power.
Do they really waste more power? Have you measured this?
Is CONFIG_PM enabled in the kernel configuration?
> This is another new problem.
>
> I think this should be corrected, so I want to apply it.
In the last email, you said that your patch did not make the machine
act different. Now you say that your patch makes the machine use less
power. Which statement is correct?
Alan Stern
On 06/28/2013 10:22 PM, Alan Stern wrote:
> On Fri, 28 Jun 2013, Li, Zhen-Hua (USL-China) wrote:
>
>> There was a problem, the warning "Controller not stopped yet".
>> And your last patch for this problem does a wrong thing:
>> It prevents all HP uhci devices from auto-stop, which make HP uhci
>> devices waste more
>> power.
> Do they really waste more power? Have you measured this?
>
> Is CONFIG_PM enabled in the kernel configuration?
>
>> This is another new problem.
>>
>> I think this should be corrected, so I want to apply it.
> In the last email, you said that your patch did not make the machine
> act different. Now you say that your patch makes the machine use less
> power. Which statement is correct?
>
> Alan Stern
>
Let's make it clear:
I said "I don't have a machine that this makes action different",
it does not mean my patch "did not make the machine act different ".
There are many kinds of machines, I have never said "my patch does
not make ALL of them act different".
On Mon, 1 Jul 2013, ZhenHua wrote:
> Let's make it clear:
> I said "I don't have a machine that this makes action different",
> it does not mean my patch "did not make the machine act different ".
Of course.
> There are many kinds of machines, I have never said "my patch does
> not make ALL of them act different".
But does it make a difference on _any_ machine?
There is a good reason for asking this question. The "auto-stopp"
mechanism in uhci-hcd was intended to be a temporary measure. It isn't
needed now that we have runtime PM. Adding a patch just to make it
work on a few systems doesn't seem worthwhile -- we would be better off
removing it entirely.
Alan Stern
On Wed, Jun 26, 2013 at 02:29:45PM +0800, Li, Zhen-Hua wrote:
> From: "Li, Zhen-Hua" <[email protected]>
>
> There's another patch trying to fix this warning:
> "Controller not stopped yet!".
> It is : 997ff893603c6455da4c5e26ba1d0f81adfecdfc .
>
> I don't think it is appropriate to avoid auto-stop for all HP uhci
> devices. So add one tag for the virtual uhci devices, it is used
> to replace "wait_for_hp" in the auto-stop case.
>
> Signed-off-by: Li, Zhen-Hua <[email protected]>
> ---
> drivers/usb/host/uhci-hcd.h | 1 +
> drivers/usb/host/uhci-hub.c | 2 +-
> drivers/usb/host/uhci-pci.c | 21 +++++++++++++++++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
Not applied due to massive confusion in this thread, sorry.
Please resend once you all work it out...
greg k-h