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