В Вт, 13/02/2024 в 13:02 +0100, Takashi Iwai пишет:
> 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;
> }
>
Thank you both for response! I'll try to simplify the logic.
