On Tue, 10 Jul 2018 22:27:43 +0200, Adam Goode wrote:

If the audio device is externally clocked and set to a rate that does not match the external clock, the clock will never be valid and we cannot set the rate successfully. To fix this, allow a rate change even if the clock is initially invalid, and validate again after the rate is changed.

This fixes problems with MOTU UltraLite AVB hardware over USB.

Signed-off-by: Adam Goode agoode@google.com

The logic looks good, but just a few minor nitpicking below.

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index c79749613fa6..9e9019464f91 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -513,14 +513,26 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, bool writeable; u32 bmControls;

• /* First, try to find a valid clock. This may trigger

• * automatic clock selection if the current clock is not
  

• * valid. */
  

The "*/" should put in another line.

clock = snd_usb_clock_find_source(chip, fmt->protocol, fmt->clock, true);

• if (clock < 0)

• return clock;
  

• if (clock < 0) {

• /* We did not find a valid clock, but that might be
  

•  * because the current sample rate does not match an
  

•  * external clock source. Try again without validation
  

•  * and we will do another validation after setting the
  

•  * rate. */
  

Ditto.

• clock = snd_usb_clock_find_source(chip, fmt->protocol,
  

• 				  fmt->clock, false);
  

• if (clock < 0)
  

• 	return clock;
  

• }

  prev_rate = get_sample_rate_v2v3(chip, iface, fmt->altsetting, clock); if (prev_rate == rate)

• return 0;
  

• return uac_clock_source_is_valid(chip, fmt->protocol, clock) ? 0 : -ENXIO;
  

It's better to have a single return path, so let's use here "goto validation", and...

if (fmt->protocol == UAC_VERSION_3) { struct uac3_clock_source_descriptor *cs_desc; @@ -577,7 +589,8 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, snd_usb_set_interface_quirk(dev); }

• return 0;

• /* validate clock after rate change */
• return uac_clock_source_is_valid(chip, fmt->protocol, clock) ? 0 : -ENXIO;

... put here like

validation: if (!uac_clock_source_is_valid(chip, fmt->protocol, clock)) return -ENXIO; return 0;

thanks,

Takashi