Re: [RFC 2/5] USB: core: OTG Supplement Revision 2.0 updates

[Pavan Kondeti <pkondeti@codeaurora.org>]

On 12/16/2010 2:59 AM, Alan Stern wrote:
> On Wed, 15 Dec 2010, Pavankumar Kondeti wrote:
> 
>> OTG supplement revision 2.0 spec introduces Attach Detection Protocol
>> (ADP) for detecting peripheral connection without applying power on
>> VBUS.  ADP is optional and is included in the OTG descriptor along with
>> SRP and HNP.
>>
>> HNP polling is introduced for peripheral to notify its wish to become
>> host.  Host polls (GET_STATUS on DEVICE) peripheral for host_request
>> and suspend the bus when peripheral returns host_request TRUE.  The spec
>> insists the polling frequency to be in 1-2 sec range and bus should be
>> suspended with in 2 sec from host_request is set.
>>
>> a_alt_hnp_support feature is obsolete and a_hnp_support feature is limited
>> to only legacy OTG B-device.  The newly introduced bcdOTG field in the OTG
>> descriptor is used for identifying the 2.0 compliant B-device.
>>
>> Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
>> ---
>>  drivers/usb/core/driver.c |   50 +++++++++++++++++++++++++++++++
>>  drivers/usb/core/hcd.c    |    3 ++
>>  drivers/usb/core/hub.c    |   71 ++++++++++++++++++++++++++++++++++++++------
>>  drivers/usb/core/usb.h    |    4 ++
>>  include/linux/usb.h       |    2 +
>>  include/linux/usb/ch9.h   |   11 ++++++-
>>  6 files changed, 129 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
>> index b9278a1..38885b6 100644
>> --- a/drivers/usb/core/driver.c
>> +++ b/drivers/usb/core/driver.c
> 
>> @@ -1270,6 +1283,43 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
>>  	return status;
>>  }
>>  
>> +#ifdef CONFIG_USB_OTG
>> +void usb_hnp_polling_work(struct work_struct *work)
>> +{
>> +	int ret;
>> +	struct usb_bus *bus =
>> +		container_of(work, struct usb_bus, hnp_polling.work);
>> +	struct usb_device *udev = bus->root_hub->children[bus->otg_port - 1];
>> +	u8 *status = kmalloc(sizeof(*status), GFP_KERNEL);
>> +
>> +	if (!status)
>> +		return;
> 
> Shouldn't you reschedule the delayed work?  A memory allocation failure 
> is likely to be temporary.
> 
Agreed. Will fix it in V2.

>> +
>> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>> +		USB_REQ_GET_STATUS, USB_DIR_IN | USB_RECIP_DEVICE,
>> +		0, OTG_STATUS_SELECTOR, status, sizeof(*status),
>> +		USB_CTRL_GET_TIMEOUT);
>> +	if (ret < 0) {
>> +		/* Peripheral may not be supporting HNP polling */
>> +		dev_vdbg(&udev->dev, "HNP polling failed. status %d\n", ret);
>> +		goto out;
>> +	}
>> +
>> +	/* Spec says host must suspend the bus with in 2 sec. */
>> +	if (*status & (1 << HOST_REQUEST_FLAG)) {
>> +		do_unbind_rebind(udev, DO_UNBIND);
> 
> You forget to set udev->do_remote_wakeup to 0.
> 
Thanks for pointing this out. I will fix it in V2.

Thanks,
Pavan

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.