Re: [PATCH 2/2] usb: ohci: add AMD remote wakeup quirk into ohci driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 23, 2013 at 01:00:29AM +0800, Alan Stern wrote:
> On Thu, 22 Aug 2013, Huang Rui wrote:
> 
> > The following patch is required to resolve remote wake issues with
> > certain devices.
> > 
> > Issue description:
> > If the remote wake is issued from the device in a specific timing
> > condition while the system is entering sleep state then it may cause
> > system to auto wake on subsequent sleep cycle.
> > 
> > Root Cause:
> > Host controller rebroadcasts the Resume signal > 100 µseconds after
> > receiving the original resume event from the device. For proper
> > function, some devices may require the rebroadcast of resume event
> > within the USB spec of 100µS.
> > 
> > This patch is for OHCI driver to add AMD remote wakeup fix.
> 
> Here you need to put a description of what the fix is and how it works.
> 

OK, I will do it in v2.

> > This should be backported to kernels as old as 3.9, that contrain the
> > commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 "usb: add
> > find_raw_port_number callback to struct hc_driver()" and the last
> > patch that AMD remote wakeup quirk for xhci.
> > 
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> > ---
> >  drivers/usb/host/ohci-hub.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  drivers/usb/host/ohci-pci.c | 13 +++++++++++++
> >  drivers/usb/host/ohci.h     | 23 ++++++++++++-----------
> >  3 files changed, 67 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
> > index 2347ab8..0af45e3 100644
> > --- a/drivers/usb/host/ohci-hub.c
> > +++ b/drivers/usb/host/ohci-hub.c
> > @@ -41,6 +41,7 @@
> >  
> >  static void dl_done_list (struct ohci_hcd *);
> >  static void finish_unlinks (struct ohci_hcd *, u16);
> > +static inline int root_port_reset (struct ohci_hcd *, unsigned);
> >  
> >  #ifdef	CONFIG_PM
> >  static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
> > @@ -293,6 +294,44 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
> >  	return rc;
> >  }
> >  
> > +/*
> > + * Reset port which attached with mouse.
> > + *
> > + * If the remote wake is issued from the device in a specific timing
> > + * condition while the system is entering sleep state then it may
> > + * cause system to auto wake on subsequent sleep cycle.
> > + *
> > + * Host controller rebroadcasts the Resume signal > 100 µseconds after
> > + * receiving the original resume event from the device. For proper
> > + * function, some devices may require the rebroadcast of resume event
> > + * within the USB spec of 100µS.
> > + *
> > + * Without this quirk, some AMD platform doesn't hold the resume right
> > + * away when there is a resume signal from LS device like mouse. So it
> > + * should reset the port which attached with mouse.
> > + */
> > +static int ohci_reset_port_by_mouse(struct ohci_hcd *ohci)
> > +{
> > +	u32 temp;
> > +	struct usb_device *hdev, *child;
> > +	int port1, wIndex;
> > +
> > +	hdev = hcd_to_bus(ohci_to_hcd(ohci))->root_hub;
> > +
> > +	usb_hub_for_each_child(hdev, port1, child) {
> > +		wIndex = port1 - 1;
> > +		temp = roothub_portstatus (ohci, wIndex);
> > +		dbg_port(ohci, "Get port status", wIndex, temp);
> > +		if (is_usb_mouse(child)) {
> > +			ohci_dbg(ohci, "Connencted mouse on port1%d.\n",
> > +					wIndex);
> > +			root_port_reset(ohci, wIndex);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> 
> Without more explanation, this seems completely wrong.  Why do you want 
> to reset the port?  What happens if you don't reset the port?
> 

The resume signal from the port attached mouse can't be cleared or
holden if wakeup from S3 with click mouse intensely, so excute S3 at
the second time, then system detects the wrong resume signal and auto
wake. If reset this port, the resume signal is cleared. Then system
doesn't auto wake at the second time.

> Wouldn't it be simpler to disable the port?

Sorry, I don't get your meaning of these words. Why?

> Then usbcore would do the 
> reset for you.  That certainly is safer than a reset.
> 

Could you point out?

> Instead of doing this always for all mice, wouldn't it be better to 
> test first and see whether it really is needed?
> 

Actually, I tested all the mice in my hand, this issue existed. Only
mice would trigger this bugy.

Thanks,
Rui

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]