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
