Re: [PATCH] ALSA: usb-audio: Ignore clock selector errors for single connection

[Takashi Iwai <tiwai@xxxxxxx>]

On Thu, 01 Feb 2024 15:12:51 +0100,
Alexander Tsoy wrote:
> 
> В Чт, 01/02/2024 в 13:51 +0100, Takashi Iwai пишет:
> > On Thu, 01 Feb 2024 12:53:08 +0100,
> > Alexander Tsoy wrote:
> > > 
> > > For devices with multiple clock sources connected to a selector, we
> > > need
> > > to check what a clock selector control request has returned. This
> > > is
> > > needed to ensure that a requested clock source is indeed selected
> > > and for
> > > autoclock feature to work.
> > > 
> > > For devices with single clock source connected, if we get an error
> > > there
> > > is nothing else we can do about it. We can't skip clock selector
> > > setup as
> > > it is required by some devices. So lets just ignore error in this
> > > case.
> > > 
> > > This should fix various buggy Mackie devices:
> > > 
> > > [  649.109785] usb 1-1.3: parse_audio_format_rates_v2v3(): unable
> > > to find clock source (clock -32)
> > > [  649.111946] usb 1-1.3: parse_audio_format_rates_v2v3(): unable
> > > to find clock source (clock -32)
> > > [  649.113822] usb 1-1.3: parse_audio_format_rates_v2v3(): unable
> > > to find clock source (clock -32)
> > > 
> > > There is also interesting info from the Windows documentation [1]
> > > (this
> > > is probably why manufacturers dont't even test this feature):
> > > 
> > > "The USB Audio 2.0 driver doesn't support clock selection. The
> > > driver
> > > uses the Clock Source Entity, which is selected by default and
> > > never
> > > issues a Clock Selector Control SET CUR request."
> > > 
> > > Link:
> > > https://learn.microsoft.com/en-us/windows-hardware/drivers/audio/usb-2-0-audio-drivers
> > >  [1]
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217314
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=218175
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=218342
> > > Signed-off-by: Alexander Tsoy <alexander@xxxxxxx>
> > > ---
> > >  sound/usb/clock.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> > > index a8204c6d6fac..60fcb872a80b 100644
> > > --- a/sound/usb/clock.c
> > > +++ b/sound/usb/clock.c
> > > @@ -347,8 +347,16 @@ static int __uac_clock_find_source(struct
> > > snd_usb_audio *chip,
> > >  			    !writeable)
> > >  				return ret;
> > >  			err = uac_clock_selector_set_val(chip,
> > > entity_id, cur);
> > > -			if (err < 0)
> > > +			if (err < 0) {
> > > +				if (pins == 1) {
> > > +					usb_audio_dbg(chip,
> > > +						      "%s():
> > > selector returned an error, "
> > > +						      "assuming a
> > > firmware bug, id %d, ret %d\n",
> > > +						      __func__,
> > > clock_id, err);
> > > +					return ret;
> > > +				}
> > >  				return err;
> > > +			}
> > 
> > Hmm, what's the difference of the behavior except for the additional
> > debug message?  Both returns ret, so I don't see how it fixes.
> 
> If pins == 1, then ret is returned, otherwise err is returned. They are
> not equal here. But yes, the code is a bit confusing.

Ah yeah, ret and err...  OK, I'll take it.


thanks,

Takashi