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

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

 



On Fri, Sep 06, 2013 at 11:42:09PM +0800, Greg Kroah-Hartman wrote:
> On Fri, Sep 06, 2013 at 06:24:44PM +0800, Huang Rui wrote:
> >  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);
> 
> How can a definition of a function be inline?  :)
> 

Sorry, I made a stupid mistake...

> Can't you just place your calling function below this one so that you
> don't need this?
> 

Yes, that's right.

> >  
> >  #ifdef	CONFIG_PM
> >  static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
> > @@ -293,6 +294,47 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
> >  	return rc;
> >  }
> >  
> > +/*
> > + * Reset port which attached with an issue device.
> > + *
> > + * 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 usb mouse controller would react
> > + * differently to this unexpected event from some AMD host controller
> 
> Which AMD host controllers have this issue?
> 

Will do it in v4.

> > + * and will result in the mouse to assert a resume event on the
> > + * subsequent S3 sleep even if the user does not initiate the wake
> > + * event by clicking on the mouse. So it should reset the port which
> > + * attached with issue mouse during S3 reusme phase.
> > + */
> > +static int ohci_reset_port_by_amd_remote_wakeup(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_issue_device_for_amd_quirk(child)) {
> > +			ohci_dbg(ohci, "Connencted issue device on port %d.\n", 
> > +					wIndex);
> 
> Please always run your patches through the scripts/checkpatch.pl tool,
> so that I don't have to point out the obvious issues that it would
> (hint, there's a problem in this function...)
> 

I remember it, will do it in future.

> > +			root_port_reset(ohci, wIndex);
> > +		}
> > +	}
> > +
> > +	return 0;
> 
> If the function can never "fail", and you never check the return value
> when you call it, why return any value at all?
>

Got it.

Thanks,
Rui

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux