All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: alsa-devel@alsa-project.org
Subject: [PATCH 1/2] ALSA: usb-audio: Refactoring UAC2/3 clock setup code
Date: Tue, 18 May 2021 17:21:11 +0200	[thread overview]
Message-ID: <20210518152112.8016-2-tiwai@suse.de> (raw)
In-Reply-To: <20210518152112.8016-1-tiwai@suse.de>

This patch just does refactoring of the UAC2/3 clock setup code.
There should be no functional changes.  The major changes are:

* Provide union objects for pointing both UAC2 and UAC3 objects
* Unify clock source, selector and multiplier helper functions
* Unify __uac_clock_find_source() to deal with both UAC2 and UAC3
  equally

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/clock.c | 285 ++++++++++++++--------------------------------
 1 file changed, 85 insertions(+), 200 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 17bbde73d4d1..48a79f1b6233 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -21,82 +21,77 @@
 #include "clock.h"
 #include "quirks.h"
 
+union uac23_clock_source_desc {
+	struct uac_clock_source_descriptor v2;
+	struct uac3_clock_source_descriptor v3;
+};
+
+union uac23_clock_selector_desc {
+	struct uac_clock_selector_descriptor v2;
+	struct uac3_clock_selector_descriptor v3;
+};
+
+union uac23_clock_multiplier_desc {
+	struct uac_clock_multiplier_descriptor v2;
+	struct uac_clock_multiplier_descriptor v3;
+};
+
+#define GET_VAL(p, proto, field) \
+	((proto) == UAC_VERSION_3 ? (p)->v3.field : (p)->v2.field)
+
 static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
-				 bool (*validator)(void *, int), u8 type)
+				 bool (*validator)(void *, int, int),
+				 u8 type, int proto)
 {
 	void *cs = NULL;
 
 	while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen,
 					     cs, type))) {
-		if (validator(cs, id))
+		if (validator(cs, id, proto))
 			return cs;
 	}
 
 	return NULL;
 }
 
-static bool validate_clock_source_v2(void *p, int id)
+static bool validate_clock_source(void *p, int id, int proto)
 {
-	struct uac_clock_source_descriptor *cs = p;
-	return cs->bClockID == id;
-}
+	union uac23_clock_source_desc *cs = p;
 
-static bool validate_clock_source_v3(void *p, int id)
-{
-	struct uac3_clock_source_descriptor *cs = p;
-	return cs->bClockID == id;
+	return GET_VAL(cs, proto, bClockID) == id;
 }
 
-static bool validate_clock_selector_v2(void *p, int id)
+static bool validate_clock_selector(void *p, int id, int proto)
 {
-	struct uac_clock_selector_descriptor *cs = p;
-	return cs->bClockID == id;
-}
+	union uac23_clock_selector_desc *cs = p;
 
-static bool validate_clock_selector_v3(void *p, int id)
-{
-	struct uac3_clock_selector_descriptor *cs = p;
-	return cs->bClockID == id;
+	return GET_VAL(cs, proto, bClockID) == id;
 }
 
-static bool validate_clock_multiplier_v2(void *p, int id)
+static bool validate_clock_multiplier(void *p, int id, int proto)
 {
-	struct uac_clock_multiplier_descriptor *cs = p;
-	return cs->bClockID == id;
-}
+	union uac23_clock_multiplier_desc *cs = p;
 
-static bool validate_clock_multiplier_v3(void *p, int id)
-{
-	struct uac3_clock_multiplier_descriptor *cs = p;
-	return cs->bClockID == id;
+	return GET_VAL(cs, proto, bClockID) == id;
 }
 
-#define DEFINE_FIND_HELPER(name, obj, validator, type)		\
-static obj *name(struct usb_host_interface *iface, int id)	\
-{								\
-	return find_uac_clock_desc(iface, id, validator, type);	\
+#define DEFINE_FIND_HELPER(name, obj, validator, type2, type3)		\
+static obj *name(struct snd_usb_audio *chip, int id, int proto)	\
+{									\
+	return find_uac_clock_desc(chip->ctrl_intf, id, validator,	\
+				   proto == UAC_VERSION_3 ? (type3) : (type2), \
+				   proto);				\
 }
 
 DEFINE_FIND_HELPER(snd_usb_find_clock_source,
-		   struct uac_clock_source_descriptor,
-		   validate_clock_source_v2, UAC2_CLOCK_SOURCE);
-DEFINE_FIND_HELPER(snd_usb_find_clock_source_v3,
-		   struct uac3_clock_source_descriptor,
-		   validate_clock_source_v3, UAC3_CLOCK_SOURCE);
-
+		   union uac23_clock_source_desc, validate_clock_source,
+		   UAC2_CLOCK_SOURCE, UAC3_CLOCK_SOURCE);
 DEFINE_FIND_HELPER(snd_usb_find_clock_selector,
-		   struct uac_clock_selector_descriptor,
-		   validate_clock_selector_v2, UAC2_CLOCK_SELECTOR);
-DEFINE_FIND_HELPER(snd_usb_find_clock_selector_v3,
-		   struct uac3_clock_selector_descriptor,
-		   validate_clock_selector_v3, UAC3_CLOCK_SELECTOR);
-
+		   union uac23_clock_selector_desc, validate_clock_selector,
+		   UAC2_CLOCK_SELECTOR, UAC3_CLOCK_SELECTOR);
 DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier,
-		   struct uac_clock_multiplier_descriptor,
-		   validate_clock_multiplier_v2, UAC2_CLOCK_MULTIPLIER);
-DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier_v3,
-		   struct uac3_clock_multiplier_descriptor,
-		   validate_clock_multiplier_v3, UAC3_CLOCK_MULTIPLIER);
+		   union uac23_clock_multiplier_desc, validate_clock_multiplier,
+		   UAC2_CLOCK_MULTIPLIER, UAC3_CLOCK_MULTIPLIER);
 
 static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
 {
@@ -159,14 +154,13 @@ static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip,
 	int count;
 	unsigned char data;
 	struct usb_device *dev = chip->dev;
+	union uac23_clock_source_desc *cs_desc;
 
-	if (fmt->protocol == UAC_VERSION_2) {
-		struct uac_clock_source_descriptor *cs_desc =
-			snd_usb_find_clock_source(chip->ctrl_intf, source_id);
-
-		if (!cs_desc)
-			return false;
+	cs_desc = snd_usb_find_clock_source(chip, source_id, fmt->protocol);
+	if (!cs_desc)
+		return false;
 
+	if (fmt->protocol == UAC_VERSION_2) {
 		/*
 		 * Assume the clock is valid if clock source supports only one
 		 * single sample rate, the terminal is connected directly to it
@@ -175,8 +169,8 @@ static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip,
 		 * reports that clock is invalid.
 		 */
 		if (fmt->nr_rates == 1 &&
-		    (fmt->clock & 0xff) == cs_desc->bClockID &&
-		    (cs_desc->bmAttributes & 0x3) !=
+		    (fmt->clock & 0xff) == cs_desc->v2.bClockID &&
+		    (cs_desc->v2.bmAttributes & 0x3) !=
 				UAC_CLOCK_SOURCE_TYPE_EXT)
 			return true;
 	}
@@ -222,22 +216,16 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip,
 	unsigned char data;
 	struct usb_device *dev = chip->dev;
 	u32 bmControls;
+	union uac23_clock_source_desc *cs_desc;
 
-	if (fmt->protocol == UAC_VERSION_3) {
-		struct uac3_clock_source_descriptor *cs_desc =
-			snd_usb_find_clock_source_v3(chip->ctrl_intf, source_id);
-
-		if (!cs_desc)
-			return false;
-		bmControls = le32_to_cpu(cs_desc->bmControls);
-	} else { /* UAC_VERSION_1/2 */
-		struct uac_clock_source_descriptor *cs_desc =
-			snd_usb_find_clock_source(chip->ctrl_intf, source_id);
+	cs_desc = snd_usb_find_clock_source(chip, source_id, fmt->protocol);
+	if (!cs_desc)
+		return false;
 
-		if (!cs_desc)
-			return false;
-		bmControls = cs_desc->bmControls;
-	}
+	if (fmt->protocol == UAC_VERSION_3)
+		bmControls = le32_to_cpu(cs_desc->v3.bmControls);
+	else
+		bmControls = cs_desc->v2.bmControls;
 
 	/* If a clock source can't tell us whether it's valid, we assume it is */
 	if (!uac_v2v3_control_is_readable(bmControls,
@@ -267,9 +255,12 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 				   const struct audioformat *fmt, int entity_id,
 				   unsigned long *visited, bool validate)
 {
-	struct uac_clock_source_descriptor *source;
-	struct uac_clock_selector_descriptor *selector;
-	struct uac_clock_multiplier_descriptor *multiplier;
+	union uac23_clock_source_desc *source;
+	union uac23_clock_selector_desc *selector;
+	union uac23_clock_multiplier_desc *multiplier;
+	int ret, i, cur, err, pins, clock_id;
+	const u8 *sources;
+	int proto = fmt->protocol;
 
 	entity_id &= 0xff;
 
@@ -281,9 +272,9 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	}
 
 	/* first, see if the ID we're looking for is a clock source already */
-	source = snd_usb_find_clock_source(chip->ctrl_intf, entity_id);
+	source = snd_usb_find_clock_source(chip, entity_id, proto);
 	if (source) {
-		entity_id = source->bClockID;
+		entity_id = GET_VAL(source, proto, bClockID);
 		if (validate && !uac_clock_source_is_valid(chip, fmt,
 								entity_id)) {
 			usb_audio_err(chip,
@@ -294,27 +285,29 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 		return entity_id;
 	}
 
-	selector = snd_usb_find_clock_selector(chip->ctrl_intf, entity_id);
+	selector = snd_usb_find_clock_selector(chip, entity_id, proto);
 	if (selector) {
-		int ret, i, cur, err;
+		pins = GET_VAL(selector, proto, bNrInPins);
+		clock_id = GET_VAL(selector, proto, bClockID);
+		sources = GET_VAL(selector, proto, baCSourceID);
 
-		if (selector->bNrInPins == 1) {
+		if (pins == 1) {
 			ret = 1;
 			goto find_source;
 		}
 
 		/* the entity ID we are looking for is a selector.
 		 * find out what it currently selects */
-		ret = uac_clock_selector_get_val(chip, selector->bClockID);
+		ret = uac_clock_selector_get_val(chip, clock_id);
 		if (ret < 0)
 			return ret;
 
 		/* Selector values are one-based */
 
-		if (ret > selector->bNrInPins || ret < 1) {
+		if (ret > pins || ret < 1) {
 			usb_audio_err(chip,
 				"%s(): selector reported illegal value, id %d, ret %d\n",
-				__func__, selector->bClockID, ret);
+				__func__, clock_id, ret);
 
 			return -EINVAL;
 		}
@@ -322,7 +315,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	find_source:
 		cur = ret;
 		ret = __uac_clock_find_source(chip, fmt,
-					      selector->baCSourceID[ret - 1],
+					      sources[ret - 1],
 					      visited, validate);
 		if (ret > 0) {
 			err = uac_clock_selector_set_val(chip, entity_id, cur);
@@ -334,12 +327,12 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 			return ret;
 
 		/* The current clock source is invalid, try others. */
-		for (i = 1; i <= selector->bNrInPins; i++) {
+		for (i = 1; i <= pins; i++) {
 			if (i == cur)
 				continue;
 
 			ret = __uac_clock_find_source(chip, fmt,
-						      selector->baCSourceID[i - 1],
+						      sources[i - 1],
 						      visited, true);
 			if (ret < 0)
 				continue;
@@ -358,112 +351,11 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	}
 
 	/* FIXME: multipliers only act as pass-thru element for now */
-	multiplier = snd_usb_find_clock_multiplier(chip->ctrl_intf, entity_id);
+	multiplier = snd_usb_find_clock_multiplier(chip, entity_id, proto);
 	if (multiplier)
 		return __uac_clock_find_source(chip, fmt,
-					       multiplier->bCSourceID,
-					       visited, validate);
-
-	return -EINVAL;
-}
-
-static int __uac3_clock_find_source(struct snd_usb_audio *chip,
-				    const struct audioformat *fmt, int entity_id,
-				    unsigned long *visited, bool validate)
-{
-	struct uac3_clock_source_descriptor *source;
-	struct uac3_clock_selector_descriptor *selector;
-	struct uac3_clock_multiplier_descriptor *multiplier;
-
-	entity_id &= 0xff;
-
-	if (test_and_set_bit(entity_id, visited)) {
-		usb_audio_warn(chip,
-			 "%s(): recursive clock topology detected, id %d.\n",
-			 __func__, entity_id);
-		return -EINVAL;
-	}
-
-	/* first, see if the ID we're looking for is a clock source already */
-	source = snd_usb_find_clock_source_v3(chip->ctrl_intf, entity_id);
-	if (source) {
-		entity_id = source->bClockID;
-		if (validate && !uac_clock_source_is_valid(chip, fmt,
-								entity_id)) {
-			usb_audio_err(chip,
-				"clock source %d is not valid, cannot use\n",
-				entity_id);
-			return -ENXIO;
-		}
-		return entity_id;
-	}
-
-	selector = snd_usb_find_clock_selector_v3(chip->ctrl_intf, entity_id);
-	if (selector) {
-		int ret, i, cur, err;
-
-		/* the entity ID we are looking for is a selector.
-		 * find out what it currently selects */
-		ret = uac_clock_selector_get_val(chip, selector->bClockID);
-		if (ret < 0)
-			return ret;
-
-		/* Selector values are one-based */
-
-		if (ret > selector->bNrInPins || ret < 1) {
-			usb_audio_err(chip,
-				"%s(): selector reported illegal value, id %d, ret %d\n",
-				__func__, selector->bClockID, ret);
-
-			return -EINVAL;
-		}
-
-		cur = ret;
-		ret = __uac3_clock_find_source(chip, fmt,
-					       selector->baCSourceID[ret - 1],
+					       GET_VAL(multiplier, proto, bCSourceID),
 					       visited, validate);
-		if (ret > 0) {
-			err = uac_clock_selector_set_val(chip, entity_id, cur);
-			if (err < 0)
-				return err;
-		}
-
-		if (!validate || ret > 0 || !chip->autoclock)
-			return ret;
-
-		/* The current clock source is invalid, try others. */
-		for (i = 1; i <= selector->bNrInPins; i++) {
-			int err;
-
-			if (i == cur)
-				continue;
-
-			ret = __uac3_clock_find_source(chip, fmt,
-						       selector->baCSourceID[i - 1],
-						       visited, true);
-			if (ret < 0)
-				continue;
-
-			err = uac_clock_selector_set_val(chip, entity_id, i);
-			if (err < 0)
-				continue;
-
-			usb_audio_info(chip,
-				 "found and selected valid clock source %d\n",
-				 ret);
-			return ret;
-		}
-
-		return -ENXIO;
-	}
-
-	/* FIXME: multipliers only act as pass-thru element for now */
-	multiplier = snd_usb_find_clock_multiplier_v3(chip->ctrl_intf,
-						      entity_id);
-	if (multiplier)
-		return __uac3_clock_find_source(chip, fmt,
-						multiplier->bCSourceID,
-						visited, validate);
 
 	return -EINVAL;
 }
@@ -487,10 +379,8 @@ int snd_usb_clock_find_source(struct snd_usb_audio *chip,
 
 	switch (fmt->protocol) {
 	case UAC_VERSION_2:
-		return __uac_clock_find_source(chip, fmt, fmt->clock, visited,
-					       validate);
 	case UAC_VERSION_3:
-		return __uac3_clock_find_source(chip, fmt, fmt->clock, visited,
+		return __uac_clock_find_source(chip, fmt, fmt->clock, visited,
 					       validate);
 	default:
 		return -EINVAL;
@@ -593,18 +483,13 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip,
 	u32 bmControls;
 	__le32 data;
 	int err;
+	union uac23_clock_source_desc *cs_desc;
 
-	if (fmt->protocol == UAC_VERSION_3) {
-		struct uac3_clock_source_descriptor *cs_desc;
-
-		cs_desc = snd_usb_find_clock_source_v3(chip->ctrl_intf, clock);
-		bmControls = le32_to_cpu(cs_desc->bmControls);
-	} else {
-		struct uac_clock_source_descriptor *cs_desc;
-
-		cs_desc = snd_usb_find_clock_source(chip->ctrl_intf, clock);
-		bmControls = cs_desc->bmControls;
-	}
+	cs_desc = snd_usb_find_clock_source(chip, clock, fmt->protocol);
+	if (fmt->protocol == UAC_VERSION_3)
+		bmControls = le32_to_cpu(cs_desc->v3.bmControls);
+	else
+		bmControls = cs_desc->v2.bmControls;
 
 	writeable = uac_v2v3_control_is_writeable(bmControls,
 						  UAC2_CS_CONTROL_SAM_FREQ);
-- 
2.26.2


  reply	other threads:[~2021-05-18 15:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18 15:21 [PATCH 0/2] ALSA: usb-audio: clock setup code refactoring Takashi Iwai
2021-05-18 15:21 ` Takashi Iwai [this message]
2021-05-18 15:21 ` [PATCH 2/2] ALSA: usb-audio: Handle error for the current selector gracefully Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210518152112.8016-2-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.