drivers: uhc: rework transfer buffer handling

The current approach is a bit impractical in the upper layer.
This patch removes the two fifos that hold the transfer buffers
and replaces them with a byte array for the setup packet and
a pointer to a data buffer. The data buffer is mandatory for
all types of transfers except control without a data stage.
The waste of eight unused bytes for non-control transfers should
be insignificant, since an additional pointer would be at least
half of it, and then there would be the overhead of handling it.

This patch also clean up the transfer flags, rename owner to callback
as it reflects the upper layer use case, and add an additional member
to hold the pointer to the USB device (peripheral on the bus).

Signed-off-by: Johann Fischer <[email protected]>

Author: jfischer-no

drivers/usb/uhc/uhc_common.c

@@ -50,7 +50,6 @@ void uhc_xfer_return(const struct device *dev,
 
 	sys_dlist_remove(&xfer->node);
 	xfer->queued = 0;
-	xfer->claimed = 0;
 	xfer->err = err;
 
 	data->event_cb(dev, &drv_evt);
@@ -81,13 +80,25 @@ int uhc_xfer_append(const struct device *dev,
 	return 0;
 }
 
+struct net_buf *uhc_xfer_buf_alloc(const struct device *dev,
+				   const size_t size)
+{
+	return net_buf_alloc_len(&uhc_ep_pool, size, K_NO_WAIT);
+}
+
+void uhc_xfer_buf_free(const struct device *dev, struct net_buf *const buf)
+{
+	net_buf_unref(buf);
+}
+
 struct uhc_transfer *uhc_xfer_alloc(const struct device *dev,
 				    const uint8_t addr,
 				    const uint8_t ep,
 				    const uint8_t attrib,
 				    const uint16_t mps,
 				    const uint16_t timeout,
-				    void *const owner)
+				    void *const udev,
+				    void *const cb)
 {
 	const struct uhc_api *api = dev->api;
 	struct uhc_transfer *xfer = NULL;
@@ -98,108 +109,93 @@ struct uhc_transfer *uhc_xfer_alloc(const struct device *dev,
 		goto xfer_alloc_error;
 	}
 
-	LOG_DBG("Allocate xfer, ep 0x%02x attrib 0x%02x owner %p",
-		ep, attrib, owner);
+	LOG_DBG("Allocate xfer, ep 0x%02x attrib 0x%02x cb %p",
+		ep, attrib, cb);
 
 	if (k_mem_slab_alloc(&uhc_xfer_pool, (void **)&xfer, K_NO_WAIT)) {
 		LOG_ERR("Failed to allocate transfer");
 		goto xfer_alloc_error;
 	}
 
 	memset(xfer, 0, sizeof(struct uhc_transfer));
-	k_fifo_init(&xfer->queue);
-	k_fifo_init(&xfer->done);
 	xfer->addr = addr;
 	xfer->ep = ep;
 	xfer->attrib = attrib;
 	xfer->mps = mps;
 	xfer->timeout = timeout;
-	xfer->owner = owner;
+	xfer->udev = udev;
+	xfer->cb = cb;
 
 xfer_alloc_error:
 	api->unlock(dev);
 
 	return xfer;
 }
 
-int uhc_xfer_free(const struct device *dev, struct uhc_transfer *const xfer)
+struct uhc_transfer *uhc_xfer_alloc_with_buf(const struct device *dev,
+					     const uint8_t addr,
+					     const uint8_t ep,
+					     const uint8_t attrib,
+					     const uint16_t mps,
+					     const uint16_t timeout,
+					     void *const udev,
+					     void *const cb,
+					     size_t size)
 {
-	const struct uhc_api *api = dev->api;
+	struct uhc_transfer *xfer;
 	struct net_buf *buf;
-	int ret = 0;
 
-	api->lock(dev);
-
-	if (xfer->queued || xfer->claimed) {
-		ret = -EBUSY;
-		LOG_ERR("Transfer is still claimed");
-		goto xfer_free_error;
-	}
-
-	while (!k_fifo_is_empty(&xfer->queue)) {
-		buf = net_buf_get(&xfer->queue, K_NO_WAIT);
-		uhc_xfer_buf_free(dev, buf);
+	buf = uhc_xfer_buf_alloc(dev, size);
+	if (buf == NULL) {
+		return NULL;
 	}
 
-	while (!k_fifo_is_empty(&xfer->done)) {
-		buf = net_buf_get(&xfer->done, K_NO_WAIT);
-		uhc_xfer_buf_free(dev, buf);
+	xfer = uhc_xfer_alloc(dev, addr, ep, attrib, mps, timeout, udev, cb);
+	if (xfer == NULL) {
+		net_buf_unref(buf);
+		return NULL;
 	}
 
-	k_mem_slab_free(&uhc_xfer_pool, (void *)xfer);
-
-xfer_free_error:
-	api->unlock(dev);
+	xfer->buf = buf;
 
-	return ret;
+	return xfer;
 }
 
-struct net_buf *uhc_xfer_buf_alloc(const struct device *dev,
-				   struct uhc_transfer *const xfer,
-				   const size_t size)
+int uhc_xfer_free(const struct device *dev, struct uhc_transfer *const xfer)
 {
 	const struct uhc_api *api = dev->api;
-	struct net_buf *buf = NULL;
+	int ret = 0;
 
 	api->lock(dev);
 
-	if (!uhc_is_initialized(dev)) {
-		goto buf_alloc_error;
-	}
-
-	if (xfer->queued || xfer->claimed) {
-		goto buf_alloc_error;
-	}
-
-	LOG_DBG("Allocate net_buf, ep 0x%02x, size %zd", xfer->ep, size);
-	buf = net_buf_alloc_len(&uhc_ep_pool, size, K_NO_WAIT);
-	if (!buf) {
-		LOG_ERR("Failed to allocate net_buf");
-		goto buf_alloc_error;
-	}
-
-	if (buf->size < size) {
-		LOG_ERR("Buffer is smaller than requested");
-		net_buf_unref(buf);
-		buf = NULL;
-		goto buf_alloc_error;
+	if (xfer->queued) {
+		ret = -EBUSY;
+		LOG_ERR("Transfer is still queued");
+		goto xfer_free_error;
 	}
 
-	k_fifo_put(&xfer->queue, &buf->node);
+	k_mem_slab_free(&uhc_xfer_pool, (void *)xfer);
 
-buf_alloc_error:
+xfer_free_error:
 	api->unlock(dev);
 
-	return buf;
+	return ret;
 }
 
-int uhc_xfer_buf_free(const struct device *dev, struct net_buf *const buf)
+int uhc_xfer_buf_add(const struct device *dev,
+		     struct uhc_transfer *const xfer,
+		     struct net_buf *buf)
 {
 	const struct uhc_api *api = dev->api;
 	int ret = 0;
 
 	api->lock(dev);
-	net_buf_unref(buf);
+	if (xfer->queued) {
+		ret = -EBUSY;
+	} else {
+		xfer->buf = buf;
+	}
+
 	api->unlock(dev);
 
 	return ret;
@@ -217,12 +213,13 @@ int uhc_ep_enqueue(const struct device *dev, struct uhc_transfer *const xfer)
 		goto ep_enqueue_error;
 	}
 
-	xfer->claimed = 1;
+	xfer->queued = 1;
 	ret = api->ep_enqueue(dev, xfer);
 	if (ret) {
-		xfer->claimed = 0;
+		xfer->queued = 0;
 	}
 
+
 ep_enqueue_error:
 	api->unlock(dev);
 
@@ -242,6 +239,7 @@ int uhc_ep_dequeue(const struct device *dev, struct uhc_transfer *const xfer)
 	}
 
 	ret = api->ep_dequeue(dev, xfer);
+	xfer->queued = 0;
 
 ep_dequeue_error:
 	api->unlock(dev);

drivers/usb/uhc/uhc_common.h

@@ -58,54 +58,6 @@ static inline int uhc_unlock_internal(const struct device *dev)
 	return k_mutex_unlock(&data->mutex);
 }
 
-/**
- * @brief Checks if the transfer is queued.
- *
- * @param[in] xfer    Pointer to UHC transfer
- *
- * @return true if transfer is queued, false otherwise
- */
-static inline bool uhc_xfer_is_queued(struct uhc_transfer *xfer)
-{
-	return xfer->queued;
-}
-
-/**
- * @brief Helper function to set queued flag
- *
- * This function can be used by the driver to set queued flag
- *
- * @param[in] xfer    Pointer to UHC transfer
- */
-static inline void uhc_xfer_queued(struct uhc_transfer *xfer)
-{
-	xfer->queued = true;
-}
-
-/**
- * @brief Checks if the setup flag is set.
- *
- * @param[in] xfer    Pointer to UHC transfer
- *
- * @return true if setup flagh is set, false otherwise
- */
-static inline bool uhc_xfer_is_setup(struct uhc_transfer *xfer)
-{
-	return xfer->setup;
-}
-
-/**
- * @brief Helper function to set setup flag
- *
- * This function can be used by the driver to set setup flag
- *
- * @param[in] xfer    Pointer to UHC transfer
- */
-static inline void uhc_xfer_setup(struct uhc_transfer *xfer)
-{
-	xfer->setup = true;
-}
-
 /**
  * @brief Helper function to return UHC transfer to a higher level.
  *
@@ -119,29 +71,6 @@ void uhc_xfer_return(const struct device *dev,
 		     struct uhc_transfer *const xfer,
 		     const int err);
 
-/**
- * @brief Helper to move current buffer in the done-FIFO.
- *
- * Helper to move current buffer (probably completed) in the
- * designated done-FIFO.
- *
- * @param[in] xfer   Pointer to UHC transfer
- *
- * @return 0 on success, all other values should be treated as error.
- * @retval -ENOMEM if there is no buffer in the queue
- */
-static inline int uhc_xfer_done(struct uhc_transfer *xfer)
-{
-	struct net_buf *buf;
-
-	buf = k_fifo_get(&xfer->queue, K_NO_WAIT);
-	if (buf) {
-		k_fifo_put(&xfer->done, &buf->node);
-	}
-
-	return buf == NULL ? -ENOMEM : 0;
-}
-
 /**
  * @brief Helper to get next transfer to process.
  *