Cr50: Support USB SETCFG/GETCFG control transfers

This adds USB support to Set and Get the Device Configuration. These control transfers are standard device requests that need to be added in order to behave properly for USB suspend/resume (and in general). Before this CL, the Get command always failed and the Set command had no effect internally. With this CL it works. Note that this particular change only supports ONE configuration for the Cr50. If/when we add additional configuration descriptors, we'll need to update it again. BUG=chrome-os-partner:50721 BRANCH=none TEST=make buildall; manual tests on Cr50 This CL includes a test program. Connect the Cr50 to the build host, and use that program to read and change the configuration. cd test/usb_test make ./device_configuration ./device_configuration 0 ./device_configuration 1 ./device_configuration 2 You may need to use sudo if your device permissions aren't sufficient. Change-Id: Id65e70265f0760b1b374005dfcddc88e66a933f6 Signed-off-by: Bill Richardson <wfrichar@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/335878 Reviewed-by: Scott Collyer <scollyer@chromium.org>
2016-03-24 19:33:31 -07:00 · 2016-03-24 19:33:31 -07:00 · 20f5efafc8
parent 06ea3b5b00
commit 20f5efafc8
4 changed files with 246 additions and 6 deletions
--- a/chip/g/usb.c
+++ b/chip/g/usb.c
@ -217,7 +217,7 @@ const struct usb_config_descriptor USB_CONF_DESC(conf) = {
 	.bDescriptorType = USB_DT_CONFIGURATION,
 	.wTotalLength = 0x0BAD,	 /* number of returned bytes, set at runtime */
 	.bNumInterfaces = USB_IFACE_COUNT,
-	.bConfigurationValue = 1,
+	.bConfigurationValue = 1,		/* Caution: hard-coded value */
 	.iConfiguration = USB_STR_VERSION,
 	.bmAttributes = 0x80,			/* bus powered */
 	.bMaxPower = 250,			/* MaxPower 500 mA */
@ -306,6 +306,15 @@ static uint8_t ep0_in_buf[IN_BUF_SIZE];
 static struct g_usb_desc ep0_in_desc[NUM_IN_PACKETS_AT_ONCE];
 static struct g_usb_desc *cur_in_desc;

+/* Overall device state (USB 2.0 spec, section 9.1.1).
+ * We only need a few, though. */
+static enum {
+	DS_DEFAULT,
+	DS_ADDRESS,
+	DS_CONFIGURED,
+} device_state;
+static uint8_t configuration_value;
+
 /* Reset all this to a good starting state. */
 static void initialize_dma_buffers(void)
 {
@ -585,8 +594,9 @@ static int handle_setup_with_in_stage(enum table_case tc,
 		break;
 	}
 	case USB_REQ_GET_CONFIGURATION:
-		/* TODO: We might need this to handle USB suspend properly */
-		return -1;
+		data = &configuration_value;
+		len = sizeof(configuration_value);
+		break;

 	case USB_REQ_SYNCH_FRAME:
 		/* Unimplemented */
@ -697,11 +707,25 @@ static int handle_setup_with_no_data_stage(enum table_case tc,
 		 */
 		GWRITE_FIELD(USB, DCFG, DEVADDR, set_addr);
 		print_later("SETAD 0x%02x (%d)", set_addr, set_addr, 0, 0, 0);
+		device_state = DS_ADDRESS;
 		break;

 	case USB_REQ_SET_CONFIGURATION:
-		/* TODO: Sanity-check this? We only have one config, right? */
 		print_later("SETCFG 0x%x", req->wValue, 0, 0, 0, 0);
+		switch (req->wValue) {
+		case 0:
+			configuration_value = req->wValue;
+			device_state = DS_ADDRESS;
+			break;
+		case 1:	    /* Caution: Only one config descriptor TODAY */
+			/* TODO: All endpoints set to DATA0 toggle state */
+			configuration_value = req->wValue;
+			device_state = DS_CONFIGURED;
+			break;
+		default:
+			/* Nope. That's a paddlin. */
+			return -1;
+		}
 		break;

 	case USB_REQ_CLEAR_FEATURE:
@ -836,7 +860,7 @@ static void ep0_interrupt(uint32_t intr_on_out, uint32_t intr_on_in)
 			/* I don't *think* we need to do this, unless we need
 			 * to transfer more data. Customer support agrees and
 			 * it shouldn't matter if the host is well-behaved, but
-			 * seems like we had issues without it.
+			 * it seems like we had issues without it.
 			 * TODO: Test this case until we know for sure. */
 			GR_USB_DIEPCTL(0) = DXEPCTL_EPENA;

@ -935,6 +959,10 @@ static void ep0_reset(void)
 	GWRITE_FIELD(USB, DCFG, DEVADDR, 0);
 	initialize_dma_buffers();
 	expect_setup_packet();
+	/* Clear our internal state */
+	device_state = DS_DEFAULT;
+	configuration_value = 0;
+
 }

 /****************************************************************************/
@ -1040,6 +1068,7 @@ static void usb_early_suspend(void)
 {
 	print_later("usb_early_suspend()", 0, 0, 0, 0, 0);
 }
+
 static void usb_suspend(void)
 {
 	print_later("usb_suspend()", 0, 0, 0, 0, 0);
@ -1147,6 +1176,9 @@ void usb_disconnect(void)
 {
 	print_later("usb_disconnect()", 0, 0, 0, 0, 0);
 	GR_USB_DCTL |= DCTL_SFTDISCON;
+
+	device_state = DS_DEFAULT;
+	configuration_value = 0;
 }

 void usb_init(void)
@ -1198,7 +1230,7 @@ void usb_init(void)
 	/* Global + DMA configuration */
 	/* TODO: What about the AHB Burst Length Field? It's 0 now. */
 	GR_USB_GAHBCFG = GAHBCFG_DMA_EN | GAHBCFG_GLB_INTR_EN |
-			 GAHBCFG_NP_TXF_EMP_LVL;
+		GAHBCFG_NP_TXF_EMP_LVL;

 	/* Be in disconnected state until we are ready */
 	usb_disconnect();
--- a/test/usb_test/Makefile
+++ b/test/usb_test/Makefile
@ -0,0 +1,34 @@
+# Copyright 2015 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+PROGRAM := device_configuration
+SOURCE  := $(PROGRAM).c
+LIBS    :=
+LFLAGS  :=
+CFLAGS  := -std=gnu99 \
+	-g3 \
+	-O3 \
+        -Wall \
+        -Werror \
+        -Wpointer-arith \
+        -Wcast-align \
+        -Wcast-qual \
+        -Wundef \
+        -Wsign-compare \
+        -Wredundant-decls \
+        -Wmissing-declarations
+
+#
+# Add libusb-1.0 required flags
+#
+LIBS    += $(shell pkg-config --libs   libusb-1.0)
+CFLAGS  += $(shell pkg-config --cflags libusb-1.0)
+
+$(PROGRAM): $(SOURCE) Makefile
+	gcc $(CFLAGS) $(SOURCE) $(LFLAGS) $(LIBS) -o $@
+
+.PHONY: clean
+
+clean:
+	rm -rf $(PROGRAM) *~
--- a/test/usb_test/README
+++ b/test/usb_test/README
@ -0,0 +1,2 @@
+These tests need to be built and run by hand, unless/until we set up a lab
+with known devices attached to a test host.
--- a/test/usb_test/device_configuration.c
+++ b/test/usb_test/device_configuration.c
@ -0,0 +1,172 @@
+/*
+ * Copyright 2016 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <libusb.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+/* Options */
+static uint16_t vid = 0x18d1;			/* Google */
+static uint16_t pid = 0x5014;			/* Cr50 */
+
+static char *progname;
+
+static void usage(int errs)
+{
+	printf("\nUsage: %s [vid:pid] [value]\n"
+	       "\n"
+	       "Set/Get the USB Device Configuration value\n"
+	       "\n"
+	       "The default vid:pid is %04x:%04x\n"
+	       "\n", progname, vid, pid);
+
+	exit(!!errs);
+}
+
+/* Globals */
+struct libusb_device_handle *devh = 0;
+
+static void stupid_usb(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+	vfprintf(stderr, format, ap);
+	va_end(ap);
+
+	if (devh)
+		libusb_close(devh);
+
+	libusb_exit(NULL);
+
+	exit(1);
+}
+
+
+int main(int argc, char *argv[])
+{
+	int r = 1;
+	int errorcnt = 0;
+	int do_set = 0;
+	uint16_t setval = 0;
+	uint8_t buf[80];			/* Arbitrary size */
+	int i;
+
+	progname = strrchr(argv[0], '/');
+	if (progname)
+		progname++;
+	else
+		progname = argv[0];
+
+	opterr = 0;				/* quiet, you */
+	while ((i = getopt(argc, argv, "")) != -1) {
+		switch (i) {
+		case 'h':
+			usage(errorcnt);
+			break;
+		case 0:				/* auto-handled option */
+			break;
+		case '?':
+			if (optopt)
+				printf("Unrecognized option: -%c\n", optopt);
+			else
+				printf("Unrecognized option: %s\n",
+				       argv[optind - 1]);
+			errorcnt++;
+			break;
+		case ':':
+			printf("Missing argument to %s\n", argv[optind - 1]);
+			errorcnt++;
+			break;
+		default:
+			printf("Internal error at %s:%d\n", __FILE__, __LINE__);
+			exit(1);
+		}
+	}
+
+	if (errorcnt)
+		usage(errorcnt);
+
+	if (optind < argc) {
+		uint16_t v, p;
+
+		if (2 == sscanf(argv[optind], "%hx:%hx", &v, &p)) {
+			vid = v;
+			pid = p;
+			optind++;
+		}
+	}
+
+	if (optind < argc) {
+		do_set = 1;
+		setval = atoi(argv[optind]);
+	}
+
+	r = libusb_init(NULL);
+	if (r) {
+		printf("libusb_init() returned 0x%x: %s\n",
+		       r, libusb_error_name(r));
+		return 1;
+	}
+
+	devh = libusb_open_device_with_vid_pid(NULL, vid, pid);
+	if (!devh) {
+		perror(progname);
+		stupid_usb("Can't open device %04x:%04x\n", vid, pid);
+	}
+
+
+	/* Set config*/
+	if (do_set) {
+		printf("SetCfg %d\n", setval);
+		r = libusb_control_transfer(
+			devh,
+			0x00,			/* bmRequestType */
+			0x09,			/* bRequest */
+			setval,			/* wValue */
+			0x0000,			/* wIndex */
+			NULL,			/* data */
+			0x0000,			/* wLength */
+			1000);			/* timeout (ms) */
+
+		if (r < 0)
+			printf("transfer returned 0x%x %s\n",
+			       r, libusb_error_name(r));
+	}
+
+	/* Get config */
+	memset(buf, 0, sizeof(buf));
+
+	r = libusb_control_transfer(
+		devh,
+		0x80,				/* bmRequestType */
+		0x08,				/* bRequest */
+		0x0000,				/* wValue */
+		0x0000,				/* wIndex */
+		buf,				/* data */
+		0x0001,				/* wLength */
+		1000);				/* timeout (ms) */
+
+	if (r <= 0)
+		stupid_usb("GetCfg transfer() returned 0x%x %s\n",
+			   r, libusb_error_name(r));
+
+	printf("GetCfg returned %d bytes:", r);
+	for (i = 0; i < r; i++)
+		printf(" 0x%02x", buf[i]);
+	printf("\n");
+
+	/* done */
+	if (devh)
+		libusb_close(devh);
+	libusb_exit(NULL);
+
+	return 0;
+}