Re: [PATCH v7 06/33] ASoC: Add SOC USB APIs for adding an USB backend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mark,

On 9/27/2023 7:48 AM, Mark Brown wrote:
On Thu, Sep 21, 2023 at 02:48:16PM -0700, Wesley Cheng wrote:

+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);

Very nitpicky but this function possibly wants a _usb_ in the name, not
that it *super* matters with it being static.  Or it could just be
inlined into the only user and not worry about the naming at all.


Thanks for the review! Sure, let me reshuffle things around a bit and just get rid of this function entirely and inline it to the API below.

+/**
+ * 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 */
+		mutex_lock(&ctx_mutex);
+		list_for_each_entry(ctx, &usb_ctx_list, list) {
+			if (dev->of_node == ctx->dev->of_node) {
+				mutex_unlock(&ctx_mutex);
+				goto out;
+			}
+		}
+		mutex_unlock(&ctx_mutex);
+		ctx = NULL;
+	}

This seems a lot more expensive than I'd expect for a get_priv_data
operation, usually it's just a container_of() or other constant time
pulling out of a pointer rather than a linked list walk - the sort of
thing that people put at the start of functions and do all the time.
If we need this I think it needs a name that's more clearly tied to the
use case.

I didn't actually find the user of this though?

So the end user of this would be the qc_audio_offload driver, within prepare_qmi_response(). This is to fetch some information about the DPCM backend during the stream enable request.

Previously, I limited the # of snd_soc_usb ports to be registered to one, but that would affect the scalability of this layer. Hence, adding a list instead increased the complexity. Will rename this accordingly.

Thanks
Wesley Cheng




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux