Re: [RFC v2 1/4] USB: core: OTG Supplement Revision 2.0 updates

[Pavan Kondeti <pkondeti@codeaurora.org>]

On 12/16/2010 9:04 PM, Alan Stern wrote:
> On Thu, 16 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 within 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.
> 
> This combination of things doesn't make sense:
> 
>> index b9278a1..baada06 100644
>> --- a/drivers/usb/core/driver.c
>> +++ b/drivers/usb/core/driver.c
>> @@ -1270,6 +1282,47 @@ 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)
>> +{
> 
> ...
> 
>> +}
>> +#endif
> 
> usb_hnp_polling_work() is defined only when CONFIG_USB_OTG is set.
> 
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index e70aeaf..5dd59e9 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -869,6 +869,7 @@ static void usb_bus_init (struct usb_bus *bus)
>>  	bus->bandwidth_isoc_reqs = 0;
>>  
>>  	INIT_LIST_HEAD (&bus->bus_list);
>> +	INIT_DELAYED_WORK(&bus->hnp_polling, usb_hnp_polling_work);
>>  }
> 
> But its address is taken regardless of CONFIG_USB_OTG.
> 
>> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
>> index b975450..ffa16e1 100644
>> --- a/drivers/usb/core/usb.h
>> +++ b/drivers/usb/core/usb.h
>> @@ -72,6 +72,14 @@ static inline int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>  
>>  #endif
>>  
>> +#ifdef CONFIG_USB_OTG
>> +extern void usb_hnp_polling_work(struct work_struct *work);
>> +#else
>> +static inline void usb_hnp_polling_work(struct work_struct *work)
>> +{
>> +}
>> +#endif
> 
> Otherwise it is an empty inline routine.
> 
> But taking the address of an inline routine forces the compiler to
> produce an out-of-line version.  Therefore you might as well explicitly
> define usb_hnp_polling_work() as an empty function when CONFIG_USB_OTG
> isn't set.
> 
Thanks. I got it. Will fix it next version.

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