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@xxxxxxxxxx>
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
