В Пт, 07/02/2020 в 09:15 +0100, 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.
Thank you for the feedback!
I just realized that we haven't checked where uac_clock_source_is_valid() is actually failing. There are two possibilities: - the Clock Validity Control is actually not present (err < 0), so there is an error in descriptors; - the Clock Validity Control request always returns FALSE (data = 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, cannotuse\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?
We can print the error only once at the end of set_sample_rate_v2v3(), where uac_clock_source_is_valid() is called for the last time:
@@ -578,6 +606,9 @@ 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)) + usb_audio_err(chip, + "clock source %d is not valid, cannot use\n", + clock); return -ENXIO; return 0; }
thanks,
Takashi