[PATCH 0/2] usb: Add support for EHSET

[PATCH 0/2] usb: Add support for EHSET

[Jack Pham]

PATCH 1/2 is a device driver supporting the Embedded Host High-Speed
Electrical Test (EHSET) test fixture. This driver supports the set of
VID/PIDs specified in the EHSET and activates the various test (TEST_J,
TEST_K, et al) modes by issuing a USB_PORT_FEAT_TEST SetFeature request
to the hub, or by directly toggling USB_PORT_FEAT_SUSPEND, in the case
of the port suspend test.

In RFC/PATCH 2/2 *[1] the SINGLE_STEP_SET_FEATURE test is implemented. The
fixture driver designates test selector=6 and sends the USB_PORT_FEAT_TEST
request directly to the root hub. The EHCI hub driver is expected to handle
this by calling functions in the queue driver to submit separate QTDs for
the SETUP and DATA/STATUS stages of a GetDescriptor request, in essence
duplicating the work of ehci_urb_enqueue(), but with the requisite added
delay.

Alan Stern wrote:
> It does sound like a lot of mucking around inside ehci-hcd for a
> relatively small benefit.  But if people want it, I guess it can be
> added.

Thus this patch is an RFC because I'm pretty sure this method of "mucking
around" isn't gonna sit well with most folks. So any alternative suggestions
are welcome.

*[1] This second patch is mostly unmodified from the original commit made on
the out-of-tree MSM kernel, so please excuse trivial style inconsistencies:
https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit?id=c2084930fd49f04571627be0762ff40e5c5d90d8

Manu Gautam (2):
  usb: misc: EHSET Test Fixture device driver for host compliance
  usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

 drivers/usb/host/ehci-hub.c |  144 ++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-q.c   |  101 ++++++++++++++++++++++++++++
 drivers/usb/misc/Kconfig    |   13 ++++
 drivers/usb/misc/Makefile   |    1 +
 drivers/usb/misc/ehset.c    |  152 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 410 insertions(+), 1 deletions(-)
 create mode 100644 drivers/usb/misc/ehset.c

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 1/2] usb: misc: EHSET Test Fixture device driver for host compliance

[Jack Pham]

From: Manu Gautam <mgautam@codeaurora.org>

An Embedded Host High-Speed Electrical Test (EHSET) test fixture is
used to initiate test modes on a host controller in order to perform
the high speed electrical testing procedure for USB-IF compliance.
When this test fixture is connected to a host, it can enumerate as
one of several selectable VID/PID pairs, each corresponding to one
of the following test modes:

* TEST_SE0_NAK
* TEST_J
* TEST_K
* TEST_PACKET
* HS_HOST_PORT_SUSPEND_RESUME
* SINGLE_STEP_GET_DEV_DESC
* SINGLE_STEP_SET_FEATURE

The USB EHSET procedure can be found here:
http://www.usb.org/developers/onthego/EHSET_v1.01.pdf

Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
[jackp@codeaurora.org: imported from commit 073c9409 on codeaurora.org;
 minor cleanup and updated author email]
Signed-off-by: Jack Pham <jackp@codeaurora.org>
---
 drivers/usb/misc/Kconfig  |   13 ++++
 drivers/usb/misc/Makefile |    1 +
 drivers/usb/misc/ehset.c  |  134 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/misc/ehset.c

diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index a51e7d6..ca91420 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -200,6 +200,19 @@ config USB_TEST
 	  See <http://www.linux-usb.org/usbtest/> for more information,
 	  including sample test device firmware and "how to use it".
 
+config USB_EHSET_TEST_FIXTURE
+        tristate "USB EHSET Test Fixture driver"
+        help
+	  Say Y here if you want to support the special test fixture device
+	  used for the USB-IF Embedded Host High-Speed Electrical Test procedure.
+
+	  When the test fixture is connected, it can enumerate as one of several
+	  VID/PID pairs. This driver then initiates a corresponding test mode on
+	  the downstream port to which the test fixture is attached.
+
+	  See <http://www.usb.org/developers/onthego/EHSET_v1.01.pdf> for more
+	  information.
+
 config USB_ISIGHTFW
 	tristate "iSight firmware loading support"
 	select FW_LOADER
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 3e1bd70..c9eafdf 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_USB_LED)			+= usbled.o
 obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
 obj-$(CONFIG_USB_RIO500)		+= rio500.o
 obj-$(CONFIG_USB_TEST)			+= usbtest.o
+obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
 obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
 obj-$(CONFIG_USB_USS720)		+= uss720.o
 obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
diff --git a/drivers/usb/misc/ehset.c b/drivers/usb/misc/ehset.c
new file mode 100644
index 0000000..d2864b3
--- /dev/null
+++ b/drivers/usb/misc/ehset.c
@@ -0,0 +1,134 @@
+/*
+ * Copyright (c) 2010-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/ch11.h>
+
+#define TEST_SE0_NAK_PID			0x0101
+#define TEST_J_PID				0x0102
+#define TEST_K_PID				0x0103
+#define TEST_PACKET_PID				0x0104
+#define TEST_HS_HOST_PORT_SUSPEND_RESUME	0x0106
+#define TEST_SINGLE_STEP_GET_DEV_DESC		0x0107
+#define TEST_SINGLE_STEP_SET_FEATURE		0x0108
+
+static int ehset_probe(struct usb_interface *intf,
+		       const struct usb_device_id *id)
+{
+	int ret = -EINVAL;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	struct usb_device *hub_udev = dev->parent;
+	struct usb_device_descriptor *buf;
+	u8 portnum = dev->portnum;
+	u16 test_pid = le16_to_cpu(dev->descriptor.idProduct);
+
+	switch (test_pid) {
+	case TEST_SE0_NAK_PID:
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_TEST,
+					(TEST_SE0_NAK << 8) | portnum,
+					NULL, 0, 1000);
+		break;
+	case TEST_J_PID:
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_TEST,
+					(TEST_J << 8) | portnum,
+					NULL, 0, 1000);
+		break;
+	case TEST_K_PID:
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_TEST,
+					(TEST_K << 8) | portnum,
+					NULL, 0, 1000);
+		break;
+	case TEST_PACKET_PID:
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_TEST,
+					(TEST_PACKET << 8) | portnum,
+					NULL, 0, 1000);
+		break;
+	case TEST_HS_HOST_PORT_SUSPEND_RESUME:
+		/* Test: wait for 15secs -> suspend -> 15secs delay -> resume */
+		msleep(15 * 1000);
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_SUSPEND, portnum,
+					NULL, 0, 1000);
+		if (ret < 0)
+			break;
+
+		msleep(15 * 1000);
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_CLEAR_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_SUSPEND, portnum,
+					NULL, 0, 1000);
+		break;
+	case TEST_SINGLE_STEP_GET_DEV_DESC:
+		/* Test: wait for 15secs -> GetDescriptor request */
+		msleep(15 * 1000);
+		buf = kmalloc(USB_DT_DEVICE_SIZE, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+
+		ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+					USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
+					USB_DT_DEVICE << 8, 0,
+					buf, USB_DT_DEVICE_SIZE,
+					USB_CTRL_GET_TIMEOUT);
+		kfree(buf);
+		break;
+	case TEST_SINGLE_STEP_SET_FEATURE:
+		/* unsupported for now */
+	default:
+		dev_err(&intf->dev, "%s: unsupported PID: 0x%x\n",
+			__func__, test_pid);
+	}
+
+	return (ret < 0) ? ret : 0;
+}
+
+static void ehset_disconnect(struct usb_interface *intf)
+{
+}
+
+static const struct usb_device_id ehset_id_table[] = {
+	{ USB_DEVICE(0x1a0a, TEST_SE0_NAK_PID) },
+	{ USB_DEVICE(0x1a0a, TEST_J_PID) },
+	{ USB_DEVICE(0x1a0a, TEST_K_PID) },
+	{ USB_DEVICE(0x1a0a, TEST_PACKET_PID) },
+	{ USB_DEVICE(0x1a0a, TEST_HS_HOST_PORT_SUSPEND_RESUME) },
+	{ USB_DEVICE(0x1a0a, TEST_SINGLE_STEP_GET_DEV_DESC) },
+	{ USB_DEVICE(0x1a0a, TEST_SINGLE_STEP_SET_FEATURE) },
+	{ }			/* Terminating entry */
+};
+MODULE_DEVICE_TABLE(usb, ehset_id_table);
+
+static struct usb_driver ehset_driver = {
+	.name =		"usb_ehset_test",
+	.probe =	ehset_probe,
+	.disconnect =	ehset_disconnect,
+	.id_table =	ehset_id_table,
+};
+
+module_usb_driver(ehset_driver);
+
+MODULE_DESCRIPTION("USB Driver for EHSET Test Fixture");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Jack Pham]

From: Manu Gautam <mgautam@codeaurora.org>

The USB Embedded High-speed Host Electrical Test (EHSET) defines the
SINGLE_STEP_SET_FEATURE test as follows:

1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
2) The host sends the SETUP stage of a GetDescriptor(Device)
3) The device ACKs the request
4) The host issues SOFs for 15 seconds allowing the test operator to
   raise the scope trigger just above the SOF voltage level
5) The host sends the IN packet
6) The device sends data in response, triggering the scope
7) The host sends an ACK in response to the data

This patch adds additional handling to the EHCI hub driver and allows
the EHSET driver to initiate this test mode by issuing a a SetFeature
request to the root hub with a Test Selector value of 0x06. From there
it mimics ehci_urb_enqueue() but separately submits QTDs for the
SETUP and DATA/STATUS stages in order to insert a delay in between.

Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
---
 drivers/usb/host/ehci-hub.c |  144 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-q.c   |  101 ++++++++++++++++++++++++++++++
 drivers/usb/misc/ehset.c    |   20 ++++++-
 3 files changed, 263 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 2b70277..8e6dc09 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -711,6 +711,142 @@ ehci_hub_descriptor (
 }
 
 /*-------------------------------------------------------------------------*/
+#define EHSET_TEST_SINGLE_STEP_SET_FEATURE 0x06
+
+static void usb_ehset_completion(struct urb *urb)
+{
+	struct completion  *done = urb->context;
+
+	complete(done);
+}
+static int submit_single_step_set_feature(
+	struct usb_hcd	*hcd,
+	struct urb	*urb,
+	int		is_setup
+);
+
+/*
+ * Allocate and initialize a control URB. This request will be used by the
+ * EHSET SINGLE_STEP_SET_FEATURE test in which the DATA and STATUS stages
+ * of the GetDescriptor request are sent 15 seconds after the SETUP stage.
+ * Return NULL if failed.
+ */
+static struct urb *request_single_step_set_feature_urb(
+	struct usb_device	*udev,
+	void			*dr,
+	void			*buf,
+	struct completion	*done
+) {
+	struct urb *urb;
+	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+	struct usb_host_endpoint *ep;
+
+	urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!urb)
+		return NULL;
+
+	urb->pipe = usb_rcvctrlpipe(udev, 0);
+	ep = (usb_pipein(urb->pipe) ? udev->ep_in : udev->ep_out)
+				[usb_pipeendpoint(urb->pipe)];
+	if (!ep) {
+		usb_free_urb(urb);
+		return NULL;
+	}
+
+	urb->ep = ep;
+	urb->dev = udev;
+	urb->setup_packet = (void *)dr;
+	urb->transfer_buffer = buf;
+	urb->transfer_buffer_length = USB_DT_DEVICE_SIZE;
+	urb->complete = usb_ehset_completion;
+	urb->status = -EINPROGRESS;
+	urb->actual_length = 0;
+	urb->transfer_flags = URB_DIR_IN;
+	usb_get_urb(urb);
+	atomic_inc(&urb->use_count);
+	atomic_inc(&urb->dev->urbnum);
+	urb->setup_dma = dma_map_single(
+			hcd->self.controller,
+			urb->setup_packet,
+			sizeof(struct usb_ctrlrequest),
+			DMA_TO_DEVICE);
+	urb->transfer_dma = dma_map_single(
+			hcd->self.controller,
+			urb->transfer_buffer,
+			urb->transfer_buffer_length,
+			DMA_FROM_DEVICE);
+	urb->context = done;
+	return urb;
+}
+
+static int ehset_single_step_set_feature(struct usb_hcd *hcd, int port)
+{
+	int retval = -ENOMEM;
+	struct usb_ctrlrequest *dr;
+	struct urb *urb;
+	struct usb_device *udev;
+	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+	struct usb_device_descriptor *buf;
+	DECLARE_COMPLETION_ONSTACK(done);
+
+	/* Obtain udev of the rhub's child port */
+	udev = hcd->self.root_hub->children[port];
+	if (!udev) {
+		ehci_err(ehci, "No device attached to the RootHub\n");
+		return -ENODEV;
+	}
+	buf = kmalloc(USB_DT_DEVICE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL);
+	if (!dr) {
+		kfree(buf);
+		return -ENOMEM;
+	}
+
+	/* Fill Setup packet for GetDescriptor */
+	dr->bRequestType = USB_DIR_IN;
+	dr->bRequest = USB_REQ_GET_DESCRIPTOR;
+	dr->wValue = cpu_to_le16(USB_DT_DEVICE << 8);
+	dr->wIndex = 0;
+	dr->wLength = cpu_to_le16(USB_DT_DEVICE_SIZE);
+	urb = request_single_step_set_feature_urb(udev, dr, buf, &done);
+	if (!urb)
+		goto cleanup;
+
+	/* Submit just the SETUP stage */
+	retval = submit_single_step_set_feature(hcd, urb, 1);
+	if (retval)
+		goto out1;
+	if (!wait_for_completion_timeout(&done, msecs_to_jiffies(2000))) {
+		usb_kill_urb(urb);
+		retval = -ETIMEDOUT;
+		ehci_err(ehci, "%s SETUP stage timed out on ep0\n", __func__);
+		goto out1;
+	}
+	msleep(15 * 1000);
+
+	/* Complete remaining DATA and STATUS stages using the same URB */
+	urb->status = -EINPROGRESS;
+	usb_get_urb(urb);
+	atomic_inc(&urb->use_count);
+	atomic_inc(&urb->dev->urbnum);
+	retval = submit_single_step_set_feature(hcd, urb, 0);
+	if (!retval && !wait_for_completion_timeout(&done,
+						msecs_to_jiffies(2000))) {
+		usb_kill_urb(urb);
+		retval = -ETIMEDOUT;
+		ehci_err(ehci, "%s IN stage timed out on ep0\n", __func__);
+	}
+out1:
+	usb_free_urb(urb);
+cleanup:
+	kfree(dr);
+	kfree(buf);
+	return retval;
+}
+/*-------------------------------------------------------------------------*/
 
 static int ehci_hub_control (
 	struct usb_hcd	*hcd,
@@ -1091,7 +1227,13 @@ static int ehci_hub_control (
 		 * about the EHCI-specific stuff.
 		 */
 		case USB_PORT_FEAT_TEST:
-			if (!selector || selector > 5)
+			if (selector == 6) {
+				spin_unlock_irqrestore(&ehci->lock, flags);
+				retval = ehset_single_step_set_feature(hcd,
+									wIndex);
+				spin_lock_irqsave(&ehci->lock, flags);
+				break;
+			} else if (!selector || selector > 5)
 				goto error;
 			spin_unlock_irqrestore(&ehci->lock, flags);
 			ehci_quiesce(ehci);
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index d34b399..702a0e9 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -1144,6 +1144,107 @@ submit_async (
 }
 
 /*-------------------------------------------------------------------------*/
+/*
+ * This function creates the qtds and submits them for the
+ * SINGLE_STEP_SET_FEATURE Test.
+ * This is done in two parts: first SETUP req for GetDesc is sent then
+ * 15 seconds later, the IN stage for GetDesc starts to req data from dev
+ *
+ * is_setup : i/p arguement decides which of the two stage needs to be
+ * performed; TRUE - SETUP and FALSE - IN+STATUS
+ * Returns 0 if success
+ */
+static int submit_single_step_set_feature(
+	struct usb_hcd  *hcd,
+	struct urb      *urb,
+	int             is_setup
+) {
+	struct ehci_hcd		*ehci = hcd_to_ehci(hcd);
+	struct list_head	qtd_list;
+	struct list_head	*head;
+
+	struct ehci_qtd		*qtd, *qtd_prev;
+	dma_addr_t		buf;
+	int			len, maxpacket;
+	u32			token;
+
+	INIT_LIST_HEAD(&qtd_list);
+	head = &qtd_list;
+
+	/* URBs map to sequences of QTDs:  one logical transaction */
+	qtd = ehci_qtd_alloc(ehci, GFP_KERNEL);
+	if (unlikely(!qtd))
+		return -1;
+	list_add_tail(&qtd->qtd_list, head);
+	qtd->urb = urb;
+
+	token = QTD_STS_ACTIVE;
+	token |= (EHCI_TUNE_CERR << 10);
+
+	len = urb->transfer_buffer_length;
+	/*
+	 * Check if the request is to perform just the SETUP stage (getDesc)
+	 * as in SINGLE_STEP_SET_FEATURE test, DATA stage (IN) happens
+	 * 15 secs after the setup
+	 */
+	if (is_setup) {
+		/* SETUP pid */
+		qtd_fill(ehci, qtd, urb->setup_dma,
+				sizeof(struct usb_ctrlrequest),
+				token | (2 /* "setup" */ << 8), 8);
+
+		submit_async(ehci, urb, &qtd_list, GFP_ATOMIC);
+		return 0; /*Return now; we shall come back after 15 seconds*/
+	}
+
+	/*
+	 * IN: data transfer stage:  buffer setup : start the IN txn phase for
+	 * the get_Desc SETUP which was sent 15seconds back
+	 */
+	token ^= QTD_TOGGLE;   /*We need to start IN with DATA-1 Pid-sequence*/
+	buf = urb->transfer_dma;
+
+	token |= (1 /* "in" */ << 8);  /*This is IN stage*/
+
+	maxpacket = max_packet(usb_maxpacket(urb->dev, urb->pipe, 0));
+
+	qtd_fill(ehci, qtd, buf, len, token, maxpacket);
+
+	/*
+	 * Our IN phase shall always be a short read; so keep the queue running
+	 * and let it advance to the next qtd which zero length OUT status
+	 */
+	qtd->hw_alt_next = EHCI_LIST_END(ehci);
+
+	/* STATUS stage for GetDesc control request */
+	token ^= 0x0100;        /* "in" <--> "out"  */
+	token |= QTD_TOGGLE;    /* force DATA1 */
+
+	qtd_prev = qtd;
+	qtd = ehci_qtd_alloc(ehci, GFP_ATOMIC);
+	if (unlikely(!qtd))
+		goto cleanup;
+	qtd->urb = urb;
+	qtd_prev->hw_next = QTD_NEXT(ehci, qtd->qtd_dma);
+	list_add_tail(&qtd->qtd_list, head);
+
+	/* dont fill any data in such packets */
+	qtd_fill(ehci, qtd, 0, 0, token, 0);
+
+	/* by default, enable interrupt on urb completion */
+	if (likely(!(urb->transfer_flags & URB_NO_INTERRUPT)))
+		qtd->hw_token |= cpu_to_hc32(ehci, QTD_IOC);
+
+	submit_async(ehci, urb, &qtd_list, GFP_KERNEL);
+
+	return 0;
+
+cleanup:
+	qtd_list_free(ehci, urb, head);
+	return -1;
+}
+
+/*-------------------------------------------------------------------------*/
 
 static void single_unlink_async(struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
diff --git a/drivers/usb/misc/ehset.c b/drivers/usb/misc/ehset.c
index d2864b3..c31b4a3 100644
--- a/drivers/usb/misc/ehset.c
+++ b/drivers/usb/misc/ehset.c
@@ -96,7 +96,25 @@ static int ehset_probe(struct usb_interface *intf,
 		kfree(buf);
 		break;
 	case TEST_SINGLE_STEP_SET_FEATURE:
-		/* unsupported for now */
+		/*
+		 * GetDescriptor SETUP request -> 15secs delay -> IN & STATUS
+		 *
+		 * Note, this test is only supported on root hubs since the
+		 * SetPortFeature handling can only be done inside the HCD's
+		 * hub_control callback function.
+		 */
+		if (hub_udev != dev->bus->root_hub) {
+			dev_err(&intf->dev, "SINGLE_STEP_SET_FEATURE test only supported on root hub\n");
+			break;
+		}
+
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_TEST,
+					(6 << 8) | portnum,
+					NULL, 0, 60 * 1000);
+
+		break;
 	default:
 		dev_err(&intf->dev, "%s: unsupported PID: 0x%x\n",
 			__func__, test_pid);
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Greg KH]

On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
> From: Manu Gautam <mgautam@codeaurora.org>
> 
> The USB Embedded High-speed Host Electrical Test (EHSET) defines the
> SINGLE_STEP_SET_FEATURE test as follows:
> 
> 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
> 2) The host sends the SETUP stage of a GetDescriptor(Device)
> 3) The device ACKs the request
> 4) The host issues SOFs for 15 seconds allowing the test operator to
>    raise the scope trigger just above the SOF voltage level
> 5) The host sends the IN packet
> 6) The device sends data in response, triggering the scope
> 7) The host sends an ACK in response to the data
> 
> This patch adds additional handling to the EHCI hub driver and allows
> the EHSET driver to initiate this test mode by issuing a a SetFeature
> request to the root hub with a Test Selector value of 0x06. From there
> it mimics ehci_urb_enqueue() but separately submits QTDs for the
> SETUP and DATA/STATUS stages in order to insert a delay in between.
> 
> Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> Signed-off-by: Jack Pham <jackp@codeaurora.org>

Alan, any thoughts about this patch?

thanks,

greg k-h

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Thu, 25 Jul 2013, Greg KH wrote:

> On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
> > From: Manu Gautam <mgautam@codeaurora.org>
> > 
> > The USB Embedded High-speed Host Electrical Test (EHSET) defines the
> > SINGLE_STEP_SET_FEATURE test as follows:
> > 
> > 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
> > 2) The host sends the SETUP stage of a GetDescriptor(Device)
> > 3) The device ACKs the request
> > 4) The host issues SOFs for 15 seconds allowing the test operator to
> >    raise the scope trigger just above the SOF voltage level
> > 5) The host sends the IN packet
> > 6) The device sends data in response, triggering the scope
> > 7) The host sends an ACK in response to the data
> > 
> > This patch adds additional handling to the EHCI hub driver and allows
> > the EHSET driver to initiate this test mode by issuing a a SetFeature
> > request to the root hub with a Test Selector value of 0x06. From there
> > it mimics ehci_urb_enqueue() but separately submits QTDs for the
> > SETUP and DATA/STATUS stages in order to insert a delay in between.
> > 
> > Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> > Signed-off-by: Jack Pham <jackp@codeaurora.org>
> 
> Alan, any thoughts about this patch?

Sorry, this slipped my mind.

It looks okay.  I haven't tested it yet (and it's so specialized that
it probably will never receive very much testing).  It is somewhat 
fragile, in that it copies part of usbcore into ehci-hcd; updates to 
the core will have to be mirrored in the driver.

On the other hand, there's no real reason to reject it, and it could 
end up helping people who want to test new USB devices.  So...

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

On Thu, Jul 25, 2013 at 03:44:20PM -0400, Alan Stern wrote:
> On Thu, 25 Jul 2013, Greg KH wrote:
> 
> > On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
> > > From: Manu Gautam <mgautam@codeaurora.org>
> > > 
> > > The USB Embedded High-speed Host Electrical Test (EHSET) defines the
> > > SINGLE_STEP_SET_FEATURE test as follows:
> > > 
> > > 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
> > > 2) The host sends the SETUP stage of a GetDescriptor(Device)
> > > 3) The device ACKs the request
> > > 4) The host issues SOFs for 15 seconds allowing the test operator to
> > >    raise the scope trigger just above the SOF voltage level
> > > 5) The host sends the IN packet
> > > 6) The device sends data in response, triggering the scope
> > > 7) The host sends an ACK in response to the data
> > > 
> > > This patch adds additional handling to the EHCI hub driver and allows
> > > the EHSET driver to initiate this test mode by issuing a a SetFeature
> > > request to the root hub with a Test Selector value of 0x06. From there
> > > it mimics ehci_urb_enqueue() but separately submits QTDs for the
> > > SETUP and DATA/STATUS stages in order to insert a delay in between.
> > > 
> > > Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> > > Signed-off-by: Jack Pham <jackp@codeaurora.org>
> > 
> > Alan, any thoughts about this patch?
> 
> Sorry, this slipped my mind.
> 
> It looks okay.  I haven't tested it yet (and it's so specialized that
> it probably will never receive very much testing).  It is somewhat 
> fragile, in that it copies part of usbcore into ehci-hcd; updates to 
> the core will have to be mirrored in the driver.
> 
> On the other hand, there's no real reason to reject it, and it could 
> end up helping people who want to test new USB devices.  So...
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Wait a minute, didn't we discuss a while back that these test features
should be built into usbcore so that we could have a usbcv clone for
linux ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Thu, 25 Jul 2013, Felipe Balbi wrote:

> On Thu, Jul 25, 2013 at 03:44:20PM -0400, Alan Stern wrote:
> > On Thu, 25 Jul 2013, Greg KH wrote:
> > 
> > > On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
> > > > From: Manu Gautam <mgautam@codeaurora.org>
> > > > 
> > > > The USB Embedded High-speed Host Electrical Test (EHSET) defines the
> > > > SINGLE_STEP_SET_FEATURE test as follows:
> > > > 
> > > > 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
> > > > 2) The host sends the SETUP stage of a GetDescriptor(Device)
> > > > 3) The device ACKs the request
> > > > 4) The host issues SOFs for 15 seconds allowing the test operator to
> > > >    raise the scope trigger just above the SOF voltage level
> > > > 5) The host sends the IN packet
> > > > 6) The device sends data in response, triggering the scope
> > > > 7) The host sends an ACK in response to the data
> > > > 
> > > > This patch adds additional handling to the EHCI hub driver and allows
> > > > the EHSET driver to initiate this test mode by issuing a a SetFeature
> > > > request to the root hub with a Test Selector value of 0x06. From there
> > > > it mimics ehci_urb_enqueue() but separately submits QTDs for the
> > > > SETUP and DATA/STATUS stages in order to insert a delay in between.
> > > > 
> > > > Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> > > > Signed-off-by: Jack Pham <jackp@codeaurora.org>
> > > 
> > > Alan, any thoughts about this patch?
> > 
> > Sorry, this slipped my mind.
> > 
> > It looks okay.  I haven't tested it yet (and it's so specialized that
> > it probably will never receive very much testing).  It is somewhat 
> > fragile, in that it copies part of usbcore into ehci-hcd; updates to 
> > the core will have to be mirrored in the driver.
> > 
> > On the other hand, there's no real reason to reject it, and it could 
> > end up helping people who want to test new USB devices.  So...
> > 
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> 
> Wait a minute, didn't we discuss a while back that these test features
> should be built into usbcore so that we could have a usbcv clone for
> linux ?

There's no way this can be built into the core.  This test requires the
behavior of the host controller to be modified at a low level
(introducing a 15-second delay between the Setup and Data stages of a
control transfer).  Only the HCD can do that.

Besides, doesn't USB-CV install its own version of Windows' EHCI driver 
while it runs its tests?

Alan Stern

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2996 bytes --]

Hi,

On Thu, Jul 25, 2013 at 05:33:48PM -0400, Alan Stern wrote:
> > On Thu, Jul 25, 2013 at 03:44:20PM -0400, Alan Stern wrote:
> > > On Thu, 25 Jul 2013, Greg KH wrote:
> > > 
> > > > On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
> > > > > From: Manu Gautam <mgautam@codeaurora.org>
> > > > > 
> > > > > The USB Embedded High-speed Host Electrical Test (EHSET) defines the
> > > > > SINGLE_STEP_SET_FEATURE test as follows:
> > > > > 
> > > > > 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
> > > > > 2) The host sends the SETUP stage of a GetDescriptor(Device)
> > > > > 3) The device ACKs the request
> > > > > 4) The host issues SOFs for 15 seconds allowing the test operator to
> > > > >    raise the scope trigger just above the SOF voltage level
> > > > > 5) The host sends the IN packet
> > > > > 6) The device sends data in response, triggering the scope
> > > > > 7) The host sends an ACK in response to the data
> > > > > 
> > > > > This patch adds additional handling to the EHCI hub driver and allows
> > > > > the EHSET driver to initiate this test mode by issuing a a SetFeature
> > > > > request to the root hub with a Test Selector value of 0x06. From there
> > > > > it mimics ehci_urb_enqueue() but separately submits QTDs for the
> > > > > SETUP and DATA/STATUS stages in order to insert a delay in between.
> > > > > 
> > > > > Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> > > > > Signed-off-by: Jack Pham <jackp@codeaurora.org>
> > > > 
> > > > Alan, any thoughts about this patch?
> > > 
> > > Sorry, this slipped my mind.
> > > 
> > > It looks okay.  I haven't tested it yet (and it's so specialized that
> > > it probably will never receive very much testing).  It is somewhat 
> > > fragile, in that it copies part of usbcore into ehci-hcd; updates to 
> > > the core will have to be mirrored in the driver.
> > > 
> > > On the other hand, there's no real reason to reject it, and it could 
> > > end up helping people who want to test new USB devices.  So...
> > > 
> > > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> > 
> > Wait a minute, didn't we discuss a while back that these test features
> > should be built into usbcore so that we could have a usbcv clone for
> > linux ?
> 
> There's no way this can be built into the core.  This test requires the
> behavior of the host controller to be modified at a low level
> (introducing a 15-second delay between the Setup and Data stages of a
> control transfer).  Only the HCD can do that.

heh, it doesn't need to be entirely in the core. Core could have the
generic calls and HCDs could implement some callbacks, but I think quite
a bit of the code will be similar if we implement the same thing on all
HCDs.

> Besides, doesn't USB-CV install its own version of Windows' EHCI driver 
> while it runs its tests?

IIRC only USB20CV, the new USB30CV (which also tests USB2) doesn't do
that.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Fri, 9 Aug 2013, Felipe Balbi wrote:

> > > Wait a minute, didn't we discuss a while back that these test features
> > > should be built into usbcore so that we could have a usbcv clone for
> > > linux ?
> > 
> > There's no way this can be built into the core.  This test requires the
> > behavior of the host controller to be modified at a low level
> > (introducing a 15-second delay between the Setup and Data stages of a
> > control transfer).  Only the HCD can do that.
> 
> heh, it doesn't need to be entirely in the core. Core could have the
> generic calls and HCDs could implement some callbacks, but I think quite
> a bit of the code will be similar if we implement the same thing on all
> HCDs.

What generic calls and callbacks would you suggest?  I assume you want 
enough to cover not just this one test but the entire USB-CV suite.

> > Besides, doesn't USB-CV install its own version of Windows' EHCI driver 
> > while it runs its tests?
> 
> IIRC only USB20CV, the new USB30CV (which also tests USB2) doesn't do
> that.

I don't have more than the foggiest notion of how these things really
work under Windows.

Alan Stern

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]

Hi,

On Fri, Aug 09, 2013 at 10:37:50AM -0400, Alan Stern wrote:
> > > > Wait a minute, didn't we discuss a while back that these test features
> > > > should be built into usbcore so that we could have a usbcv clone for
> > > > linux ?
> > > 
> > > There's no way this can be built into the core.  This test requires the
> > > behavior of the host controller to be modified at a low level
> > > (introducing a 15-second delay between the Setup and Data stages of a
> > > control transfer).  Only the HCD can do that.
> > 
> > heh, it doesn't need to be entirely in the core. Core could have the
> > generic calls and HCDs could implement some callbacks, but I think quite
> > a bit of the code will be similar if we implement the same thing on all
> > HCDs.
> 
> What generic calls and callbacks would you suggest?  I assume you want 
> enough to cover not just this one test but the entire USB-CV suite.

maybe a single callback for supporting 'testmodes' ? which receives the
test mode as argument ?

> > > Besides, doesn't USB-CV install its own version of Windows' EHCI driver 
> > > while it runs its tests?
> > 
> > IIRC only USB20CV, the new USB30CV (which also tests USB2) doesn't do
> > that.
> 
> I don't have more than the foggiest notion of how these things really
> work under Windows.

and that's hardly necessary to understand...

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Fri, 9 Aug 2013, Felipe Balbi wrote:

> > > heh, it doesn't need to be entirely in the core. Core could have the
> > > generic calls and HCDs could implement some callbacks, but I think quite
> > > a bit of the code will be similar if we implement the same thing on all
> > > HCDs.
> > 
> > What generic calls and callbacks would you suggest?  I assume you want 
> > enough to cover not just this one test but the entire USB-CV suite.
> 
> maybe a single callback for supporting 'testmodes' ? which receives the
> test mode as argument ?

I don't have a clear picture of how you would apply such an approach to 
this case.  There would have to be a way to tell the HCD to insert a
15-second delay between the Setup and Data stages of a particular
control URB.  How would you do that?  Whatever method you choose,
implementing it in every HCD would be a huge amount of work.

What other test modes would you want to support?

Is it worth adding this support to the standard host controller
drivers, or should there be a special version (a Kconfig option like
CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
in distribution kernels where it will never be used seems like a big
waste.

Alan Stern

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

Hi,

On Fri, Aug 09, 2013 at 11:04:48AM -0400, Alan Stern wrote:
> > > > heh, it doesn't need to be entirely in the core. Core could have the
> > > > generic calls and HCDs could implement some callbacks, but I think quite
> > > > a bit of the code will be similar if we implement the same thing on all
> > > > HCDs.
> > > 
> > > What generic calls and callbacks would you suggest?  I assume you want 
> > > enough to cover not just this one test but the entire USB-CV suite.
> > 
> > maybe a single callback for supporting 'testmodes' ? which receives the
> > test mode as argument ?
> 
> I don't have a clear picture of how you would apply such an approach to 
> this case.  There would have to be a way to tell the HCD to insert a
> 15-second delay between the Setup and Data stages of a particular
> control URB.  How would you do that?  Whatever method you choose,

each test is started after enumerating a known Vid/Pid pair. Based on
that, you *know* which test to run.

> implementing it in every HCD would be a huge amount of work.

sure, we can support different HCDs with time, but once SINGLE_STEP is
implemented in e.g. EHCI, it should be simple to port it to
OHCI/xHCI/MUSB/etc.

> What other test modes would you want to support?

anything that USB[23]0CV supports today. There are even link layer tests
for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
of a large(-ish) test suite which needs to be supported.

> Is it worth adding this support to the standard host controller
> drivers, or should there be a special version (a Kconfig option like
> CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
> in distribution kernels where it will never be used seems like a big
> waste.

right, I think it should be hidden by Kconfig, not arguing against that.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Mon, 12 Aug 2013, Felipe Balbi wrote:

> > > maybe a single callback for supporting 'testmodes' ? which receives the
> > > test mode as argument ?
> > 
> > I don't have a clear picture of how you would apply such an approach to 
> > this case.  There would have to be a way to tell the HCD to insert a
> > 15-second delay between the Setup and Data stages of a particular
> > control URB.  How would you do that?  Whatever method you choose,
> 
> each test is started after enumerating a known Vid/Pid pair. Based on
> that, you *know* which test to run.

That's not what I meant.  Yes, the test-device driver knows what test
it wants to run, based on the VID/PID.  I was asking how it would
communicate this knowledge to the HCD.

For example, it doesn't make sense to have a callback that means 
"Insert a 15-second delay into the next URB that I submit", because the 
HCD doesn't know where URBs come from.

> > What other test modes would you want to support?
> 
> anything that USB[23]0CV supports today. There are even link layer tests
> for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
> of a large(-ish) test suite which needs to be supported.

That's what I'm trying to find out.  What are the special features that 
we would need to implement in order to support the entire test suite?

> > Is it worth adding this support to the standard host controller
> > drivers, or should there be a special version (a Kconfig option like
> > CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
> > in distribution kernels where it will never be used seems like a big
> > waste.
> 
> right, I think it should be hidden by Kconfig, not arguing against that.

Therefore we both agree the $SUBJECT patch should not be accepted in
its current form.  At the very least, the new code in ehci-hcd should
be conditional on a CONFIG_USB_HCD_TEST_MODE option.  In addition, we
may want some of the work (though at this point I'm not still clear on
exactly what parts) to be moved into usbcore.

Alan Stern

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2725 bytes --]

On Mon, Aug 12, 2013 at 02:52:33PM -0400, Alan Stern wrote:
> On Mon, 12 Aug 2013, Felipe Balbi wrote:
> 
> > > > maybe a single callback for supporting 'testmodes' ? which receives the
> > > > test mode as argument ?
> > > 
> > > I don't have a clear picture of how you would apply such an approach to 
> > > this case.  There would have to be a way to tell the HCD to insert a
> > > 15-second delay between the Setup and Data stages of a particular
> > > control URB.  How would you do that?  Whatever method you choose,
> > 
> > each test is started after enumerating a known Vid/Pid pair. Based on
> > that, you *know* which test to run.
> 
> That's not what I meant.  Yes, the test-device driver knows what test
> it wants to run, based on the VID/PID.  I was asking how it would
> communicate this knowledge to the HCD.
> 
> For example, it doesn't make sense to have a callback that means 
> "Insert a 15-second delay into the next URB that I submit", because the 
> HCD doesn't know where URBs come from.

static int ehci_test_mode(struct usb_hcd *hcd, unsigned int test)
{
	struct ehci_hcd *ehci = to_ehci(hcd);

	....

	switch (test) {
	case USB_TEST_SINGLE_STEP_GET_DESC:
		start_test();
		wait_15_seconds();
		finish_test();
		break;
	...

	default:
		return -ENOTSUP;
	}

	return ret;
}

...

static struct hc_driver ehci_hcd_driver = {
	....

	.test_mode	= ehci_test_mode,

	...
};

> > > What other test modes would you want to support?
> > 
> > anything that USB[23]0CV supports today. There are even link layer tests
> > for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
> > of a large(-ish) test suite which needs to be supported.
> 
> That's what I'm trying to find out.  What are the special features that 
> we would need to implement in order to support the entire test suite?

I haven't looked at USB??CV spec for a while, maybe Jack knows better ?

> > > Is it worth adding this support to the standard host controller
> > > drivers, or should there be a special version (a Kconfig option like
> > > CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
> > > in distribution kernels where it will never be used seems like a big
> > > waste.
> > 
> > right, I think it should be hidden by Kconfig, not arguing against that.
> 
> Therefore we both agree the $SUBJECT patch should not be accepted in
> its current form.  At the very least, the new code in ehci-hcd should
> be conditional on a CONFIG_USB_HCD_TEST_MODE option.  In addition, we
> may want some of the work (though at this point I'm not still clear on
> exactly what parts) to be moved into usbcore.

right

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Tue, 13 Aug 2013, Felipe Balbi wrote:

> > That's not what I meant.  Yes, the test-device driver knows what test
> > it wants to run, based on the VID/PID.  I was asking how it would
> > communicate this knowledge to the HCD.
> > 
> > For example, it doesn't make sense to have a callback that means 
> > "Insert a 15-second delay into the next URB that I submit", because the 
> > HCD doesn't know where URBs come from.
> 
> static int ehci_test_mode(struct usb_hcd *hcd, unsigned int test)
> {
> 	struct ehci_hcd *ehci = to_ehci(hcd);
> 
> 	....
> 
> 	switch (test) {
> 	case USB_TEST_SINGLE_STEP_GET_DESC:
> 		start_test();
> 		wait_15_seconds();
> 		finish_test();
> 		break;
> 	...
> 
> 	default:
> 		return -ENOTSUP;
> 	}
> 
> 	return ret;
> }
> 
> ...
> 
> static struct hc_driver ehci_hcd_driver = {
> 	....
> 
> 	.test_mode	= ehci_test_mode,
> 
> 	...
> };

This is almost exactly the same as the way it is done in the newly
merged code.  The only difference is that it uses a special test_mode 
callback instead of a special selector value.

Didn't you say you wanted a large part of the support moved into
usbcore?  For example, the routines that create and enqueue this "URB
with a delay in the middle" are little more than copies of the core
code.

Alan Stern

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Jack Pham]

On Tue, Aug 13, 2013 at 10:32:57AM -0500, Felipe Balbi wrote:
> On Mon, Aug 12, 2013 at 02:52:33PM -0400, Alan Stern wrote:
> > On Mon, 12 Aug 2013, Felipe Balbi wrote:
> > > anything that USB[23]0CV supports today. There are even link layer tests
> > > for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
> > > of a large(-ish) test suite which needs to be supported.
> > 
> > That's what I'm trying to find out.  What are the special features that 
> > we would need to implement in order to support the entire test suite?
> 
> I haven't looked at USB??CV spec for a while, maybe Jack knows better ?

I've been going off of
http://www.usb.org/developers/onthego/EHSET_v1.01.pdf which is specific
to OTG and Embedded hosts, but I think it overlaps pretty close with
with what USB20CV/HSET is trying to cover.

So far the SINGLE_STEP_GET_DEVICE_DESCRIPTOR and SINGLE_STEP_SET_FEATURE
tests are the only ones we've had to implement in software for, and
obviously with the latter requiring further modification in the HCD
itself for that 15-second delay insertion.

The other test modes that the USB20CV/HSETT tool appears to invoke (TEST_J,
TEST_K, etc.) are actually defined in USB 2.0 Section 7.1.20 and also
reserved in EHCI's PORTSC bits [19:16], so whatever invokes these tests
modes--unlike SINGLE_STEP--simply issue a SET_FEATURE(PORT_FEAT_TEST)
and can expect the hardware to handle it.
 
> > Therefore we both agree the $SUBJECT patch should not be accepted in
> > its current form.  At the very least, the new code in ehci-hcd should
> > be conditional on a CONFIG_USB_HCD_TEST_MODE option.

Already submitted a follow-on patch for that.

> > In addition, we may want some of the work (though at this point I'm
> > not still clear on exactly what parts) to be moved into usbcore.

+1. I am open to suggestions on how best to achieve this.  I have a
another patch in the for doing the same 15-second delay in xHCI but am
hesitant to submit it just yet, pending ideas on a better way to do it
in the common HCD core.

Thanks,
Jack
-- 
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Alan Stern]

On Tue, 13 Aug 2013, Jack Pham wrote:

> > > In addition, we may want some of the work (though at this point I'm
> > > not still clear on exactly what parts) to be moved into usbcore.
> 
> +1. I am open to suggestions on how best to achieve this.  I have a
> another patch in the for doing the same 15-second delay in xHCI but am
> hesitant to submit it just yet, pending ideas on a better way to do it
> in the common HCD core.

Well, here's one possibility, not fully thought out.  Suppose there was
a way to tell the HCD to carry out _just_ the Setup stage of a control
transfer.  Or to omit the Setup stage and go directly to the Data
stage.  Then the SINGLE_STEP_GET_DEVICE_DESCRIPTOR test could be
implemented very simply by submitting two URBs with a delay between.

Alan Stern

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Jack Pham]

On Wed, Aug 14, 2013 at 10:10:23AM -0400, Alan Stern wrote:
> On Tue, 13 Aug 2013, Jack Pham wrote:
> 
> > > > In addition, we may want some of the work (though at this point I'm
> > > > not still clear on exactly what parts) to be moved into usbcore.
> > 
> > +1. I am open to suggestions on how best to achieve this.  I have a
> > another patch in the for doing the same 15-second delay in xHCI but am
> > hesitant to submit it just yet, pending ideas on a better way to do it
> > in the common HCD core.
> 
> Well, here's one possibility, not fully thought out.  Suppose there was
> a way to tell the HCD to carry out _just_ the Setup stage of a control
> transfer.  Or to omit the Setup stage and go directly to the Data
> stage.  Then the SINGLE_STEP_GET_DEVICE_DESCRIPTOR test could be
> implemented very simply by submitting two URBs with a delay between.

Ok, but that would be in the form of a hook from usbcore to the HCD
driver or an URB flag right?  In both ehci-hcd and xhci-hcd, setup
transfers are handled in their entirety and my work was simply
duplicating those functions to be able to split them up.  To do this in
a generic fashion would still need HCD-specific implementation.  But it
would at least still allow moving the "submit setup urb", "wait", and
"submit data/status urb" steps into usbcore instead, which is much
cleaner.

Jack
-- 
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Jack Pham]

On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
> The USB Embedded High-speed Host Electrical Test (EHSET) defines the
> SINGLE_STEP_SET_FEATURE test as follows:

> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index 2b70277..8e6dc09 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> +static int ehset_single_step_set_feature(struct usb_hcd *hcd, int port)
> +{
> +	int retval = -ENOMEM;
> +	struct usb_ctrlrequest *dr;
> +	struct urb *urb;
> +	struct usb_device *udev;
> +	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> +	struct usb_device_descriptor *buf;
> +	DECLARE_COMPLETION_ONSTACK(done);
> +
> +	/* Obtain udev of the rhub's child port */
> +	udev = hcd->self.root_hub->children[port];

Oops, I forgot to add this additional fix needed after refreshing to
latest:

-       udev = hcd->self.root_hub->children[port];
+       udev = usb_hub_find_child(hcd->self.root_hub, port);

I'll wait for Felipe's response regarding whether it's appropriate to do
this in the HCD, and then will re-send without the RFC tag.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Jack Pham]

From: Manu Gautam <mgautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

The USB Embedded High-speed Host Electrical Test (EHSET) defines the
SINGLE_STEP_SET_FEATURE test as follows:

1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
2) The host sends the SETUP stage of a GetDescriptor(Device)
3) The device ACKs the request
4) The host issues SOFs for 15 seconds allowing the test operator to
   raise the scope trigger just above the SOF voltage level
5) The host sends the IN packet
6) The device sends data in response, triggering the scope
7) The host sends an ACK in response to the data

This patch adds additional handling to the EHCI hub driver and allows
the EHSET driver to initiate this test mode by issuing a a SetFeature
request to the root hub with a Test Selector value of 0x06. From there
it mimics ehci_urb_enqueue() but separately submits QTDs for the
SETUP and DATA/STATUS stages in order to insert a delay in between.

Signed-off-by: Manu Gautam <mgautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
[jackp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org: imported from commit c2084930 on codeaurora.org;
 minor cleanup and updated author email]
Signed-off-by: Jack Pham <jackp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
Changes from RFC:
- replaced reference to non-existent root_hub->children with call to
   usb_hub_find_child()
- use EHSET_TEST_SINGLE_STEP_SET_FEATURE constant

This should be applied to usb-next atop or after commit 1353aa53.

 drivers/usb/host/ehci-hub.c |  144 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-q.c   |  101 ++++++++++++++++++++++++++++++
 drivers/usb/misc/ehset.c    |   20 ++++++-
 3 files changed, 263 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 2b70277..d0fd0be 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -711,6 +711,142 @@ ehci_hub_descriptor (
 }
 
 /*-------------------------------------------------------------------------*/
+#define EHSET_TEST_SINGLE_STEP_SET_FEATURE 0x06
+
+static void usb_ehset_completion(struct urb *urb)
+{
+	struct completion  *done = urb->context;
+
+	complete(done);
+}
+static int submit_single_step_set_feature(
+	struct usb_hcd	*hcd,
+	struct urb	*urb,
+	int		is_setup
+);
+
+/*
+ * Allocate and initialize a control URB. This request will be used by the
+ * EHSET SINGLE_STEP_SET_FEATURE test in which the DATA and STATUS stages
+ * of the GetDescriptor request are sent 15 seconds after the SETUP stage.
+ * Return NULL if failed.
+ */
+static struct urb *request_single_step_set_feature_urb(
+	struct usb_device	*udev,
+	void			*dr,
+	void			*buf,
+	struct completion	*done
+) {
+	struct urb *urb;
+	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+	struct usb_host_endpoint *ep;
+
+	urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!urb)
+		return NULL;
+
+	urb->pipe = usb_rcvctrlpipe(udev, 0);
+	ep = (usb_pipein(urb->pipe) ? udev->ep_in : udev->ep_out)
+				[usb_pipeendpoint(urb->pipe)];
+	if (!ep) {
+		usb_free_urb(urb);
+		return NULL;
+	}
+
+	urb->ep = ep;
+	urb->dev = udev;
+	urb->setup_packet = (void *)dr;
+	urb->transfer_buffer = buf;
+	urb->transfer_buffer_length = USB_DT_DEVICE_SIZE;
+	urb->complete = usb_ehset_completion;
+	urb->status = -EINPROGRESS;
+	urb->actual_length = 0;
+	urb->transfer_flags = URB_DIR_IN;
+	usb_get_urb(urb);
+	atomic_inc(&urb->use_count);
+	atomic_inc(&urb->dev->urbnum);
+	urb->setup_dma = dma_map_single(
+			hcd->self.controller,
+			urb->setup_packet,
+			sizeof(struct usb_ctrlrequest),
+			DMA_TO_DEVICE);
+	urb->transfer_dma = dma_map_single(
+			hcd->self.controller,
+			urb->transfer_buffer,
+			urb->transfer_buffer_length,
+			DMA_FROM_DEVICE);
+	urb->context = done;
+	return urb;
+}
+
+static int ehset_single_step_set_feature(struct usb_hcd *hcd, int port)
+{
+	int retval = -ENOMEM;
+	struct usb_ctrlrequest *dr;
+	struct urb *urb;
+	struct usb_device *udev;
+	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+	struct usb_device_descriptor *buf;
+	DECLARE_COMPLETION_ONSTACK(done);
+
+	/* Obtain udev of the rhub's child port */
+	udev = usb_hub_find_child(hcd->self.root_hub, port);
+	if (!udev) {
+		ehci_err(ehci, "No device attached to the RootHub\n");
+		return -ENODEV;
+	}
+	buf = kmalloc(USB_DT_DEVICE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL);
+	if (!dr) {
+		kfree(buf);
+		return -ENOMEM;
+	}
+
+	/* Fill Setup packet for GetDescriptor */
+	dr->bRequestType = USB_DIR_IN;
+	dr->bRequest = USB_REQ_GET_DESCRIPTOR;
+	dr->wValue = cpu_to_le16(USB_DT_DEVICE << 8);
+	dr->wIndex = 0;
+	dr->wLength = cpu_to_le16(USB_DT_DEVICE_SIZE);
+	urb = request_single_step_set_feature_urb(udev, dr, buf, &done);
+	if (!urb)
+		goto cleanup;
+
+	/* Submit just the SETUP stage */
+	retval = submit_single_step_set_feature(hcd, urb, 1);
+	if (retval)
+		goto out1;
+	if (!wait_for_completion_timeout(&done, msecs_to_jiffies(2000))) {
+		usb_kill_urb(urb);
+		retval = -ETIMEDOUT;
+		ehci_err(ehci, "%s SETUP stage timed out on ep0\n", __func__);
+		goto out1;
+	}
+	msleep(15 * 1000);
+
+	/* Complete remaining DATA and STATUS stages using the same URB */
+	urb->status = -EINPROGRESS;
+	usb_get_urb(urb);
+	atomic_inc(&urb->use_count);
+	atomic_inc(&urb->dev->urbnum);
+	retval = submit_single_step_set_feature(hcd, urb, 0);
+	if (!retval && !wait_for_completion_timeout(&done,
+						msecs_to_jiffies(2000))) {
+		usb_kill_urb(urb);
+		retval = -ETIMEDOUT;
+		ehci_err(ehci, "%s IN stage timed out on ep0\n", __func__);
+	}
+out1:
+	usb_free_urb(urb);
+cleanup:
+	kfree(dr);
+	kfree(buf);
+	return retval;
+}
+/*-------------------------------------------------------------------------*/
 
 static int ehci_hub_control (
 	struct usb_hcd	*hcd,
@@ -1091,7 +1227,13 @@ static int ehci_hub_control (
 		 * about the EHCI-specific stuff.
 		 */
 		case USB_PORT_FEAT_TEST:
-			if (!selector || selector > 5)
+			if (selector == EHSET_TEST_SINGLE_STEP_SET_FEATURE) {
+				spin_unlock_irqrestore(&ehci->lock, flags);
+				retval = ehset_single_step_set_feature(hcd,
+									wIndex);
+				spin_lock_irqsave(&ehci->lock, flags);
+				break;
+			} else if (!selector || selector > 5)
 				goto error;
 			spin_unlock_irqrestore(&ehci->lock, flags);
 			ehci_quiesce(ehci);
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index d34b399..702a0e9 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -1144,6 +1144,107 @@ submit_async (
 }
 
 /*-------------------------------------------------------------------------*/
+/*
+ * This function creates the qtds and submits them for the
+ * SINGLE_STEP_SET_FEATURE Test.
+ * This is done in two parts: first SETUP req for GetDesc is sent then
+ * 15 seconds later, the IN stage for GetDesc starts to req data from dev
+ *
+ * is_setup : i/p arguement decides which of the two stage needs to be
+ * performed; TRUE - SETUP and FALSE - IN+STATUS
+ * Returns 0 if success
+ */
+static int submit_single_step_set_feature(
+	struct usb_hcd  *hcd,
+	struct urb      *urb,
+	int             is_setup
+) {
+	struct ehci_hcd		*ehci = hcd_to_ehci(hcd);
+	struct list_head	qtd_list;
+	struct list_head	*head;
+
+	struct ehci_qtd		*qtd, *qtd_prev;
+	dma_addr_t		buf;
+	int			len, maxpacket;
+	u32			token;
+
+	INIT_LIST_HEAD(&qtd_list);
+	head = &qtd_list;
+
+	/* URBs map to sequences of QTDs:  one logical transaction */
+	qtd = ehci_qtd_alloc(ehci, GFP_KERNEL);
+	if (unlikely(!qtd))
+		return -1;
+	list_add_tail(&qtd->qtd_list, head);
+	qtd->urb = urb;
+
+	token = QTD_STS_ACTIVE;
+	token |= (EHCI_TUNE_CERR << 10);
+
+	len = urb->transfer_buffer_length;
+	/*
+	 * Check if the request is to perform just the SETUP stage (getDesc)
+	 * as in SINGLE_STEP_SET_FEATURE test, DATA stage (IN) happens
+	 * 15 secs after the setup
+	 */
+	if (is_setup) {
+		/* SETUP pid */
+		qtd_fill(ehci, qtd, urb->setup_dma,
+				sizeof(struct usb_ctrlrequest),
+				token | (2 /* "setup" */ << 8), 8);
+
+		submit_async(ehci, urb, &qtd_list, GFP_ATOMIC);
+		return 0; /*Return now; we shall come back after 15 seconds*/
+	}
+
+	/*
+	 * IN: data transfer stage:  buffer setup : start the IN txn phase for
+	 * the get_Desc SETUP which was sent 15seconds back
+	 */
+	token ^= QTD_TOGGLE;   /*We need to start IN with DATA-1 Pid-sequence*/
+	buf = urb->transfer_dma;
+
+	token |= (1 /* "in" */ << 8);  /*This is IN stage*/
+
+	maxpacket = max_packet(usb_maxpacket(urb->dev, urb->pipe, 0));
+
+	qtd_fill(ehci, qtd, buf, len, token, maxpacket);
+
+	/*
+	 * Our IN phase shall always be a short read; so keep the queue running
+	 * and let it advance to the next qtd which zero length OUT status
+	 */
+	qtd->hw_alt_next = EHCI_LIST_END(ehci);
+
+	/* STATUS stage for GetDesc control request */
+	token ^= 0x0100;        /* "in" <--> "out"  */
+	token |= QTD_TOGGLE;    /* force DATA1 */
+
+	qtd_prev = qtd;
+	qtd = ehci_qtd_alloc(ehci, GFP_ATOMIC);
+	if (unlikely(!qtd))
+		goto cleanup;
+	qtd->urb = urb;
+	qtd_prev->hw_next = QTD_NEXT(ehci, qtd->qtd_dma);
+	list_add_tail(&qtd->qtd_list, head);
+
+	/* dont fill any data in such packets */
+	qtd_fill(ehci, qtd, 0, 0, token, 0);
+
+	/* by default, enable interrupt on urb completion */
+	if (likely(!(urb->transfer_flags & URB_NO_INTERRUPT)))
+		qtd->hw_token |= cpu_to_hc32(ehci, QTD_IOC);
+
+	submit_async(ehci, urb, &qtd_list, GFP_KERNEL);
+
+	return 0;
+
+cleanup:
+	qtd_list_free(ehci, urb, head);
+	return -1;
+}
+
+/*-------------------------------------------------------------------------*/
 
 static void single_unlink_async(struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
diff --git a/drivers/usb/misc/ehset.c b/drivers/usb/misc/ehset.c
index d2864b3..c31b4a3 100644
--- a/drivers/usb/misc/ehset.c
+++ b/drivers/usb/misc/ehset.c
@@ -96,7 +96,25 @@ static int ehset_probe(struct usb_interface *intf,
 		kfree(buf);
 		break;
 	case TEST_SINGLE_STEP_SET_FEATURE:
-		/* unsupported for now */
+		/*
+		 * GetDescriptor SETUP request -> 15secs delay -> IN & STATUS
+		 *
+		 * Note, this test is only supported on root hubs since the
+		 * SetPortFeature handling can only be done inside the HCD's
+		 * hub_control callback function.
+		 */
+		if (hub_udev != dev->bus->root_hub) {
+			dev_err(&intf->dev, "SINGLE_STEP_SET_FEATURE test only supported on root hub\n");
+			break;
+		}
+
+		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
+					USB_REQ_SET_FEATURE, USB_RT_PORT,
+					USB_PORT_FEAT_TEST,
+					(6 << 8) | portnum,
+					NULL, 0, 60 * 1000);
+
+		break;
 	default:
 		dev_err(&intf->dev, "%s: unsupported PID: 0x%x\n",
 			__func__, test_pid);
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Greg KH]

On Thu, Aug 08, 2013 at 04:49:24PM -0700, Jack Pham wrote:
> From: Manu Gautam <mgautam@codeaurora.org>

I don't seem to have a patch 1/2 from you, did it not get sent somehow?

thanks,

greg k-h

Re: [PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Jack Pham]

Hi Greg,

On Thu, Aug 08, 2013 at 05:05:41PM -0700, Greg KH wrote:
> I don't seem to have a patch 1/2 from you, did it not get sent somehow?

Here was patch 1/2: http://marc.info/?l=linux-usb&m=137282127113409&w=2
which you've applied to usb-next already as commit 1353aa53. I kept the
sequence numbering in the subject as it was in response to the RFC
version I originally sent which was the 2/2. Should I resend this as a
standalone (i.e. with a revised subject tag) instead?

Jack
-- 
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Greg KH]

On Thu, Aug 08, 2013 at 07:12:13PM -0700, Jack Pham wrote:
> Hi Greg,
> 
> On Thu, Aug 08, 2013 at 05:05:41PM -0700, Greg KH wrote:
> > I don't seem to have a patch 1/2 from you, did it not get sent somehow?
> 
> Here was patch 1/2: http://marc.info/?l=linux-usb&m=137282127113409&w=2
> which you've applied to usb-next already as commit 1353aa53. I kept the
> sequence numbering in the subject as it was in response to the RFC
> version I originally sent which was the 2/2. Should I resend this as a
> standalone (i.e. with a revised subject tag) instead?

Ah, ok.  Given that this was about 500+ patches ago in my review queue,
I didn't remember it at all :)

In the future, when resending something like this, when the 1/2 patch is
already accepted, just send it as 1/1, as there is no patch it depends
on outstanding.

thanks,

greg k-h