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

[Felipe Balbi <balbi@ti.com>]

Hi,

On Wed, Dec 15, 2010 at 04:44:11PM +0530, 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.

within ?

>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
>@@ -1207,6 +1207,19 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
> 	 * and flush any outstanding URBs.
> 	 */
> 	} else {
>+#ifdef CONFIG_USB_OTG
>+		/* According to OTG supplement Rev 2.0 section 6.3
>+		 * Unless an A-device enables b_hnp_enable before entering
>+		 * suspend it shall also continue polling while the bus is
>+		 * suspended.
>+		 *
>+		 * We don't have to perform HNP polling, as we are going to
>+		 * enable b_hnp_enable before suspending.
>+		 */
>+		if (udev->bus->hnp_support &&
>+			udev->portnum == udev->bus->otg_port)
>+			cancel_delayed_work(&udev->bus->hnp_polling);
>+#endif
> 		udev->can_submit = 0;
> 		for (i = 0; i < 16; ++i) {
> 			usb_hcd_flush_endpoint(udev, udev->ep_out[i]);
>@@ -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)

do you really need this to be exported ??

>+{
>+	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);

how about:

u8 status;

and...

>+	if (!status)
>+		return;
>+
>+	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),

0, OTG_STATUS_SELECTOR, &status, sizeof(status);
??

-- 
balbi