Re: [PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

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

 



On 2013年01月18日 02:49, Sarah Sharp wrote:
> Hi Tianyu,
> 
> This patch looks fine, however, this doesn't apply against Greg's
> usb-linus branch.  Can you please fix and resubmit?
> 
Yeah. I made this patch based on usb-next tree. I will resend soon.

> Thanks,
> Sarah Sharp
> 
> On Thu, Jan 17, 2013 at 03:47:51PM +0800, Lan Tianyu wrote:
>> Usb3.0 device defines function remote wakeup which is only for interface
>> recipient rather than device recipient. This is different with usb2.0 device's
>> remote wakeup feature which is defined for device recipient. According usb3.0
>> spec 9.4.5, the function remote wakeup can be modified by the SetFeature()
>> requests using the FUNCTION_SUSPEND feature selector. This patch is to use
>> correct way to disable usb3.0 device's function remote wakeup after suspend
>> error and resuming.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>> ---
>>  drivers/usb/core/hub.c       |   71 +++++++++++++++++++++++++++++++-----------
>>  include/uapi/linux/usb/ch9.h |    6 ++++
>>  2 files changed, 59 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index 0ef512a..08c9c04 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -2852,6 +2852,24 @@ void usb_enable_ltm(struct usb_device *udev)
>>  }
>>  EXPORT_SYMBOL_GPL(usb_enable_ltm);
>>  
>> +/*
>> + * usb_disable_function_remotewakeup - disable usb3.0
>> + * device's function remote wakeup
>> + * @udev: target device
>> + *
>> + * Assume there's only one function on the USB 3.0
>> + * device and disable remote wake for the first
>> + * interface. FIXME if the interface association
>> + * descriptor shows there's more than one function.
>> + */
>> +static int usb_disable_function_remotewakeup(struct usb_device *udev)
>> +{
>> +	return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>> +				USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
>> +				USB_INTRF_FUNC_SUSPEND,	0, NULL, 0,
>> +				USB_CTRL_SET_TIMEOUT);
>> +}
>> +
>>  #ifdef	CONFIG_USB_SUSPEND
>>  
>>  /*
>> @@ -2968,12 +2986,19 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
>>  		dev_dbg(hub->intfdev, "can't suspend port %d, status %d\n",
>>  				port1, status);
>>  		/* paranoia:  "should not happen" */
>> -		if (udev->do_remote_wakeup)
>> -			(void) usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>> -				USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
>> -				USB_DEVICE_REMOTE_WAKEUP, 0,
>> -				NULL, 0,
>> -				USB_CTRL_SET_TIMEOUT);
>> +		if (udev->do_remote_wakeup) {
>> +			if (!hub_is_superspeed(hub->hdev)) {
>> +				(void) usb_control_msg(udev,
>> +						usb_sndctrlpipe(udev, 0),
>> +						USB_REQ_CLEAR_FEATURE,
>> +						USB_RECIP_DEVICE,
>> +						USB_DEVICE_REMOTE_WAKEUP, 0,
>> +						NULL, 0,
>> +						USB_CTRL_SET_TIMEOUT);
>> +			} else
>> +				(void) usb_disable_function_remotewakeup(udev);
>> +
>> +		}
>>  
>>  		/* Try to enable USB2 hardware LPM again */
>>  		if (udev->usb2_hw_lpm_capable == 1)
>> @@ -3059,20 +3084,30 @@ static int finish_port_resume(struct usb_device *udev)
>>  		dev_dbg(&udev->dev, "gone after usb resume? status %d\n",
>>  				status);
>>  	} else if (udev->actconfig) {
>> -		le16_to_cpus(&devstatus);
>> -		if (devstatus & (1 << USB_DEVICE_REMOTE_WAKEUP)) {
>> -			status = usb_control_msg(udev,
>> -					usb_sndctrlpipe(udev, 0),
>> -					USB_REQ_CLEAR_FEATURE,
>> +		if (!hub_is_superspeed(udev->parent)) {
>> +			le16_to_cpus(&devstatus);
>> +			if (devstatus & (1 << USB_DEVICE_REMOTE_WAKEUP))
>> +				status = usb_control_msg(udev,
>> +						usb_sndctrlpipe(udev, 0),
>> +						USB_REQ_CLEAR_FEATURE,
>>  						USB_RECIP_DEVICE,
>> -					USB_DEVICE_REMOTE_WAKEUP, 0,
>> -					NULL, 0,
>> -					USB_CTRL_SET_TIMEOUT);
>> -			if (status)
>> -				dev_dbg(&udev->dev,
>> -					"disable remote wakeup, status %d\n",
>> -					status);
>> +						USB_DEVICE_REMOTE_WAKEUP, 0,
>> +						NULL, 0,
>> +						USB_CTRL_SET_TIMEOUT);
>> +		} else {
>> +			status = usb_get_status(udev, USB_RECIP_INTERFACE, 0,
>> +					&devstatus);
>> +			le16_to_cpus(&devstatus);
>> +			if (!status && devstatus & (USB_INTRF_STAT_FUNC_RW_CAP
>> +					| USB_INTRF_STAT_FUNC_RW))
>> +				status =
>> +					usb_disable_function_remotewakeup(udev);
>>  		}
>> +
>> +		if (status)
>> +			dev_dbg(&udev->dev,
>> +				"disable remote wakeup, status %d\n",
>> +				status);
>>  		status = 0;
>>  	}
>>  	return status;
>> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
>> index 5059847..f738e25 100644
>> --- a/include/uapi/linux/usb/ch9.h
>> +++ b/include/uapi/linux/usb/ch9.h
>> @@ -152,6 +152,12 @@
>>  #define USB_INTRF_FUNC_SUSPEND_LP	(1 << (8 + 0))
>>  #define USB_INTRF_FUNC_SUSPEND_RW	(1 << (8 + 1))
>>  
>> +/*
>> + * Interface status, Figure 9-5 USB 3.0 spec
>> + */
>> +#define USB_INTRF_STAT_FUNC_RW_CAP     1
>> +#define USB_INTRF_STAT_FUNC_RW         2
>> +
>>  #define USB_ENDPOINT_HALT		0	/* IN/OUT will STALL */
>>  
>>  /* Bit array elements as returned by the USB_REQ_GET_STATUS request. */
>> -- 
>> 1.7.9.5
>>


-- 
Best regards
Tianyu Lan
--
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