[PATCH v1 0/3] Exynos Usb Audio Offloading Support

[PATCH v1 0/3] Exynos Usb Audio Offloading Support

[Oh Eomji]

Exynos uses the usb audio offloading functions to save power consumption
in the usb audio device. The audio control interface is processed in the
existing usb path, and the audio stream interface is processed in the
audio path.

Through communication between AP usb and audio usb f/w, usb audio device
connection information, xhci memory information, etc. are notified to usb
audio f/w so that the abox directly controls xhci transmission.

Vendor's hooking interface is required for this functions. Throught this
interface, information such as usb audio device connection information,
pcm interface information, sample rate setting, xhci memory, full descriptor
of connected device, and setting point can be transmitted to usb audio f/w.

the usb audio f/w can set up and interface, transmit audio data, etc. through
the received information.

This patchset includes the following for using usb audio offloading:
- vendor's hooking interface
- vendor's hooking interface calling point
- user using vendor's hooking interface


Oh Eomji (3):
  sound: usb: Add vendor's hooking interface
  sound: usb: Calling vendor's call-back function within usb audio
    operation.
  sound: usb: Exynos usb audio offloading driver

 sound/usb/Kconfig            |   9 +
 sound/usb/Makefile           |   2 +
 sound/usb/card.c             | 119 +++++++++
 sound/usb/card.h             |  20 ++
 sound/usb/exynos_usb_audio.c | 560 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/exynos_usb_audio.h | 150 ++++++++++++
 sound/usb/pcm.c              |  37 +++
 sound/usb/stream.c           |   2 +
 sound/usb/usbaudio.h         |  45 ++++
 9 files changed, 944 insertions(+)
 create mode 100644 sound/usb/exynos_usb_audio.c
 create mode 100644 sound/usb/exynos_usb_audio.h

-- 
2.7.4

[PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Oh Eomji]

In mobile, a co-processor can be used with USB audio to improve power
consumption.  To support this type of hardware, hooks need to be added
to the USB audio subsystem to be able to call into the hardware when
needed.

The main operation of the call-backs are:
  - Initialize the co-processor by transmitting data when initializing.
  - Change the co-processor setting value through the interface
    function.
  - Configure sampling rate
  - pcm open/close
  - other housekeeping

Known issues:
  - This only supports one set of callback hooks, meaning that this only
    works if there is one type of USB controller in the system.  This
    should be changed to be a per-host-controller interface instead of
    one global set of callbacks.

Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>
Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
---
 sound/usb/card.c     | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++
 sound/usb/card.h     |  20 +++++++++
 sound/usb/usbaudio.h |  45 +++++++++++++++++++
 3 files changed, 184 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 3769622..bd59311 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -117,6 +117,117 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no)
 static DEFINE_MUTEX(register_mutex);
 static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
 static struct usb_driver usb_audio_driver;
+static struct snd_usb_audio_vendor_ops *usb_vendor_ops;
+
+int snd_vendor_set_ops(struct snd_usb_audio_vendor_ops *ops)
+{
+	if ((!ops->connect) ||
+	    (!ops->disconnect) ||
+	    (!ops->set_interface) ||
+	    (!ops->set_rate) ||
+	    (!ops->set_pcm_buf) ||
+	    (!ops->set_pcm_intf) ||
+	    (!ops->set_pcm_connection) ||
+	    (!ops->set_pcm_binterval) ||
+	    (!ops->usb_add_ctls))
+		return -EINVAL;
+
+	usb_vendor_ops = ops;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_vendor_set_ops);
+
+struct snd_usb_audio_vendor_ops *snd_vendor_get_ops(void)
+{
+	return usb_vendor_ops;
+}
+
+static int snd_vendor_connect(struct usb_interface *intf)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->connect(intf);
+	return 0;
+}
+
+static void snd_vendor_disconnect(struct usb_interface *intf)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		ops->disconnect(intf);
+}
+
+int snd_vendor_set_interface(struct usb_device *udev,
+			     struct usb_host_interface *intf,
+			     int iface, int alt)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->set_interface(udev, intf, iface, alt);
+	return 0;
+}
+
+int snd_vendor_set_rate(int iface, int rate, int alt)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->set_rate(iface, rate, alt);
+	return 0;
+}
+
+int snd_vendor_set_pcm_buf(struct usb_device *udev, int iface)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		ops->set_pcm_buf(udev, iface);
+	return 0;
+}
+
+int snd_vendor_set_pcm_intf(struct usb_interface *intf, int iface, int alt,
+			    int direction)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->set_pcm_intf(intf, iface, alt, direction);
+	return 0;
+}
+
+int snd_vendor_set_pcm_connection(struct usb_device *udev,
+				  enum snd_vendor_pcm_open_close onoff,
+				  int direction)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->set_pcm_connection(udev, onoff, direction);
+	return 0;
+}
+
+int snd_vendor_set_pcm_binterval(const struct audioformat *fp,
+				 const struct audioformat *found,
+				 int *cur_attr, int *attr)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->set_pcm_binterval(fp, found, cur_attr, attr);
+	return 0;
+}
+
+static int snd_vendor_usb_add_ctls(struct snd_usb_audio *chip)
+{
+	struct snd_usb_audio_vendor_ops *ops = snd_vendor_get_ops();
+
+	if (ops)
+		return ops->usb_add_ctls(chip);
+	return 0;
+}
 
 /*
  * disconnect streams
@@ -753,6 +864,10 @@ static int usb_audio_probe(struct usb_interface *intf,
 	if (err < 0)
 		return err;
 
+	err = snd_vendor_connect(intf);
+	if (err)
+		return err;
+
 	/*
 	 * found a config.  now register to ALSA
 	 */
@@ -820,6 +935,8 @@ static int usb_audio_probe(struct usb_interface *intf,
 	if (chip->quirk_flags & QUIRK_FLAG_DISABLE_AUTOSUSPEND)
 		usb_disable_autosuspend(interface_to_usbdev(intf));
 
+	snd_vendor_usb_add_ctls(chip);
+
 	/*
 	 * For devices with more than one control interface, we assume the
 	 * first contains the audio controls. We might need a more specific
@@ -907,6 +1024,8 @@ static void usb_audio_disconnect(struct usb_interface *intf)
 
 	card = chip->card;
 
+	snd_vendor_disconnect(intf);
+
 	mutex_lock(&register_mutex);
 	if (atomic_inc_return(&chip->shutdown) == 1) {
 		struct snd_usb_stream *as;
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 87f042d0..81280ac 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -204,4 +204,24 @@ struct snd_usb_stream {
 	struct list_head list;
 };
 
+struct snd_usb_substream *find_snd_usb_substream(unsigned int card_num,
+	unsigned int pcm_idx, unsigned int direction, struct snd_usb_audio
+	**uchip, void (*disconnect_cb)(struct snd_usb_audio *chip));
+
+int snd_vendor_set_ops(struct snd_usb_audio_vendor_ops *vendor_ops);
+struct snd_usb_audio_vendor_ops *snd_vendor_get_ops(void);
+int snd_vendor_set_interface(struct usb_device *udev,
+			     struct usb_host_interface *alts,
+			     int iface, int alt);
+int snd_vendor_set_rate(int iface, int rate, int alt);
+int snd_vendor_set_pcm_buf(struct usb_device *udev, int iface);
+int snd_vendor_set_pcm_intf(struct usb_interface *intf, int iface, int alt,
+			    int direction);
+int snd_vendor_set_pcm_connection(struct usb_device *udev,
+				  enum snd_vendor_pcm_open_close onoff,
+				  int direction);
+int snd_vendor_set_pcm_binterval(const struct audioformat *fp,
+				 const struct audioformat *found,
+				 int *cur_attr, int *attr);
+
 #endif /* __USBAUDIO_CARD_H */
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 1678341..90c68cb 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -184,4 +184,49 @@ extern bool snd_usb_skip_validation;
 #define QUIRK_FLAG_DSD_RAW		(1U << 15)
 #define QUIRK_FLAG_SET_IFACE_FIRST	(1U << 16)
 
+struct audioformat;
+
+enum snd_vendor_pcm_open_close {
+	SOUND_PCM_CLOSE = 0,
+	SOUND_PCM_OPEN,
+};
+
+/**
+ * struct snd_usb_audio_vendor_ops - function callbacks for USB audio accelerators
+ * @connect: called when a new interface is found
+ * @disconnect: called when an interface is removed
+ * @set_interface: called when an interface is initialized
+ * @set_rate: called when the rate is set
+ * @set_pcm_buf: called when the pcm buffer is set
+ * @set_pcm_intf: called when the pcm interface is set
+ * @set_pcm_connection: called when pcm is opened/closed
+ * @set_pcm_binterval: called when the pcm binterval is set
+ * @usb_add_ctls: called when USB controls are added
+ *
+ * Set of callbacks for some accelerated USB audio streaming hardware.
+ *
+ * TODO: make this USB host-controller specific, right now this only works for
+ * one USB controller in the system at a time, which is only realistic for
+ * self-contained systems like phones.
+ */
+struct snd_usb_audio_vendor_ops {
+	int (*connect)(struct usb_interface *intf);
+	void (*disconnect)(struct usb_interface *intf);
+
+	int (*set_interface)(struct usb_device *udev,
+			     struct usb_host_interface *alts,
+			     int iface, int alt);
+	int (*set_rate)(int iface, int rate, int alt);
+	int (*set_pcm_buf)(struct usb_device *udev, int iface);
+	int (*set_pcm_intf)(struct usb_interface *intf, int iface, int alt,
+			    int direction);
+	int (*set_pcm_connection)(struct usb_device *udev,
+				  enum snd_vendor_pcm_open_close onoff,
+				  int direction);
+	int (*set_pcm_binterval)(const struct audioformat *fp,
+				 const struct audioformat *found,
+				 int *cur_attr, int *attr);
+	int (*usb_add_ctls)(struct snd_usb_audio *chip);
+};
+
 #endif /* __USBAUDIO_H */
-- 
2.7.4

[PATCH v1 2/3] sound: usb: Calling vendor's call-back function within usb audio operation.

[Oh Eomji]

When a new interface is connected or removed, the call-back functions
are called to transmit a command to the hardware.

Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
---
 sound/usb/pcm.c    | 37 +++++++++++++++++++++++++++++++++++++
 sound/usb/stream.c |  2 ++
 2 files changed, 39 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index cec6e91a..4bae4ba 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -144,6 +144,8 @@ find_format(struct list_head *fmt_list_head, snd_pcm_format_t format,
 			found = fp;
 			cur_attr = attr;
 		}
+
+		snd_vendor_set_pcm_binterval(fp, found, &cur_attr, &attr);
 	}
 	return found;
 }
@@ -434,6 +436,7 @@ static int configure_endpoints(struct snd_usb_audio *chip,
 			       struct snd_usb_substream *subs)
 {
 	int err;
+	struct usb_interface *iface;
 
 	if (subs->data_endpoint->need_setup) {
 		/* stop any running stream beforehand */
@@ -442,6 +445,13 @@ static int configure_endpoints(struct snd_usb_audio *chip,
 		err = snd_usb_endpoint_configure(chip, subs->data_endpoint);
 		if (err < 0)
 			return err;
+
+		iface = usb_ifnum_to_if(chip->dev, subs->data_endpoint->iface);
+		err = snd_vendor_set_pcm_intf(iface, subs->data_endpoint->iface,
+				subs->data_endpoint->altsetting, subs->direction);
+		if (err < 0)
+			return err;
+
 		snd_usb_set_format_quirk(subs, subs->cur_audiofmt);
 	}
 
@@ -616,8 +626,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_usb_substream *subs = runtime->private_data;
 	struct snd_usb_audio *chip = subs->stream->chip;
+	struct snd_usb_endpoint *ep = subs->data_endpoint;
 	int ret;
 
+	ret = snd_vendor_set_pcm_buf(subs->dev, subs->cur_audiofmt->iface);
+	if (ret)
+		return ret;
+
+	if (!subs->cur_audiofmt) {
+		dev_err(&subs->dev->dev, "no format is specified\n");
+		return -ENXIO;
+	}
+
 	ret = snd_usb_lock_shutdown(chip);
 	if (ret < 0)
 		return ret;
@@ -630,6 +650,13 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	if (ret < 0)
 		goto unlock;
 
+	if (snd_vendor_get_ops()) {
+		ret = snd_vendor_set_rate(ep->cur_audiofmt->iface,
+				ep->cur_rate, ep->cur_audiofmt->altsetting);
+		if (!ret)
+			goto unlock;
+	}
+
 	/* reset the pointer */
 	subs->buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size);
 	subs->inflight_bytes = 0;
@@ -1104,6 +1131,11 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
 	struct snd_usb_substream *subs = &as->substream[direction];
 	int ret;
 
+	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_OPEN,
+					    direction);
+	if (ret)
+		return ret;
+
 	runtime->hw = snd_usb_hardware;
 	/* need an explicit sync to catch applptr update in low-latency mode */
 	if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
@@ -1137,6 +1169,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_usb_substream *subs = &as->substream[direction];
 	int ret;
 
+	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_CLOSE,
+					    direction);
+	if (ret)
+		return ret;
+
 	snd_media_stop_pipeline(subs);
 
 	if (!snd_usb_lock_shutdown(subs->stream->chip)) {
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index ceb93d7..26ca696 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -1227,6 +1227,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
 		snd_usb_init_pitch(chip, fp);
 		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
 		usb_set_interface(chip->dev, iface_no, altno);
+		if (protocol > UAC_VERSION_1)
+			snd_vendor_set_interface(chip->dev, alts, iface_no, 0);
 	}
 	return 0;
 }
-- 
2.7.4

[PATCH v1 3/3] sound: usb: Exynos usb audio offloading driver

[Oh Eomji]

This is for usb audio offloading. usb audio offloading processes usb
audio stream data directly from the audio box even if ap usb enters
suspend, there is no problem in stream data transmission. This obtains
power saving effect while using usb audio device.

AP usb and audio usb f/w communicate via AUDIO IPC. By performing AUDIO
IPC in the vendor operations, abox can access and control the xhci to
transmit the data directly.

The types of commands and data delivered through AUDIO IPC include the
following (AP USB <-> AUDIO USB f/w) :
1. usb audio connection/disconnection status
2. xhci memory information
3. full descriptor for usb audio device
4. UAC(usb audio class) control command

Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
---
 sound/usb/Kconfig            |   9 +
 sound/usb/Makefile           |   2 +
 sound/usb/exynos_usb_audio.c | 560 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/exynos_usb_audio.h | 150 ++++++++++++
 4 files changed, 721 insertions(+)
 create mode 100644 sound/usb/exynos_usb_audio.c
 create mode 100644 sound/usb/exynos_usb_audio.h

diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
index 059242f..70252a3 100644
--- a/sound/usb/Kconfig
+++ b/sound/usb/Kconfig
@@ -27,6 +27,15 @@ config SND_USB_AUDIO
 config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
 	bool
 
+config SND_EXYNOS_USB_AUDIO
+	tristate "EXYNOS USB Audio offloading module"
+	depends on SND_USB_AUDIO
+	help
+	 Say Y here to include support for Exynos USB Audio ABOX offloading.
+
+	 To compile this driver as a module, choose M here: the module
+	 will be called exynos-usb-audio-offloading.
+
 config SND_USB_UA101
 	tristate "Edirol UA-101/UA-1000 driver"
 	select SND_PCM
diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 9ccb21a..bf019c7 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -28,6 +28,8 @@ snd-usbmidi-lib-objs := midi.o
 
 # Toplevel Module Dependency
 obj-$(CONFIG_SND_USB_AUDIO) += snd-usb-audio.o snd-usbmidi-lib.o
+obj-$(CONFIG_SND_EXYNOS_USB_AUDIO) += exynos-usb-audio-offloading.o
+exynos-usb-audio-offloading-y += exynos_usb_audio.o
 
 obj-$(CONFIG_SND_USB_UA101) += snd-usbmidi-lib.o
 obj-$(CONFIG_SND_USB_USX2Y) += snd-usbmidi-lib.o
diff --git a/sound/usb/exynos_usb_audio.c b/sound/usb/exynos_usb_audio.c
new file mode 100644
index 0000000..456cc78
--- /dev/null
+++ b/sound/usb/exynos_usb_audio.c
@@ -0,0 +1,560 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *   USB Audio offloading Driver for Exynos
+ *
+ *   Copyright (c) 2017 by Kyounghye Yun <k-hye.yun@samsung.com>
+ *
+ */
+
+
+#include <linux/bitops.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/ctype.h>
+#include <linux/usb.h>
+#include <linux/usb/audio.h>
+#include <linux/usb/audio-v2.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/completion.h>
+
+#include <sound/pcm.h>
+#include "../../../sound/usb/exynos_usb_audio.h"
+#include "usbaudio.h"
+#include "helper.h"
+#include "card.h"
+#include "quirks.h"
+
+static struct exynos_usb_audio *usb_audio;
+static struct snd_usb_audio_vendor_ops exynos_usb_audio_ops;
+
+struct hcd_hw_info *g_hwinfo;
+EXPORT_SYMBOL_GPL(g_hwinfo);
+int otg_connection;
+EXPORT_SYMBOL_GPL(otg_connection);
+int usb_audio_connection;
+EXPORT_SYMBOL_GPL(usb_audio_connection);
+
+static void exynos_usb_audio_set_device(struct usb_device *udev)
+{
+	usb_audio->udev = udev;
+	usb_audio->is_audio = 1;
+}
+
+static int exynos_usb_audio_unmap_all(void)
+{
+	/*
+	 * TODO: unmapping pcm buffer, usb descriptor, Device Context,
+	 * Input Context, ERST, URAM.
+	 */
+
+	return 0;
+}
+
+static int exynos_usb_audio_pcmbuf(struct usb_device *udev, int iface)
+{
+	if (!usb_audio->is_audio || !otg_connection)
+		return -1;
+
+	/*
+	 * TODO: Transmit the DRAM address that contains the xhci device
+	 * information,and the DMA address required for operation to the abox
+	 * usb f/w.
+	 */
+
+	return 0;
+}
+
+static int exynos_usb_audio_setrate(int iface, int rate, int alt)
+{
+	if (!usb_audio->is_audio || !otg_connection)
+		return -1;
+
+	/*
+	 * TODO: Notify the abox usb f/w the audio sample rate supported by
+	 * the interface of the connected audio device.
+	 */
+
+	return 0;
+}
+
+static int exynos_usb_audio_setintf(struct usb_device *udev, int iface, int alt, int direction)
+{
+	struct hcd_hw_info *hwinfo = g_hwinfo;
+	u64 in_offset, out_offset;
+
+	if (!usb_audio->pcm_open_done)
+		return -EPERM;
+
+	if (!usb_audio->is_audio || !otg_connection)
+		return -1;
+
+	if (direction) {
+		/* IN EP */
+		if (!usb_audio->indeq_map_done ||
+			(hwinfo->in_deq != hwinfo->old_in_deq)) {
+			/* TODO: Transmit pcm interface number, alt setting
+			 * number to abox usb f/w
+			 */
+			usb_audio->indeq_map_done = 1;
+			in_offset = hwinfo->in_deq % PAGE_SIZE;
+		}
+
+		if (hwinfo->fb_out_deq) {
+			if (!usb_audio->fb_outdeq_map_done ||
+					(hwinfo->fb_out_deq != hwinfo->fb_old_out_deq)) {
+				/* TODO: Transmit pcm interface number,
+				 * alt setting number to abox usb f/w
+				 */
+				usb_audio->fb_outdeq_map_done = 1;
+				out_offset = hwinfo->fb_out_deq % PAGE_SIZE;
+			}
+		}
+	} else {
+		/* OUT EP */
+		if (!usb_audio->outdeq_map_done ||
+			(hwinfo->out_deq != hwinfo->old_out_deq)) {
+			/* TODO: Transmit pcm interface number, alt setting
+			 * number to abox usb f/w
+			 */
+			usb_audio->outdeq_map_done = 1;
+			out_offset = hwinfo->out_deq % PAGE_SIZE;
+		}
+
+		if (hwinfo->fb_in_deq) {
+			if (!usb_audio->fb_indeq_map_done ||
+					(hwinfo->fb_in_deq != hwinfo->fb_old_in_deq)) {
+				/* TODO: Transmit pcm interface number,
+				 * alt setting number to abox usb f/w
+				 */
+				usb_audio->fb_indeq_map_done = 1;
+				in_offset = hwinfo->fb_in_deq % PAGE_SIZE;
+			}
+		}
+	}
+
+	/* one more check connection to prevent kernel panic */
+	if (!usb_audio->is_audio || !otg_connection)
+		return -1;
+
+	/* TODO: Notify abox usb f/w a dequeue pointer */
+
+	return 0;
+}
+
+static int exynos_usb_audio_hcd(struct usb_device *udev)
+{
+	struct hcd_hw_info *hwinfo = g_hwinfo;
+
+	/* back up each address for unmap */
+	usb_audio->dcbaa_dma = hwinfo->dcbaa_dma;
+	usb_audio->save_dma = hwinfo->save_dma;
+	usb_audio->in_ctx = hwinfo->in_ctx;
+	usb_audio->out_ctx = hwinfo->out_ctx;
+	usb_audio->erst_addr = hwinfo->erst_addr;
+	usb_audio->speed = hwinfo->speed;
+	usb_audio->use_uram = hwinfo->use_uram;
+
+	/*
+	 * TODO: DCBAA, Device Context, Input Context, URAM, ERST mapping
+	 * and notify abox usb f/w the address about xhci h/w resource to
+	 * directly control the xhci in abox.
+	 */
+
+	return 0;
+}
+
+static int exynos_usb_audio_desc(struct usb_device *udev)
+{
+	int configuration, cfgno, i;
+	unsigned char *buffer;
+	u64 desc_addr;
+	u64 offset;
+
+	configuration = usb_choose_configuration(udev);
+
+	cfgno = -1;
+	for (i = 0; i < udev->descriptor.bNumConfigurations; i++) {
+		if (udev->config[i].desc.bConfigurationValue ==
+				configuration) {
+			cfgno = i;
+			break;
+		}
+	}
+
+	if (cfgno == -1)
+		cfgno = 0;
+
+	/* need to memory mapping for usb descriptor */
+	buffer = udev->rawdescriptors[cfgno];
+	desc_addr = virt_to_phys(buffer);
+	offset = desc_addr % PAGE_SIZE;
+
+	/* store address information */
+	usb_audio->desc_addr = desc_addr;
+	usb_audio->offset = offset;
+
+	desc_addr -= offset;
+
+	/*
+	 * TODO: Notify the abox usb f/w that all descriptors have been recived
+	 * from the connected usb audio device, and that copy and memory mapping
+	 * have beed completed so that it can be used in abox usb f/w
+	 */
+
+	return 0;
+}
+
+static int exynos_usb_audio_conn(struct usb_device *udev, int is_conn)
+{
+
+	/* TODO: Notify abox usb f/w whether usb device is connected or not */
+	if (!is_conn) {
+		if (usb_audio->is_audio) {
+			usb_audio->is_audio = 0;
+			usb_audio->usb_audio_state = USB_AUDIO_REMOVING;
+		}
+	} else {
+		cancel_work_sync(&usb_audio->usb_work);
+		usb_audio->indeq_map_done = 0;
+		usb_audio->outdeq_map_done = 0;
+		usb_audio->fb_indeq_map_done = 0;
+		usb_audio->fb_outdeq_map_done = 0;
+		usb_audio->pcm_open_done = 0;
+		reinit_completion(&usb_audio->discon_done);
+		usb_audio->usb_audio_state = USB_AUDIO_CONNECT;
+		usb_audio_connection = 1;
+	}
+
+	return 0;
+}
+
+static int exynos_usb_audio_pcm(bool is_open, bool direction)
+{
+	if (!usb_audio->is_audio || !otg_connection)
+		return -1;
+
+	if (is_open)
+		usb_audio->pcm_open_done = 1;
+
+	/* TODO: Notify the abox usb f/w the pcm open/close status */
+
+	return 0;
+}
+
+static void exynos_usb_audio_work(struct work_struct *w)
+{
+	/* Don't unmap in USB_AUDIO_TIMEOUT_PROBE state */
+	if (usb_audio->usb_audio_state != USB_AUDIO_REMOVING)
+		return;
+
+	exynos_usb_audio_unmap_all();
+	usb_audio->usb_audio_state = USB_AUDIO_DISCONNECT;
+	usb_audio_connection = 0;
+	complete(&usb_audio->discon_done);
+}
+
+static int exynos_usb_scenario_ctl_info(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = AUDIO_MODE_NORMAL;
+	uinfo->value.integer.max = AUDIO_MODE_CALL_SCREEN;
+	return 0;
+}
+
+static int exynos_usb_scenario_ctl_get(struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	struct exynos_usb_audio *usb = snd_kcontrol_chip(kcontrol);
+
+	ucontrol->value.integer.value[0] = usb->user_scenario;
+	return 0;
+}
+
+static int exynos_usb_scenario_ctl_put(struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	struct exynos_usb_audio *usb = snd_kcontrol_chip(kcontrol);
+	int changed = 0;
+
+	if (usb->user_scenario !=
+	     ucontrol->value.integer.value[0]) {
+		usb->user_scenario = ucontrol->value.integer.value[0];
+		changed = 1;
+	}
+
+	return changed;
+}
+
+static int exynos_usb_add_ctls(struct snd_usb_audio *chip,
+				unsigned long private_value)
+{
+	struct snd_kcontrol_new knew = {
+		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
+		.name = "USB Audio Mode",
+		.info = exynos_usb_scenario_ctl_info,
+		.get = exynos_usb_scenario_ctl_get,
+		.put = exynos_usb_scenario_ctl_put,
+	};
+
+	int err;
+
+	if (!chip)
+		return -ENODEV;
+
+	knew.private_value = private_value;
+	usb_audio->kctl = snd_ctl_new1(&knew, usb_audio);
+	if (!usb_audio->kctl)
+		return -ENOMEM;
+
+	err = snd_ctl_add(chip->card, usb_audio->kctl);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+int exynos_usb_audio_init(struct device *dev, struct platform_device *pdev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *np_abox;
+	struct platform_device *pdev_abox;
+	int ret = 0;
+
+	if (!usb_audio) {
+		usb_audio = kmalloc(sizeof(struct exynos_usb_audio), GFP_KERNEL);
+		if (!usb_audio)
+			return -ENOMEM;
+	}
+
+	np_abox = of_parse_phandle(np, "abox", 0);
+	if (!np_abox)
+		return -EPROBE_DEFER;
+
+	pdev_abox = of_find_device_by_node(np_abox);
+	if (!pdev_abox)
+		return -EPROBE_DEFER;
+
+	init_completion(&usb_audio->in_conn_stop);
+	init_completion(&usb_audio->out_conn_stop);
+	init_completion(&usb_audio->discon_done);
+	usb_audio->abox = pdev_abox;
+	usb_audio->hcd_pdev = pdev;
+	usb_audio->udev = NULL;
+	usb_audio->is_audio = 0;
+	usb_audio->is_first_probe = 1;
+	usb_audio->user_scenario = AUDIO_MODE_NORMAL;
+	usb_audio->usb_audio_state = USB_AUDIO_DISCONNECT;
+	usb_audio_connection = 0;
+
+	INIT_WORK(&usb_audio->usb_work, exynos_usb_audio_work);
+
+	/* interface function mapping */
+	ret = snd_vendor_set_ops(&exynos_usb_audio_ops);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(exynos_usb_audio_init);
+
+/* card */
+static int exynos_usb_audio_connect(struct usb_interface *intf)
+{
+	struct usb_interface_descriptor *altsd;
+	struct usb_host_interface *alts;
+	struct usb_device *udev = interface_to_usbdev(intf);
+	int timeout = 0;
+
+	alts = &intf->altsetting[0];
+	altsd = get_iface_desc(alts);
+
+	if ((altsd->bInterfaceClass == USB_CLASS_AUDIO ||
+		altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC) &&
+		altsd->bInterfaceSubClass == USB_SUBCLASS_MIDISTREAMING) {
+	} else {
+		if (usb_audio->usb_audio_state == USB_AUDIO_REMOVING) {
+			timeout = wait_for_completion_timeout(
+				&usb_audio->discon_done,
+				msecs_to_jiffies(DISCONNECT_TIMEOUT));
+
+			if ((usb_audio->usb_audio_state == USB_AUDIO_REMOVING)
+					&& !timeout) {
+				usb_audio->usb_audio_state =
+					USB_AUDIO_TIMEOUT_PROBE;
+			}
+		}
+
+		if ((usb_audio->usb_audio_state == USB_AUDIO_DISCONNECT)
+			|| (usb_audio->usb_audio_state == USB_AUDIO_TIMEOUT_PROBE)) {
+			exynos_usb_audio_set_device(udev);
+			exynos_usb_audio_hcd(udev);
+			exynos_usb_audio_conn(udev, 1);
+			exynos_usb_audio_desc(udev);
+		} else {
+			return -EPERM;
+		}
+	}
+
+	return 0;
+}
+
+static void exynos_usb_audio_disconn(struct usb_interface *intf)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+
+	exynos_usb_audio_conn(udev, 0);
+}
+
+/* clock */
+static int exynos_usb_audio_set_interface(struct usb_device *udev,
+		struct usb_host_interface *alts, int iface, int alt)
+{
+	unsigned char ep;
+	unsigned char numEndpoints;
+	int direction;
+	int i;
+	int ret = 0;
+
+	if (alts != NULL) {
+		numEndpoints = get_iface_desc(alts)->bNumEndpoints;
+		if (numEndpoints < 1)
+			return -22;
+		if (numEndpoints == 1)
+			ep = get_endpoint(alts, 0)->bEndpointAddress;
+		else {
+			for (i = 0; i < numEndpoints; i++) {
+				ep = get_endpoint(alts, i)->bmAttributes;
+				if (!(ep & 0x30)) {
+					ep = get_endpoint(alts, i)->bEndpointAddress;
+					break;
+				}
+			}
+		}
+		if (ep & USB_DIR_IN)
+			direction = SNDRV_PCM_STREAM_CAPTURE;
+		else
+			direction = SNDRV_PCM_STREAM_PLAYBACK;
+
+		ret = exynos_usb_audio_setintf(udev, iface, alt, direction);
+	}
+
+	return ret;
+}
+
+/* pcm */
+static int exynos_usb_audio_set_rate(int iface, int rate, int alt)
+{
+	int ret;
+
+	ret = exynos_usb_audio_setrate(iface, rate, alt);
+
+	return ret;
+}
+
+static int exynos_usb_audio_set_pcmbuf(struct usb_device *dev, int iface)
+{
+	int ret;
+
+	ret = exynos_usb_audio_pcmbuf(dev, iface);
+
+	return ret;
+}
+
+static int exynos_usb_audio_set_pcm_intf(struct usb_interface *intf,
+					int iface, int alt, int direction)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	int ret;
+
+	ret = exynos_usb_audio_setintf(udev, iface, alt, direction);
+
+	return ret;
+}
+
+static int exynos_usb_audio_pcm_control(struct usb_device *udev,
+			enum snd_vendor_pcm_open_close onoff, int direction)
+{
+	int ret = 0;
+
+	if (onoff == 1) {
+		ret = exynos_usb_audio_pcm(1, direction);
+	} else if (onoff == 0) {
+		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
+			reinit_completion(&usb_audio->out_conn_stop);
+		else if (direction == SNDRV_PCM_STREAM_CAPTURE)
+			reinit_completion(&usb_audio->in_conn_stop);
+
+		if (!usb_audio->pcm_open_done)
+			return 0;
+
+		ret = exynos_usb_audio_pcm(0, direction);
+	}
+
+	return ret;
+}
+
+static int exynos_usb_audio_add_control(struct snd_usb_audio *chip)
+{
+	int ret;
+
+	if (chip != NULL)
+		ret = exynos_usb_add_ctls(chip, 0);
+	else
+		ret = usb_audio->user_scenario;
+
+	return ret;
+}
+
+static int exynos_usb_audio_set_pcm_binterval(const struct audioformat *fp,
+				 const struct audioformat *found,
+				 int *cur_attr, int *attr)
+{
+	if (usb_audio->user_scenario >= AUDIO_MODE_IN_CALL) {
+		if (fp->datainterval < found->datainterval) {
+			found = fp;
+			cur_attr = attr;
+		}
+	} else {
+		if (fp->datainterval > found->datainterval) {
+			found = fp;
+			cur_attr = attr;
+		}
+	}
+
+	return 0;
+}
+
+/* Set interface function */
+static struct snd_usb_audio_vendor_ops exynos_usb_audio_ops = {
+	/* card */
+	.connect = exynos_usb_audio_connect,
+	.disconnect = exynos_usb_audio_disconn,
+	/* clock */
+	.set_interface = exynos_usb_audio_set_interface,
+	/* pcm */
+	.set_rate = exynos_usb_audio_set_rate,
+	.set_pcm_buf = exynos_usb_audio_set_pcmbuf,
+	.set_pcm_intf = exynos_usb_audio_set_pcm_intf,
+	.set_pcm_connection = exynos_usb_audio_pcm_control,
+	.set_pcm_binterval = exynos_usb_audio_set_pcm_binterval,
+	.usb_add_ctls = exynos_usb_audio_add_control,
+};
+
+int exynos_usb_audio_exit(void)
+{
+	/* future use */
+	return 0;
+}
+EXPORT_SYMBOL_GPL(exynos_usb_audio_exit);
+
+MODULE_AUTHOR("Jaehun Jung <jh0801.jung@samsung.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Exynos USB Audio offloading driver");
+
diff --git a/sound/usb/exynos_usb_audio.h b/sound/usb/exynos_usb_audio.h
new file mode 100644
index 0000000..13707744
--- /dev/null
+++ b/sound/usb/exynos_usb_audio.h
@@ -0,0 +1,150 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*   USB Audio Driver for Exynos
+ *
+ *   Copyright (c) 2017 by Kyounghye Yun <k-hye.yun@samsung.com>
+ *
+ */
+
+#ifndef __LINUX_USB_EXYNOS_AUDIO_H
+#define __LINUX_USB_EXYNOS_AUDIO_H
+
+#include "../../../sound/usb/usbaudio.h"
+
+/* for KM */
+
+#define USB_AUDIO_MEM_BASE	0xC0000000
+
+#define USB_AUDIO_SAVE_RESTORE	(USB_AUDIO_MEM_BASE)
+#define USB_AUDIO_DEV_CTX	(USB_AUDIO_SAVE_RESTORE+PAGE_SIZE)
+#define USB_AUDIO_INPUT_CTX	(USB_AUDIO_DEV_CTX+PAGE_SIZE)
+#define USB_AUDIO_OUT_DEQ	(USB_AUDIO_INPUT_CTX+PAGE_SIZE)
+#define USB_AUDIO_IN_DEQ	(USB_AUDIO_OUT_DEQ+PAGE_SIZE)
+#define USB_AUDIO_FBOUT_DEQ	(USB_AUDIO_IN_DEQ+PAGE_SIZE)
+#define USB_AUDIO_FBIN_DEQ	(USB_AUDIO_FBOUT_DEQ+PAGE_SIZE)
+#define USB_AUDIO_ERST		(USB_AUDIO_FBIN_DEQ+PAGE_SIZE)
+#define USB_AUDIO_DESC		(USB_AUDIO_ERST+PAGE_SIZE)
+#define USB_AUDIO_PCM_OUTBUF	(USB_AUDIO_MEM_BASE+0x100000)
+#define USB_AUDIO_PCM_INBUF	(USB_AUDIO_MEM_BASE+0x800000)
+
+#if defined(CONFIG_SOC_S5E9815)
+#define USB_AUDIO_XHCI_BASE	0x12210000
+#define USB_URAM_BASE		0x122a0000
+#define USB_URAM_SIZE		(SZ_1K * 24)
+#elif defined(CONFIG_SOC_S5E9935)
+#define USB_AUDIO_XHCI_BASE	0x10B00000
+#define USB_URAM_BASE		0x02a00000
+#define USB_URAM_SIZE		(SZ_1K * 24)
+#else
+#error
+#endif
+
+#define USB_AUDIO_CONNECT		(1 << 0)
+#define USB_AUDIO_REMOVING		(1 << 1)
+#define USB_AUDIO_DISCONNECT		(1 << 2)
+#define USB_AUDIO_TIMEOUT_PROBE	(1 << 3)
+
+#define DISCONNECT_TIMEOUT	(500)
+
+#define AUDIO_MODE_NORMAL		0
+#define AUDIO_MODE_RINGTONE		1
+#define AUDIO_MODE_IN_CALL		2
+#define AUDIO_MODE_IN_COMMUNICATION	3
+#define AUDIO_MODE_CALL_SCREEN		4
+
+#define	CALL_INTERVAL_THRESHOLD		3
+
+#define USB_AUDIO_CONNECT		(1 << 0)
+#define USB_AUDIO_REMOVING		(1 << 1)
+#define USB_AUDIO_DISCONNECT		(1 << 2)
+#define USB_AUDIO_TIMEOUT_PROBE	(1 << 3)
+
+#define DISCONNECT_TIMEOUT	(500)
+
+struct host_data {
+	dma_addr_t out_data_dma;
+	dma_addr_t in_data_dma;
+	void *out_data_addr;
+	void *in_data_addr;
+};
+
+extern struct host_data xhci_data;
+
+struct exynos_usb_audio {
+	struct usb_device *udev;
+	struct platform_device *abox;
+	struct platform_device *hcd_pdev;
+	struct mutex    lock;
+	struct work_struct usb_work;
+	struct completion in_conn_stop;
+	struct completion out_conn_stop;
+	struct completion discon_done;
+
+	u64 out_buf_addr;
+	u64 in_buf_addr;
+	u64 pcm_offset;
+	u64 desc_addr;
+	u64 offset;
+
+	/* for hw_info */
+	u64 dcbaa_dma;
+	u64 in_ctx;
+	u64 out_ctx;
+	u64 erst_addr;
+
+	int speed;
+	/* 1: in, 0: out */
+	int set_ep;
+	int is_audio;
+	int is_first_probe;
+	u8 indeq_map_done;
+	u8 outdeq_map_done;
+	u8 fb_indeq_map_done;
+	u8 fb_outdeq_map_done;
+	u32 pcm_open_done;
+	u32 usb_audio_state;
+
+	struct snd_kcontrol *kctl;
+	u32 user_scenario;
+
+	void *pcm_buf;
+	u64 save_dma;
+
+	bool use_uram;
+};
+
+struct hcd_hw_info {
+	/* for XHCI */
+	int slot_id;
+	dma_addr_t erst_addr;
+	dma_addr_t dcbaa_dma;
+	dma_addr_t in_ctx;
+	dma_addr_t out_ctx;
+	dma_addr_t save_dma;
+	u64 cmd_ring;
+	/* Data Stream EP */
+	u64 old_out_deq;
+	u64 old_in_deq;
+	u64 out_deq;
+	u64 in_deq;
+	int in_ep;
+	int out_ep;
+	/* feedback ep */
+	int fb_in_ep;
+	int fb_out_ep;
+	u64 fb_old_out_deq;
+	u64 fb_old_in_deq;
+	u64 fb_out_deq;
+	u64 fb_in_deq;
+	/* Device Common Information */
+	int speed;
+	void *out_buf;
+	u64 out_dma;
+	void *in_buf;
+	u64 in_dma;
+	int use_uram;
+	int rawdesc_length;
+};
+
+int exynos_usb_audio_init(struct device *dev, struct platform_device *pdev);
+int exynos_usb_audio_exit(void);
+#endif /* __LINUX_USB_EXYNOS_AUDIO_H */
-- 
2.7.4

Re: [PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Greg Kroah-Hartman]

On Thu, Mar 24, 2022 at 05:10:42PM +0900, Oh Eomji wrote:
> In mobile, a co-processor can be used with USB audio to improve power
> consumption.  To support this type of hardware, hooks need to be added
> to the USB audio subsystem to be able to call into the hardware when
> needed.
> 
> The main operation of the call-backs are:
>   - Initialize the co-processor by transmitting data when initializing.
>   - Change the co-processor setting value through the interface
>     function.
>   - Configure sampling rate
>   - pcm open/close
>   - other housekeeping
> 
> Known issues:
>   - This only supports one set of callback hooks, meaning that this only
>     works if there is one type of USB controller in the system.  This
>     should be changed to be a per-host-controller interface instead of
>     one global set of callbacks.

Sorry, but this limitation alone means that this is not going to be able
to be accepted.  Almost all real systems have multiple USB controllers
in the system and so, this will break in very bad ways on the majority
of devices in the world.

Please fix this up and make this per-USB-controller, as was requested
the last time this series was published.

thanks,

greg k-h

Re: [PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Greg Kroah-Hartman]

On Thu, Mar 24, 2022 at 05:10:42PM +0900, Oh Eomji wrote:
> In mobile, a co-processor can be used with USB audio to improve power
> consumption.  To support this type of hardware, hooks need to be added
> to the USB audio subsystem to be able to call into the hardware when
> needed.
> 
> The main operation of the call-backs are:
>   - Initialize the co-processor by transmitting data when initializing.
>   - Change the co-processor setting value through the interface
>     function.
>   - Configure sampling rate
>   - pcm open/close
>   - other housekeeping
> 
> Known issues:
>   - This only supports one set of callback hooks, meaning that this only
>     works if there is one type of USB controller in the system.  This
>     should be changed to be a per-host-controller interface instead of
>     one global set of callbacks.
> 
> Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>
> Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> ---
>  sound/usb/card.c     | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/card.h     |  20 +++++++++
>  sound/usb/usbaudio.h |  45 +++++++++++++++++++
>  3 files changed, 184 insertions(+)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 3769622..bd59311 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -117,6 +117,117 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no)
>  static DEFINE_MUTEX(register_mutex);
>  static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
>  static struct usb_driver usb_audio_driver;
> +static struct snd_usb_audio_vendor_ops *usb_vendor_ops;
> +
> +int snd_vendor_set_ops(struct snd_usb_audio_vendor_ops *ops)
> +{
> +	if ((!ops->connect) ||
> +	    (!ops->disconnect) ||
> +	    (!ops->set_interface) ||
> +	    (!ops->set_rate) ||
> +	    (!ops->set_pcm_buf) ||
> +	    (!ops->set_pcm_intf) ||
> +	    (!ops->set_pcm_connection) ||
> +	    (!ops->set_pcm_binterval) ||
> +	    (!ops->usb_add_ctls))
> +		return -EINVAL;
> +
> +	usb_vendor_ops = ops;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_vendor_set_ops);
> +
> +struct snd_usb_audio_vendor_ops *snd_vendor_get_ops(void)
> +{
> +	return usb_vendor_ops;
> +}

This is the function you need to fix up, and add proper reference
counting to, in order to solve your "this breaks with multiple USB
controllers" problem.  So this really should not be all that difficult
of a task.  Why has it taken years to do so?

thanks,

greg k-h

Re: [PATCH v1 2/3] sound: usb: Calling vendor's call-back function within usb audio operation.

[Greg Kroah-Hartman]

On Thu, Mar 24, 2022 at 05:10:43PM +0900, Oh Eomji wrote:
> When a new interface is connected or removed, the call-back functions
> are called to transmit a command to the hardware.
> 
> Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> ---
>  sound/usb/pcm.c    | 37 +++++++++++++++++++++++++++++++++++++
>  sound/usb/stream.c |  2 ++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index cec6e91a..4bae4ba 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -144,6 +144,8 @@ find_format(struct list_head *fmt_list_head, snd_pcm_format_t format,
>  			found = fp;
>  			cur_attr = attr;
>  		}
> +
> +		snd_vendor_set_pcm_binterval(fp, found, &cur_attr, &attr);
>  	}
>  	return found;
>  }
> @@ -434,6 +436,7 @@ static int configure_endpoints(struct snd_usb_audio *chip,
>  			       struct snd_usb_substream *subs)
>  {
>  	int err;
> +	struct usb_interface *iface;
>  
>  	if (subs->data_endpoint->need_setup) {
>  		/* stop any running stream beforehand */
> @@ -442,6 +445,13 @@ static int configure_endpoints(struct snd_usb_audio *chip,
>  		err = snd_usb_endpoint_configure(chip, subs->data_endpoint);
>  		if (err < 0)
>  			return err;
> +
> +		iface = usb_ifnum_to_if(chip->dev, subs->data_endpoint->iface);
> +		err = snd_vendor_set_pcm_intf(iface, subs->data_endpoint->iface,
> +				subs->data_endpoint->altsetting, subs->direction);
> +		if (err < 0)
> +			return err;
> +
>  		snd_usb_set_format_quirk(subs, subs->cur_audiofmt);
>  	}
>  
> @@ -616,8 +626,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct snd_usb_substream *subs = runtime->private_data;
>  	struct snd_usb_audio *chip = subs->stream->chip;
> +	struct snd_usb_endpoint *ep = subs->data_endpoint;
>  	int ret;
>  
> +	ret = snd_vendor_set_pcm_buf(subs->dev, subs->cur_audiofmt->iface);
> +	if (ret)
> +		return ret;
> +
> +	if (!subs->cur_audiofmt) {
> +		dev_err(&subs->dev->dev, "no format is specified\n");
> +		return -ENXIO;
> +	}
> +
>  	ret = snd_usb_lock_shutdown(chip);
>  	if (ret < 0)
>  		return ret;
> @@ -630,6 +650,13 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>  	if (ret < 0)
>  		goto unlock;
>  
> +	if (snd_vendor_get_ops()) {
> +		ret = snd_vendor_set_rate(ep->cur_audiofmt->iface,
> +				ep->cur_rate, ep->cur_audiofmt->altsetting);
> +		if (!ret)
> +			goto unlock;
> +	}
> +
>  	/* reset the pointer */
>  	subs->buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size);
>  	subs->inflight_bytes = 0;
> @@ -1104,6 +1131,11 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>  	struct snd_usb_substream *subs = &as->substream[direction];
>  	int ret;
>  
> +	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_OPEN,
> +					    direction);
> +	if (ret)
> +		return ret;
> +
>  	runtime->hw = snd_usb_hardware;
>  	/* need an explicit sync to catch applptr update in low-latency mode */
>  	if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
> @@ -1137,6 +1169,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>  	struct snd_usb_substream *subs = &as->substream[direction];
>  	int ret;
>  
> +	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_CLOSE,
> +					    direction);
> +	if (ret)
> +		return ret;
> +
>  	snd_media_stop_pipeline(subs);
>  
>  	if (!snd_usb_lock_shutdown(subs->stream->chip)) {
> diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> index ceb93d7..26ca696 100644
> --- a/sound/usb/stream.c
> +++ b/sound/usb/stream.c
> @@ -1227,6 +1227,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
>  		snd_usb_init_pitch(chip, fp);
>  		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
>  		usb_set_interface(chip->dev, iface_no, altno);
> +		if (protocol > UAC_VERSION_1)

Why the protocol check?  That's not documented in your changelog
anywhere :(

Re: [PATCH v1 3/3] sound: usb: Exynos usb audio offloading driver

[Greg Kroah-Hartman]

On Thu, Mar 24, 2022 at 05:10:44PM +0900, Oh Eomji wrote:
> This is for usb audio offloading. usb audio offloading processes usb
> audio stream data directly from the audio box even if ap usb enters
> suspend, there is no problem in stream data transmission. This obtains
> power saving effect while using usb audio device.
> 
> AP usb and audio usb f/w communicate via AUDIO IPC. By performing AUDIO
> IPC in the vendor operations, abox can access and control the xhci to
> transmit the data directly.
> 
> The types of commands and data delivered through AUDIO IPC include the
> following (AP USB <-> AUDIO USB f/w) :
> 1. usb audio connection/disconnection status
> 2. xhci memory information
> 3. full descriptor for usb audio device
> 4. UAC(usb audio class) control command
> 
> Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> ---
>  sound/usb/Kconfig            |   9 +
>  sound/usb/Makefile           |   2 +
>  sound/usb/exynos_usb_audio.c | 560 +++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/exynos_usb_audio.h | 150 ++++++++++++
>  4 files changed, 721 insertions(+)
>  create mode 100644 sound/usb/exynos_usb_audio.c
>  create mode 100644 sound/usb/exynos_usb_audio.h
> 
> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> index 059242f..70252a3 100644
> --- a/sound/usb/Kconfig
> +++ b/sound/usb/Kconfig
> @@ -27,6 +27,15 @@ config SND_USB_AUDIO
>  config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
>  	bool
>  
> +config SND_EXYNOS_USB_AUDIO
> +	tristate "EXYNOS USB Audio offloading module"
> +	depends on SND_USB_AUDIO
> +	help
> +	 Say Y here to include support for Exynos USB Audio ABOX offloading.
> +
> +	 To compile this driver as a module, choose M here: the module
> +	 will be called exynos-usb-audio-offloading.
> +
>  config SND_USB_UA101
>  	tristate "Edirol UA-101/UA-1000 driver"
>  	select SND_PCM
> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
> index 9ccb21a..bf019c7 100644
> --- a/sound/usb/Makefile
> +++ b/sound/usb/Makefile
> @@ -28,6 +28,8 @@ snd-usbmidi-lib-objs := midi.o
>  
>  # Toplevel Module Dependency
>  obj-$(CONFIG_SND_USB_AUDIO) += snd-usb-audio.o snd-usbmidi-lib.o
> +obj-$(CONFIG_SND_EXYNOS_USB_AUDIO) += exynos-usb-audio-offloading.o
> +exynos-usb-audio-offloading-y += exynos_usb_audio.o
>  
>  obj-$(CONFIG_SND_USB_UA101) += snd-usbmidi-lib.o
>  obj-$(CONFIG_SND_USB_USX2Y) += snd-usbmidi-lib.o
> diff --git a/sound/usb/exynos_usb_audio.c b/sound/usb/exynos_usb_audio.c
> new file mode 100644
> index 0000000..456cc78
> --- /dev/null
> +++ b/sound/usb/exynos_usb_audio.c
> @@ -0,0 +1,560 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

Are you sure about this license?  I have to ask, sorry.

> +/*
> + *   USB Audio offloading Driver for Exynos
> + *
> + *   Copyright (c) 2017 by Kyounghye Yun <k-hye.yun@samsung.com>
> + *
> + */
> +
> +
> +#include <linux/bitops.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/ctype.h>
> +#include <linux/usb.h>
> +#include <linux/usb/audio.h>
> +#include <linux/usb/audio-v2.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/completion.h>
> +
> +#include <sound/pcm.h>
> +#include "../../../sound/usb/exynos_usb_audio.h"

Did this patch just break the build?  It should not need this type of
.../../../ mess.

> +#include "usbaudio.h"
> +#include "helper.h"
> +#include "card.h"
> +#include "quirks.h"
> +
> +static struct exynos_usb_audio *usb_audio;
> +static struct snd_usb_audio_vendor_ops exynos_usb_audio_ops;
> +
> +struct hcd_hw_info *g_hwinfo;
> +EXPORT_SYMBOL_GPL(g_hwinfo);
> +int otg_connection;
> +EXPORT_SYMBOL_GPL(otg_connection);
> +int usb_audio_connection;
> +EXPORT_SYMBOL_GPL(usb_audio_connection);

Why are these exported?  Who uses them?  And why generic names for a
driver-specific variable?  And what do they do and how are they
accessed?

No one in this patch series needs these exports, so why are they
exported?

> +
> +static void exynos_usb_audio_set_device(struct usb_device *udev)
> +{
> +	usb_audio->udev = udev;
> +	usb_audio->is_audio = 1;

boolean?

How do you know?  Did you check?

And why a single static variable?

> +}
> +
> +static int exynos_usb_audio_unmap_all(void)
> +{
> +	/*
> +	 * TODO: unmapping pcm buffer, usb descriptor, Device Context,
> +	 * Input Context, ERST, URAM.
> +	 */

If this is not completed, just don't have the function at all.

Why isn't this done?

> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_pcmbuf(struct usb_device *udev, int iface)
> +{
> +	if (!usb_audio->is_audio || !otg_connection)
> +		return -1;

Do not make up error values.  Use correct ones.


> +
> +	/*
> +	 * TODO: Transmit the DRAM address that contains the xhci device
> +	 * information,and the DMA address required for operation to the abox
> +	 * usb f/w.
> +	 */

So this function does nothing?

Does this driver even work properly?  Where is the  missing logic?

> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_setrate(int iface, int rate, int alt)
> +{
> +	if (!usb_audio->is_audio || !otg_connection)
> +		return -1;
> +
> +	/*
> +	 * TODO: Notify the abox usb f/w the audio sample rate supported by
> +	 * the interface of the connected audio device.
> +	 */

Same as above.

Does this driver actually work on real hardware?  Or is this just a
"fake" driver that is never intended for anyone to use just to allow the
hooks to be added to the kernel tree?

What hardware has this driver worked on?  Can I take the pixel6 device
today and replace their out-of-tree driver with this one and will it
work properly?  If not, what is missing?

> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_setintf(struct usb_device *udev, int iface, int alt, int direction)
> +{
> +	struct hcd_hw_info *hwinfo = g_hwinfo;
> +	u64 in_offset, out_offset;
> +
> +	if (!usb_audio->pcm_open_done)
> +		return -EPERM;

How is that a permission error?

> +
> +	if (!usb_audio->is_audio || !otg_connection)
> +		return -1;

Again, no fake error numbers.

> +
> +	if (direction) {
> +		/* IN EP */
> +		if (!usb_audio->indeq_map_done ||
> +			(hwinfo->in_deq != hwinfo->old_in_deq)) {
> +			/* TODO: Transmit pcm interface number, alt setting
> +			 * number to abox usb f/w
> +			 */

Fix all TODO please before resubmitting.

> +			usb_audio->indeq_map_done = 1;
> +			in_offset = hwinfo->in_deq % PAGE_SIZE;

Why does PAGE_SIZE matter?

> +		}
> +
> +		if (hwinfo->fb_out_deq) {
> +			if (!usb_audio->fb_outdeq_map_done ||
> +					(hwinfo->fb_out_deq != hwinfo->fb_old_out_deq)) {
> +				/* TODO: Transmit pcm interface number,
> +				 * alt setting number to abox usb f/w
> +				 */
> +				usb_audio->fb_outdeq_map_done = 1;
> +				out_offset = hwinfo->fb_out_deq % PAGE_SIZE;
> +			}
> +		}
> +	} else {
> +		/* OUT EP */
> +		if (!usb_audio->outdeq_map_done ||
> +			(hwinfo->out_deq != hwinfo->old_out_deq)) {
> +			/* TODO: Transmit pcm interface number, alt setting
> +			 * number to abox usb f/w
> +			 */
> +			usb_audio->outdeq_map_done = 1;
> +			out_offset = hwinfo->out_deq % PAGE_SIZE;
> +		}
> +
> +		if (hwinfo->fb_in_deq) {
> +			if (!usb_audio->fb_indeq_map_done ||
> +					(hwinfo->fb_in_deq != hwinfo->fb_old_in_deq)) {
> +				/* TODO: Transmit pcm interface number,
> +				 * alt setting number to abox usb f/w
> +				 */
> +				usb_audio->fb_indeq_map_done = 1;
> +				in_offset = hwinfo->fb_in_deq % PAGE_SIZE;
> +			}
> +		}
> +	}
> +
> +	/* one more check connection to prevent kernel panic */
> +	if (!usb_audio->is_audio || !otg_connection)
> +		return -1;
> +
> +	/* TODO: Notify abox usb f/w a dequeue pointer */
> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_hcd(struct usb_device *udev)
> +{
> +	struct hcd_hw_info *hwinfo = g_hwinfo;
> +
> +	/* back up each address for unmap */
> +	usb_audio->dcbaa_dma = hwinfo->dcbaa_dma;
> +	usb_audio->save_dma = hwinfo->save_dma;
> +	usb_audio->in_ctx = hwinfo->in_ctx;
> +	usb_audio->out_ctx = hwinfo->out_ctx;
> +	usb_audio->erst_addr = hwinfo->erst_addr;
> +	usb_audio->speed = hwinfo->speed;
> +	usb_audio->use_uram = hwinfo->use_uram;
> +
> +	/*
> +	 * TODO: DCBAA, Device Context, Input Context, URAM, ERST mapping
> +	 * and notify abox usb f/w the address about xhci h/w resource to
> +	 * directly control the xhci in abox.
> +	 */
> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_desc(struct usb_device *udev)
> +{
> +	int configuration, cfgno, i;
> +	unsigned char *buffer;
> +	u64 desc_addr;
> +	u64 offset;
> +
> +	configuration = usb_choose_configuration(udev);
> +
> +	cfgno = -1;
> +	for (i = 0; i < udev->descriptor.bNumConfigurations; i++) {
> +		if (udev->config[i].desc.bConfigurationValue ==
> +				configuration) {
> +			cfgno = i;
> +			break;
> +		}
> +	}
> +
> +	if (cfgno == -1)
> +		cfgno = 0;
> +
> +	/* need to memory mapping for usb descriptor */
> +	buffer = udev->rawdescriptors[cfgno];
> +	desc_addr = virt_to_phys(buffer);
> +	offset = desc_addr % PAGE_SIZE;
> +
> +	/* store address information */
> +	usb_audio->desc_addr = desc_addr;
> +	usb_audio->offset = offset;
> +
> +	desc_addr -= offset;
> +
> +	/*
> +	 * TODO: Notify the abox usb f/w that all descriptors have been recived
> +	 * from the connected usb audio device, and that copy and memory mapping
> +	 * have beed completed so that it can be used in abox usb f/w
> +	 */
> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_conn(struct usb_device *udev, int is_conn)
> +{
> +
> +	/* TODO: Notify abox usb f/w whether usb device is connected or not */
> +	if (!is_conn) {
> +		if (usb_audio->is_audio) {
> +			usb_audio->is_audio = 0;
> +			usb_audio->usb_audio_state = USB_AUDIO_REMOVING;
> +		}
> +	} else {
> +		cancel_work_sync(&usb_audio->usb_work);
> +		usb_audio->indeq_map_done = 0;
> +		usb_audio->outdeq_map_done = 0;
> +		usb_audio->fb_indeq_map_done = 0;
> +		usb_audio->fb_outdeq_map_done = 0;
> +		usb_audio->pcm_open_done = 0;
> +		reinit_completion(&usb_audio->discon_done);
> +		usb_audio->usb_audio_state = USB_AUDIO_CONNECT;
> +		usb_audio_connection = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int exynos_usb_audio_pcm(bool is_open, bool direction)
> +{
> +	if (!usb_audio->is_audio || !otg_connection)
> +		return -1;
> +
> +	if (is_open)
> +		usb_audio->pcm_open_done = 1;
> +
> +	/* TODO: Notify the abox usb f/w the pcm open/close status */
> +
> +	return 0;
> +}
> +
> +static void exynos_usb_audio_work(struct work_struct *w)
> +{
> +	/* Don't unmap in USB_AUDIO_TIMEOUT_PROBE state */
> +	if (usb_audio->usb_audio_state != USB_AUDIO_REMOVING)
> +		return;
> +
> +	exynos_usb_audio_unmap_all();
> +	usb_audio->usb_audio_state = USB_AUDIO_DISCONNECT;
> +	usb_audio_connection = 0;
> +	complete(&usb_audio->discon_done);
> +}
> +
> +static int exynos_usb_scenario_ctl_info(struct snd_kcontrol *kcontrol,
> +			      struct snd_ctl_elem_info *uinfo)
> +{
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +	uinfo->count = 1;
> +	uinfo->value.integer.min = AUDIO_MODE_NORMAL;
> +	uinfo->value.integer.max = AUDIO_MODE_CALL_SCREEN;
> +	return 0;
> +}
> +
> +static int exynos_usb_scenario_ctl_get(struct snd_kcontrol *kcontrol,
> +			 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct exynos_usb_audio *usb = snd_kcontrol_chip(kcontrol);
> +
> +	ucontrol->value.integer.value[0] = usb->user_scenario;
> +	return 0;
> +}
> +
> +static int exynos_usb_scenario_ctl_put(struct snd_kcontrol *kcontrol,
> +			 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct exynos_usb_audio *usb = snd_kcontrol_chip(kcontrol);
> +	int changed = 0;
> +
> +	if (usb->user_scenario !=
> +	     ucontrol->value.integer.value[0]) {
> +		usb->user_scenario = ucontrol->value.integer.value[0];
> +		changed = 1;
> +	}
> +
> +	return changed;
> +}
> +
> +static int exynos_usb_add_ctls(struct snd_usb_audio *chip,
> +				unsigned long private_value)
> +{
> +	struct snd_kcontrol_new knew = {
> +		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
> +		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> +		.name = "USB Audio Mode",
> +		.info = exynos_usb_scenario_ctl_info,
> +		.get = exynos_usb_scenario_ctl_get,
> +		.put = exynos_usb_scenario_ctl_put,
> +	};
> +
> +	int err;
> +
> +	if (!chip)
> +		return -ENODEV;
> +
> +	knew.private_value = private_value;
> +	usb_audio->kctl = snd_ctl_new1(&knew, usb_audio);
> +	if (!usb_audio->kctl)
> +		return -ENOMEM;
> +
> +	err = snd_ctl_add(chip->card, usb_audio->kctl);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +
> +int exynos_usb_audio_init(struct device *dev, struct platform_device *pdev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct device_node *np_abox;
> +	struct platform_device *pdev_abox;
> +	int ret = 0;
> +
> +	if (!usb_audio) {
> +		usb_audio = kmalloc(sizeof(struct exynos_usb_audio), GFP_KERNEL);
> +		if (!usb_audio)
> +			return -ENOMEM;
> +	}
> +
> +	np_abox = of_parse_phandle(np, "abox", 0);
> +	if (!np_abox)
> +		return -EPROBE_DEFER;
> +
> +	pdev_abox = of_find_device_by_node(np_abox);
> +	if (!pdev_abox)
> +		return -EPROBE_DEFER;
> +
> +	init_completion(&usb_audio->in_conn_stop);
> +	init_completion(&usb_audio->out_conn_stop);
> +	init_completion(&usb_audio->discon_done);
> +	usb_audio->abox = pdev_abox;
> +	usb_audio->hcd_pdev = pdev;
> +	usb_audio->udev = NULL;
> +	usb_audio->is_audio = 0;
> +	usb_audio->is_first_probe = 1;
> +	usb_audio->user_scenario = AUDIO_MODE_NORMAL;
> +	usb_audio->usb_audio_state = USB_AUDIO_DISCONNECT;
> +	usb_audio_connection = 0;
> +
> +	INIT_WORK(&usb_audio->usb_work, exynos_usb_audio_work);
> +
> +	/* interface function mapping */
> +	ret = snd_vendor_set_ops(&exynos_usb_audio_ops);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(exynos_usb_audio_init);

Why are you exporting functions that are never called?


> +
> +/* card */
> +static int exynos_usb_audio_connect(struct usb_interface *intf)
> +{
> +	struct usb_interface_descriptor *altsd;
> +	struct usb_host_interface *alts;
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	int timeout = 0;
> +
> +	alts = &intf->altsetting[0];
> +	altsd = get_iface_desc(alts);
> +
> +	if ((altsd->bInterfaceClass == USB_CLASS_AUDIO ||
> +		altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC) &&
> +		altsd->bInterfaceSubClass == USB_SUBCLASS_MIDISTREAMING) {
> +	} else {
> +		if (usb_audio->usb_audio_state == USB_AUDIO_REMOVING) {
> +			timeout = wait_for_completion_timeout(
> +				&usb_audio->discon_done,
> +				msecs_to_jiffies(DISCONNECT_TIMEOUT));
> +
> +			if ((usb_audio->usb_audio_state == USB_AUDIO_REMOVING)
> +					&& !timeout) {
> +				usb_audio->usb_audio_state =
> +					USB_AUDIO_TIMEOUT_PROBE;
> +			}
> +		}
> +
> +		if ((usb_audio->usb_audio_state == USB_AUDIO_DISCONNECT)
> +			|| (usb_audio->usb_audio_state == USB_AUDIO_TIMEOUT_PROBE)) {
> +			exynos_usb_audio_set_device(udev);
> +			exynos_usb_audio_hcd(udev);
> +			exynos_usb_audio_conn(udev, 1);
> +			exynos_usb_audio_desc(udev);
> +		} else {
> +			return -EPERM;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void exynos_usb_audio_disconn(struct usb_interface *intf)
> +{
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +
> +	exynos_usb_audio_conn(udev, 0);
> +}
> +
> +/* clock */
> +static int exynos_usb_audio_set_interface(struct usb_device *udev,
> +		struct usb_host_interface *alts, int iface, int alt)
> +{
> +	unsigned char ep;
> +	unsigned char numEndpoints;
> +	int direction;
> +	int i;
> +	int ret = 0;
> +
> +	if (alts != NULL) {
> +		numEndpoints = get_iface_desc(alts)->bNumEndpoints;
> +		if (numEndpoints < 1)
> +			return -22;
> +		if (numEndpoints == 1)
> +			ep = get_endpoint(alts, 0)->bEndpointAddress;
> +		else {
> +			for (i = 0; i < numEndpoints; i++) {
> +				ep = get_endpoint(alts, i)->bmAttributes;
> +				if (!(ep & 0x30)) {
> +					ep = get_endpoint(alts, i)->bEndpointAddress;
> +					break;
> +				}
> +			}
> +		}
> +		if (ep & USB_DIR_IN)
> +			direction = SNDRV_PCM_STREAM_CAPTURE;
> +		else
> +			direction = SNDRV_PCM_STREAM_PLAYBACK;
> +
> +		ret = exynos_usb_audio_setintf(udev, iface, alt, direction);
> +	}
> +
> +	return ret;
> +}
> +
> +/* pcm */
> +static int exynos_usb_audio_set_rate(int iface, int rate, int alt)
> +{
> +	int ret;
> +
> +	ret = exynos_usb_audio_setrate(iface, rate, alt);
> +
> +	return ret;
> +}
> +
> +static int exynos_usb_audio_set_pcmbuf(struct usb_device *dev, int iface)
> +{
> +	int ret;
> +
> +	ret = exynos_usb_audio_pcmbuf(dev, iface);
> +
> +	return ret;
> +}
> +
> +static int exynos_usb_audio_set_pcm_intf(struct usb_interface *intf,
> +					int iface, int alt, int direction)
> +{
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	int ret;
> +
> +	ret = exynos_usb_audio_setintf(udev, iface, alt, direction);
> +
> +	return ret;
> +}
> +
> +static int exynos_usb_audio_pcm_control(struct usb_device *udev,
> +			enum snd_vendor_pcm_open_close onoff, int direction)
> +{
> +	int ret = 0;
> +
> +	if (onoff == 1) {
> +		ret = exynos_usb_audio_pcm(1, direction);
> +	} else if (onoff == 0) {
> +		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
> +			reinit_completion(&usb_audio->out_conn_stop);
> +		else if (direction == SNDRV_PCM_STREAM_CAPTURE)
> +			reinit_completion(&usb_audio->in_conn_stop);
> +
> +		if (!usb_audio->pcm_open_done)
> +			return 0;
> +
> +		ret = exynos_usb_audio_pcm(0, direction);
> +	}
> +
> +	return ret;
> +}
> +
> +static int exynos_usb_audio_add_control(struct snd_usb_audio *chip)
> +{
> +	int ret;
> +
> +	if (chip != NULL)
> +		ret = exynos_usb_add_ctls(chip, 0);
> +	else
> +		ret = usb_audio->user_scenario;
> +
> +	return ret;
> +}
> +
> +static int exynos_usb_audio_set_pcm_binterval(const struct audioformat *fp,
> +				 const struct audioformat *found,
> +				 int *cur_attr, int *attr)
> +{
> +	if (usb_audio->user_scenario >= AUDIO_MODE_IN_CALL) {
> +		if (fp->datainterval < found->datainterval) {
> +			found = fp;
> +			cur_attr = attr;
> +		}
> +	} else {
> +		if (fp->datainterval > found->datainterval) {
> +			found = fp;
> +			cur_attr = attr;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/* Set interface function */
> +static struct snd_usb_audio_vendor_ops exynos_usb_audio_ops = {
> +	/* card */
> +	.connect = exynos_usb_audio_connect,
> +	.disconnect = exynos_usb_audio_disconn,
> +	/* clock */
> +	.set_interface = exynos_usb_audio_set_interface,
> +	/* pcm */
> +	.set_rate = exynos_usb_audio_set_rate,
> +	.set_pcm_buf = exynos_usb_audio_set_pcmbuf,
> +	.set_pcm_intf = exynos_usb_audio_set_pcm_intf,
> +	.set_pcm_connection = exynos_usb_audio_pcm_control,
> +	.set_pcm_binterval = exynos_usb_audio_set_pcm_binterval,
> +	.usb_add_ctls = exynos_usb_audio_add_control,
> +};
> +
> +int exynos_usb_audio_exit(void)
> +{
> +	/* future use */

What needs to be done here?

> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(exynos_usb_audio_exit);

Why did you export a function that does nothing?

> +
> +MODULE_AUTHOR("Jaehun Jung <jh0801.jung@samsung.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Exynos USB Audio offloading driver");
> +
> diff --git a/sound/usb/exynos_usb_audio.h b/sound/usb/exynos_usb_audio.h
> new file mode 100644
> index 0000000..13707744
> --- /dev/null
> +++ b/sound/usb/exynos_usb_audio.h
> @@ -0,0 +1,150 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*   USB Audio Driver for Exynos
> + *
> + *   Copyright (c) 2017 by Kyounghye Yun <k-hye.yun@samsung.com>
> + *
> + */
> +
> +#ifndef __LINUX_USB_EXYNOS_AUDIO_H
> +#define __LINUX_USB_EXYNOS_AUDIO_H
> +
> +#include "../../../sound/usb/usbaudio.h"
> +
> +/* for KM */
> +
> +#define USB_AUDIO_MEM_BASE	0xC0000000
> +
> +#define USB_AUDIO_SAVE_RESTORE	(USB_AUDIO_MEM_BASE)
> +#define USB_AUDIO_DEV_CTX	(USB_AUDIO_SAVE_RESTORE+PAGE_SIZE)
> +#define USB_AUDIO_INPUT_CTX	(USB_AUDIO_DEV_CTX+PAGE_SIZE)
> +#define USB_AUDIO_OUT_DEQ	(USB_AUDIO_INPUT_CTX+PAGE_SIZE)
> +#define USB_AUDIO_IN_DEQ	(USB_AUDIO_OUT_DEQ+PAGE_SIZE)
> +#define USB_AUDIO_FBOUT_DEQ	(USB_AUDIO_IN_DEQ+PAGE_SIZE)
> +#define USB_AUDIO_FBIN_DEQ	(USB_AUDIO_FBOUT_DEQ+PAGE_SIZE)
> +#define USB_AUDIO_ERST		(USB_AUDIO_FBIN_DEQ+PAGE_SIZE)
> +#define USB_AUDIO_DESC		(USB_AUDIO_ERST+PAGE_SIZE)
> +#define USB_AUDIO_PCM_OUTBUF	(USB_AUDIO_MEM_BASE+0x100000)
> +#define USB_AUDIO_PCM_INBUF	(USB_AUDIO_MEM_BASE+0x800000)
> +
> +#if defined(CONFIG_SOC_S5E9815)
> +#define USB_AUDIO_XHCI_BASE	0x12210000
> +#define USB_URAM_BASE		0x122a0000
> +#define USB_URAM_SIZE		(SZ_1K * 24)
> +#elif defined(CONFIG_SOC_S5E9935)
> +#define USB_AUDIO_XHCI_BASE	0x10B00000
> +#define USB_URAM_BASE		0x02a00000
> +#define USB_URAM_SIZE		(SZ_1K * 24)

Shouldn't this information come from a DT file?


> +#else
> +#error

Error what?

> +#endif
> +
> +#define USB_AUDIO_CONNECT		(1 << 0)
> +#define USB_AUDIO_REMOVING		(1 << 1)
> +#define USB_AUDIO_DISCONNECT		(1 << 2)
> +#define USB_AUDIO_TIMEOUT_PROBE	(1 << 3)

BIT()?

> +
> +#define DISCONNECT_TIMEOUT	(500)
> +
> +#define AUDIO_MODE_NORMAL		0
> +#define AUDIO_MODE_RINGTONE		1
> +#define AUDIO_MODE_IN_CALL		2
> +#define AUDIO_MODE_IN_COMMUNICATION	3
> +#define AUDIO_MODE_CALL_SCREEN		4
> +
> +#define	CALL_INTERVAL_THRESHOLD		3
> +
> +#define USB_AUDIO_CONNECT		(1 << 0)
> +#define USB_AUDIO_REMOVING		(1 << 1)
> +#define USB_AUDIO_DISCONNECT		(1 << 2)
> +#define USB_AUDIO_TIMEOUT_PROBE	(1 << 3)
> +
> +#define DISCONNECT_TIMEOUT	(500)

What units is this in?

> +
> +struct host_data {
> +	dma_addr_t out_data_dma;
> +	dma_addr_t in_data_dma;
> +	void *out_data_addr;
> +	void *in_data_addr;

Why void pointers?  io pointers?

Again, does this code even work?

confused,

greg k-h

Re: [PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Oh Eomji]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

On Thu, Mar 24, 2022 at 09:33:40AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 24, 2022 at 05:10:42PM +0900, Oh Eomji wrote:
> > In mobile, a co-processor can be used with USB audio to improve power
> > consumption.  To support this type of hardware, hooks need to be added
> > to the USB audio subsystem to be able to call into the hardware when
> > needed.
> > 
> > The main operation of the call-backs are:
> >   - Initialize the co-processor by transmitting data when initializing.
> >   - Change the co-processor setting value through the interface
> >     function.
> >   - Configure sampling rate
> >   - pcm open/close
> >   - other housekeeping
> > 
> > Known issues:
> >   - This only supports one set of callback hooks, meaning that this only
> >     works if there is one type of USB controller in the system.  This
> >     should be changed to be a per-host-controller interface instead of
> >     one global set of callbacks.
> > 
> > Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>
> > Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> > ---
> >  sound/usb/card.c     | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  sound/usb/card.h     |  20 +++++++++
> >  sound/usb/usbaudio.h |  45 +++++++++++++++++++
> >  3 files changed, 184 insertions(+)
> > 
> > diff --git a/sound/usb/card.c b/sound/usb/card.c
> > index 3769622..bd59311 100644
> > --- a/sound/usb/card.c
> > +++ b/sound/usb/card.c
> > @@ -117,6 +117,117 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no)
> >  static DEFINE_MUTEX(register_mutex);
> >  static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
> >  static struct usb_driver usb_audio_driver;
> > +static struct snd_usb_audio_vendor_ops *usb_vendor_ops;
> > +
> > +int snd_vendor_set_ops(struct snd_usb_audio_vendor_ops *ops)
> > +{
> > +	if ((!ops->connect) ||
> > +	    (!ops->disconnect) ||
> > +	    (!ops->set_interface) ||
> > +	    (!ops->set_rate) ||
> > +	    (!ops->set_pcm_buf) ||
> > +	    (!ops->set_pcm_intf) ||
> > +	    (!ops->set_pcm_connection) ||
> > +	    (!ops->set_pcm_binterval) ||
> > +	    (!ops->usb_add_ctls))
> > +		return -EINVAL;
> > +
> > +	usb_vendor_ops = ops;
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(snd_vendor_set_ops);
> > +
> > +struct snd_usb_audio_vendor_ops *snd_vendor_get_ops(void)
> > +{
> > +	return usb_vendor_ops;
> > +}
> 
> This is the function you need to fix up, and add proper reference
> counting to, in order to solve your "this breaks with multiple USB
> controllers" problem.  So this really should not be all that difficult
> of a task.  Why has it taken years to do so?
> 
> thanks,
> 
> greg k-h
>
Hi,

Is that mean the scenario when two or more usb audio devices are
connected through the hub?

Thanks,
Eomji Oh

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Greg Kroah-Hartman]

On Fri, Mar 25, 2022 at 03:44:13PM +0900, Oh Eomji wrote:
> On Thu, Mar 24, 2022 at 09:33:40AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Mar 24, 2022 at 05:10:42PM +0900, Oh Eomji wrote:
> > > In mobile, a co-processor can be used with USB audio to improve power
> > > consumption.  To support this type of hardware, hooks need to be added
> > > to the USB audio subsystem to be able to call into the hardware when
> > > needed.
> > > 
> > > The main operation of the call-backs are:
> > >   - Initialize the co-processor by transmitting data when initializing.
> > >   - Change the co-processor setting value through the interface
> > >     function.
> > >   - Configure sampling rate
> > >   - pcm open/close
> > >   - other housekeeping
> > > 
> > > Known issues:
> > >   - This only supports one set of callback hooks, meaning that this only
> > >     works if there is one type of USB controller in the system.  This
> > >     should be changed to be a per-host-controller interface instead of
> > >     one global set of callbacks.
> > > 
> > > Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>
> > > Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> > > ---
> > >  sound/usb/card.c     | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  sound/usb/card.h     |  20 +++++++++
> > >  sound/usb/usbaudio.h |  45 +++++++++++++++++++
> > >  3 files changed, 184 insertions(+)
> > > 
> > > diff --git a/sound/usb/card.c b/sound/usb/card.c
> > > index 3769622..bd59311 100644
> > > --- a/sound/usb/card.c
> > > +++ b/sound/usb/card.c
> > > @@ -117,6 +117,117 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no)
> > >  static DEFINE_MUTEX(register_mutex);
> > >  static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
> > >  static struct usb_driver usb_audio_driver;
> > > +static struct snd_usb_audio_vendor_ops *usb_vendor_ops;
> > > +
> > > +int snd_vendor_set_ops(struct snd_usb_audio_vendor_ops *ops)
> > > +{
> > > +	if ((!ops->connect) ||
> > > +	    (!ops->disconnect) ||
> > > +	    (!ops->set_interface) ||
> > > +	    (!ops->set_rate) ||
> > > +	    (!ops->set_pcm_buf) ||
> > > +	    (!ops->set_pcm_intf) ||
> > > +	    (!ops->set_pcm_connection) ||
> > > +	    (!ops->set_pcm_binterval) ||
> > > +	    (!ops->usb_add_ctls))
> > > +		return -EINVAL;
> > > +
> > > +	usb_vendor_ops = ops;
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(snd_vendor_set_ops);
> > > +
> > > +struct snd_usb_audio_vendor_ops *snd_vendor_get_ops(void)
> > > +{
> > > +	return usb_vendor_ops;
> > > +}
> > 
> > This is the function you need to fix up, and add proper reference
> > counting to, in order to solve your "this breaks with multiple USB
> > controllers" problem.  So this really should not be all that difficult
> > of a task.  Why has it taken years to do so?
> > 
> > thanks,
> > 
> > greg k-h
> >
> Hi,
> 
> Is that mean the scenario when two or more usb audio devices are
> connected through the hub?

I have no idea, it's your hardware and code to test, not mine :)

greg k-h

Re: [PATCH v1 2/3] sound: usb: Calling vendor's call-back function within usb audio operation.

[Oh Eomji]

[-- Attachment #1: Type: text/plain, Size: 4581 bytes --]

On Thu, Mar 24, 2022 at 09:34:22AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 24, 2022 at 05:10:43PM +0900, Oh Eomji wrote:
> > When a new interface is connected or removed, the call-back functions
> > are called to transmit a command to the hardware.
> > 
> > Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> > ---
> >  sound/usb/pcm.c    | 37 +++++++++++++++++++++++++++++++++++++
> >  sound/usb/stream.c |  2 ++
> >  2 files changed, 39 insertions(+)
> > 
> > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> > index cec6e91a..4bae4ba 100644
> > --- a/sound/usb/pcm.c
> > +++ b/sound/usb/pcm.c
> > @@ -144,6 +144,8 @@ find_format(struct list_head *fmt_list_head, snd_pcm_format_t format,
> >  			found = fp;
> >  			cur_attr = attr;
> >  		}
> > +
> > +		snd_vendor_set_pcm_binterval(fp, found, &cur_attr, &attr);
> >  	}
> >  	return found;
> >  }
> > @@ -434,6 +436,7 @@ static int configure_endpoints(struct snd_usb_audio *chip,
> >  			       struct snd_usb_substream *subs)
> >  {
> >  	int err;
> > +	struct usb_interface *iface;
> >  
> >  	if (subs->data_endpoint->need_setup) {
> >  		/* stop any running stream beforehand */
> > @@ -442,6 +445,13 @@ static int configure_endpoints(struct snd_usb_audio *chip,
> >  		err = snd_usb_endpoint_configure(chip, subs->data_endpoint);
> >  		if (err < 0)
> >  			return err;
> > +
> > +		iface = usb_ifnum_to_if(chip->dev, subs->data_endpoint->iface);
> > +		err = snd_vendor_set_pcm_intf(iface, subs->data_endpoint->iface,
> > +				subs->data_endpoint->altsetting, subs->direction);
> > +		if (err < 0)
> > +			return err;
> > +
> >  		snd_usb_set_format_quirk(subs, subs->cur_audiofmt);
> >  	}
> >  
> > @@ -616,8 +626,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
> >  	struct snd_pcm_runtime *runtime = substream->runtime;
> >  	struct snd_usb_substream *subs = runtime->private_data;
> >  	struct snd_usb_audio *chip = subs->stream->chip;
> > +	struct snd_usb_endpoint *ep = subs->data_endpoint;
> >  	int ret;
> >  
> > +	ret = snd_vendor_set_pcm_buf(subs->dev, subs->cur_audiofmt->iface);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (!subs->cur_audiofmt) {
> > +		dev_err(&subs->dev->dev, "no format is specified\n");
> > +		return -ENXIO;
> > +	}
> > +
> >  	ret = snd_usb_lock_shutdown(chip);
> >  	if (ret < 0)
> >  		return ret;
> > @@ -630,6 +650,13 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
> >  	if (ret < 0)
> >  		goto unlock;
> >  
> > +	if (snd_vendor_get_ops()) {
> > +		ret = snd_vendor_set_rate(ep->cur_audiofmt->iface,
> > +				ep->cur_rate, ep->cur_audiofmt->altsetting);
> > +		if (!ret)
> > +			goto unlock;
> > +	}
> > +
> >  	/* reset the pointer */
> >  	subs->buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size);
> >  	subs->inflight_bytes = 0;
> > @@ -1104,6 +1131,11 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
> >  	struct snd_usb_substream *subs = &as->substream[direction];
> >  	int ret;
> >  
> > +	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_OPEN,
> > +					    direction);
> > +	if (ret)
> > +		return ret;
> > +
> >  	runtime->hw = snd_usb_hardware;
> >  	/* need an explicit sync to catch applptr update in low-latency mode */
> >  	if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
> > @@ -1137,6 +1169,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
> >  	struct snd_usb_substream *subs = &as->substream[direction];
> >  	int ret;
> >  
> > +	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_CLOSE,
> > +					    direction);
> > +	if (ret)
> > +		return ret;
> > +
> >  	snd_media_stop_pipeline(subs);
> >  
> >  	if (!snd_usb_lock_shutdown(subs->stream->chip)) {
> > diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> > index ceb93d7..26ca696 100644
> > --- a/sound/usb/stream.c
> > +++ b/sound/usb/stream.c
> > @@ -1227,6 +1227,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
> >  		snd_usb_init_pitch(chip, fp);
> >  		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> >  		usb_set_interface(chip->dev, iface_no, altno);
> > +		if (protocol > UAC_VERSION_1)
> 
> Why the protocol check?  That's not documented in your changelog
> anywhere :(
> 
>
Hi,

In kernel 5.10, set_interface is performed when the protocol is more
than UAC_VERSION_1 in the snd_usb_init_sample_rate function.
There was an issue here, so there is a history of adding to perform
snd_vendor_set_interface when the protocol is more than UAC_VERSION_1.
But I don't think I need this in kerenel 5.15, so I'll delete it.

Thanks,
Eomji Oh

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1 2/3] sound: usb: Calling vendor's call-back function within usb audio operation.

[Greg Kroah-Hartman]

On Fri, Mar 25, 2022 at 04:13:57PM +0900, Oh Eomji wrote:
> On Thu, Mar 24, 2022 at 09:34:22AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Mar 24, 2022 at 05:10:43PM +0900, Oh Eomji wrote:
> > > When a new interface is connected or removed, the call-back functions
> > > are called to transmit a command to the hardware.
> > > 
> > > Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> > > ---
> > >  sound/usb/pcm.c    | 37 +++++++++++++++++++++++++++++++++++++
> > >  sound/usb/stream.c |  2 ++
> > >  2 files changed, 39 insertions(+)
> > > 
> > > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> > > index cec6e91a..4bae4ba 100644
> > > --- a/sound/usb/pcm.c
> > > +++ b/sound/usb/pcm.c
> > > @@ -144,6 +144,8 @@ find_format(struct list_head *fmt_list_head, snd_pcm_format_t format,
> > >  			found = fp;
> > >  			cur_attr = attr;
> > >  		}
> > > +
> > > +		snd_vendor_set_pcm_binterval(fp, found, &cur_attr, &attr);
> > >  	}
> > >  	return found;
> > >  }
> > > @@ -434,6 +436,7 @@ static int configure_endpoints(struct snd_usb_audio *chip,
> > >  			       struct snd_usb_substream *subs)
> > >  {
> > >  	int err;
> > > +	struct usb_interface *iface;
> > >  
> > >  	if (subs->data_endpoint->need_setup) {
> > >  		/* stop any running stream beforehand */
> > > @@ -442,6 +445,13 @@ static int configure_endpoints(struct snd_usb_audio *chip,
> > >  		err = snd_usb_endpoint_configure(chip, subs->data_endpoint);
> > >  		if (err < 0)
> > >  			return err;
> > > +
> > > +		iface = usb_ifnum_to_if(chip->dev, subs->data_endpoint->iface);
> > > +		err = snd_vendor_set_pcm_intf(iface, subs->data_endpoint->iface,
> > > +				subs->data_endpoint->altsetting, subs->direction);
> > > +		if (err < 0)
> > > +			return err;
> > > +
> > >  		snd_usb_set_format_quirk(subs, subs->cur_audiofmt);
> > >  	}
> > >  
> > > @@ -616,8 +626,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
> > >  	struct snd_pcm_runtime *runtime = substream->runtime;
> > >  	struct snd_usb_substream *subs = runtime->private_data;
> > >  	struct snd_usb_audio *chip = subs->stream->chip;
> > > +	struct snd_usb_endpoint *ep = subs->data_endpoint;
> > >  	int ret;
> > >  
> > > +	ret = snd_vendor_set_pcm_buf(subs->dev, subs->cur_audiofmt->iface);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (!subs->cur_audiofmt) {
> > > +		dev_err(&subs->dev->dev, "no format is specified\n");
> > > +		return -ENXIO;
> > > +	}
> > > +
> > >  	ret = snd_usb_lock_shutdown(chip);
> > >  	if (ret < 0)
> > >  		return ret;
> > > @@ -630,6 +650,13 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
> > >  	if (ret < 0)
> > >  		goto unlock;
> > >  
> > > +	if (snd_vendor_get_ops()) {
> > > +		ret = snd_vendor_set_rate(ep->cur_audiofmt->iface,
> > > +				ep->cur_rate, ep->cur_audiofmt->altsetting);
> > > +		if (!ret)
> > > +			goto unlock;
> > > +	}
> > > +
> > >  	/* reset the pointer */
> > >  	subs->buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size);
> > >  	subs->inflight_bytes = 0;
> > > @@ -1104,6 +1131,11 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
> > >  	struct snd_usb_substream *subs = &as->substream[direction];
> > >  	int ret;
> > >  
> > > +	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_OPEN,
> > > +					    direction);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > >  	runtime->hw = snd_usb_hardware;
> > >  	/* need an explicit sync to catch applptr update in low-latency mode */
> > >  	if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
> > > @@ -1137,6 +1169,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
> > >  	struct snd_usb_substream *subs = &as->substream[direction];
> > >  	int ret;
> > >  
> > > +	ret = snd_vendor_set_pcm_connection(subs->dev, SOUND_PCM_CLOSE,
> > > +					    direction);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > >  	snd_media_stop_pipeline(subs);
> > >  
> > >  	if (!snd_usb_lock_shutdown(subs->stream->chip)) {
> > > diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> > > index ceb93d7..26ca696 100644
> > > --- a/sound/usb/stream.c
> > > +++ b/sound/usb/stream.c
> > > @@ -1227,6 +1227,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
> > >  		snd_usb_init_pitch(chip, fp);
> > >  		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> > >  		usb_set_interface(chip->dev, iface_no, altno);
> > > +		if (protocol > UAC_VERSION_1)
> > 
> > Why the protocol check?  That's not documented in your changelog
> > anywhere :(
> > 
> >
> Hi,
> 
> In kernel 5.10, set_interface is performed when the protocol is more
> than UAC_VERSION_1 in the snd_usb_init_sample_rate function.
> There was an issue here, so there is a history of adding to perform
> snd_vendor_set_interface when the protocol is more than UAC_VERSION_1.
> But I don't think I need this in kerenel 5.15, so I'll delete it.

5.15 is very old, you are working on 5.18-rc1 now :)

Re: [PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Christoph Hellwig]

Independ of the actual usefulness of thi: vendor is a reallly bogus
naming choice for device specific hooks.  No one cares what vendor
produces a given device, it is all about the interface of it.  One
vendor might have different interfaces over time, from different
divisons or thrugh acquisitions, and multiple vendors can agree to
a single interface through open standards or IP licensing.

Re: [PATCH v1 3/3] sound: usb: Exynos usb audio offloading driver

[Krzysztof Kozlowski]

On 24/03/2022 09:10, Oh Eomji wrote:
> This is for usb audio offloading. usb audio offloading processes usb
> audio stream data directly from the audio box even if ap usb enters
> suspend, there is no problem in stream data transmission. This obtains
> power saving effect while using usb audio device.
> 
> AP usb and audio usb f/w communicate via AUDIO IPC. By performing AUDIO
> IPC in the vendor operations, abox can access and control the xhci to
> transmit the data directly.
> 
> The types of commands and data delivered through AUDIO IPC include the
> following (AP USB <-> AUDIO USB f/w) :
> 1. usb audio connection/disconnection status
> 2. xhci memory information
> 3. full descriptor for usb audio device
> 4. UAC(usb audio class) control command
> 
> Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> ---
>  sound/usb/Kconfig            |   9 +
>  sound/usb/Makefile           |   2 +
>  sound/usb/exynos_usb_audio.c | 560 +++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/exynos_usb_audio.h | 150 ++++++++++++
>  4 files changed, 721 insertions(+)
>  create mode 100644 sound/usb/exynos_usb_audio.c
>  create mode 100644 sound/usb/exynos_usb_audio.h

Similar pattern as XHCI submission - it looks like you do not work on
mainline kernel, but some other, private tree with other modifications.
It seems you created this series based on that private tree.

This is not the proper process.

Please rebase all your work on recent mainline kernel (v5.18-rc1,
linux-next) and work there.

Then you can start using get_maintainers.pl...

BTW, this v2, not v1, so please version it correctly. Add also changelog
to your series in cover letter.

Best regards,
Krzysztof

Re: [PATCH v1 1/3] sound: usb: Add vendor's hooking interface

[Krzysztof Kozlowski]

On 24/03/2022 09:32, Greg Kroah-Hartman wrote:
> On Thu, Mar 24, 2022 at 05:10:42PM +0900, Oh Eomji wrote:
>> In mobile, a co-processor can be used with USB audio to improve
>> power consumption.  To support this type of hardware, hooks need to
>> be added to the USB audio subsystem to be able to call into the
>> hardware when needed.
>> 
>> The main operation of the call-backs are: - Initialize the
>> co-processor by transmitting data when initializing. - Change the
>> co-processor setting value through the interface function. -
>> Configure sampling rate - pcm open/close - other housekeeping
>> 
>> Known issues: - This only supports one set of callback hooks,
>> meaning that this only works if there is one type of USB controller
>> in the system.  This should be changed to be a per-host-controller
>> interface instead of one global set of callbacks.
> 
> Sorry, but this limitation alone means that this is not going to be
> able to be accepted.  Almost all real systems have multiple USB
> controllers in the system and so, this will break in very bad ways on
> the majority of devices in the world.
> 
> Please fix this up and make this per-USB-controller, as was
> requested the last time this series was published.

This is a v2 (not v1) and Greg asked this already:
https://lore.kernel.org/all/YiW6ZqnINsOSyN+z@kroah.com/
That time, it was left without an answer.

Ignoring feedback and resending does not help in getting patches
mainlined. :(

Best regards,
Krzysztof

Re: [PATCH v1 3/3] sound: usb: Exynos usb audio offloading driver

[Oh Eomji]

[-- Attachment #1: Type: text/plain, Size: 25098 bytes --]

On Thu, Mar 24, 2022 at 09:41:26AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 24, 2022 at 05:10:44PM +0900, Oh Eomji wrote:
> > This is for usb audio offloading. usb audio offloading processes usb
> > audio stream data directly from the audio box even if ap usb enters
> > suspend, there is no problem in stream data transmission. This obtains
> > power saving effect while using usb audio device.
> > 
> > AP usb and audio usb f/w communicate via AUDIO IPC. By performing AUDIO
> > IPC in the vendor operations, abox can access and control the xhci to
> > transmit the data directly.
> > 
> > The types of commands and data delivered through AUDIO IPC include the
> > following (AP USB <-> AUDIO USB f/w) :
> > 1. usb audio connection/disconnection status
> > 2. xhci memory information
> > 3. full descriptor for usb audio device
> > 4. UAC(usb audio class) control command
> > 
> > Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> > ---
> >  sound/usb/Kconfig            |   9 +
> >  sound/usb/Makefile           |   2 +
> >  sound/usb/exynos_usb_audio.c | 560 +++++++++++++++++++++++++++++++++++++++++++
> >  sound/usb/exynos_usb_audio.h | 150 ++++++++++++
> >  4 files changed, 721 insertions(+)
> >  create mode 100644 sound/usb/exynos_usb_audio.c
> >  create mode 100644 sound/usb/exynos_usb_audio.h
> > 
> > diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> > index 059242f..70252a3 100644
> > --- a/sound/usb/Kconfig
> > +++ b/sound/usb/Kconfig
> > @@ -27,6 +27,15 @@ config SND_USB_AUDIO
> >  config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> >  	bool
> >  
> > +config SND_EXYNOS_USB_AUDIO
> > +	tristate "EXYNOS USB Audio offloading module"
> > +	depends on SND_USB_AUDIO
> > +	help
> > +	 Say Y here to include support for Exynos USB Audio ABOX offloading.
> > +
> > +	 To compile this driver as a module, choose M here: the module
> > +	 will be called exynos-usb-audio-offloading.
> > +
> >  config SND_USB_UA101
> >  	tristate "Edirol UA-101/UA-1000 driver"
> >  	select SND_PCM
> > diff --git a/sound/usb/Makefile b/sound/usb/Makefile
> > index 9ccb21a..bf019c7 100644
> > --- a/sound/usb/Makefile
> > +++ b/sound/usb/Makefile
> > @@ -28,6 +28,8 @@ snd-usbmidi-lib-objs := midi.o
> >  
> >  # Toplevel Module Dependency
> >  obj-$(CONFIG_SND_USB_AUDIO) += snd-usb-audio.o snd-usbmidi-lib.o
> > +obj-$(CONFIG_SND_EXYNOS_USB_AUDIO) += exynos-usb-audio-offloading.o
> > +exynos-usb-audio-offloading-y += exynos_usb_audio.o
> >  
> >  obj-$(CONFIG_SND_USB_UA101) += snd-usbmidi-lib.o
> >  obj-$(CONFIG_SND_USB_USX2Y) += snd-usbmidi-lib.o
> > diff --git a/sound/usb/exynos_usb_audio.c b/sound/usb/exynos_usb_audio.c
> > new file mode 100644
> > index 0000000..456cc78
> > --- /dev/null
> > +++ b/sound/usb/exynos_usb_audio.c
> > @@ -0,0 +1,560 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> 
> Are you sure about this license?  I have to ask, sorry.
> 
It's my mistake. I will fix it.

> > +/*
> > + *   USB Audio offloading Driver for Exynos
> > + *
> > + *   Copyright (c) 2017 by Kyounghye Yun <k-hye.yun@samsung.com>
> > + *
> > + */
> > +
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/ctype.h>
> > +#include <linux/usb.h>
> > +#include <linux/usb/audio.h>
> > +#include <linux/usb/audio-v2.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/io.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/completion.h>
> > +
> > +#include <sound/pcm.h>
> > +#include "../../../sound/usb/exynos_usb_audio.h"
> 
> Did this patch just break the build?  It should not need this type of
> .../../../ mess.
> 
Build succedded. But yes it's mess. I will fix it.

> > +#include "usbaudio.h"
> > +#include "helper.h"
> > +#include "card.h"
> > +#include "quirks.h"
> > +
> > +static struct exynos_usb_audio *usb_audio;
> > +static struct snd_usb_audio_vendor_ops exynos_usb_audio_ops;
> > +
> > +struct hcd_hw_info *g_hwinfo;
> > +EXPORT_SYMBOL_GPL(g_hwinfo);
> > +int otg_connection;
> > +EXPORT_SYMBOL_GPL(otg_connection);
> > +int usb_audio_connection;
> > +EXPORT_SYMBOL_GPL(usb_audio_connection);
> 
> Why are these exported?  Who uses them?  And why generic names for a
> driver-specific variable?  And what do they do and how are they
> accessed?
> 
> No one in this patch series needs these exports, so why are they
> exported?
> 
Right. I'll fix it.

> > +
> > +static void exynos_usb_audio_set_device(struct usb_device *udev)
> > +{
> > +	usb_audio->udev = udev;
> > +	usb_audio->is_audio = 1;
> 
> boolean?
> 
> How do you know?  Did you check?
> 
> And why a single static variable?
> 
1 : connection / 0 : disconnection
Should I change this type to boolean?

> > +}
> > +
> > +static int exynos_usb_audio_unmap_all(void)
> > +{
> > +	/*
> > +	 * TODO: unmapping pcm buffer, usb descriptor, Device Context,
> > +	 * Input Context, ERST, URAM.
> > +	 */
> 
> If this is not completed, just don't have the function at all.
> 
> Why isn't this done?
>
Unmapping is performed using the AUDIO_IPC of the exynos audio module.
As mentioned in commit msg, AUDIO_IPC is a function for ap usb and audio
usb f/w communication, which is defined in exynos audio module.
So the contents performed through AUDIO_IPC were written in TODO.

> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_pcmbuf(struct usb_device *udev, int iface)
> > +{
> > +	if (!usb_audio->is_audio || !otg_connection)
> > +		return -1;
> 
> Do not make up error values.  Use correct ones.
> 
>
Okay, I will fix it.

> > +
> > +	/*
> > +	 * TODO: Transmit the DRAM address that contains the xhci device
> > +	 * information,and the DMA address required for operation to the abox
> > +	 * usb f/w.
> > +	 */
> 
> So this function does nothing?
> 
> Does this driver even work properly?  Where is the  missing logic?
> 
This is also performed using the AUDIO_IPC of the exynos audio module.
As mentioned above, the contents performed through AUDIO_IPC were
written in TODO.

> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_setrate(int iface, int rate, int alt)
> > +{
> > +	if (!usb_audio->is_audio || !otg_connection)
> > +		return -1;
> > +
> > +	/*
> > +	 * TODO: Notify the abox usb f/w the audio sample rate supported by
> > +	 * the interface of the connected audio device.
> > +	 */
> 
> Same as above.
> 
> Does this driver actually work on real hardware?  Or is this just a
> "fake" driver that is never intended for anyone to use just to allow the
> hooks to be added to the kernel tree?
> 
> What hardware has this driver worked on?  Can I take the pixel6 device
> today and replace their out-of-tree driver with this one and will it
> work properly?  If not, what is missing?
> 
This is also performed using the AUDIO_IPC of the exynos audio module.
As mentioned above, the contents performed through AUDIO_IPC were
written in TODO.
> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_setintf(struct usb_device *udev, int iface, int alt, int direction)
> > +{
> > +	struct hcd_hw_info *hwinfo = g_hwinfo;
> > +	u64 in_offset, out_offset;
> > +
> > +	if (!usb_audio->pcm_open_done)
> > +		return -EPERM;
> 
> How is that a permission error?
>
This means that the operation was not allowed because the pcm open was
not finished, so would it be better to change it to EBUSY or EIO?
> > +
> > +	if (!usb_audio->is_audio || !otg_connection)
> > +		return -1;
> 
> Again, no fake error numbers.
> 
Okay, I'll fix it.

> > +	if (direction) {
> > +		/* IN EP */
> > +		if (!usb_audio->indeq_map_done ||
> > +			(hwinfo->in_deq != hwinfo->old_in_deq)) {
> > +			/* TODO: Transmit pcm interface number, alt setting
> > +			 * number to abox usb f/w
> > +			 */
> 
> Fix all TODO please before resubmitting.
> 
This is also performed using the AUDIO_IPC of the exynos audio module.
As mentioned above, the contents performed through AUDIO_IPC were
written in TODO.
> > +			usb_audio->indeq_map_done = 1;
> > +			in_offset = hwinfo->in_deq % PAGE_SIZE;
> 
> Why does PAGE_SIZE matter?
> 
This is for page align. Audio maps memory for usb ipc. From the base
address, each memory is mapped in 4KB(page size) units.
When allocated through dma alloc, address is fiteed in page align, but
when allocated through trb buffer and kmalloc, address may not be fitted
in page align. So, the offset value (PA % PAGE_SIZE) is transmitted for
page align.
> > +		}
> > +
> > +		if (hwinfo->fb_out_deq) {
> > +			if (!usb_audio->fb_outdeq_map_done ||
> > +					(hwinfo->fb_out_deq != hwinfo->fb_old_out_deq)) {
> > +				/* TODO: Transmit pcm interface number,
> > +				 * alt setting number to abox usb f/w
> > +				 */
> > +				usb_audio->fb_outdeq_map_done = 1;
> > +				out_offset = hwinfo->fb_out_deq % PAGE_SIZE;
> > +			}
> > +		}
> > +	} else {
> > +		/* OUT EP */
> > +		if (!usb_audio->outdeq_map_done ||
> > +			(hwinfo->out_deq != hwinfo->old_out_deq)) {
> > +			/* TODO: Transmit pcm interface number, alt setting
> > +			 * number to abox usb f/w
> > +			 */
> > +			usb_audio->outdeq_map_done = 1;
> > +			out_offset = hwinfo->out_deq % PAGE_SIZE;
> > +		}
> > +
> > +		if (hwinfo->fb_in_deq) {
> > +			if (!usb_audio->fb_indeq_map_done ||
> > +					(hwinfo->fb_in_deq != hwinfo->fb_old_in_deq)) {
> > +				/* TODO: Transmit pcm interface number,
> > +				 * alt setting number to abox usb f/w
> > +				 */
> > +				usb_audio->fb_indeq_map_done = 1;
> > +				in_offset = hwinfo->fb_in_deq % PAGE_SIZE;
> > +			}
> > +		}
> > +	}
> > +
> > +	/* one more check connection to prevent kernel panic */
> > +	if (!usb_audio->is_audio || !otg_connection)
> > +		return -1;
> > +
> > +	/* TODO: Notify abox usb f/w a dequeue pointer */
> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_hcd(struct usb_device *udev)
> > +{
> > +	struct hcd_hw_info *hwinfo = g_hwinfo;
> > +
> > +	/* back up each address for unmap */
> > +	usb_audio->dcbaa_dma = hwinfo->dcbaa_dma;
> > +	usb_audio->save_dma = hwinfo->save_dma;
> > +	usb_audio->in_ctx = hwinfo->in_ctx;
> > +	usb_audio->out_ctx = hwinfo->out_ctx;
> > +	usb_audio->erst_addr = hwinfo->erst_addr;
> > +	usb_audio->speed = hwinfo->speed;
> > +	usb_audio->use_uram = hwinfo->use_uram;
> > +
> > +	/*
> > +	 * TODO: DCBAA, Device Context, Input Context, URAM, ERST mapping
> > +	 * and notify abox usb f/w the address about xhci h/w resource to
> > +	 * directly control the xhci in abox.
> > +	 */
> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_desc(struct usb_device *udev)
> > +{
> > +	int configuration, cfgno, i;
> > +	unsigned char *buffer;
> > +	u64 desc_addr;
> > +	u64 offset;
> > +
> > +	configuration = usb_choose_configuration(udev);
> > +
> > +	cfgno = -1;
> > +	for (i = 0; i < udev->descriptor.bNumConfigurations; i++) {
> > +		if (udev->config[i].desc.bConfigurationValue ==
> > +				configuration) {
> > +			cfgno = i;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (cfgno == -1)
> > +		cfgno = 0;
> > +
> > +	/* need to memory mapping for usb descriptor */
> > +	buffer = udev->rawdescriptors[cfgno];
> > +	desc_addr = virt_to_phys(buffer);
> > +	offset = desc_addr % PAGE_SIZE;
> > +
> > +	/* store address information */
> > +	usb_audio->desc_addr = desc_addr;
> > +	usb_audio->offset = offset;
> > +
> > +	desc_addr -= offset;
> > +
> > +	/*
> > +	 * TODO: Notify the abox usb f/w that all descriptors have been recived
> > +	 * from the connected usb audio device, and that copy and memory mapping
> > +	 * have beed completed so that it can be used in abox usb f/w
> > +	 */
> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_conn(struct usb_device *udev, int is_conn)
> > +{
> > +
> > +	/* TODO: Notify abox usb f/w whether usb device is connected or not */
> > +	if (!is_conn) {
> > +		if (usb_audio->is_audio) {
> > +			usb_audio->is_audio = 0;
> > +			usb_audio->usb_audio_state = USB_AUDIO_REMOVING;
> > +		}
> > +	} else {
> > +		cancel_work_sync(&usb_audio->usb_work);
> > +		usb_audio->indeq_map_done = 0;
> > +		usb_audio->outdeq_map_done = 0;
> > +		usb_audio->fb_indeq_map_done = 0;
> > +		usb_audio->fb_outdeq_map_done = 0;
> > +		usb_audio->pcm_open_done = 0;
> > +		reinit_completion(&usb_audio->discon_done);
> > +		usb_audio->usb_audio_state = USB_AUDIO_CONNECT;
> > +		usb_audio_connection = 1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_audio_pcm(bool is_open, bool direction)
> > +{
> > +	if (!usb_audio->is_audio || !otg_connection)
> > +		return -1;
> > +
> > +	if (is_open)
> > +		usb_audio->pcm_open_done = 1;
> > +
> > +	/* TODO: Notify the abox usb f/w the pcm open/close status */
> > +
> > +	return 0;
> > +}
> > +
> > +static void exynos_usb_audio_work(struct work_struct *w)
> > +{
> > +	/* Don't unmap in USB_AUDIO_TIMEOUT_PROBE state */
> > +	if (usb_audio->usb_audio_state != USB_AUDIO_REMOVING)
> > +		return;
> > +
> > +	exynos_usb_audio_unmap_all();
> > +	usb_audio->usb_audio_state = USB_AUDIO_DISCONNECT;
> > +	usb_audio_connection = 0;
> > +	complete(&usb_audio->discon_done);
> > +}
> > +
> > +static int exynos_usb_scenario_ctl_info(struct snd_kcontrol *kcontrol,
> > +			      struct snd_ctl_elem_info *uinfo)
> > +{
> > +	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> > +	uinfo->count = 1;
> > +	uinfo->value.integer.min = AUDIO_MODE_NORMAL;
> > +	uinfo->value.integer.max = AUDIO_MODE_CALL_SCREEN;
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_scenario_ctl_get(struct snd_kcontrol *kcontrol,
> > +			 struct snd_ctl_elem_value *ucontrol)
> > +{
> > +	struct exynos_usb_audio *usb = snd_kcontrol_chip(kcontrol);
> > +
> > +	ucontrol->value.integer.value[0] = usb->user_scenario;
> > +	return 0;
> > +}
> > +
> > +static int exynos_usb_scenario_ctl_put(struct snd_kcontrol *kcontrol,
> > +			 struct snd_ctl_elem_value *ucontrol)
> > +{
> > +	struct exynos_usb_audio *usb = snd_kcontrol_chip(kcontrol);
> > +	int changed = 0;
> > +
> > +	if (usb->user_scenario !=
> > +	     ucontrol->value.integer.value[0]) {
> > +		usb->user_scenario = ucontrol->value.integer.value[0];
> > +		changed = 1;
> > +	}
> > +
> > +	return changed;
> > +}
> > +
> > +static int exynos_usb_add_ctls(struct snd_usb_audio *chip,
> > +				unsigned long private_value)
> > +{
> > +	struct snd_kcontrol_new knew = {
> > +		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
> > +		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> > +		.name = "USB Audio Mode",
> > +		.info = exynos_usb_scenario_ctl_info,
> > +		.get = exynos_usb_scenario_ctl_get,
> > +		.put = exynos_usb_scenario_ctl_put,
> > +	};
> > +
> > +	int err;
> > +
> > +	if (!chip)
> > +		return -ENODEV;
> > +
> > +	knew.private_value = private_value;
> > +	usb_audio->kctl = snd_ctl_new1(&knew, usb_audio);
> > +	if (!usb_audio->kctl)
> > +		return -ENOMEM;
> > +
> > +	err = snd_ctl_add(chip->card, usb_audio->kctl);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	return 0;
> > +}
> > +
> > +int exynos_usb_audio_init(struct device *dev, struct platform_device *pdev)
> > +{
> > +	struct device_node *np = dev->of_node;
> > +	struct device_node *np_abox;
> > +	struct platform_device *pdev_abox;
> > +	int ret = 0;
> > +
> > +	if (!usb_audio) {
> > +		usb_audio = kmalloc(sizeof(struct exynos_usb_audio), GFP_KERNEL);
> > +		if (!usb_audio)
> > +			return -ENOMEM;
> > +	}
> > +
> > +	np_abox = of_parse_phandle(np, "abox", 0);
> > +	if (!np_abox)
> > +		return -EPROBE_DEFER;
> > +
> > +	pdev_abox = of_find_device_by_node(np_abox);
> > +	if (!pdev_abox)
> > +		return -EPROBE_DEFER;
> > +
> > +	init_completion(&usb_audio->in_conn_stop);
> > +	init_completion(&usb_audio->out_conn_stop);
> > +	init_completion(&usb_audio->discon_done);
> > +	usb_audio->abox = pdev_abox;
> > +	usb_audio->hcd_pdev = pdev;
> > +	usb_audio->udev = NULL;
> > +	usb_audio->is_audio = 0;
> > +	usb_audio->is_first_probe = 1;
> > +	usb_audio->user_scenario = AUDIO_MODE_NORMAL;
> > +	usb_audio->usb_audio_state = USB_AUDIO_DISCONNECT;
> > +	usb_audio_connection = 0;
> > +
> > +	INIT_WORK(&usb_audio->usb_work, exynos_usb_audio_work);
> > +
> > +	/* interface function mapping */
> > +	ret = snd_vendor_set_ops(&exynos_usb_audio_ops);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(exynos_usb_audio_init);
> 
> Why are you exporting functions that are never called?
> 
>
This function can be called exynos usb module driver.
> > +
> > +/* card */
> > +static int exynos_usb_audio_connect(struct usb_interface *intf)
> > +{
> > +	struct usb_interface_descriptor *altsd;
> > +	struct usb_host_interface *alts;
> > +	struct usb_device *udev = interface_to_usbdev(intf);
> > +	int timeout = 0;
> > +
> > +	alts = &intf->altsetting[0];
> > +	altsd = get_iface_desc(alts);
> > +
> > +	if ((altsd->bInterfaceClass == USB_CLASS_AUDIO ||
> > +		altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC) &&
> > +		altsd->bInterfaceSubClass == USB_SUBCLASS_MIDISTREAMING) {
> > +	} else {
> > +		if (usb_audio->usb_audio_state == USB_AUDIO_REMOVING) {
> > +			timeout = wait_for_completion_timeout(
> > +				&usb_audio->discon_done,
> > +				msecs_to_jiffies(DISCONNECT_TIMEOUT));
> > +
> > +			if ((usb_audio->usb_audio_state == USB_AUDIO_REMOVING)
> > +					&& !timeout) {
> > +				usb_audio->usb_audio_state =
> > +					USB_AUDIO_TIMEOUT_PROBE;
> > +			}
> > +		}
> > +
> > +		if ((usb_audio->usb_audio_state == USB_AUDIO_DISCONNECT)
> > +			|| (usb_audio->usb_audio_state == USB_AUDIO_TIMEOUT_PROBE)) {
> > +			exynos_usb_audio_set_device(udev);
> > +			exynos_usb_audio_hcd(udev);
> > +			exynos_usb_audio_conn(udev, 1);
> > +			exynos_usb_audio_desc(udev);
> > +		} else {
> > +			return -EPERM;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void exynos_usb_audio_disconn(struct usb_interface *intf)
> > +{
> > +	struct usb_device *udev = interface_to_usbdev(intf);
> > +
> > +	exynos_usb_audio_conn(udev, 0);
> > +}
> > +
> > +/* clock */
> > +static int exynos_usb_audio_set_interface(struct usb_device *udev,
> > +		struct usb_host_interface *alts, int iface, int alt)
> > +{
> > +	unsigned char ep;
> > +	unsigned char numEndpoints;
> > +	int direction;
> > +	int i;
> > +	int ret = 0;
> > +
> > +	if (alts != NULL) {
> > +		numEndpoints = get_iface_desc(alts)->bNumEndpoints;
> > +		if (numEndpoints < 1)
> > +			return -22;
> > +		if (numEndpoints == 1)
> > +			ep = get_endpoint(alts, 0)->bEndpointAddress;
> > +		else {
> > +			for (i = 0; i < numEndpoints; i++) {
> > +				ep = get_endpoint(alts, i)->bmAttributes;
> > +				if (!(ep & 0x30)) {
> > +					ep = get_endpoint(alts, i)->bEndpointAddress;
> > +					break;
> > +				}
> > +			}
> > +		}
> > +		if (ep & USB_DIR_IN)
> > +			direction = SNDRV_PCM_STREAM_CAPTURE;
> > +		else
> > +			direction = SNDRV_PCM_STREAM_PLAYBACK;
> > +
> > +		ret = exynos_usb_audio_setintf(udev, iface, alt, direction);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +/* pcm */
> > +static int exynos_usb_audio_set_rate(int iface, int rate, int alt)
> > +{
> > +	int ret;
> > +
> > +	ret = exynos_usb_audio_setrate(iface, rate, alt);
> > +
> > +	return ret;
> > +}
> > +
> > +static int exynos_usb_audio_set_pcmbuf(struct usb_device *dev, int iface)
> > +{
> > +	int ret;
> > +
> > +	ret = exynos_usb_audio_pcmbuf(dev, iface);
> > +
> > +	return ret;
> > +}
> > +
> > +static int exynos_usb_audio_set_pcm_intf(struct usb_interface *intf,
> > +					int iface, int alt, int direction)
> > +{
> > +	struct usb_device *udev = interface_to_usbdev(intf);
> > +	int ret;
> > +
> > +	ret = exynos_usb_audio_setintf(udev, iface, alt, direction);
> > +
> > +	return ret;
> > +}
> > +
> > +static int exynos_usb_audio_pcm_control(struct usb_device *udev,
> > +			enum snd_vendor_pcm_open_close onoff, int direction)
> > +{
> > +	int ret = 0;
> > +
> > +	if (onoff == 1) {
> > +		ret = exynos_usb_audio_pcm(1, direction);
> > +	} else if (onoff == 0) {
> > +		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
> > +			reinit_completion(&usb_audio->out_conn_stop);
> > +		else if (direction == SNDRV_PCM_STREAM_CAPTURE)
> > +			reinit_completion(&usb_audio->in_conn_stop);
> > +
> > +		if (!usb_audio->pcm_open_done)
> > +			return 0;
> > +
> > +		ret = exynos_usb_audio_pcm(0, direction);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int exynos_usb_audio_add_control(struct snd_usb_audio *chip)
> > +{
> > +	int ret;
> > +
> > +	if (chip != NULL)
> > +		ret = exynos_usb_add_ctls(chip, 0);
> > +	else
> > +		ret = usb_audio->user_scenario;
> > +
> > +	return ret;
> > +}
> > +
> > +static int exynos_usb_audio_set_pcm_binterval(const struct audioformat *fp,
> > +				 const struct audioformat *found,
> > +				 int *cur_attr, int *attr)
> > +{
> > +	if (usb_audio->user_scenario >= AUDIO_MODE_IN_CALL) {
> > +		if (fp->datainterval < found->datainterval) {
> > +			found = fp;
> > +			cur_attr = attr;
> > +		}
> > +	} else {
> > +		if (fp->datainterval > found->datainterval) {
> > +			found = fp;
> > +			cur_attr = attr;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* Set interface function */
> > +static struct snd_usb_audio_vendor_ops exynos_usb_audio_ops = {
> > +	/* card */
> > +	.connect = exynos_usb_audio_connect,
> > +	.disconnect = exynos_usb_audio_disconn,
> > +	/* clock */
> > +	.set_interface = exynos_usb_audio_set_interface,
> > +	/* pcm */
> > +	.set_rate = exynos_usb_audio_set_rate,
> > +	.set_pcm_buf = exynos_usb_audio_set_pcmbuf,
> > +	.set_pcm_intf = exynos_usb_audio_set_pcm_intf,
> > +	.set_pcm_connection = exynos_usb_audio_pcm_control,
> > +	.set_pcm_binterval = exynos_usb_audio_set_pcm_binterval,
> > +	.usb_add_ctls = exynos_usb_audio_add_control,
> > +};
> > +
> > +int exynos_usb_audio_exit(void)
> > +{
> > +	/* future use */
> 
> What needs to be done here?
> 
It needs kfree. I wil ladd it to the next patch.
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(exynos_usb_audio_exit);
> 
> Why did you export a function that does nothing?
> 
I'll fix it.
> > +
> > +MODULE_AUTHOR("Jaehun Jung <jh0801.jung@samsung.com>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Exynos USB Audio offloading driver");
> > +
> > diff --git a/sound/usb/exynos_usb_audio.h b/sound/usb/exynos_usb_audio.h
> > new file mode 100644
> > index 0000000..13707744
> > --- /dev/null
> > +++ b/sound/usb/exynos_usb_audio.h
> > @@ -0,0 +1,150 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*   USB Audio Driver for Exynos
> > + *
> > + *   Copyright (c) 2017 by Kyounghye Yun <k-hye.yun@samsung.com>
> > + *
> > + */
> > +
> > +#ifndef __LINUX_USB_EXYNOS_AUDIO_H
> > +#define __LINUX_USB_EXYNOS_AUDIO_H
> > +
> > +#include "../../../sound/usb/usbaudio.h"
> > +
> > +/* for KM */
> > +
> > +#define USB_AUDIO_MEM_BASE	0xC0000000
> > +
> > +#define USB_AUDIO_SAVE_RESTORE	(USB_AUDIO_MEM_BASE)
> > +#define USB_AUDIO_DEV_CTX	(USB_AUDIO_SAVE_RESTORE+PAGE_SIZE)
> > +#define USB_AUDIO_INPUT_CTX	(USB_AUDIO_DEV_CTX+PAGE_SIZE)
> > +#define USB_AUDIO_OUT_DEQ	(USB_AUDIO_INPUT_CTX+PAGE_SIZE)
> > +#define USB_AUDIO_IN_DEQ	(USB_AUDIO_OUT_DEQ+PAGE_SIZE)
> > +#define USB_AUDIO_FBOUT_DEQ	(USB_AUDIO_IN_DEQ+PAGE_SIZE)
> > +#define USB_AUDIO_FBIN_DEQ	(USB_AUDIO_FBOUT_DEQ+PAGE_SIZE)
> > +#define USB_AUDIO_ERST		(USB_AUDIO_FBIN_DEQ+PAGE_SIZE)
> > +#define USB_AUDIO_DESC		(USB_AUDIO_ERST+PAGE_SIZE)
> > +#define USB_AUDIO_PCM_OUTBUF	(USB_AUDIO_MEM_BASE+0x100000)
> > +#define USB_AUDIO_PCM_INBUF	(USB_AUDIO_MEM_BASE+0x800000)
> > +
> > +#if defined(CONFIG_SOC_S5E9815)
> > +#define USB_AUDIO_XHCI_BASE	0x12210000
> > +#define USB_URAM_BASE		0x122a0000
> > +#define USB_URAM_SIZE		(SZ_1K * 24)
> > +#elif defined(CONFIG_SOC_S5E9935)
> > +#define USB_AUDIO_XHCI_BASE	0x10B00000
> > +#define USB_URAM_BASE		0x02a00000
> > +#define USB_URAM_SIZE		(SZ_1K * 24)
> 
> Shouldn't this information come from a DT file?
> 
>
Okay, I'll fix it.
> > +#else
> > +#error
> 
> Error what?
> 
I'll fix it
> > +#endif
> > +
> > +#define USB_AUDIO_CONNECT		(1 << 0)
> > +#define USB_AUDIO_REMOVING		(1 << 1)
> > +#define USB_AUDIO_DISCONNECT		(1 << 2)
> > +#define USB_AUDIO_TIMEOUT_PROBE	(1 << 3)
> 
> BIT()?
> 
Okay, I'll fix it.
> > +
> > +#define DISCONNECT_TIMEOUT	(500)
> > +
> > +#define AUDIO_MODE_NORMAL		0
> > +#define AUDIO_MODE_RINGTONE		1
> > +#define AUDIO_MODE_IN_CALL		2
> > +#define AUDIO_MODE_IN_COMMUNICATION	3
> > +#define AUDIO_MODE_CALL_SCREEN		4
> > +
> > +#define	CALL_INTERVAL_THRESHOLD		3
> > +
> > +#define USB_AUDIO_CONNECT		(1 << 0)
> > +#define USB_AUDIO_REMOVING		(1 << 1)
> > +#define USB_AUDIO_DISCONNECT		(1 << 2)
> > +#define USB_AUDIO_TIMEOUT_PROBE	(1 << 3)
> > +
> > +#define DISCONNECT_TIMEOUT	(500)
> 
> What units is this in?
> 
I'll fix it.
> > +
> > +struct host_data {
> > +	dma_addr_t out_data_dma;
> > +	dma_addr_t in_data_dma;
> > +	void *out_data_addr;
> > +	void *in_data_addr;
> 
> Why void pointers?  io pointers?
> 
I'll fix it.
> Again, does this code even work?
> 
> confused,
> 
> greg k-h
> 
This file shows the user functions using the audio vendor hooking interface.
Each function uses the AUDIO_IPC defined in the exynos audio module.
Since the exynos audio module is not in the mainline, the contents
performed in each function are writeen in TODO.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1 3/3] sound: usb: Exynos usb audio offloading driver

[Oh Eomji]

[-- Attachment #1: Type: text/plain, Size: 1924 bytes --]

On Fri, Mar 25, 2022 at 07:44:31PM +0100, Krzysztof Kozlowski wrote:
> On 24/03/2022 09:10, Oh Eomji wrote:
> > This is for usb audio offloading. usb audio offloading processes usb
> > audio stream data directly from the audio box even if ap usb enters
> > suspend, there is no problem in stream data transmission. This obtains
> > power saving effect while using usb audio device.
> > 
> > AP usb and audio usb f/w communicate via AUDIO IPC. By performing AUDIO
> > IPC in the vendor operations, abox can access and control the xhci to
> > transmit the data directly.
> > 
> > The types of commands and data delivered through AUDIO IPC include the
> > following (AP USB <-> AUDIO USB f/w) :
> > 1. usb audio connection/disconnection status
> > 2. xhci memory information
> > 3. full descriptor for usb audio device
> > 4. UAC(usb audio class) control command
> > 
> > Signed-off-by: Oh Eomji <eomji.oh@samsung.com>
> > ---
> >  sound/usb/Kconfig            |   9 +
> >  sound/usb/Makefile           |   2 +
> >  sound/usb/exynos_usb_audio.c | 560 +++++++++++++++++++++++++++++++++++++++++++
> >  sound/usb/exynos_usb_audio.h | 150 ++++++++++++
> >  4 files changed, 721 insertions(+)
> >  create mode 100644 sound/usb/exynos_usb_audio.c
> >  create mode 100644 sound/usb/exynos_usb_audio.h
> 
> Similar pattern as XHCI submission - it looks like you do not work on
> mainline kernel, but some other, private tree with other modifications.
> It seems you created this series based on that private tree.
> 
> This is not the proper process.
> 
> Please rebase all your work on recent mainline kernel (v5.18-rc1,
> linux-next) and work there.
> 
> Then you can start using get_maintainers.pl...
> 
> BTW, this v2, not v1, so please version it correctly. Add also changelog
> to your series in cover letter.
> 
> Best regards,
> Krzysztof
>
Okay, I will rebase and modify the patch version in the next patch.

Thanks,
Eomji Oh

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]