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@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel