* [PATCH v2 1/3] usb: host: add xhci hooks for USB offload
2022-11-10 8:00 [PATCH v2 0/3] add xhci hooks for USB offload Albert Wang
@ 2022-11-10 8:00 ` Albert Wang
2022-11-10 8:00 ` [PATCH v2 2/3] usb: xhci-plat: add xhci_plat_priv_overwrite Albert Wang
2022-11-10 8:00 ` [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations Albert Wang
2 siblings, 0 replies; 7+ messages in thread
From: Albert Wang @ 2022-11-10 8:00 UTC (permalink / raw)
To: mathias.nyman, gregkh; +Cc: badhri, howardyen, linux-kernel, linux-usb
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>
---
Changes in v2:
- Remove irq work, device context synchronization
- Remove export symbols
- Use 'offload_ops' instead of 'vendor_ops'
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 81ca2bc1f0be..ab0ef19d4fa3 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);
@@ -1487,8 +1523,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;
@@ -1832,6 +1876,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;
@@ -1883,9 +1944,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);
@@ -2422,15 +2487,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 79d7931c048a..75d39fe0d44d 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"
@@ -1669,6 +1670,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) &&
@@ -4441,6 +4447,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 cc084d9505cd..d67bea2c180d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2229,6 +2229,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.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/3] usb: xhci-plat: add xhci_plat_priv_overwrite
2022-11-10 8:00 [PATCH v2 0/3] add xhci hooks for USB offload Albert Wang
2022-11-10 8:00 ` [PATCH v2 1/3] usb: host: " Albert Wang
@ 2022-11-10 8:00 ` Albert Wang
2022-11-10 8:00 ` [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations Albert Wang
2 siblings, 0 replies; 7+ messages in thread
From: Albert Wang @ 2022-11-10 8:00 UTC (permalink / raw)
To: mathias.nyman, gregkh; +Cc: badhri, howardyen, linux-kernel, linux-usb
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>
---
Changes in v2:
- Use 'offload_ops' instead of 'vendor_ops'
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.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations
2022-11-10 8:00 [PATCH v2 0/3] add xhci hooks for USB offload Albert Wang
2022-11-10 8:00 ` [PATCH v2 1/3] usb: host: " Albert Wang
2022-11-10 8:00 ` [PATCH v2 2/3] usb: xhci-plat: add xhci_plat_priv_overwrite Albert Wang
@ 2022-11-10 8:00 ` Albert Wang
2022-11-10 8:17 ` Greg KH
2 siblings, 1 reply; 7+ messages in thread
From: Albert Wang @ 2022-11-10 8:00 UTC (permalink / raw)
To: mathias.nyman, gregkh
Cc: badhri, howardyen, linux-kernel, linux-usb, Albert Wang
Add the offload hooks implementations which are used in the xHCI driver
for vendor offload function, and some functions will call to
co-processor driver for further offload operations.
Signed-off-by: Albert Wang <albertccwang@google.com>
Signed-off-by: Howard Yen <howardyen@google.com>
---
Changes in v2:
- New in v2
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.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations
2022-11-10 8:00 ` [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations Albert Wang
@ 2022-11-10 8:17 ` Greg KH
2022-11-24 6:47 ` Albert Wang
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2022-11-10 8:17 UTC (permalink / raw)
To: Albert Wang; +Cc: mathias.nyman, badhri, howardyen, linux-kernel, linux-usb
On Thu, Nov 10, 2022 at 04:00:06PM +0800, Albert Wang wrote:
> Add the offload hooks implementations which are used in the xHCI driver
> for vendor offload function, and some functions will call to
> co-processor driver for further offload operations.
Where is the users for these hooks? We can't add code that doesn't have
users as stated before.
> Signed-off-by: Albert Wang <albertccwang@google.com>
> Signed-off-by: Howard Yen <howardyen@google.com>
> ---
> Changes in v2:
> - New in v2
>
> 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.
I don't think it's 2020 anymore :)
> + *
> + * 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;
Don't just silence compiler warnings for no reason.
Again, this does not actually do anything at all. So how can we accept
this code?
> +
> + 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;
Why are you setting a stack variable to NULL?
Anyway, this looks much better overall than the previous submissions,
but we need a real user of this code otherwise it can not be accepted.
Please submit the driver that uses this api as part of your next
submission.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations
2022-11-10 8:17 ` Greg KH
@ 2022-11-24 6:47 ` Albert Wang
2022-11-24 9:08 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Albert Wang @ 2022-11-24 6:47 UTC (permalink / raw)
To: Greg KH; +Cc: mathias.nyman, badhri, howardyen, linux-kernel, linux-usb
On Thu, Nov 10, 2022 at 4:17 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Nov 10, 2022 at 04:00:06PM +0800, Albert Wang wrote:
> > Add the offload hooks implementations which are used in the xHCI driver
> > for vendor offload function, and some functions will call to
> > co-processor driver for further offload operations.
>
> Where is the users for these hooks? We can't add code that doesn't have
> users as stated before.
>
> > Signed-off-by: Albert Wang <albertccwang@google.com>
> > Signed-off-by: Howard Yen <howardyen@google.com>
> > ---
> > Changes in v2:
> > - New in v2
> >
> > 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.
>
> I don't think it's 2020 anymore :)
Thanks, I will correct it.
>
> > + *
> > + * 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;
>
> Don't just silence compiler warnings for no reason.
>
> Again, this does not actually do anything at all. So how can we accept
> this code?
>
This is the driver call to our co-processor which is a specific
hardware, so I don't submit it
and make it silent here.
> > +
> > + 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 accesconfirm ifs 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;
>
> Why are you setting a stack variable to NULL?
Thanks, I will remove it.
>
> Anyway, this looks much better overall than the previous submissions,
> but we need a real user of this code otherwise it can not be accepted.
> Please submit the driver that uses this api as part of your next
> submission.
>
We define and use those hook apis in the common xhci driver to offload
some memory
manipulation to a co-processor. So these apis will be executed to
allocate or free memory,
like dcbaa, transfer ring, in the co-processor memory space when
offlooffload_driver_callad enabled. The file,
xhci-offload-impl.c, shows how we use those xHCI hook apis for the
offload implementation.
Here is the flow diagram:
xHCI common driver xHCI offload implement driver
co-processor driver
hooks
offload_driver_call()
----------------------------
----------------------------------------
--------------------------------------------------------------
offload_init usb_audio_offload_init
offload_cleanup usb_audio_offload_cleanup
is_offload_enabled is_offload_enabled
alloc_dcbaa alloc_dcbaa
offload_driver_call(SET_DCBAA_PTR, &dcbaa_ptr);
offload_driver_call(SETUP_DONE, NULL);
free_dcbaa free_dcbaa
alloc_transfer_ring alloc_transfer_ring
offload_driver_call(SET_ISOC_TR_INFO, &tr_info);
free_transfer_ring free_transfer_ring
usb_offload_skip_urb offload_skip_urb
Thanks!
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations
2022-11-24 6:47 ` Albert Wang
@ 2022-11-24 9:08 ` Greg KH
0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2022-11-24 9:08 UTC (permalink / raw)
To: Albert Wang; +Cc: mathias.nyman, badhri, howardyen, linux-kernel, linux-usb
On Thu, Nov 24, 2022 at 02:47:22PM +0800, Albert Wang wrote:
> > > +/*
> > > + * 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;
> >
> > Don't just silence compiler warnings for no reason.
> >
> > Again, this does not actually do anything at all. So how can we accept
> > this code?
> >
>
> This is the driver call to our co-processor which is a specific
> hardware, so I don't submit it
> and make it silent here.
"specific hardware" is what Linux is all about! Please submit your
actual drivers for this hardware, otherwise there is no way we can even
review properly this type of code, let alone accept it.
You all know this in great detail, I've been saying this for many years
now. It is very frustrating on my end to constantly have to reject this
type of change all the time.
What would you do if you were on the reviewer's side? Would you accept
this type of submission after constantly saying "I will only accept this
if you do X" and you get another patch that does NOT do "X"?
> We define and use those hook apis in the common xhci driver to offload
> some memory
> manipulation to a co-processor. So these apis will be executed to
> allocate or free memory,
> like dcbaa, transfer ring, in the co-processor memory space when
> offlooffload_driver_callad enabled. The file,
> xhci-offload-impl.c, shows how we use those xHCI hook apis for the
> offload implementation.
>
> Here is the flow diagram:
> xHCI common driver xHCI offload implement driver
> co-processor driver
> hooks
> offload_driver_call()
> ----------------------------
> ----------------------------------------
> --------------------------------------------------------------
> offload_init usb_audio_offload_init
> offload_cleanup usb_audio_offload_cleanup
> is_offload_enabled is_offload_enabled
> alloc_dcbaa alloc_dcbaa
> offload_driver_call(SET_DCBAA_PTR, &dcbaa_ptr);
>
> offload_driver_call(SETUP_DONE, NULL);
> free_dcbaa free_dcbaa
> alloc_transfer_ring alloc_transfer_ring
> offload_driver_call(SET_ISOC_TR_INFO, &tr_info);
> free_transfer_ring free_transfer_ring
> usb_offload_skip_urb offload_skip_urb
This does not make any sense, sorry. Perhaps your lines got wrapped
incorrectly?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread