On Mon, Feb 12, 2024 at 06:28:48PM +0300, Alexander Tsoy wrote: > Config with the highest supported UAC version is always preferable because > it usually provides more features. > > Main motivation for this change is to improve support for several UAC2 > devices which have UAC1 config as the first one for compatibility reasons. > UAC2 mode provides a wider range of supported sampling rates on these > devices. Also when UAC4 support is implemented, it will need just one > additional case line without changing the logic. > > Signed-off-by: Alexander Tsoy <alexander@xxxxxxx> > --- > drivers/usb/core/generic.c | 39 +++++++++++++++++++++++++------------- > 1 file changed, 26 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c > index b134bff5c3fe..8aeb180e1cf9 100644 > --- a/drivers/usb/core/generic.c > +++ b/drivers/usb/core/generic.c > @@ -48,9 +48,16 @@ static bool is_audio(struct usb_interface_descriptor *desc) > return desc->bInterfaceClass == USB_CLASS_AUDIO; > } > > -static bool is_uac3_config(struct usb_interface_descriptor *desc) > +static bool is_supported_uac(struct usb_interface_descriptor *desc) > { > - return desc->bInterfaceProtocol == UAC_VERSION_3; > + switch(desc->bInterfaceProtocol) { > + case UAC_VERSION_1: > + case UAC_VERSION_2: > + case UAC_VERSION_3: > + return true; > + default: > + return false; > + } > } > > int usb_choose_configuration(struct usb_device *udev) > @@ -135,22 +142,28 @@ int usb_choose_configuration(struct usb_device *udev) > } > > /* > - * Select first configuration as default for audio so that > - * devices that don't comply with UAC3 protocol are supported. > - * But, still iterate through other configurations and > - * select UAC3 compliant config if present. > + * Iterate through audio configurations and select the first of > + * the highest supported UAC versions. Select the first config > + * if no supported UAC configs are found. > */ > if (desc && is_audio(desc)) { > - /* Always prefer the first found UAC3 config */ > - if (is_uac3_config(desc)) { > - best = c; > - break; > - } > + struct usb_interface_descriptor *best_desc = NULL; > + > + if (best) > + best_desc = &best->intf_cache[0]->altsetting->desc; Are you sure you always have a intf_cache[0] pointer? What about altsetting? Remember, invalid descriptors happen all the time, and are a known "attack vector" against the USB stack. > > - /* If there is no UAC3 config, prefer the first config */ > - else if (i == 0) > + if (i == 0) > best = c; > > + /* Assume that bInterfaceProtocol value is always > + * growing when UAC versions are incremented, so that > + * the direct comparison is possible. */ How do we know this assumption is always true? What happens when it is not? > + else if (is_supported_uac(desc) && best_desc && > + (!is_supported_uac(best_desc) || > + (desc->bInterfaceProtocol > > + best_desc->bInterfaceProtocol))) > + best = c; I really can't understand this if logic, sorry, can you describe it better so that we can maintain it over time? thanks, greg k-h