Cr50: Cleaner API for USB_DECLARE_IFACE callbacks

The control endpoint (EP0) can receive some Setup packets that
are specific to individual Interfaces. The USB_DECLARE_IFACE
macro is used to register the callbacks that an interface
implementation provides to handle those Setup packets.

This change cleans up the callback API a bit, so that we don't
have to export the internal workings of the Cr50's EP0 interrupt
handler.

BUG=chrome-os-partner:34893
BRANCH=none
TEST=make buildall, manual

Connect the Cr50 to my workstation via USB:
* /bin/dmesg reports no errors
* verify EP0 with lsusb -v -d 18d1:5014
* verify EP1 with './extra/usb_console -e 1 -p 5014' (reverses
  case of input text)
* verify EP2 with the 'hid' command on the EC console (types a 'g')

Change-Id: I9ac22f6a74f360f201c58e9ef39e3576834578a8
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/317269
Reviewed-by: Dominic Rizzo <domrizzo@google.com>

3 files changed, 74 insertions, 32 deletions

diff --git a/chip/g/usb.c b/chip/g/usb.c
index a5026dcd1d..18f5a39dd1 100644
--- a/chip/g/usb.c
+++ b/chip/g/usb.c
@@ -244,8 +244,8 @@ const uint8_t usb_string_desc[] = {
 };
 
 /* Descriptors for USB controller S/G DMA */
-struct g_usb_desc ep0_out_desc;
-struct g_usb_desc ep0_in_desc;
+static struct g_usb_desc ep0_out_desc;
+static struct g_usb_desc ep0_in_desc;
 
 /* Control endpoint (EP0) buffers */
 static uint8_t ep0_buf_tx[USB_MAX_PACKET_SIZE];
@@ -257,6 +257,29 @@ static int desc_left;
 /* pointer to descriptor data if any */
 static const uint8_t *desc_ptr;
 
+/* Load the EP0 IN FIFO buffer with some data (zero-length works too). Returns
+ * len, or negative on error. */
+int load_in_fifo(const void *source, uint32_t len)
+{
+	if (len > sizeof(ep0_buf_tx))
+		return -1;
+
+	memcpy(ep0_buf_tx, source, len);
+	ep0_in_desc.flags = DIEPDMA_LAST | DIEPDMA_BS_HOST_RDY |
+		DIEPDMA_IOC | DIEPDMA_TXBYTES(len);
+	GR_USB_DIEPCTL(0) |= DXEPCTL_CNAK | DXEPCTL_EPENA;
+
+	return len;
+}
+
+/* Prepare the EP0 OUT FIFO buffer to accept some data. Returns len, or
+ * negative on error. */
+int accept_out_fifo(uint32_t len)
+{
+	/* TODO: This is not yet implemented */
+	return -1;
+}
+
 /*
  * Requests on the control endpoint (aka EP0). The USB spec mandates that all
  * values are little-endian over the wire. Since this file is intentionally
@@ -282,9 +305,18 @@ static void ep0_rx(void)
 	/* interface specific requests */
 	if ((req->bmRequestType & USB_RECIP_MASK) == USB_RECIP_INTERFACE) {
 		uint8_t iface = req->wIndex & 0xff;
-		if (iface < USB_IFACE_COUNT &&
-		    usb_iface_request[iface](ep0_buf_rx, ep0_buf_tx))
+		int bytes;
+
+		if (iface >= USB_IFACE_COUNT)
+			goto unknown_req;
+
+		bytes = usb_iface_request[iface](req);
+		if (bytes < 0)
 			goto unknown_req;
+
+		ep0_out_desc.flags = DOEPDMA_RXBYTES(64) | DOEPDMA_LAST
+			| DOEPDMA_BS_HOST_RDY | DOEPDMA_IOC;
+		GR_USB_DOEPCTL(0) |= DXEPCTL_CNAK | DXEPCTL_EPENA;
 		return;
 	}
 
diff --git a/chip/g/usb_hid.c b/chip/g/usb_hid.c
index 27393525b5..c00d3d20cb 100644
--- a/chip/g/usb_hid.c
+++ b/chip/g/usb_hid.c
@@ -119,33 +119,21 @@ static void hid_reset(void)
 
 USB_DECLARE_EP(USB_EP_HID, hid_tx, hid_tx, hid_reset);
 
-extern struct g_usb_desc ep0_in_desc;
-extern struct g_usb_desc ep0_out_desc;
-
-static int hid_iface_request(uint8_t *ep0_buf_rx, uint8_t *ep0_buf_tx)
+static int hid_iface_request(struct usb_setup_packet *req)
 {
-	/* This chip and buffer data are all little-endian, so this works */
-	struct usb_setup_packet *req = (struct usb_setup_packet *)ep0_buf_rx;
-	int len;
-
-	if (req->bmRequestType == (USB_DIR_IN | USB_RECIP_INTERFACE) &&
+	if ((req->bmRequestType & USB_DIR_IN) &&
 	    req->bRequest == USB_REQ_GET_DESCRIPTOR &&
 	    req->wValue == (USB_HID_DT_REPORT << 8)) {
 		/* Setup : HID specific : Get Report descriptor */
-		memcpy(ep0_buf_tx, report_desc, sizeof(report_desc));
-		len = MIN(req->wLength, sizeof(report_desc));
-		ep0_in_desc.flags = DIEPDMA_LAST | DIEPDMA_BS_HOST_RDY |
-			DIEPDMA_IOC | DIEPDMA_TXBYTES(len);
-		GR_USB_DIEPCTL(0) |= DXEPCTL_CNAK | DXEPCTL_EPENA;
-		ep0_out_desc.flags = DOEPDMA_RXBYTES(64) | DOEPDMA_LAST
-			| DOEPDMA_BS_HOST_RDY | DOEPDMA_IOC;
-		GR_USB_DOEPCTL(0) |= DXEPCTL_CNAK | DXEPCTL_EPENA;
-		return 0;
+		return load_in_fifo(report_desc,
+				    MIN(req->wLength,
+					sizeof(report_desc)));
 	}
 
-	return 1;
+	/* Anything else we'll stall */
+	return -1;
 }
-USB_DECLARE_IFACE(USB_IFACE_HID, hid_iface_request)
+USB_DECLARE_IFACE(USB_IFACE_HID, hid_iface_request);
 
 static int command_hid(int argc, char **argv)
 {
diff --git a/chip/g/usb_hw.h b/chip/g/usb_hw.h
index 2602e51e60..90d23f2bd7 100644
--- a/chip/g/usb_hw.h
+++ b/chip/g/usb_hw.h
@@ -18,19 +18,41 @@
 	void _EP_RX_HANDLER(num)(void)                            \
 		__attribute__ ((alias(STRINGIFY(rx_handler))));	  \
 	void _EP_RESET_HANDLER(num)(void)                         \
-		__attribute__ ((alias(STRINGIFY(rst_handler))));
+		__attribute__ ((alias(STRINGIFY(rst_handler))))
 
-/* arrays with all endpoint callbacks */
+/* Endpoint callbacks */
 extern void (*usb_ep_tx[]) (void);
 extern void (*usb_ep_rx[]) (void);
 extern void (*usb_ep_reset[]) (void);
-/* array with interface-specific control request callbacks */
-extern int (*usb_iface_request[]) (uint8_t *ep0_buf_rx, uint8_t *ep0_buf_tx);
+struct usb_setup_packet;
+/* EP0 Interface handler callbacks */
+static int (*usb_iface_request[]) (struct usb_setup_packet *req);
 
+/*
+ * Declare any interface-specific control request handlers. These Setup packets
+ * arrive on the control endpoint (EP0), but are handled by the interface code.
+ * The callback must prepare the EP0 IN or OUT FIFOs and return the number of
+ * bytes placed in the IN FIFO. A negative return value will STALL the response
+ * (and thus indicate error to the host).
+ */
 #define _IFACE_HANDLER(num) CONCAT3(iface_, num, _request)
-#define USB_DECLARE_IFACE(num, handler)					\
-	int _IFACE_HANDLER(num)(uint8_t *ep0_buf_rx,			\
-				uint8_t *epo_buf_tx)			\
-	__attribute__ ((alias(STRINGIFY(handler))));
+#define USB_DECLARE_IFACE(num, handler)				\
+	int _IFACE_HANDLER(num)(struct usb_setup_packet *req)	\
+		__attribute__ ((alias(STRINGIFY(handler))))
+
+/*
+ * The interface handler can call this to put <len> bytes into the EP0 TX FIFO
+ * (zero is acceptable). It returns (int)<len> on success, -1 if <len> is too
+ * large.
+ */
+int load_in_fifo(const void *source, uint32_t len);
+
+/*
+ * The interface handler can call this to enable the EP0 RX FIFO to receive
+ * <len> bytes of data for a Control Write request. This is not needed to
+ * prepare for the Status phase of a Control Read. It will return (int)<len> on
+ * success, -1 if <len> is too large.
+ */
+int accept_out_fifo(uint32_t len);
 
 #endif	/* __CROS_EC_USB_HW_H */