[PATCH 0/3] add xhci hooks for USB offload

[PATCH 0/3] add xhci hooks for USB offload

[Albert Wang]

This patchset is to proviode the USB offload function which allows to
offload some xHCI operations on co-processor.

*** BLURB HERE ***

Albert Wang (1):
  usb: host: add the xhci offload hooks implementations

Howard Yen (2):
  usb: host: add xhci hooks for USB offload
  usb: xhci-plat: add xhci_plat_priv_overwrite

 drivers/usb/host/xhci-mem.c          |  97 +++++-
 drivers/usb/host/xhci-offload-impl.c | 492 +++++++++++++++++++++++++++
 drivers/usb/host/xhci-plat.c         |  43 +++
 drivers/usb/host/xhci-plat.h         |   8 +
 drivers/usb/host/xhci.c              |  21 ++
 drivers/usb/host/xhci.h              |  31 ++
 6 files changed, 679 insertions(+), 13 deletions(-)
 create mode 100644 drivers/usb/host/xhci-offload-impl.c

-- 
2.38.0.135.g90850a2211-goog

[PATCH 1/3] usb: host: add xhci hooks for USB offload

[Albert Wang]

From: Howard Yen <howardyen@google.com>

This change is to provide USB offload function which allows to offload some
xHCI operations on co-processor. This is especially designed for USB audio
usecase. The co-processor is able to manipulate some USB structures in his
own memory, like SRAM.

There are several offload_ops introduced by this patch:

struct xhci_offload_ops - function callbacks for offlad specific operations
{
	@offload_init:
		- called for vendor init process during xhci-plat-hcd
		  probe.
	@offload_cleanup:
		- called for vendor cleanup process during xhci-plat-hcd
		  remove.
	@is_usb_offload_enabled:
		- called to check if usb offload enabled.
	@alloc_dcbaa:
		- called when allocating vendor specific dcbaa during
		  memory initializtion.
	@free_dcbaa:
		- called to free vendor specific dcbaa when cleanup the
		  memory.
	@alloc_transfer_ring:
		- called when vendor specific transfer ring allocation is required
	@free_transfer_ring:
		- called to free vendor specific transfer ring
	@usb_offload_skip_urb:
		- called to check if need to skip urb enqueue
}

The xhci hooks with prefix "xhci_vendor_" on the ops in xhci_offload_ops.
For example, offload_init ops will be invoked by xhci_vendor_offload_init()
hook,is_usb_offload_enabled ops will be invoked by
xhci_vendor_is_usb_offload_enabled(), and so on.

Signed-off-by: Howard Yen <howardyen@google.com>
Link: https://lore.kernel.org/r/20210119101044.1637023-1-howardyen@google.com
Signed-off-by: Greg Kroah-Harktman <gregkh@google.com>
---
 drivers/usb/host/xhci-mem.c  | 97 +++++++++++++++++++++++++++++++-----
 drivers/usb/host/xhci-plat.c | 23 +++++++++
 drivers/usb/host/xhci-plat.h |  1 +
 drivers/usb/host/xhci.c      | 21 ++++++++
 drivers/usb/host/xhci.h      | 31 ++++++++++++
 5 files changed, 160 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 9e56aa28efcd..0449109a973d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -361,6 +361,38 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
 	return 0;
 }
 
+static struct xhci_ring *xhci_vendor_alloc_transfer_ring(struct xhci_hcd *xhci,
+		u32 endpoint_type, enum xhci_ring_type ring_type,
+		unsigned int max_packet, gfp_t mem_flags)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->alloc_transfer_ring)
+		return ops->alloc_transfer_ring(xhci, endpoint_type, ring_type,
+						max_packet, mem_flags);
+	return 0;
+}
+
+static void xhci_vendor_free_transfer_ring(struct xhci_hcd *xhci,
+		struct xhci_virt_device *virt_dev, unsigned int ep_index)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->free_transfer_ring)
+		ops->free_transfer_ring(xhci, virt_dev, ep_index);
+}
+
+static bool xhci_vendor_is_offload_enabled(struct xhci_hcd *xhci,
+		struct xhci_virt_device *virt_dev, unsigned int ep_index)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->is_offload_enabled)
+		return ops->is_offload_enabled(xhci, virt_dev, ep_index);
+
+	return false;
+}
+
 /*
  * Create a new ring with zero or more segments.
  *
@@ -412,7 +444,11 @@ void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
 		struct xhci_virt_device *virt_dev,
 		unsigned int ep_index)
 {
-	xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
+	if (xhci_vendor_is_offload_enabled(xhci, virt_dev, ep_index))
+		xhci_vendor_free_transfer_ring(xhci, virt_dev, ep_index);
+	else
+		xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
+
 	virt_dev->eps[ep_index].ring = NULL;
 }
 
@@ -885,7 +921,7 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id)
 
 	for (i = 0; i < 31; i++) {
 		if (dev->eps[i].ring)
-			xhci_ring_free(xhci, dev->eps[i].ring);
+			xhci_free_endpoint_ring(xhci, dev, i);
 		if (dev->eps[i].stream_info)
 			xhci_free_stream_info(xhci,
 					dev->eps[i].stream_info);
@@ -1483,8 +1519,16 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 		mult = 0;
 
 	/* Set up the endpoint ring */
-	virt_dev->eps[ep_index].new_ring =
-		xhci_ring_alloc(xhci, 2, 1, ring_type, max_packet, mem_flags);
+	if (xhci_vendor_is_offload_enabled(xhci, virt_dev, ep_index) &&
+	    usb_endpoint_xfer_isoc(&ep->desc)) {
+		virt_dev->eps[ep_index].new_ring =
+			xhci_vendor_alloc_transfer_ring(xhci, endpoint_type, ring_type,
+							max_packet, mem_flags);
+	} else {
+		virt_dev->eps[ep_index].new_ring =
+			xhci_ring_alloc(xhci, 2, 1, ring_type, max_packet, mem_flags);
+	}
+
 	if (!virt_dev->eps[ep_index].new_ring)
 		return -ENOMEM;
 
@@ -1828,6 +1872,23 @@ void xhci_free_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
 	erst->entries = NULL;
 }
 
+static void xhci_vendor_alloc_dcbaa(
+		struct xhci_hcd *xhci, gfp_t flags)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->alloc_dcbaa)
+		return ops->alloc_dcbaa(xhci, flags);
+}
+
+static void xhci_vendor_free_dcbaa(struct xhci_hcd *xhci)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->free_dcbaa)
+		ops->free_dcbaa(xhci);
+}
+
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
@@ -1879,9 +1940,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"Freed medium stream array pool");
 
-	if (xhci->dcbaa)
-		dma_free_coherent(dev, sizeof(*xhci->dcbaa),
-				xhci->dcbaa, xhci->dcbaa->dma);
+	if (xhci_vendor_is_offload_enabled(xhci, NULL, 0)) {
+		xhci_vendor_free_dcbaa(xhci);
+	} else {
+		if (xhci->dcbaa)
+			dma_free_coherent(dev, sizeof(*xhci->dcbaa),
+					xhci->dcbaa, xhci->dcbaa->dma);
+	}
 	xhci->dcbaa = NULL;
 
 	scratchpad_free(xhci);
@@ -2418,15 +2483,21 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	 * xHCI section 5.4.6 - Device Context array must be
 	 * "physically contiguous and 64-byte (cache line) aligned".
 	 */
-	xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma,
-			flags);
-	if (!xhci->dcbaa)
-		goto fail;
-	xhci->dcbaa->dma = dma;
+	if (xhci_vendor_is_offload_enabled(xhci, NULL, 0)) {
+		xhci_vendor_alloc_dcbaa(xhci, flags);
+		if (!xhci->dcbaa)
+			goto fail;
+	} else {
+		xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma,
+				flags);
+		if (!xhci->dcbaa)
+			goto fail;
+		xhci->dcbaa->dma = dma;
+	}
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"// Device context base array address = 0x%llx (DMA), %p (virt)",
 			(unsigned long long)xhci->dcbaa->dma, xhci->dcbaa);
-	xhci_write_64(xhci, dma, &xhci->op_regs->dcbaa_ptr);
+	xhci_write_64(xhci, xhci->dcbaa->dma, &xhci->op_regs->dcbaa_ptr);
 
 	/*
 	 * Initialize the ring segment pool.  The ring must be a contiguous
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5fb55bf19493..2f04acb42fa6 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -173,6 +173,23 @@ static const struct of_device_id usb_xhci_of_match[] = {
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 #endif
 
+static int xhci_vendor_init(struct xhci_hcd *xhci)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->offload_init)
+		return ops->offload_init(xhci);
+	return 0;
+}
+
+static void xhci_vendor_cleanup(struct xhci_hcd *xhci)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->offload_cleanup)
+		ops->offload_cleanup(xhci);
+}
+
 static int xhci_plat_probe(struct platform_device *pdev)
 {
 	const struct xhci_plat_priv *priv_match;
@@ -317,6 +334,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto disable_clk;
 	}
 
+	ret = xhci_vendor_init(xhci);
+	if (ret)
+		goto disable_usb_phy;
+
 	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
 
 	if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
@@ -410,6 +431,8 @@ static int xhci_plat_remove(struct platform_device *dev)
 	if (shared_hcd)
 		usb_put_hcd(shared_hcd);
 
+	xhci_vendor_cleanup(xhci);
+
 	clk_disable_unprepare(clk);
 	clk_disable_unprepare(reg_clk);
 	usb_put_hcd(hcd);
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 1fb149d1fbce..5aa0d38fa01a 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -13,6 +13,7 @@
 struct xhci_plat_priv {
 	const char *firmware_name;
 	unsigned long long quirks;
+	struct xhci_offload_ops *offload_ops;
 	void (*plat_start)(struct usb_hcd *);
 	int (*init_quirk)(struct usb_hcd *);
 	int (*suspend_quirk)(struct usb_hcd *);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5176765c4013..aa3a44733bbe 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -22,6 +22,7 @@
 #include "xhci-trace.h"
 #include "xhci-debugfs.h"
 #include "xhci-dbgcap.h"
+#include "xhci-plat.h"
 
 #define DRIVER_AUTHOR "Sarah Sharp"
 #define DRIVER_DESC "'eXtensible' Host Controller (xHC) Driver"
@@ -1663,6 +1664,11 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
 		return -ENODEV;
 	}
 
+	if (xhci_vendor_usb_offload_skip_urb(xhci, urb)) {
+		xhci_dbg(xhci, "skip urb for usb offload\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (usb_endpoint_xfer_isoc(&urb->ep->desc))
 		num_tds = urb->number_of_packets;
 	else if (usb_endpoint_is_bulk_out(&urb->ep->desc) &&
@@ -4435,6 +4441,21 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
 	return ret;
 }
 
+struct xhci_offload_ops *xhci_offload_get_ops(struct xhci_hcd *xhci)
+{
+	return xhci_to_priv(xhci)->offload_ops;
+}
+EXPORT_SYMBOL_GPL(xhci_offload_get_ops);
+
+bool xhci_vendor_usb_offload_skip_urb(struct xhci_hcd *xhci, struct urb *urb)
+{
+	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+
+	if (ops && ops->usb_offload_skip_urb)
+		return ops->usb_offload_skip_urb(xhci, urb);
+	return false;
+}
+
 #ifdef CONFIG_PM
 
 /* BESL to HIRD Encoding array for USB2 LPM */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c0964fe8ac12..5368c66f5cec 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2228,6 +2228,37 @@ static inline struct xhci_ring *xhci_urb_to_transfer_ring(struct xhci_hcd *xhci,
 					urb->stream_id);
 }
 
+/**
+ * struct xhci_offload_ops - function callbacks for offload specific operations
+ * @offload_init: called for offload init process
+ * @offload_cleanup: called for offload cleanup process
+ * @is_usb_offload_enabled: called to check if xhci offload enabled
+ * @alloc_dcbaa: called when allocating specific dcbaa for offload
+ * @free_dcbaa: called to free specific dcbaa for offload
+ * @alloc_transfer_ring: called when remote transfer ring allocation is required
+ * @free_transfer_ring: called to free specific transfer ring for offload
+ * @usb_offload_skip_urb: called to check if need to skip urb
+ */
+struct xhci_offload_ops {
+	int (*offload_init)(struct xhci_hcd *xhci);
+	void (*offload_cleanup)(struct xhci_hcd *xhci);
+	bool (*is_offload_enabled)(struct xhci_hcd *xhci,
+				       struct xhci_virt_device *vdev,
+				       unsigned int ep_index);
+	void (*alloc_dcbaa)(struct xhci_hcd *xhci, gfp_t flags);
+	void (*free_dcbaa)(struct xhci_hcd *xhci);
+
+	struct xhci_ring *(*alloc_transfer_ring)(struct xhci_hcd *xhci,
+			u32 endpoint_type, enum xhci_ring_type ring_type,
+			unsigned int max_packet, gfp_t mem_flags);
+	void (*free_transfer_ring)(struct xhci_hcd *xhci,
+			struct xhci_virt_device *virt_dev, unsigned int ep_index);
+	bool (*usb_offload_skip_urb)(struct xhci_hcd *xhci, struct urb *urb);
+};
+
+struct xhci_offload_ops *xhci_offload_get_ops(struct xhci_hcd *xhci);
+bool xhci_vendor_usb_offload_skip_urb(struct xhci_hcd *xhci, struct urb *urb);
+
 /*
  * TODO: As per spec Isochronous IDT transmissions are supported. We bypass
  * them anyways as we where unable to find a device that matches the
-- 
2.38.0.135.g90850a2211-goog

Re: [PATCH 1/3] usb: host: add xhci hooks for USB offload

[Greg KH]

On Thu, Oct 27, 2022 at 08:40:48AM +0800, Albert Wang wrote:
> From: Howard Yen <howardyen@google.com>
> 
> This change is to provide USB offload function which allows to offload some
> xHCI operations on co-processor. This is especially designed for USB audio
> usecase. The co-processor is able to manipulate some USB structures in his
> own memory, like SRAM.
> 
> There are several offload_ops introduced by this patch:
> 
> struct xhci_offload_ops - function callbacks for offlad specific operations
> {
> 	@offload_init:
> 		- called for vendor init process during xhci-plat-hcd
> 		  probe.
> 	@offload_cleanup:
> 		- called for vendor cleanup process during xhci-plat-hcd
> 		  remove.
> 	@is_usb_offload_enabled:
> 		- called to check if usb offload enabled.
> 	@alloc_dcbaa:
> 		- called when allocating vendor specific dcbaa during
> 		  memory initializtion.
> 	@free_dcbaa:
> 		- called to free vendor specific dcbaa when cleanup the
> 		  memory.
> 	@alloc_transfer_ring:
> 		- called when vendor specific transfer ring allocation is required
> 	@free_transfer_ring:
> 		- called to free vendor specific transfer ring
> 	@usb_offload_skip_urb:
> 		- called to check if need to skip urb enqueue
> }
> 
> The xhci hooks with prefix "xhci_vendor_" on the ops in xhci_offload_ops.
> For example, offload_init ops will be invoked by xhci_vendor_offload_init()
> hook,is_usb_offload_enabled ops will be invoked by
> xhci_vendor_is_usb_offload_enabled(), and so on.
> 
> Signed-off-by: Howard Yen <howardyen@google.com>
> Link: https://lore.kernel.org/r/20210119101044.1637023-1-howardyen@google.com
> Signed-off-by: Greg Kroah-Harktman <gregkh@google.com>

Sorry, but no, I did NOT sign of on this patch for submission here.

And if you read the link above, you will see that I explicitly rejected
this commit for inclusion.

What changed from this previous series?  Is any of the issues raised
there now resolved?  If so, how?  If not, why not?

thanks,

greg k-h

[PATCH 2/3] usb: xhci-plat: add xhci_plat_priv_overwrite

[Albert Wang]

From: Howard Yen <howardyen@google.com>

Add an overwrite to platform specific callback for setting up the
xhci_offload_ops, allow vendor to store the xhci_offload_ops and
overwrite them when xhci_plat_probe invoked.

Signed-off-by: Howard Yen <howardyen@google.com>
Link: https://lore.kernel.org/r/20210119101044.1637023-1-howardyen@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
---
 drivers/usb/host/xhci-plat.c | 20 ++++++++++++++++++++
 drivers/usb/host/xhci-plat.h |  7 +++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 2f04acb42fa6..11ff89f722b7 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -173,9 +173,26 @@ static const struct of_device_id usb_xhci_of_match[] = {
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 #endif
 
+static struct xhci_plat_priv_overwrite xhci_plat_vendor_overwrite;
+
+int xhci_plat_register_offload_ops(struct xhci_offload_ops *offload_ops)
+{
+	if (offload_ops == NULL)
+		return -EINVAL;
+
+	xhci_plat_vendor_overwrite.offload_ops = offload_ops;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_plat_register_offload_ops);
+
 static int xhci_vendor_init(struct xhci_hcd *xhci)
 {
 	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+	struct xhci_plat_priv *priv = xhci_to_priv(xhci);
+
+	if (xhci_plat_vendor_overwrite.offload_ops)
+		ops = priv->offload_ops = xhci_plat_vendor_overwrite.offload_ops;
 
 	if (ops && ops->offload_init)
 		return ops->offload_init(xhci);
@@ -185,9 +202,12 @@ static int xhci_vendor_init(struct xhci_hcd *xhci)
 static void xhci_vendor_cleanup(struct xhci_hcd *xhci)
 {
 	struct xhci_offload_ops *ops = xhci_offload_get_ops(xhci);
+	struct xhci_plat_priv *priv = xhci_to_priv(xhci);
 
 	if (ops && ops->offload_cleanup)
 		ops->offload_cleanup(xhci);
+
+	priv->offload_ops = NULL;
 }
 
 static int xhci_plat_probe(struct platform_device *pdev)
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 5aa0d38fa01a..0656d6daa194 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -22,4 +22,11 @@ struct xhci_plat_priv {
 
 #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
 #define xhci_to_priv(x) ((struct xhci_plat_priv *)(x)->priv)
+
+struct xhci_plat_priv_overwrite {
+	struct xhci_offload_ops *offload_ops;
+};
+
+int xhci_plat_register_offload_ops(struct xhci_offload_ops *offload_ops);
+
 #endif	/* _XHCI_PLAT_H */
-- 
2.38.0.135.g90850a2211-goog

Re: [PATCH 2/3] usb: xhci-plat: add xhci_plat_priv_overwrite

[Greg KH]

On Thu, Oct 27, 2022 at 08:40:49AM +0800, Albert Wang wrote:
> From: Howard Yen <howardyen@google.com>
> 
> Add an overwrite to platform specific callback for setting up the
> xhci_offload_ops, allow vendor to store the xhci_offload_ops and
> overwrite them when xhci_plat_probe invoked.
> 
> Signed-off-by: Howard Yen <howardyen@google.com>
> Link: https://lore.kernel.org/r/20210119101044.1637023-1-howardyen@google.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

Again, no, I do NOT sign off on this submission.

Also, you did not sign off on it, yet you forwarded it on.  That's not
allowed either, and makes this whole series not even able to be
accepted, if it were a valid set of changes :(

Please fix.

thanks,

greg k-h

Re: [PATCH 2/3] usb: xhci-plat: add xhci_plat_priv_overwrite

[Albert Wang]

Understood, will fix and re-upload v2 patch set.

Thanks,
Albert

Albert Wang | Pixel USB Software  | albertccwang@google.com | +886-918-695-245


On Thu, Oct 27, 2022 at 2:22 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Oct 27, 2022 at 08:40:49AM +0800, Albert Wang wrote:
> > From: Howard Yen <howardyen@google.com>
> >
> > Add an overwrite to platform specific callback for setting up the
> > xhci_offload_ops, allow vendor to store the xhci_offload_ops and
> > overwrite them when xhci_plat_probe invoked.
> >
> > Signed-off-by: Howard Yen <howardyen@google.com>
> > Link: https://lore.kernel.org/r/20210119101044.1637023-1-howardyen@google.com
> > Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
>
> Again, no, I do NOT sign off on this submission.
>
> Also, you did not sign off on it, yet you forwarded it on.  That's not
> allowed either, and makes this whole series not even able to be
> accepted, if it were a valid set of changes :(
>
> Please fix.
>
> thanks,
>
> greg k-h

[PATCH 3/3] usb: host: add the xhci offload hooks implementations

[Albert Wang]

Add the offload hooks implementations and call to co-processor for
offload operations.

Signed-off-by: Albert Wang <albertccwang@google.com>
---
 drivers/usb/host/xhci-offload-impl.c | 492 +++++++++++++++++++++++++++
 1 file changed, 492 insertions(+)
 create mode 100644 drivers/usb/host/xhci-offload-impl.c

diff --git a/drivers/usb/host/xhci-offload-impl.c b/drivers/usb/host/xhci-offload-impl.c
new file mode 100644
index 000000000000..90e546d63fbe
--- /dev/null
+++ b/drivers/usb/host/xhci-offload-impl.c
@@ -0,0 +1,492 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Google Corp.
+ *
+ * Author:
+ *  Howard.Yen <howardyen@google.com>
+ */
+
+#include <linux/dmapool.h>
+#include <linux/dma-mapping.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/pm_wakeup.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+#include <linux/usb/hcd.h>
+
+#include "xhci.h"
+#include "xhci-plat.h"
+
+enum usb_offload_op_mode {
+	USB_OFFLOAD_STOP,
+	USB_OFFLOAD_DRAM
+};
+
+enum usb_state {
+	USB_DISCONNECTED,
+	USB_CONNECTED
+};
+
+enum usb_offload_msg {
+	SET_DCBAA_PTR,
+	SETUP_DONE,
+	SET_ISOC_TR_INFO,
+	SYNC_CONN_STAT,
+	SET_OFFLOAD_STATE
+};
+
+struct conn_stat_args {
+	u16 bus_id;
+	u16 dev_num;
+	u16 slot_id;
+	u32 conn_stat;
+};
+
+struct get_isoc_tr_info_args {
+	u16 ep_id;
+	u16 dir;
+	u32 type;
+	u32 num_segs;
+	u32 seg_ptr;
+	u32 max_packet;
+	u32 deq_ptr;
+	u32 enq_ptr;
+	u32 cycle_state;
+	u32 num_trbs_free;
+};
+
+struct xhci_offload_data {
+	struct xhci_hcd *xhci;
+
+	bool usb_accessory_enabled;
+	bool usb_audio_offload;
+	bool dt_direct_usb_access;
+	bool offload_state;
+
+	enum usb_offload_op_mode op_mode;
+};
+
+static struct xhci_offload_data *offload_data;
+struct xhci_offload_data *xhci_get_offload_data(void)
+{
+	return offload_data;
+}
+
+/*
+ * Determine if an USB device is a compatible devices:
+ *     True: Devices are audio class and they contain ISOC endpoint
+ *    False: Devices are not audio class or they're audio class but no ISOC endpoint or
+ *           they have at least one interface is video class
+ */
+static bool is_compatible_with_usb_audio_offload(struct usb_device *udev)
+{
+	struct usb_endpoint_descriptor *epd;
+	struct usb_host_config *config;
+	struct usb_host_interface *alt;
+	struct usb_interface_cache *intfc;
+	int i, j, k;
+	bool is_audio = false;
+
+	config = udev->config;
+	for (i = 0; i < config->desc.bNumInterfaces; i++) {
+		intfc = config->intf_cache[i];
+		for (j = 0; j < intfc->num_altsetting; j++) {
+			alt = &intfc->altsetting[j];
+
+			if (alt->desc.bInterfaceClass == USB_CLASS_VIDEO) {
+				is_audio = false;
+				goto out;
+			}
+
+			if (alt->desc.bInterfaceClass == USB_CLASS_AUDIO) {
+				for (k = 0; k < alt->desc.bNumEndpoints; k++) {
+					epd = &alt->endpoint[k].desc;
+					if (usb_endpoint_xfer_isoc(epd)) {
+						is_audio = true;
+						break;
+					}
+				}
+			}
+		}
+	}
+
+out:
+	return is_audio;
+}
+
+/*
+ * check the usb device including the video class:
+ *     True: Devices contain video class
+ *    False: Device doesn't contain video class
+ */
+static bool is_usb_video_device(struct usb_device *udev)
+{
+	struct usb_host_config *config;
+	struct usb_host_interface *alt;
+	struct usb_interface_cache *intfc;
+	int i, j;
+	bool is_video = false;
+
+	if (!udev || !udev->config)
+		return is_video;
+
+	config = udev->config;
+
+	for (i = 0; i < config->desc.bNumInterfaces; i++) {
+		intfc = config->intf_cache[i];
+		for (j = 0; j < intfc->num_altsetting; j++) {
+			alt = &intfc->altsetting[j];
+
+			if (alt->desc.bInterfaceClass == USB_CLASS_VIDEO) {
+				is_video = true;
+				goto out;
+			}
+		}
+	}
+
+out:
+	return is_video;
+}
+
+/*
+ * This is the driver call to co-processor for offload operations.
+ */
+int offload_driver_call(enum usb_offload_msg msg, void *ptr)
+{
+	enum usb_offload_msg offload_msg;
+	void *argptr;
+
+	offload_msg = msg;
+	argptr = ptr;
+
+	return 0;
+}
+
+static int xhci_sync_conn_stat(unsigned int bus_id, unsigned int dev_num, unsigned int slot_id,
+				unsigned int conn_stat)
+{
+	struct conn_stat_args conn_args;
+
+	conn_args.bus_id = bus_id;
+	conn_args.dev_num = dev_num;
+	conn_args.slot_id = slot_id;
+	conn_args.conn_stat = conn_stat;
+
+	return offload_driver_call(SYNC_CONN_STAT, &conn_args);
+}
+
+static int usb_host_mode_state_notify(enum usb_state usb_state)
+{
+	return xhci_sync_conn_stat(0, 0, 0, usb_state);
+}
+
+static int xhci_udev_notify(struct notifier_block *self, unsigned long action,
+				void *dev)
+{
+	struct usb_device *udev = dev;
+	struct xhci_offload_data *offload_data = xhci_get_offload_data();
+
+	switch (action) {
+	case USB_DEVICE_ADD:
+		if (is_compatible_with_usb_audio_offload(udev)) {
+			dev_dbg(&udev->dev, "Compatible with usb audio offload\n");
+			if (offload_data->op_mode == USB_OFFLOAD_DRAM) {
+				xhci_sync_conn_stat(udev->bus->busnum, udev->devnum, udev->slot_id,
+						    USB_CONNECTED);
+			}
+		}
+		offload_data->usb_accessory_enabled = false;
+		break;
+	case USB_DEVICE_REMOVE:
+		if (is_compatible_with_usb_audio_offload(udev) &&
+		    (offload_data->op_mode == USB_OFFLOAD_DRAM)) {
+			xhci_sync_conn_stat(udev->bus->busnum, udev->devnum, udev->slot_id,
+					    USB_DISCONNECTED);
+		}
+		offload_data->usb_accessory_enabled = false;
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block xhci_udev_nb = {
+	.notifier_call = xhci_udev_notify,
+};
+
+static int usb_audio_offload_init(struct xhci_hcd *xhci)
+{
+	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+	struct xhci_offload_data *offload_data = xhci_get_offload_data();
+	int ret;
+	u32 out_val;
+
+	offload_data = kzalloc(sizeof(struct xhci_offload_data), GFP_KERNEL);
+	if (!offload_data)
+		return -ENOMEM;
+
+	if (!of_property_read_u32(dev->of_node, "offload", &out_val))
+		offload_data->usb_audio_offload = (out_val == 1) ? true : false;
+
+	ret = of_reserved_mem_device_init(dev);
+	if (ret) {
+		dev_err(dev, "Could not get reserved memory\n");
+		kfree(offload_data);
+		return ret;
+	}
+
+	offload_data->dt_direct_usb_access =
+		of_property_read_bool(dev->of_node, "direct-usb-access") ? true : false;
+	if (!offload_data->dt_direct_usb_access)
+		dev_warn(dev, "Direct USB access is not supported\n");
+
+	offload_data->offload_state = true;
+
+	usb_register_notify(&xhci_udev_nb);
+	offload_data->op_mode = USB_OFFLOAD_DRAM;
+	offload_data->xhci = xhci;
+
+	return 0;
+}
+
+static void usb_audio_offload_cleanup(struct xhci_hcd *xhci)
+{
+	struct xhci_offload_data *offload_data = xhci_get_offload_data();
+
+	offload_data->usb_audio_offload = false;
+	offload_data->op_mode = USB_OFFLOAD_STOP;
+	offload_data->xhci = NULL;
+
+	usb_unregister_notify(&xhci_udev_nb);
+
+	/* Notification for xhci driver removing */
+	usb_host_mode_state_notify(USB_DISCONNECTED);
+
+	kfree(offload_data);
+	offload_data = NULL;
+}
+
+static bool is_offload_enabled(struct xhci_hcd *xhci,
+		struct xhci_virt_device *vdev, unsigned int ep_index)
+{
+	struct usb_device *udev;
+	struct xhci_offload_data *offload_data = xhci_get_offload_data();
+	bool global_enabled = offload_data->op_mode != USB_OFFLOAD_STOP;
+	struct xhci_ring *ep_ring;
+
+	if (vdev == NULL || vdev->eps[ep_index].ring == NULL)
+		return global_enabled;
+
+	udev = vdev->udev;
+
+	if (global_enabled) {
+		ep_ring = vdev->eps[ep_index].ring;
+		if (offload_data->op_mode == USB_OFFLOAD_DRAM) {
+			if (is_usb_video_device(udev))
+				return false;
+			else if (ep_ring->type == TYPE_ISOC)
+				return offload_data->offload_state;
+		}
+	}
+
+	return false;
+}
+
+static bool is_usb_bulk_transfer_enabled(struct xhci_hcd *xhci, struct urb *urb)
+{
+	struct xhci_offload_data *offload_data = xhci_get_offload_data();
+	struct usb_endpoint_descriptor *desc = &urb->ep->desc;
+	int ep_type = usb_endpoint_type(desc);
+	struct usb_ctrlrequest *cmd;
+	bool skip_bulk = false;
+
+	cmd = (struct usb_ctrlrequest *) urb->setup_packet;
+
+	if (ep_type == USB_ENDPOINT_XFER_CONTROL) {
+		if (!usb_endpoint_dir_in(desc) && cmd->bRequest == 0x35)
+			offload_data->usb_accessory_enabled = true;
+		else
+			offload_data->usb_accessory_enabled = false;
+	}
+
+	if (ep_type == USB_ENDPOINT_XFER_BULK && !usb_endpoint_dir_in(desc))
+		skip_bulk = offload_data->usb_accessory_enabled;
+
+	return skip_bulk;
+}
+
+static int xhci_set_dcbaa_ptr(u64 dcbaa_ptr)
+{
+	return offload_driver_call(SET_DCBAA_PTR, &dcbaa_ptr);
+}
+
+static int xhci_setup_done(void)
+{
+	return offload_driver_call(SETUP_DONE, NULL);
+}
+
+static void alloc_dcbaa(struct xhci_hcd *xhci, gfp_t flags)
+{
+	dma_addr_t dma;
+	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+	struct xhci_offload_data *offload_data = xhci_get_offload_data();
+
+	if (offload_data->op_mode == USB_OFFLOAD_DRAM) {
+		xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa),
+						 &dma, flags);
+		if (!xhci->dcbaa)
+			return;
+
+		xhci->dcbaa->dma = dma;
+		if (xhci_set_dcbaa_ptr(xhci->dcbaa->dma) != 0) {
+			xhci_err(xhci, "Set DCBAA pointer failed\n");
+			xhci->dcbaa = NULL;
+			return;
+		}
+		xhci_setup_done();
+
+		xhci_dbg(xhci, "Set dcbaa_ptr=%llx to AoC\n", xhci->dcbaa->dma);
+	} else {
+		xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa),
+						 &dma, flags);
+		if (!xhci->dcbaa)
+			return;
+
+		xhci->dcbaa->dma = dma;
+	}
+}
+
+static void free_dcbaa(struct xhci_hcd *xhci)
+{
+	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+
+	if (!xhci->dcbaa)
+		return;
+
+	dma_free_coherent(dev, sizeof(*xhci->dcbaa),
+			  xhci->dcbaa, xhci->dcbaa->dma);
+
+	xhci->dcbaa = NULL;
+}
+
+static int xhci_set_isoc_tr_info(u16 ep_id, u16 dir, struct xhci_ring *ep_ring)
+{
+	struct get_isoc_tr_info_args tr_info;
+
+	tr_info.ep_id = ep_id;
+	tr_info.dir = dir;
+	tr_info.num_segs = ep_ring->num_segs;
+	tr_info.max_packet = ep_ring->bounce_buf_len;
+	tr_info.type = ep_ring->type;
+	tr_info.seg_ptr = ep_ring->first_seg->dma;
+	tr_info.cycle_state = ep_ring->cycle_state;
+	tr_info.num_trbs_free = ep_ring->num_trbs_free;
+
+	return offload_driver_call(SET_ISOC_TR_INFO, &tr_info);
+}
+
+static struct xhci_ring *alloc_transfer_ring(struct xhci_hcd *xhci,
+		u32 endpoint_type, enum xhci_ring_type ring_type,
+		unsigned int max_packet, gfp_t mem_flags)
+{
+	struct xhci_ring *ep_ring;
+	u16 dir;
+
+	ep_ring = xhci_ring_alloc(xhci, 1, 1, ring_type, max_packet, mem_flags);
+	dir = endpoint_type == ISOC_IN_EP ? 0 : 1;
+
+	xhci_set_isoc_tr_info(0, dir, ep_ring);
+
+	return ep_ring;
+}
+
+static void free_transfer_ring(struct xhci_hcd *xhci, struct xhci_virt_device *virt_dev,
+				unsigned int ep_index)
+{
+	struct xhci_ring *ring, *new_ring;
+	struct xhci_ep_ctx *ep_ctx;
+	struct xhci_input_control_ctx *ctrl_ctx;
+	u32 ep_type;
+	u32 ep_is_added, ep_is_dropped;
+
+	ring = virt_dev->eps[ep_index].ring;
+	new_ring = virt_dev->eps[ep_index].new_ring;
+	ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, ep_index);
+	ep_type = CTX_TO_EP_TYPE(le32_to_cpu(ep_ctx->ep_info2));
+
+	ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx);
+	if (!ctrl_ctx) {
+		xhci_warn(xhci, "%s: Could not get input context, bad type.\n", __func__);
+		return;
+	}
+	ep_is_added = EP_IS_ADDED(ctrl_ctx, ep_index);
+	ep_is_dropped = EP_IS_DROPPED(ctrl_ctx, ep_index);
+
+	xhci_dbg(xhci, "%s: ep %u is added(0x%x), is dropped(0x%x)\n", __func__, ep_index,
+		 ep_is_added, ep_is_dropped);
+
+	if (ring) {
+		xhci_dbg(xhci, "%s: ep_index=%u, ep_type=%u, ring type=%u, new_ring=%pK\n",
+			 __func__, ep_index, ep_type, ring->type, new_ring);
+
+		xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
+
+		virt_dev->eps[ep_index].ring = NULL;
+
+		if (ep_is_added == 0 && ep_is_dropped == 0)
+			return;
+	}
+
+	if (new_ring) {
+		xhci_dbg(xhci, "%s: ep_index=%u, ep_type=%u, new_ring type=%u\n", __func__,
+			ep_index, ep_type, new_ring->type);
+
+		xhci_ring_free(xhci, virt_dev->eps[ep_index].new_ring);
+
+		virt_dev->eps[ep_index].new_ring = NULL;
+
+		return;
+	}
+}
+
+static bool offload_skip_urb(struct xhci_hcd *xhci, struct urb *urb)
+{
+	struct xhci_virt_device *vdev = xhci->devs[urb->dev->slot_id];
+	struct usb_endpoint_descriptor *desc = &urb->ep->desc;
+	int ep_type = usb_endpoint_type(desc);
+	unsigned int ep_index;
+
+	if (ep_type == USB_ENDPOINT_XFER_CONTROL)
+		ep_index = (unsigned int)(usb_endpoint_num(desc)*2);
+	else
+		ep_index = (unsigned int)(usb_endpoint_num(desc)*2) +
+			   (usb_endpoint_dir_in(desc) ? 1 : 0) - 1;
+
+	xhci_dbg(xhci, "%s: ep_index=%u, ep_type=%d\n", __func__, ep_index, ep_type);
+
+	if (is_offload_enabled(xhci, vdev, ep_index))
+		return true;
+
+	if (is_usb_bulk_transfer_enabled(xhci, urb))
+		return true;
+
+	return false;
+}
+
+static struct xhci_offload_ops offload_ops = {
+	.offload_init = usb_audio_offload_init,
+	.offload_cleanup = usb_audio_offload_cleanup,
+	.is_offload_enabled = is_offload_enabled,
+	.alloc_dcbaa = alloc_dcbaa,
+	.free_dcbaa = free_dcbaa,
+	.alloc_transfer_ring = alloc_transfer_ring,
+	.free_transfer_ring = free_transfer_ring,
+	.usb_offload_skip_urb = offload_skip_urb,
+};
+
+int xhci_offload_helper_init(void)
+{
+	return xhci_plat_register_offload_ops(&offload_ops);
+}
-- 
2.38.0.135.g90850a2211-goog

Re: [PATCH 3/3] usb: host: add the xhci offload hooks implementations

[Greg KH]

On Thu, Oct 27, 2022 at 08:40:50AM +0800, Albert Wang wrote:
> Add the offload hooks implementations and call to co-processor for
> offload operations.

You forgot to submit the user for all of these hooks at the same time.

For obvious reasons (and this was raised every time this patch series
has been submitted), we can not take hooks or apis that do not have a
real user of them at the same time.

And you don't want us to do that either, right?  How would you maintain
a chunk of code that has calls to other code that is not even in the
same repo and that you can never seee?  Would you be able to do that?

Again, please read the comments that came up when this was submitted
last time, and work to resolve them before submitting this series again.

I do not see any changes here from the last submission.  If that is
incorrect, then you need to document what changed, as our kernel
documentation asks you to do.

thanks,

greg k-h

Re: [PATCH 0/3] add xhci hooks for USB offload

[Greg KH]

On Thu, Oct 27, 2022 at 08:40:47AM +0800, Albert Wang wrote:
> This patchset is to proviode the USB offload function which allows to
> offload some xHCI operations on co-processor.
> 
> *** BLURB HERE ***

No blurb?

And isn't this a v2 of this patchset?  What changed from the previous
submission?

thanks,

greg k-h