Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAEsQvctJDnsaRTXAGAJ6==juKazoo2=AJrWabLzqE=jCfg5EEA@mail.gmail.com>
 <20210727102744.30364-1-chihhao.chen@mediatek.com>
In-Reply-To: <20210727102744.30364-1-chihhao.chen@mediatek.com>
From:   Geraldo Nascimento <geraldogabriel@gmail.com>
Date:   Tue, 27 Jul 2021 17:56:33 +0000
Message-ID: <CAEsQvcuOQ_Tg0bRbsegECdBUsGxa61ei9-Z2bRbGvQ9FmEUPbA@mail.gmail.com>
Subject: Re: [PATCH] ALSA: usb-audio: fix incorrect clock source setting
To:     chihhao chen <chihhao.chen@mediatek.com>
Cc:     alsa-devel@alsa-project.org, damien@zamaudio.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
        tiwai@suse.com, Takashi Iwai <tiwai@suse.de>,
        wsd_upstream@mediatek.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jul 27, 2021 at 10:28 AM <chihhao.chen@mediatek.com> wrote:
>
> From: chihhao chen <chihhao.chen@mediatek.com>
>
> Hello
>
> <6>[  150.347456][ T2768] __uac_clock_find_source: FOR EP 81: Clock Selector c has pin 2 for Clock Source ID a selected
> <6>[  150.347517][ T2768] __uac_clock_find_source: FOR EP 81: Found Source! Clock Selector c has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.384289][ T2768] __uac_clock_find_source: FOR EP 81: Clock Selector c has pin 2 for Clock Source ID a selected
> <6>[  150.390920][ T2768] __uac_clock_find_source: FOR EP 81: Found Source! Clock Selector c has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.438156][ T2768] __uac_clock_find_source: FOR EP 81: Clock Selector c has pin 2 for Clock Source ID a selected
> <6>[  150.438226][ T2768] __uac_clock_find_source: FOR EP 81: Found Source! Clock Selector c has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.473547][ T2768] __uac_clock_find_source: FOR EP 81: Clock Selector c has pin 2 for Clock Source ID a selected
> <6>[  150.480165][ T2768] __uac_clock_find_source: FOR EP 81: Found Source! Clock Selector c has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.513375][ T2768] __uac_clock_find_source: FOR EP 1: Clock Selector b has pin 2 for Clock Source ID a selected
> <6>[  150.513439][ T2768] __uac_clock_find_source: FOR EP 1: Found Source! Clock Selector b has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.546161][ T2768] __uac_clock_find_source: FOR EP 1: Clock Selector b has pin 2 for Clock Source ID a selected
> <6>[  150.552678][ T2768] __uac_clock_find_source: FOR EP 1: Found Source! Clock Selector b has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.584347][ T2768] __uac_clock_find_source: FOR EP 1: Clock Selector b has pin 2 for Clock Source ID a selected
> <6>[  150.584418][ T2768] __uac_clock_find_source: FOR EP 1: Found Source! Clock Selector b has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.617760][ T2768] __uac_clock_find_source: FOR EP 1: Clock Selector b has pin 2 for Clock Source ID a selected
> <6>[  150.624253][ T2768] __uac_clock_find_source: FOR EP 1: Found Source! Clock Selector b has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.657906][ T2768] __uac_clock_find_source: FOR EP 1: Clock Selector b has pin 2 for Clock Source ID a selected
> <6>[  150.657982][ T2768] __uac_clock_find_source: FOR EP 1: Found Source! Clock Selector b has pin 2 for Clock Source ID a about to be reselected
> <6>[  150.689571][ T2768] __uac_clock_find_source: FOR EP 1: Clock Selector b has pin 2 for Clock Source ID a selected
> <6>[  150.696109][ T2768] __uac_clock_find_source: FOR EP 1: Found Source! Clock Selector b has pin 2 for Clock Source ID a about to be reselected
>
> Chihhao
>

Thank you. Chihhao Chen!

I see both EPs have their Clock Selectors (ID 0xC for EP 81 and ID 0xB
for EP 1) selected to pin 2, i.e. Clock Source ID 0xA.

I'm assuming this log is for the vanilla kernel without Chihhao's fix.
Please correct me if I'm wrong, Chihhao.

From the original commit message for the fix, we know the correct
setting for Clock Selector 0xB should be pin 1, with Clock Source ID
0x9.


Takashi Iwai already shared his perspective that this is a firmware
bug on the device.

I have a hunch that the firmware bug is setting both Clock Selectors
at the same time regardless of which one we want to select
specifically.


Chihhao, please try the below patch and perform another echo test.

Let us know if the echo test works or if it still fails and please
remember to share with us the relevant dmesg logs.


Thank you,
Geraldo Nascimento


--- clock.c.orig        2021-07-17 12:15:06.416028360 -0000
+++ clock.c     2021-07-27 17:36:16.954774954 -0000
@@ -324,9 +324,45 @@ static int __uac_clock_find_source(struc
                                              sources[ret - 1],
                                              visited, validate);
                if (ret > 0) {
-                       err = uac_clock_selector_set_val(chip, entity_id, cur);
-                       if (err < 0)
-                               return err;
+                       if (chip->usb_id == USB_ID(0x04e8, 0xa051)) {
+                               if (entity_id == 0xc) {
+                                       err =
uac_clock_selector_set_val(chip, entity_id, 2);
+                                       if (err < 0)
+                                               return err;
+
+                                       err =
uac_clock_selector_get_val(chip, 0xc);
+                                       if (err > 0) {
+                                               printk(KERN_ERR
"__uac_clock_find_source: Clock Selector 0xc has pin %d selected",
err);
+                                       }
+
+                                       err =
uac_clock_selector_get_val(chip, 0xb);
+                                       if (err > 0) {
+                                               printk(KERN_ERR
"__uac_clock_find_source: Clock Selector 0xb has pin %d selected",
err);
+                                       }
+                               }
+
+                               else if (entity_id == 0xb) {
+                                       err =
uac_clock_selector_set_val(chip, entity_id, 1);
+                                       if (err < 0)
+                                               return err;
+
+                                       err =
uac_clock_selector_get_val(chip, 0xc);
+                                       if (err > 0) {
+                                               printk(KERN_ERR
"__uac_clock_find_source: Clock Selector 0xc has pin %d selected",
err);
+                                       }
+
+                                       err =
uac_clock_selector_get_val(chip, 0xb);
+                                       if (err > 0) {
+                                               printk(KERN_ERR
"__uac_clock_find_source: Clock Selector 0xb has pin %d selected",
err);
+                                       }
+                               }
+                       }
+
+                       else {
+                               err = uac_clock_selector_set_val(chip,
entity_id, cur);
+                               if (err < 0)
+                                       return err;
+                       }
                }

                if (!validate || ret > 0 || !chip->autoclock)