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@xxxxxxxxxx>
> Signed-off-by: Howard Yen <howardyen@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> + */
> +
> +#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
