From: Nikolay Yakimov <root@livid.pp.ru> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Nikolay Yakimov <root@livid.pp.ru>, Saranya Gopal <saranya.gopal@intel.com>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] USB: Fix configuration selection issues introduced in v4.20.0 Date: Tue, 15 Jan 2019 19:13:54 +0300 [thread overview] Message-ID: <20190115161354.6806-1-root@livid.pp.ru> (raw) Commit f13912d3f014a introduced changes to the usb_choose_configuration function to better support USB Audio UAC3-compatible devices. However, there are a few problems with this patch. First of all, it adds new "if" clauses in the middle of an existing "if"/"else if" tree, which obviously breaks pre-existing logic. Secondly, since it continues iterating over configurations in one of the branches, other code in the loop can choose an unintended configuration. Finally, if an audio device's first configuration is UAC3-compatible, and there are multiple UAC3 configurations, the second one would be chosen, due to the first configuration never being checked for UAC3-compatibility. Commit ff2a8c532c14 tries to fix the second issue, but it goes about it in a somewhat unnecessarily convoluted way, in my opinion, and does nothing to fix the first or the last one. This patch tries to rectify problems described by essentially rewriting code introduced in f13912d3f014a. Notice the code was moved to *before* the "if"/"else if" tree. Signed-off-by: Nikolay Yakimov <root@livid.pp.ru> --- drivers/usb/core/generic.c | 44 ++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index f713cecc1f41..1ac9c1e5f773 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -118,6 +118,31 @@ int usb_choose_configuration(struct usb_device *udev) continue; } + /* + * 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. + */ + if (desc && is_audio(desc)) { + /* Always prefer the first found UAC3 config */ + if (is_uac3_config(desc)) { + best = c; + break; + } + + /* If there is no UAC3 config, prefer the first config */ + else if (i == 0) + best = c; + + /* Unconditional continue, because the rest of the code + * in the loop is irrelevant for audio devices, and + * because it can reassign best, which for audio devices + * we don't want. + */ + continue; + } + /* When the first config's first interface is one of Microsoft's * pet nonstandard Ethernet-over-USB protocols, ignore it unless * this kernel has enabled the necessary host side driver. @@ -132,25 +157,6 @@ int usb_choose_configuration(struct usb_device *udev) #endif } - /* - * 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. - */ - if (i == 0 && num_configs > 1 && desc && is_audio(desc)) { - best = c; - continue; - } - - if (i > 0 && desc && is_audio(desc)) { - if (is_uac3_config(desc)) { - best = c; - break; - } - continue; - } - /* From the remaining configs, choose the first one whose * first interface is for a non-vendor-specific class. * Reason: Linux is more likely to have a class driver -- 2.20.1
WARNING: multiple messages have this Message-ID (diff)
From: Nikolay Yakimov <root@livid.pp.ru> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Nikolay Yakimov <root@livid.pp.ru>, Saranya Gopal <saranya.gopal@intel.com>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: USB: Fix configuration selection issues introduced in v4.20.0 Date: Tue, 15 Jan 2019 19:13:54 +0300 [thread overview] Message-ID: <20190115161354.6806-1-root@livid.pp.ru> (raw) Commit f13912d3f014a introduced changes to the usb_choose_configuration function to better support USB Audio UAC3-compatible devices. However, there are a few problems with this patch. First of all, it adds new "if" clauses in the middle of an existing "if"/"else if" tree, which obviously breaks pre-existing logic. Secondly, since it continues iterating over configurations in one of the branches, other code in the loop can choose an unintended configuration. Finally, if an audio device's first configuration is UAC3-compatible, and there are multiple UAC3 configurations, the second one would be chosen, due to the first configuration never being checked for UAC3-compatibility. Commit ff2a8c532c14 tries to fix the second issue, but it goes about it in a somewhat unnecessarily convoluted way, in my opinion, and does nothing to fix the first or the last one. This patch tries to rectify problems described by essentially rewriting code introduced in f13912d3f014a. Notice the code was moved to *before* the "if"/"else if" tree. Signed-off-by: Nikolay Yakimov <root@livid.pp.ru> --- drivers/usb/core/generic.c | 44 ++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index f713cecc1f41..1ac9c1e5f773 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -118,6 +118,31 @@ int usb_choose_configuration(struct usb_device *udev) continue; } + /* + * 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. + */ + if (desc && is_audio(desc)) { + /* Always prefer the first found UAC3 config */ + if (is_uac3_config(desc)) { + best = c; + break; + } + + /* If there is no UAC3 config, prefer the first config */ + else if (i == 0) + best = c; + + /* Unconditional continue, because the rest of the code + * in the loop is irrelevant for audio devices, and + * because it can reassign best, which for audio devices + * we don't want. + */ + continue; + } + /* When the first config's first interface is one of Microsoft's * pet nonstandard Ethernet-over-USB protocols, ignore it unless * this kernel has enabled the necessary host side driver. @@ -132,25 +157,6 @@ int usb_choose_configuration(struct usb_device *udev) #endif } - /* - * 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. - */ - if (i == 0 && num_configs > 1 && desc && is_audio(desc)) { - best = c; - continue; - } - - if (i > 0 && desc && is_audio(desc)) { - if (is_uac3_config(desc)) { - best = c; - break; - } - continue; - } - /* From the remaining configs, choose the first one whose * first interface is for a non-vendor-specific class. * Reason: Linux is more likely to have a class driver
next reply other threads:[~2019-01-15 16:14 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-15 16:13 Nikolay Yakimov [this message] 2019-01-15 16:13 ` USB: Fix configuration selection issues introduced in v4.20.0 Nikolay Yakimov 2019-01-15 16:40 ` [PATCH] " Greg Kroah-Hartman 2019-01-15 16:40 ` Greg Kroah-Hartman 2019-01-15 18:12 ` [PATCH] " Nikolay Yakimov 2019-01-15 18:12 ` Nikolay Yakimov 2019-01-16 4:24 ` [PATCH] " Gopal, Saranya 2019-01-16 4:24 ` saranya.gopal 2019-01-16 19:13 ` [PATCH] " Nikolay Yakimov 2019-01-16 19:13 ` Nikolay Yakimov 2019-01-17 3:53 ` [PATCH] " Gopal, Saranya 2019-01-17 3:53 ` saranya.gopal 2019-02-06 12:01 ` [PATCH] " Nikolay Yakimov 2019-02-06 12:01 ` Nikolay Yakimov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190115161354.6806-1-root@livid.pp.ru \ --to=root@livid.pp.ru \ --cc=gregkh@linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=saranya.gopal@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.