USB-Stream: Switch to handling packets in a deferred hook

Previously the TX and RX queues were being accessed from two
different locations without locking, which is wrong.  This
moves the access to a single location in a deffered hook and
calls that hook from the old locations.  The result is
correct, simpler, and not much slower.  It also reduces time
in the USB interrupt handler by moving the memcpy from packet
to queue out to the deferred hook.

Signed-off-by: Anton Staaf <robotboy@chromium.org>

BRANCH=None
BUG=None
TEST=make buildall -j
     Verify that USB streams still work on Ryu and discovery-stm32f072

Change-Id: I6ea53d7c40b42c6112e86a7886f3b888408f72b7
Reviewed-on: https://chromium-review.googlesource.com/264763
Tested-by: Anton Staaf <robotboy@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Commit-Queue: Anton Staaf <robotboy@chromium.org>
Trybot-Ready: Anton Staaf <robotboy@chromium.org>

3 files changed, 53 insertions, 46 deletions

diff --git a/board/ryu/board.h b/board/ryu/board.h
index 95e5f2582e..6ded87e243 100644
--- a/board/ryu/board.h
+++ b/board/ryu/board.h
@@ -123,7 +123,7 @@
 
 /* Maximum number of deferrable functions */
 #undef  DEFERRABLE_MAX_COUNT
-#define DEFERRABLE_MAX_COUNT 12
+#define DEFERRABLE_MAX_COUNT 14
 
 #ifndef __ASSEMBLER__
 
diff --git a/chip/stm32/usb-stream.c b/chip/stm32/usb-stream.c
index 05351a8ca3..f5487372bb 100644
--- a/chip/stm32/usb-stream.c
+++ b/chip/stm32/usb-stream.c
@@ -44,21 +44,22 @@ static size_t tx_write(struct usb_stream_config const *config)
 	return count;
 }
 
-static void usb_read(struct producer const *producer, size_t count)
+static int tx_valid(struct usb_stream_config const *config)
 {
-	struct usb_stream_config const *config =
-		DOWNCAST(producer, struct usb_stream_config, producer);
-
-	if (config->state->rx_waiting && rx_read(config)) {
-		config->state->rx_waiting = 0;
+	return (STM32_USB_EP(config->endpoint) & EP_TX_MASK) == EP_TX_VALID;
+}
 
-		STM32_TOGGLE_EP(config->endpoint, EP_RX_MASK, EP_RX_VALID, 0);
-	}
+static int rx_valid(struct usb_stream_config const *config)
+{
+	return (STM32_USB_EP(config->endpoint) & EP_RX_MASK) == EP_RX_VALID;
 }
 
-static int tx_valid(struct usb_stream_config const *config)
+static void usb_read(struct producer const *producer, size_t count)
 {
-	return (STM32_USB_EP(config->endpoint) & EP_TX_MASK) == EP_TX_VALID;
+	struct usb_stream_config const *config =
+		DOWNCAST(producer, struct usb_stream_config, producer);
+
+	hook_call_deferred(config->deferred, 0);
 }
 
 static void usb_written(struct consumer const *consumer, size_t count)
@@ -66,12 +67,7 @@ static void usb_written(struct consumer const *consumer, size_t count)
 	struct usb_stream_config const *config =
 		DOWNCAST(consumer, struct usb_stream_config, consumer);
 
-	/*
-	 * If we are not currently in a valid transmission state and we had
-	 * something for the TX buffer, then mark the TX endpoint as valid.
-	 */
-	if (!tx_valid(config) && tx_write(config))
-		STM32_TOGGLE_EP(config->endpoint, EP_TX_MASK, EP_TX_VALID, 0);
+	hook_call_deferred(config->deferred, 0);
 }
 
 static void usb_flush(struct consumer const *consumer)
@@ -92,32 +88,27 @@ struct consumer_ops const usb_stream_consumer_ops = {
 	.flush   = usb_flush,
 };
 
-void usb_stream_tx(struct usb_stream_config const *config)
+void usb_stream_deferred(struct usb_stream_config const *config)
 {
-	if (tx_write(config))
+	if (!tx_valid(config) && tx_write(config))
 		STM32_TOGGLE_EP(config->endpoint, EP_TX_MASK, EP_TX_VALID, 0);
-	else
-		STM32_TOGGLE_EP(config->endpoint, 0, 0, 0);
+
+	if (!rx_valid(config) && rx_read(config))
+		STM32_TOGGLE_EP(config->endpoint, EP_RX_MASK, EP_RX_VALID, 0);
+}
+
+void usb_stream_tx(struct usb_stream_config const *config)
+{
+	STM32_TOGGLE_EP(config->endpoint, 0, 0, 0);
+
+	hook_call_deferred(config->deferred, 0);
 }
 
 void usb_stream_rx(struct usb_stream_config const *config)
 {
-	if (rx_read(config)) {
-		/*
-		 * RX packet consumed, mark the packet as VALID.
-		 */
-		STM32_TOGGLE_EP(config->endpoint, EP_RX_MASK, EP_RX_VALID, 0);
-	} else {
-		/*
-		 * There is not enough space in the RX queue to receive this
-		 * packet.  Leave the RX endpoint in a NAK state, clear the
-		 * interrupt, and indicate to the usb_read function that when
-		 * there is enough space in the queue to hold it there is an
-		 * RX packet waiting.
-		 */
-		config->state->rx_waiting = 1;
-		STM32_TOGGLE_EP(config->endpoint, 0, 0, 0);
-	}
+	STM32_TOGGLE_EP(config->endpoint, 0, 0, 0);
+
+	hook_call_deferred(config->deferred, 0);
 }
 
 static usb_uint usb_ep_rx_size(size_t bytes)
diff --git a/chip/stm32/usb-stream.h b/chip/stm32/usb-stream.h
index cca1523ab1..dd575d5a43 100644
--- a/chip/stm32/usb-stream.h
+++ b/chip/stm32/usb-stream.h
@@ -9,6 +9,7 @@
 
 #include "compile_time_macros.h"
 #include "consumer.h"
+#include "hooks.h"
 #include "in_stream.h"
 #include "out_stream.h"
 #include "producer.h"
@@ -51,6 +52,11 @@ struct usb_stream_config {
 	 */
 	int endpoint;
 
+	/*
+	 * Deferred function to call to handle USB and Queue request.
+	 */
+	void (*deferred)(void);
+
 	size_t rx_size;
 	size_t tx_size;
 
@@ -129,19 +135,21 @@ extern struct producer_ops const usb_stream_producer_ops;
 	static usb_uint CONCAT2(NAME, _ep_rx_buffer)[RX_SIZE / 2] __usb_ram; \
 	static usb_uint CONCAT2(NAME, _ep_tx_buffer)[TX_SIZE / 2] __usb_ram; \
 	static struct usb_stream_state CONCAT2(NAME, _state);		\
+	static void CONCAT2(NAME, _deferred_)(void);			\
 	struct usb_stream_config const NAME = {				\
-		.state    = &CONCAT2(NAME, _state),			\
-		.endpoint = ENDPOINT,					\
-		.rx_size  = RX_SIZE,					\
-		.tx_size  = TX_SIZE,					\
-		.rx_ram   = CONCAT2(NAME, _ep_rx_buffer),		\
-		.tx_ram   = CONCAT2(NAME, _ep_tx_buffer),		\
-		.consumer = {						\
+		.state     = &CONCAT2(NAME, _state),			\
+		.endpoint  = ENDPOINT,					\
+		.deferred  = CONCAT2(NAME, _deferred_),			\
+		.rx_size   = RX_SIZE,					\
+		.tx_size   = TX_SIZE,					\
+		.rx_ram    = CONCAT2(NAME, _ep_rx_buffer),		\
+		.tx_ram    = CONCAT2(NAME, _ep_tx_buffer),		\
+		.consumer  = {						\
 			.producer = &PRODUCER,				\
 			.queue    = &TX_QUEUE,				\
 			.ops      = &usb_stream_consumer_ops,		\
 		},							\
-		.producer = {						\
+		.producer  = {						\
 			.consumer = &CONSUMER,				\
 			.queue    = &RX_QUEUE,				\
 			.ops      = &usb_stream_producer_ops,		\
@@ -192,7 +200,15 @@ extern struct producer_ops const usb_stream_producer_ops;
 	USB_DECLARE_EP(ENDPOINT,					\
 		       CONCAT2(NAME, _ep_tx),				\
 		       CONCAT2(NAME, _ep_rx),				\
-		       CONCAT2(NAME, _ep_reset));
+		       CONCAT2(NAME, _ep_reset));			\
+	static void CONCAT2(NAME, _deferred_)(void)			\
+	{ usb_stream_deferred(&NAME); }					\
+	DECLARE_DEFERRED(CONCAT2(NAME, _deferred_));
+
+/*
+ * Handle USB and Queue request in a deferred callback.
+ */
+void usb_stream_deferred(struct usb_stream_config const *config);
 
 /*
  * These functions are used by the trampoline functions defined above to