Re: [PATCH] USB: Always select config with the highest supported UAC version

[Takashi Iwai <tiwai@xxxxxxx>]

On Tue, 13 Feb 2024 12:05:47 +0100,
Greg Kroah-Hartman wrote:
> On Mon, Feb 12, 2024 at 06:28:48PM +0300, Alexander Tsoy wrote:
> >  
> > -			/* 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?

I believe this assumption is acceptable.  It's all about the protocol
number from 1 to 3, so far.  If UAC4 is ever supported in future,
it'll be highly probably the number 4.  (If not and keeping the same
protocol number 3, we'll need a different check in anyway.)
And the other numbers are excluded already in is_supported_uac()
check.

> > +			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?

The condition looks cryptic, though, yes.

Maybe the check should be factored out, e.g.

/* return true if the new config has a higher priority then the old config */
static bool check_uac_desc_priority(struct usb_host_config *old,
				struct usb_host_config *new)
{
	if (!is_supported_uac(new))
		return false;

	if (!is_supported_uac(old))
		return true;

	/*
	 * Assume that bInterfaceProtocol value is always growing;
	 * so far, it's true from UAC1 to UAC3 (1..3)
	 */
	if (new->bInterfaceProtocol > old->bInterfaceProtocol)
		return true;

	return false;
}


thanks,

Takashi