Re: [alsa-devel] [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers
On Tue, 03 Apr 2018 19:13:51 +0200, Andrew Chant wrote:
On Tue, Apr 3, 2018 at 8:48 AM, Takashi Iwai tiwai@suse.de wrote:
There are lots of open-coded functions to find a clock source, selector and multiplier. Now there are both v2 and v3, so six variants.
This patch refactors the code to use a common helper for the main loop, and define each validator function for each target. There is no functional change.
Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support") Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/clock.c | 127 +++++++++++++++++++++++------------------------------- 1 file changed, 53 insertions(+), 74 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index ab39ccb974c6..c5f0cf532c0c 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -35,105 +35,84 @@ #include "clock.h" #include "quirks.h"
-static struct uac_clock_source_descriptor *
snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
int clock_id)
+static void *find_descriptor(struct usb_host_interface *iface, int id,
bool (*validator)(void *, int), u8 type)
this comment isn't very important, and might be wrong: find_descriptor is a very generic name, it might make grepping a little confusing. Could you use find_clock_descriptor? find_uac_clock_descriptor?
Makes sense. Now I renamed it with a bit shorter one, find_uac_clock_desc(). The revised patch is below.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH v2 1/2] ALSA: usb-audio: Refactor clock finder helpers
There are lots of open-coded functions to find a clock source, selector and multiplier. Now there are both v2 and v3, so six variants.
This patch refactors the code to use a common helper for the main loop, and define each validator function for each target. There is no functional change.
Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support") Signed-off-by: Takashi Iwai tiwai@suse.de --- v1->v2: rename find_descriptor() with find_uac_clock_desc()
sound/usb/clock.c | 127 +++++++++++++++++++++++------------------------------- 1 file changed, 53 insertions(+), 74 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index ab39ccb974c6..27c2275a2505 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -35,105 +35,84 @@ #include "clock.h" #include "quirks.h"
-static struct uac_clock_source_descriptor * - snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface, - int clock_id) +static void *find_uac_clock_desc(struct usb_host_interface *iface, int id, + bool (*validator)(void *, int), u8 type) { - struct uac_clock_source_descriptor *cs = NULL; + void *cs = NULL;
- while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra, - ctrl_iface->extralen, - cs, UAC2_CLOCK_SOURCE))) { - if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) + while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen, + cs, type))) { + if (validator(cs, id)) return cs; }
return NULL; }
-static struct uac3_clock_source_descriptor * - snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface, - int clock_id) +static bool validate_clock_source_v2(void *p, int id) { - struct uac3_clock_source_descriptor *cs = NULL; - - while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra, - ctrl_iface->extralen, - cs, UAC3_CLOCK_SOURCE))) { - if (cs->bClockID == clock_id) - return cs; - } - - return NULL; + struct uac_clock_source_descriptor *cs = p; + return cs->bLength >= sizeof(*cs) && cs->bClockID == id; }
-static struct uac_clock_selector_descriptor * - snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface, - int clock_id) +static bool validate_clock_source_v3(void *p, int id) { - struct uac_clock_selector_descriptor *cs = NULL; - - while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra, - ctrl_iface->extralen, - cs, UAC2_CLOCK_SELECTOR))) { - if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) { - if (cs->bLength < 5 + cs->bNrInPins) - return NULL; - return cs; - } - } - - return NULL; + struct uac3_clock_source_descriptor *cs = p; + return cs->bClockID == id; }
-static struct uac3_clock_selector_descriptor * - snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface, - int clock_id) +static bool validate_clock_selector_v2(void *p, int id) { - struct uac3_clock_selector_descriptor *cs = NULL; - - while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra, - ctrl_iface->extralen, - cs, UAC3_CLOCK_SELECTOR))) { - if (cs->bClockID == clock_id) - return cs; - } - - return NULL; + struct uac_clock_selector_descriptor *cs = p; + return cs->bLength >= sizeof(*cs) && cs->bClockID == id && + cs->bLength >= 5 + cs->bNrInPins; }
-static struct uac_clock_multiplier_descriptor * - snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface, - int clock_id) +static bool validate_clock_selector_v3(void *p, int id) { - struct uac_clock_multiplier_descriptor *cs = NULL; - - while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra, - ctrl_iface->extralen, - cs, UAC2_CLOCK_MULTIPLIER))) { - if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) - return cs; - } - - return NULL; + struct uac3_clock_selector_descriptor *cs = p; + return cs->bClockID == id; }
-static struct uac3_clock_multiplier_descriptor * - snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface, - int clock_id) +static bool validate_clock_multiplier_v2(void *p, int id) { - struct uac3_clock_multiplier_descriptor *cs = NULL; + struct uac_clock_multiplier_descriptor *cs = p; + return cs->bLength >= sizeof(*cs) && cs->bClockID == id; +}
- while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra, - ctrl_iface->extralen, - cs, UAC3_CLOCK_MULTIPLIER))) { - if (cs->bClockID == clock_id) - return cs; - } +static bool validate_clock_multiplier_v3(void *p, int id) +{ + struct uac3_clock_multiplier_descriptor *cs = p; + return cs->bClockID == id; +}
- return NULL; +#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_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); + +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); + +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); + static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id) { unsigned char buf;
On Tue, Apr 3, 2018 at 9:02 PM, Takashi Iwai tiwai@suse.de wrote:
On Tue, 03 Apr 2018 19:13:51 +0200, Andrew Chant wrote:
On Tue, Apr 3, 2018 at 8:48 AM, Takashi Iwai tiwai@suse.de wrote:
There are lots of open-coded functions to find a clock source, selector and multiplier. Now there are both v2 and v3, so six variants.
This patch refactors the code to use a common helper for the main loop, and define each validator function for each target. There is no functional change.
Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support") Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/clock.c | 127 +++++++++++++++++++++++------------------------------- 1 file changed, 53 insertions(+), 74 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index ab39ccb974c6..c5f0cf532c0c 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -35,105 +35,84 @@ #include "clock.h" #include "quirks.h"
-static struct uac_clock_source_descriptor *
snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
int clock_id)
+static void *find_descriptor(struct usb_host_interface *iface, int id,
bool (*validator)(void *, int), u8 type)
this comment isn't very important, and might be wrong: find_descriptor is a very generic name, it might make grepping a little confusing. Could you use find_clock_descriptor? find_uac_clock_descriptor?
Makes sense. Now I renamed it with a bit shorter one, find_uac_clock_desc(). The revised patch is below.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH v2 1/2] ALSA: usb-audio: Refactor clock finder helpers
There are lots of open-coded functions to find a clock source, selector and multiplier. Now there are both v2 and v3, so six variants.
This patch refactors the code to use a common helper for the main loop, and define each validator function for each target. There is no functional change.
Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support") Signed-off-by: Takashi Iwai tiwai@suse.de
Good improvement.
Reviewed-by: Ruslan Bilovol ruslan.bilovol@gmail.com
v1->v2: rename find_descriptor() with find_uac_clock_desc()
sound/usb/clock.c | 127 +++++++++++++++++++++++------------------------------- 1 file changed, 53 insertions(+), 74 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index ab39ccb974c6..27c2275a2505 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -35,105 +35,84 @@ #include "clock.h" #include "quirks.h"
-static struct uac_clock_source_descriptor *
snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
int clock_id)
+static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
bool (*validator)(void *, int), u8 type)
{
struct uac_clock_source_descriptor *cs = NULL;
void *cs = NULL;
while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
ctrl_iface->extralen,
cs, UAC2_CLOCK_SOURCE))) {
if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen,
cs, type))) {
if (validator(cs, id)) return cs; } return NULL;
}
-static struct uac3_clock_source_descriptor *
snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
int clock_id)
+static bool validate_clock_source_v2(void *p, int id) {
struct uac3_clock_source_descriptor *cs = NULL;
while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
ctrl_iface->extralen,
cs, UAC3_CLOCK_SOURCE))) {
if (cs->bClockID == clock_id)
return cs;
}
return NULL;
struct uac_clock_source_descriptor *cs = p;
return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
}
-static struct uac_clock_selector_descriptor *
snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
int clock_id)
+static bool validate_clock_source_v3(void *p, int id) {
struct uac_clock_selector_descriptor *cs = NULL;
while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
ctrl_iface->extralen,
cs, UAC2_CLOCK_SELECTOR))) {
if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) {
if (cs->bLength < 5 + cs->bNrInPins)
return NULL;
return cs;
}
}
return NULL;
struct uac3_clock_source_descriptor *cs = p;
return cs->bClockID == id;
}
-static struct uac3_clock_selector_descriptor *
snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
int clock_id)
+static bool validate_clock_selector_v2(void *p, int id) {
struct uac3_clock_selector_descriptor *cs = NULL;
while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
ctrl_iface->extralen,
cs, UAC3_CLOCK_SELECTOR))) {
if (cs->bClockID == clock_id)
return cs;
}
return NULL;
struct uac_clock_selector_descriptor *cs = p;
return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
cs->bLength >= 5 + cs->bNrInPins;
}
-static struct uac_clock_multiplier_descriptor *
snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
int clock_id)
+static bool validate_clock_selector_v3(void *p, int id) {
struct uac_clock_multiplier_descriptor *cs = NULL;
while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
ctrl_iface->extralen,
cs, UAC2_CLOCK_MULTIPLIER))) {
if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
return cs;
}
return NULL;
struct uac3_clock_selector_descriptor *cs = p;
return cs->bClockID == id;
}
-static struct uac3_clock_multiplier_descriptor *
snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
int clock_id)
+static bool validate_clock_multiplier_v2(void *p, int id) {
struct uac3_clock_multiplier_descriptor *cs = NULL;
struct uac_clock_multiplier_descriptor *cs = p;
return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
+}
while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
ctrl_iface->extralen,
cs, UAC3_CLOCK_MULTIPLIER))) {
if (cs->bClockID == clock_id)
return cs;
}
+static bool validate_clock_multiplier_v3(void *p, int id) +{
struct uac3_clock_multiplier_descriptor *cs = p;
return cs->bClockID == id;
+}
return NULL;
+#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_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);
+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);
+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);
static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id) { unsigned char buf; -- 2.16.2
participants (2)
-
Ruslan Bilovol
-
Takashi Iwai