Re: [PATCH v3 04/28] ASoC: Add SOC USB APIs for adding an USB backend

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Mar 08, 2023 at 03:57:27PM -0800, Wesley Cheng wrote:
> Some platforms may have support for offloading USB audio devices to a
> dedicated audio DSP.  Introduce a set of APIs that allow for management of
> USB sound card and PCM devices enumerated by the USB SND class driver.
> This allows for the ASoC components to be aware of what USB devices are
> available for offloading.
> 
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
>  include/sound/soc-usb.h |  35 ++++++++
>  sound/soc/Makefile      |   2 +-
>  sound/soc/soc-usb.c     | 180 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 216 insertions(+), 1 deletion(-)
>  create mode 100644 include/sound/soc-usb.h
>  create mode 100644 sound/soc/soc-usb.c
> 
> diff --git a/include/sound/soc-usb.h b/include/sound/soc-usb.h
> new file mode 100644
> index 000000000000..378992ea07bd
> --- /dev/null
> +++ b/include/sound/soc-usb.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef __LINUX_SND_SOC_USB_H
> +#define __LINUX_SND_SOC_USB_H
> +
> +/**
> + * struct snd_soc_usb
> + * @list - list head for SND SOC struct list
> + * @dev - USB backend device reference
> + * @component - reference to DAPM component
> + * @connection_status_cb - callback to notify connection events
> + * @priv_data - driver data
> + **/
> +struct snd_soc_usb {
> +	struct list_head list;
> +	struct device *dev;

If this is a USB device, then make it a pointer to the real structure,
not just struct device.


> +	struct snd_soc_component *component;
> +	int (*connection_status_cb)(struct snd_soc_usb *usb, int card_idx,
> +				int connected);
> +	void *priv_data;
> +};
> +
> +int snd_soc_usb_connect(struct device *usbdev, int card_idx);
> +int snd_soc_usb_disconnect(struct device *usbdev);
> +void snd_soc_usb_set_priv_data(struct device *dev, void *priv);
> +void *snd_soc_usb_get_priv_data(struct device *usbdev);

Same here, you mix "dev" and "usbdev" in the names, make them real USB
devices please.


> +
> +struct snd_soc_usb *snd_soc_usb_add_port(struct device *dev, void *priv,

And here.

> +			int (*connection_cb)(struct snd_soc_usb *usb, int card_idx,
> +			int connected));
> +int snd_soc_usb_remove_port(struct device *dev);
> +#endif
> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
> index 507eaed1d6a1..3305ceb59d84 100644
> --- a/sound/soc/Makefile
> +++ b/sound/soc/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-utils.o soc-dai.o soc-component.o
> +snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-usb.o soc-utils.o soc-dai.o soc-component.o
>  snd-soc-core-objs += soc-pcm.o soc-devres.o soc-ops.o soc-link.o soc-card.o
>  snd-soc-core-$(CONFIG_SND_SOC_COMPRESS) += soc-compress.o
>  
> diff --git a/sound/soc/soc-usb.c b/sound/soc/soc-usb.c
> new file mode 100644
> index 000000000000..4293451cdd49
> --- /dev/null
> +++ b/sound/soc/soc-usb.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#include <linux/of.h>
> +#include <linux/usb.h>
> +#include <sound/soc.h>
> +#include <sound/soc-usb.h>
> +#include "../usb/card.h"
> +
> +static DEFINE_MUTEX(ctx_mutex);
> +static LIST_HEAD(usb_ctx_list);
> +
> +static struct device_node *snd_soc_find_phandle(struct device *dev)
> +{
> +	struct device_node *node;
> +
> +	node = of_parse_phandle(dev->of_node, "usb-soc-be", 0);
> +	if (!node)
> +		return ERR_PTR(-ENODEV);
> +
> +	return node;
> +}
> +
> +static struct snd_soc_usb *snd_soc_find_usb_ctx(struct device *dev)
> +{
> +	struct device_node *node;
> +	struct snd_soc_usb *ctx = NULL;
> +
> +	node = snd_soc_find_phandle(dev);
> +	if (IS_ERR(node))
> +		return NULL;
> +
> +	mutex_lock(&ctx_mutex);
> +	list_for_each_entry(ctx, &usb_ctx_list, list) {
> +		if (ctx->dev->of_node == node) {
> +			of_node_put(node);
> +			mutex_unlock(&ctx_mutex);
> +			return ctx;
> +		}
> +	}
> +	of_node_put(node);
> +	mutex_unlock(&ctx_mutex);
> +
> +	return NULL;
> +}
> +
> +/**
> + * snd_soc_usb_get_priv_data() - Retrieve private data stored
> + * @dev: device reference
> + *
> + * Fetch the private data stored in the USB SND SOC structure.
> + *
> + */
> +void *snd_soc_usb_get_priv_data(struct device *dev)
> +{
> +	struct snd_soc_usb *ctx;
> +
> +	ctx = snd_soc_find_usb_ctx(dev);
> +	if (!ctx) {
> +		/* Check if backend device */
> +		list_for_each_entry(ctx, &usb_ctx_list, list) {
> +			if (dev->of_node == ctx->dev->of_node)
> +				goto out;

No locking for this list traversal?

thanks,

greg k-h