From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wolverine01.qualcomm.com ([199.106.114.254]:62030 "EHLO wolverine01.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756629Ab0LPPxv (ORCPT ); Thu, 16 Dec 2010 10:53:51 -0500 Message-ID: <4D0A3609.7000002@codeaurora.org> Date: Thu, 16 Dec 2010 21:23:45 +0530 From: Pavan Kondeti MIME-Version: 1.0 Subject: Re: [RFC v2 1/4] USB: core: OTG Supplement Revision 2.0 updates References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-arm-msm-owner@vger.kernel.org List-ID: To: Alan Stern Cc: linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.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.