LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Geraldo Nascimento <geraldogabriel@gmail.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: chihhao.chen@mediatek.com, alsa-devel@alsa-project.org,
	wsd_upstream@mediatek.com, damien@zamaudio.com, tiwai@suse.com,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	matthias.bgg@gmail.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ALSA: usb-audio: fix incorrect clock source setting
Date: Sat, 24 Jul 2021 18:42:49 +0000	[thread overview]
Message-ID: <CAEsQvcuHyaWQQKYqydgv-XhFo7byaARYFGrPmpCu8XSRgTPDTg@mail.gmail.com> (raw)
In-Reply-To: <CAEsQvcu3jXa4fXJAu7nvT+G+-S2bZ1BPkcuwnT4VjbrnW1Pkog@mail.gmail.com>

I tried to convey in code what I had in mind.

It's a rough sketch and very much untested.

--- clock.5.14-rc2.c    2021-07-24 18:30:09.773718208 -0000
+++ clock-one-to-one.c  2021-07-24 18:35:52.276412366 -0000
@@ -54,6 +54,61 @@ static void *find_uac_clock_desc(struct
        return NULL;
 }

+/* Behringer UFX1604 / UFX1204 have a simple one-to-one
+ * topology where there is only one Clock Selector, only
+ * one Clock Source linked to USB SOF and no Clock Multipliers.
+ *
+ * This function checks for the presence of such a
+ * one-to-one clock selector / clock source topology
+ * so that it's possible to safely set the one and only
+ * Clock Selector to the one and only Clock Source
+ * upon sample rate change without breaking devices
+ * with more complicated topologies.
+ */
+
+static bool one_to_one_clock_topology(struct usb_host_interface
*iface, int proto)
+{
+        int clock_sources, clock_selectors, clock_multipliers = 0;
+       int source_version, selector_version, multiplier_version;
+       int found_count;
+
+       void *cs = NULL;
+
+       if (proto == UAC_VERSION_3) {
+               source_version = UAC3_CLOCK_SOURCE;
+               selector_version = UAC3_CLOCK_SELECTOR;
+               multiplier_version = UAC3_CLOCK_MULTIPLIER;
+       }
+
+       else {
+               source_version = UAC2_CLOCK_SOURCE;
+               selector_version = UAC2_CLOCK_SELECTOR;
+               multiplier_version = UAC2_CLOCK_MULTIPLIER;
+       }
+
+        if ((found_count = snd_usb_count_csint_desc(iface->extra,
iface->extralen,
+                                             cs, source_version)) > 0) {
+               clock_sources = found_count;
+        }
+
+        if ((found_count = snd_usb_count_csint_desc(iface->extra,
iface->extralen,
+                                             cs, selector_version)) > 0) {
+               clock_selectors = found_count;
+        }
+
+        if ((found_count = snd_usb_count_csint_desc(iface->extra,
iface->extralen,
+                                             cs, multiplier_version)) > 0) {
+               clock_multipliers = found_count;
+        }
+
+       if ((clock_sources == 1) && (clock_selectors == 1) &&
(clock_multipliers == 0)) {
+               return true;
+       }
+
+       return false;
+}
+
+
 static bool validate_clock_source(void *p, int id, int proto)
 {
        union uac23_clock_source_desc *cs = p;
@@ -323,7 +378,7 @@ static int __uac_clock_find_source(struc
                ret = __uac_clock_find_source(chip, fmt,
                                              sources[ret - 1],
                                              visited, validate);
-               if (ret > 0) {
+               if (ret > 0 &&
one_to_one_clock_topology(chip->ctrl_intf, proto)) {
                        err = uac_clock_selector_set_val(chip, entity_id, cur);
                        if (err < 0)
                                return err;



--- helper.5.14-rc2.c   2021-07-24 18:30:25.042526253 -0000
+++ helper-one-to-one.c 2021-07-24 18:35:45.019503597 -0000
@@ -64,6 +64,29 @@ void *snd_usb_find_csint_desc(void *buff
 }

 /*
+ * find every class-specified interface descriptor with the given subtype
+ * and return how many did it find
+ */
+int snd_usb_count_csint_desc(void *buffer, int buflen, void *after,
u8 dsubtype)
+{
+       int count = 0;
+        unsigned char *p = after;
+
+        while ((p = snd_usb_find_desc(buffer, buflen, p,
+                                      USB_DT_CS_INTERFACE)) != NULL) {
+                if (p[0] >= 3 && p[2] == dsubtype)
+                        count++;
+        }
+
+       if (count > 0) {
+               return count;
+       }
+
+        return 0;
+}
+
+
+/*
  * Wrapper for usb_control_msg().
  * Allocates a temp buffer to prevent dmaing from/to the stack.
  */



--- helper.5.14-rc2.h   2021-07-24 18:30:35.219398312 -0000
+++ helper-one-to-one.h 2021-07-24 18:29:34.139166195 -0000
@@ -7,6 +7,8 @@ unsigned int snd_usb_combine_bytes(unsig
 void *snd_usb_find_desc(void *descstart, int desclen, void *after, u8 dtype);
 void *snd_usb_find_csint_desc(void *descstart, int desclen, void
*after, u8 dsubtype);

+int snd_usb_count_csint_desc(void *descstart, int desclen, void
*after, u8 dsubtype);
+
 int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe,
                    __u8 request, __u8 requesttype, __u16 value, __u16 index,
                    void *data, __u16 size);

On Sat, Jul 24, 2021 at 3:20 PM Geraldo Nascimento
<geraldogabriel@gmail.com> wrote:
>
> > Dr. Iwai, perhaps we could restrict the generalized fix for the
> > Behringer UFX1604 / UFX1204 with some simple logic to devices that
> > only have *one* clock source.
>
> Okay, rereading the original commit log from Cihhao Chen I gather
> Samsung USBC Headset (AKG) with VID/PID (0x04e8/0xa051) actually has
> two clock selectors and only one clock source.
>
> Correct me if I'm wrong.
>
> This is complicated by the fact I haven't been able to find a lsusb -v
> of Samsung USBC Headset (AKG) with VID/PID (0x04e8/0xa051)
>
> Even so, my proposition still stands: devices with only one clock
> source and only one clock selector should be able to handle us
> selecting the clock selector to the only clock source.

  reply	other threads:[~2021-07-24 21:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-24  4:23 chihhao.chen
2021-07-24  8:04 ` Takashi Iwai
2021-07-24 15:04   ` Geraldo Nascimento
2021-07-24 15:20     ` Geraldo Nascimento
2021-07-24 18:42       ` Geraldo Nascimento [this message]
2021-07-25  7:44     ` Takashi Iwai
2021-07-26  2:16       ` Geraldo Nascimento
2021-07-26  8:42         ` chihhao chen
2021-07-26 20:57           ` Geraldo Nascimento
2021-07-27 10:27             ` chihhao.chen
2021-07-27 17:56               ` Geraldo Nascimento
2021-07-28  1:19                 ` Geraldo Nascimento
2021-08-05  7:54             ` chihhao.chen
2021-08-05 15:50               ` Geraldo Nascimento
     [not found]                 ` <CAEsQvcvJeAXoVE9FE9vsKNvXMaQYgHZBoPyKfZLT=UA-4BMe_Q@mail.gmail.com>
2021-08-05 22:03                   ` Geraldo Nascimento

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=CAEsQvcuHyaWQQKYqydgv-XhFo7byaARYFGrPmpCu8XSRgTPDTg@mail.gmail.com \
    --to=geraldogabriel@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=chihhao.chen@mediatek.com \
    --cc=damien@zamaudio.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    --cc=wsd_upstream@mediatek.com \
    --subject='Re: [PATCH] ALSA: usb-audio: fix incorrect clock source setting' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).