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

[Tobias <toszlanyi@yahoo.de>]

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

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