On Thu, 16 Jan 2020 12:58:00 +0100, Tobias wrote:

Thank you for taking care of this...

I have tried with the latest Kernel 5.4.11 in Ubuntu 16.04 and $ dmesg still shows "clock source 65 is not valid, cannot use"

My current running stable system is

$ uname -a $ Linux tobias-V130 4.15.0-23-generic #25~16.04.1 SMP Fri Dec 20 20:16:19 CET 2019 x86_64 x86_64 x86_64 GNU/Linux

in where I applied the chane described in here: https://alsa-user.narkive.com/2tDAO87f/troubleshooting-new-usb-audio-device#...

Just for my understanding what you now need me to do... deleting the line in /sound/usb/clock.c that states "return -ENXIO;" and compile the kernel again.

Yes, and with that, the error message still remains. That's not wrong. The question is to identify who calls this function. So...

Clemes mentioned in his last post to add logging but I have no idea what he means by that. Can you briefly guide me what I would need to do?

... try the patch below instead. This should work for the recent kernels. It'll give Oops-like kernel WARNING with stack traces, so show them. There can be multiple occurrences.

Takashi

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..8d92a946f978 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -219,10 +219,10 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, entity_id = source->bClockID; if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_2, entity_id)) { - usb_audio_err(chip, + WARN(1, "clock source %d is not valid, cannot use\n", entity_id); - return -ENXIO; + /* return -ENXIO; */ } return entity_id; }

Hello Alexander. I compiled the kernel with your patch but the result unfortunately it's the same. The audio device is still not working, dmesg shows the same message that clock source is not valid. Any other idea? Thanks a lotTobias

Gesendet von Yahoo Mail auf Android

Am Di., Jan. 21, 2020 at 12:00 schrieb Alexander Tsoyalexander@tsoy.me: Please try the patch below. Make sure that no other patches are applied.

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 82184036437b..7264f57d3188 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1553,6 +1553,13 @@ void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,         && (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)         msleep(20);

+    /* +    * Denon MC7000 +    */ +    if (chip->usb_id == USB_ID(0x15e4, 0x8004) && +        (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS) +        msleep(20); +     /* Zoom R16/24, Logitech H650e, Jabra 550a needs a tiny delay here,     * otherwise requests like get/set frequency return as failed despite     * actually succeeding.

В Вт, 21/01/2020 в 09:17 +0100, Tobias пишет:

Thanks a lot for the hint Alexander. Anyhow, I am not sure where to put that. Also Takashi Iwai was involved already and I would like to prevent mixing up things or double work.

Please advise in detail what I would need to do as I am not a programmer.

Thanks a lot. Tobias

Am 20.01.20 um 09:22 schrieb Alexander Tsoy:

...

В Сб, 14/12/2019 в 09:24 +0100, Tobias пишет:

...

Hello dear ALSA developers.

I have purchased a MC7000 controller in order to control MIXXX on Ubuntu 16.04. According to the Denon specification the controller should have been class compliant but there is an issue with the Audio interface to work properly giving following message ...

$ dmesg ...

...

[  74.522831] usb 1-1.3: new high-speed USB device number 6 using xhci_hcd

[  74.623784] usb 1-1.3: New USB device found, idVendor=15e4, idProduct=8004 [  74.623789] usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [  74.623793] usb 1-1.3: Product: DENON DJ MC7000 [  74.623796] usb 1-1.3: Manufacturer: DENON DJ [  74.623798] usb 1-1.3: SerialNumber: 201603 [  74.625134] usb 1-1.3: clock source 65 is not valid, cannot use

"|||clock source 65 is not valid, cannot use" is repeated uncountable times then

Please try to add delay after each class control request in snd_usb_ctl_msg_quirk():

if (chip->usb_id == USB_ID(0x15e4, 0x8004) &&           (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)           msleep(20);

В Пн, 03/02/2020 в 11:23 +0100, Tobias пишет:

Dear Alexander - unfortunately I did hot hear back from you. I guess this may not be your highest priority but still - do you have any other idea to make the MC7000 sound device working? Thanks a lot.

I think it should be safe to ignore clock validity check result if: - only one single sample rate is supported; - the clock source is internal, - there is no clock selector.

Could you try the following patch?

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..636c340d4f9f 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -197,6 +197,38 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, return data ? true : false; }

+/* + * Assume the clock is valid if clock source supports only one single sample + * rate, its type is not external and there is no clock selector. This is needed + * for some Denon DJ controllers, that always report that clock is invalid. + */ +static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + int clock) +{ + if (fmt->protocol == UAC_VERSION_3) { + struct uac3_clock_source_descriptor *cs_desc = + snd_usb_find_clock_source_v3(chip->ctrl_intf, clock); + + if (!cs_desc) + return false; + + return (fmt->nr_rates == 1 && + (fmt->clock & 0xff) == cs_desc->bClockID && + (cs_desc->bmAttributes & 0x1) != UAC3_CLOCK_SOURCE_TYPE_EXT); + } else { /* UAC_VERSION_2 */ + struct uac_clock_source_descriptor *cs_desc = + snd_usb_find_clock_source(chip->ctrl_intf, clock); + + if (!cs_desc) + return false; + + return (fmt->nr_rates == 1 && + (fmt->clock & 0xff) == cs_desc->bClockID && + (cs_desc->bmAttributes & 0x3) != UAC_CLOCK_SOURCE_TYPE_EXT); + } +} + static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, unsigned long *visited, bool validate) { @@ -577,7 +609,8 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,

validation: /* validate clock after rate change */ - if (!uac_clock_source_is_valid(chip, fmt->protocol, clock)) + if (!uac_clock_source_is_valid(chip, fmt->protocol, clock) && + !uac_clock_source_is_valid_quirk(chip, fmt, clock)) return -ENXIO; return 0; }

В Чт, 06/02/2020 в 11:06 +0100, Tobias пишет:

Thank you so much Alexander! I used latest Kernel and patched as you suggested. The Device is working now giving sound on all 4 channels, even though dmesg still shows the error message as you can see here:

uname -a: Linux tobias-V130 5.5.2 #1 SMP Thu Feb 6 09:41:57 CET 2020 x86_64 x86_64 x86_64 GNU/Linux

dmesg: [ 62.918777] usb 1-1.3: new high-speed USB device number 6 using xhci_hcd [ 62.939293] usb 1-1.3: New USB device found, idVendor=15e4, idProduct=8004, bcdDevice=11.10 [ 62.939295] usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 62.939297] usb 1-1.3: Product: DENON DJ MC7000 [ 62.939298] usb 1-1.3: Manufacturer: DENON DJ [ 62.939299] usb 1-1.3: SerialNumber: 201603 [ 62.942232] usb 1-1.3: clock source 65 is not valid, cannot use [ 62.943998] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.013306] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.028912] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.029675] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.037813] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.063865] usb 1-1.3: clock source 65 is not valid, cannot use

Yes, this is expected.

I checked in file /sound/usb/clock.c that within functions

static int __uac_clock_find_source static int __uac3_clock_find_source

there is another check that possibly gives the warning.

Maybe the warning "cannot use" should not be displayed when a Denon Audio device is attached as it is misleading.

Please try the patch below. I've dropped UAC3 support and changed __uac_clock_find_source() and __uac3_clock_find_source() to print errors only in debug mode, as we make the final decision about clock validity in set_sample_rate_v2v3().

Dear Takashi, what do you think about this approach. Is it acceptable?

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..e978b46efc85 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -197,6 +197,32 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, return data ? true : false; }

+/* + * Assume the clock is valid if clock source supports only one single sample + * rate, its type is not external and a terminal is connected directly to it + * (there is no clock selector). This is needed for some Denon DJ controllers, + * that always reports that clock is invalid. + */ +static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + int clock) +{ + if (fmt->protocol == UAC_VERSION_2) { + struct uac_clock_source_descriptor *cs_desc = + snd_usb_find_clock_source(chip->ctrl_intf, clock); + + if (!cs_desc) + return false; + + return (fmt->nr_rates == 1 && + (fmt->clock & 0xff) == cs_desc->bClockID && + (cs_desc->bmAttributes & 0x3) != + UAC_CLOCK_SOURCE_TYPE_EXT); + } + + return false; +} + static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, unsigned long *visited, bool validate) { @@ -219,7 +245,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, entity_id = source->bClockID; if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_2, entity_id)) { - usb_audio_err(chip, + usb_audio_dbg(chip, "clock source %d is not valid, cannot use\n", entity_id); return -ENXIO; @@ -309,7 +335,7 @@ static int __uac3_clock_find_source(struct snd_usb_audio *chip, int entity_id, entity_id = source->bClockID; if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_3, entity_id)) { - usb_audio_err(chip, + usb_audio_dbg(chip, "clock source %d is not valid, cannot use\n", entity_id); return -ENXIO; @@ -577,7 +603,11 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,

validation: /* validate clock after rate change */ - if (!uac_clock_source_is_valid(chip, fmt->protocol, clock)) + if (!uac_clock_source_is_valid(chip, fmt->protocol, clock) && + !uac_clock_source_is_valid_quirk(chip, fmt, clock)) + usb_audio_err(chip, + "clock source %d is not valid, cannot use\n", + entity_id); return -ENXIO; return 0; }

Thanks a lot for your next patch that I would like to test. Tobias

Am 05.02.20 um 20:07 schrieb Alexander Tsoy:

...

В Пн, 03/02/2020 в 11:23 +0100, Tobias пишет:

...

Dear Alexander - unfortunately I did hot hear back from you. I guess this may not be your highest priority but still - do you have any other idea to make the MC7000 sound device working? Thanks a lot.

I think it should be safe to ignore clock validity check result if:

• only one single sample rate is supported;
• the clock source is internal,
• there is no clock selector.

Could you try the following patch?

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..636c340d4f9f 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -197,6 +197,38 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, return data ? true : false; }

+/*

• • Assume the clock is valid if clock source supports only one

single sample

• • rate, its type is not external and there is no clock selector.

This is needed

• • for some Denon DJ controllers, that always report that clock is

invalid.

• */

+static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip,

• 			    struct audioformat *fmt,
  

• 			    int clock)
  

+{

• if (fmt->protocol == UAC_VERSION_3) {

• struct uac3_clock_source_descriptor *cs_desc =
  

• 	snd_usb_find_clock_source_v3(chip->ctrl_intf,
  

clock);

• if (!cs_desc)
  

• 	return false;
  

• return (fmt->nr_rates == 1 &&
  

• 	(fmt->clock & 0xff) == cs_desc->bClockID &&
  

• 	(cs_desc->bmAttributes & 0x1) !=
  

UAC3_CLOCK_SOURCE_TYPE_EXT);

• } else { /* UAC_VERSION_2 */

• struct uac_clock_source_descriptor *cs_desc =
  

• 	snd_usb_find_clock_source(chip->ctrl_intf,
  

clock);

• if (!cs_desc)
  

• 	return false;
  

• return (fmt->nr_rates == 1 &&
  

• 	(fmt->clock & 0xff) == cs_desc->bClockID &&
  

• 	(cs_desc->bmAttributes & 0x3) !=
  

UAC_CLOCK_SOURCE_TYPE_EXT);

• }

+}

• static int __uac_clock_find_source(struct snd_usb_audio *chip,

int entity_id, unsigned long *visited, bool validate) { @@ -577,7 +609,8 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,

validation: /* validate clock after rate change */

• if (!uac_clock_source_is_valid(chip, fmt->protocol, clock))

• if (!uac_clock_source_is_valid(chip, fmt->protocol, clock) &&

•    !uac_clock_source_is_valid_quirk(chip, fmt, clock))
  

  return -ENXIO; return 0; }

Thank you very much again for your quick input. Unfortunately the new patch caused a compilation error so I tried to compile the module where the kernel stopped at first place which gave following message.

$ sudo make modules

CALL    scripts/checksyscalls.sh   CALL    scripts/atomic/check-atomics.sh   DESCEND  objtool   CC [M]  sound/usb/clock.o In file included from ./include/linux/usb/ch9.h:36:0,                  from ./include/linux/usb.h:6,                  from sound/usb/clock.c:9: sound/usb/clock.c: In function ‘set_sample_rate_v2v3’: sound/usb/clock.c:610:10: error: ‘entity_id’ undeclared (first use in this function)           entity_id);           ^ ./include/linux/device.h:1774:32: note: in definition of macro ‘dev_err’   _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)                                 ^ sound/usb/clock.c:608:3: note: in expansion of macro ‘usb_audio_err’    usb_audio_err(chip,    ^ sound/usb/clock.c:610:10: note: each undeclared identifier is reported only once for each function it appears in           entity_id);           ^ ./include/linux/device.h:1774:32: note: in definition of macro ‘dev_err’   _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)                                 ^ sound/usb/clock.c:608:3: note: in expansion of macro ‘usb_audio_err’    usb_audio_err(chip,    ^ scripts/Makefile.build:265: die Regel für Ziel „sound/usb/clock.o“ scheiterte make[2]: *** [sound/usb/clock.o] Fehler 1 scripts/Makefile.build:503: die Regel für Ziel „sound/usb“ scheiterte make[1]: *** [sound/usb] Fehler 2 Makefile:1693: die Regel für Ziel „sound“ scheiterte make: *** [sound] Fehler 2

Hope that helps to determine what went wrong. If you need anything else, then please let me know.

Cheers Tobias

Am 07.02.20 um 09:15 schrieb Takashi Iwai:

On Thu, 06 Feb 2020 23:09:33 +0100, Alexander Tsoy wrote:

...

В Чт, 06/02/2020 в 11:06 +0100, Tobias пишет:

...

Thank you so much Alexander! I used latest Kernel and patched as you suggested. The Device is working now giving sound on all 4 channels, even though dmesg still shows the error message as you can see here:

uname -a: Linux tobias-V130 5.5.2 #1 SMP Thu Feb 6 09:41:57 CET 2020 x86_64 x86_64 x86_64 GNU/Linux

dmesg: [ 62.918777] usb 1-1.3: new high-speed USB device number 6 using xhci_hcd [ 62.939293] usb 1-1.3: New USB device found, idVendor=15e4, idProduct=8004, bcdDevice=11.10 [ 62.939295] usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 62.939297] usb 1-1.3: Product: DENON DJ MC7000 [ 62.939298] usb 1-1.3: Manufacturer: DENON DJ [ 62.939299] usb 1-1.3: SerialNumber: 201603 [ 62.942232] usb 1-1.3: clock source 65 is not valid, cannot use [ 62.943998] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.013306] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.028912] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.029675] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.037813] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.063865] usb 1-1.3: clock source 65 is not valid, cannot use

Yes, this is expected.

...

I checked in file /sound/usb/clock.c that within functions

static int __uac_clock_find_source static int __uac3_clock_find_source

there is another check that possibly gives the warning.

Maybe the warning "cannot use" should not be displayed when a Denon Audio device is attached as it is misleading.

Please try the patch below. I've dropped UAC3 support and changed __uac_clock_find_source() and __uac3_clock_find_source() to print errors only in debug mode, as we make the final decision about clock validity in set_sample_rate_v2v3().

Dear Takashi, what do you think about this approach. Is it acceptable?

Yes, the approach looks good to me. Just a few comments:

...

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..e978b46efc85 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -197,6 +197,32 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, return data ? true : false; }

+/*

• • Assume the clock is valid if clock source supports only one single sample
• • rate, its type is not external and a terminal is connected directly to it
• • (there is no clock selector). This is needed for some Denon DJ controllers,
• • that always reports that clock is invalid.
• */

+static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip,

• 			    struct audioformat *fmt,
  

• 			    int clock)
  

+{

• if (fmt->protocol == UAC_VERSION_2) {

• struct uac_clock_source_descriptor *cs_desc =
  

• 	snd_usb_find_clock_source(chip->ctrl_intf, clock);
  

• if (!cs_desc)
  

• 	return false;
  

• return (fmt->nr_rates == 1 &&
  

• 	(fmt->clock & 0xff) == cs_desc->bClockID &&
  

• 	(cs_desc->bmAttributes & 0x3) !=
  

• 		UAC_CLOCK_SOURCE_TYPE_EXT);
  

• }
• return false;

IMO it's safer to call from the specific failure path, i.e.

static bool uac_clock_source_is_valid(....) { .... err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR, USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN, UAC2_CS_CONTROL_CLOCK_VALID << 8, snd_usb_ctrl_intf(chip) | (source_id << 8), &data, sizeof(data));

if (err < 0) {

if (uac_clock_source_is_valid_quirk(....))
	return true;

dev_warn(&dev->dev,
	 "%s(): cannot get clock validity for id %d\n",
	   __func__, source_id);
return false;

}

Then you can pass cs_desc there, too.

...

+}

• static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, unsigned long *visited, bool validate) {

@@ -219,7 +245,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, entity_id = source->bClockID; if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_2, entity_id)) {

• 	usb_audio_err(chip,
  

• 	usb_audio_dbg(chip,
  	"clock source %d is not valid, cannot use\n",
  	entity_id);
  return -ENXIO;
  

Hm, it's not good to hide the error message always. This is a common error on many devices and suppressing it would look cleaner but also hide what's the reason. Maybe we can add nowarn bool flag for certain code paths?

thanks,

Takashi

Dear Alexander - unfortunately the patch doesn't want to be applied.

$ patch -p1 < ../denon-4.patch patching file sound/usb/clock.c Hunk #1 FAILED at 187. 1 out of 1 hunk FAILED -- saving rejects to file sound/usb/clock.c.rej

I guess it was not your intention only adding one line to /sound/usb/clock.c. so what am I missing here?

Thanks a lot!

Am 07.02.20 um 18:45 schrieb Alexander Tsoy:

В Пт, 07/02/2020 в 15:39 +0100, Tobias пишет:

...

Thank you very much again for your quick input. Unfortunately the new patch caused a compilation error so I tried to compile the module where the kernel stopped at first place which gave following message.

$ sudo make modules

CALL    scripts/checksyscalls.sh
CALL    scripts/atomic/check-atomics.sh
DESCEND  objtool
CC [M]  sound/usb/clock.o

In file included from ./include/linux/usb/ch9.h:36:0, from ./include/linux/usb.h:6, from sound/usb/clock.c:9: sound/usb/clock.c: In function ‘set_sample_rate_v2v3’: sound/usb/clock.c:610:10: error: ‘entity_id’ undeclared (first use in this function) entity_id); ^ ./include/linux/device.h:1774:32: note: in definition of macro ‘dev_err’ _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__) ^ sound/usb/clock.c:608:3: note: in expansion of macro ‘usb_audio_err’ usb_audio_err(chip, ^ sound/usb/clock.c:610:10: note: each undeclared identifier is reported only once for each function it appears in entity_id); ^ ./include/linux/device.h:1774:32: note: in definition of macro ‘dev_err’ _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__) ^ sound/usb/clock.c:608:3: note: in expansion of macro ‘usb_audio_err’ usb_audio_err(chip, ^ scripts/Makefile.build:265: die Regel für Ziel „sound/usb/clock.o“ scheiterte make[2]: *** [sound/usb/clock.o] Fehler 1 scripts/Makefile.build:503: die Regel für Ziel „sound/usb“ scheiterte make[1]: *** [sound/usb] Fehler 2 Makefile:1693: die Regel für Ziel „sound“ scheiterte make: *** [sound] Fehler 2

Hope that helps to determine what went wrong. If you need anything else, then please let me know.

Sorry, this was a copy-paste error, the argument should be "clock", not "entity_id".

Please try the patch below. It should just print

... uac_clock_source_is_valid(): err: X ; data: X

to the kernel log. Hopefully this is a final peace of data I need.

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..65ee5c24c5d1 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -187,6 +187,8 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, snd_usb_ctrl_intf(chip) | (source_id << 8), &data, sizeof(data));

• dev_info(&dev->dev, "%s(): err: %d ; data: %d\n", __func__, err, data);
• if (err < 0) { dev_warn(&dev->dev, "%s(): cannot get clock validity for id %d\n",

...

Cheers Tobias

Am 07.02.20 um 09:15 schrieb Takashi Iwai:

...

On Thu, 06 Feb 2020 23:09:33 +0100, Alexander Tsoy wrote:

...

В Чт, 06/02/2020 в 11:06 +0100, Tobias пишет:

...

Thank you so much Alexander! I used latest Kernel and patched as you suggested. The Device is working now giving sound on all 4 channels, even though dmesg still shows the error message as you can see here:

uname -a: Linux tobias-V130 5.5.2 #1 SMP Thu Feb 6 09:41:57 CET 2020 x86_64 x86_64 x86_64 GNU/Linux

dmesg: [ 62.918777] usb 1-1.3: new high-speed USB device number 6 using xhci_hcd [ 62.939293] usb 1-1.3: New USB device found, idVendor=15e4, idProduct=8004, bcdDevice=11.10 [ 62.939295] usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 62.939297] usb 1-1.3: Product: DENON DJ MC7000 [ 62.939298] usb 1-1.3: Manufacturer: DENON DJ [ 62.939299] usb 1-1.3: SerialNumber: 201603 [ 62.942232] usb 1-1.3: clock source 65 is not valid, cannot use [ 62.943998] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.013306] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.028912] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.029675] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.037813] usb 1-1.3: clock source 65 is not valid, cannot use [ 63.063865] usb 1-1.3: clock source 65 is not valid, cannot use

Yes, this is expected.

...

I checked in file /sound/usb/clock.c that within functions

static int __uac_clock_find_source static int __uac3_clock_find_source

there is another check that possibly gives the warning.

Maybe the warning "cannot use" should not be displayed when a Denon Audio device is attached as it is misleading.

Please try the patch below. I've dropped UAC3 support and changed __uac_clock_find_source() and __uac3_clock_find_source() to print errors only in debug mode, as we make the final decision about clock validity in set_sample_rate_v2v3().

Dear Takashi, what do you think about this approach. Is it acceptable?

Yes, the approach looks good to me. Just a few comments:

...

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..e978b46efc85 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -197,6 +197,32 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, return data ? true : false; }

+/*

• • Assume the clock is valid if clock source supports only one

single sample

• • rate, its type is not external and a terminal is connected

directly to it

• • (there is no clock selector). This is needed for some Denon

DJ controllers,

• • that always reports that clock is invalid.
• */

+static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip,

• 			    struct audioformat *fmt,
  

• 			    int clock)
  

+{

• if (fmt->protocol == UAC_VERSION_2) {

• struct uac_clock_source_descriptor *cs_desc =
  

• 	snd_usb_find_clock_source(chip->ctrl_intf,
  

clock);

• if (!cs_desc)
  

• 	return false;
  

• return (fmt->nr_rates == 1 &&
  

• 	(fmt->clock & 0xff) == cs_desc->bClockID &&
  

• 	(cs_desc->bmAttributes & 0x3) !=
  

• 		UAC_CLOCK_SOURCE_TYPE_EXT);
  

• }
• return false;

IMO it's safer to call from the specific failure path, i.e.

static bool uac_clock_source_is_valid(....) { .... err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR, USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN, UAC2_CS_CONTROL_CLOCK_VALID << 8, snd_usb_ctrl_intf(chip) | (source_id << 8), &data, sizeof(data));

if (err < 0) {

if (uac_clock_source_is_valid_quirk(....))
	return true;

dev_warn(&dev->dev,
	 "%s(): cannot get clock validity for id %d\n",
	   __func__, source_id);
return false;

}

Then you can pass cs_desc there, too.

...

+}

• static int __uac_clock_find_source(struct snd_usb_audio *chip,

int entity_id, unsigned long *visited, bool validate) { @@ -219,7 +245,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id, entity_id = source->bClockID; if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_2, entity_id)) {

• 	usb_audio_err(chip,
  

• 	usb_audio_dbg(chip,
  		"clock source %d is not valid,
  

cannot use\n", entity_id); return -ENXIO;

Hm, it's not good to hide the error message always. This is a common error on many devices and suppressing it would look cleaner but also hide what's the reason. Maybe we can add nowarn bool flag for certain code paths?

thanks,

Takashi

Alexander - here comes the dmesg output you are looking for:

35.175855] usb 1-2: new high-speed USB device number 6 using xhci_hcd [   35.196647] usb 1-2: New USB device found, idVendor=15e4, idProduct=8004, bcdDevice=11.10 [   35.196649] usb 1-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [   35.196650] usb 1-2: Product: DENON DJ MC7000 [   35.196650] usb 1-2: Manufacturer: DENON DJ [   35.196651] usb 1-2: SerialNumber: 201603 [   35.198291] usb 1-2: uac_clock_source_is_valid(): err: 1 ; data: 0 [   35.198292] usb 1-2: clock source 65 is not valid, cannot use [   35.198370] usb 1-2: uac_clock_source_is_valid(): err: 1 ; data: 0 [   35.199052] usb 1-2: uac_clock_source_is_valid(): err: 1 ; data: 0 [   35.199053] usb 1-2: clock source 65 is not valid, cannot use [   35.199132] usb 1-2: uac_clock_source_is_valid(): err: 1 ; data: 0 ... ... repeated several 100 times.

Hope this is useful data for you.

Cheers! Tobias

Am 07.02.20 um 20:11 schrieb Alexander Tsoy:

В Пт, 07/02/2020 в 19:49 +0100, Tobias пишет:

...

Dear Alexander - unfortunately the patch doesn't want to be applied.

It seems I copied it from less with different tab width. The patch below should be OK.

...

$ patch -p1 < ../denon-4.patch patching file sound/usb/clock.c Hunk #1 FAILED at 187. 1 out of 1 hunk FAILED -- saving rejects to file sound/usb/clock.c.rej

I guess it was not your intention only adding one line to /sound/usb/clock.c. so what am I missing here?

No, it was intentional. This patch just adds printing of some info I need in the following format:

uac_clock_source_is_valid(): err: X ; data: X

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 018b1ecb5404..65ee5c24c5d1 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -187,6 +187,8 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, snd_usb_ctrl_intf(chip) | (source_id << 8), &data, sizeof(data));

• dev_info(&dev->dev, "%s(): err: %d ; data: %d\n", __func__, err, data);
• if (err < 0) { dev_warn(&dev->dev, "%s(): cannot get clock validity for id %d\n",