Re: [alsa-devel] USB Audio Interface / Denon MC7000 and MC8000 controller

[Alexander Tsoy <alexander@tsoy.me>]

В Чт, 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;
> >   }
> > 

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel