| Message ID | 20240609213449.194762-5-marek.vasut+renesas@mailbox.org |
|---|---|
| State | Accepted |
| Commit | bd7ec7b04f877b8b4a88d4367f100dc3f0af27a3 |
| Delegated to: | Mattijs Korpershoek |
| Headers | show |
| Series | [1/6] usb: gadget: g_dnl: Drop usb_gadget_controller_number() | expand |
Hi Marek, Thank you for the patch. On dim., juin 09, 2024 at 23:32, Marek Vasut <marek.vasut+renesas@mailbox.org> wrote: > If .match_ep() callback returns non-NULL endpoint, immediately check > its usability and if the returned endpoint is usable, stop search and > return the endpoint. Otherwise, continue with best effort search for > usable endpoint. > > Currently the code would attempt the best effort search in any case, > which may find another unexpected endpoint. It is likely that the > intention of the original code was to stop the search early. > > Fixes: 77dcbdf3c1ce ("usb: gadget: Add match_ep() op to usb_gadget_ops") > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org> I've added Vignesh to the CC list since he is the author of 77dcbdf3c1ce. He might be able to comment if this was indeed a mistake. It looks like a good fix to me as well. With this change we match more closely the linux implementation (usb_ep_autoconfig_ss()). Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3 > --- > Cc: Alexander Sverdlin <alexander.sverdlin@siemens.com> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com> > Cc: Lukasz Majewski <lukma@denx.de> > Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com> > Cc: Nishanth Menon <nm@ti.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: u-boot@lists.denx.de > --- > drivers/usb/gadget/epautoconf.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c > index 09950ceeaed..66599ce8efa 100644 > --- a/drivers/usb/gadget/epautoconf.c > +++ b/drivers/usb/gadget/epautoconf.c > @@ -247,8 +247,11 @@ struct usb_ep *usb_ep_autoconfig( > return ep; > } > > - if (gadget->ops->match_ep) > + if (gadget->ops->match_ep) { > ep = gadget->ops->match_ep(gadget, desc, NULL); > + if (ep && ep_matches(gadget, ep, desc)) > + return ep; > + } > > /* Second, look at endpoints until an unclaimed one looks usable */ > list_for_each_entry(ep, &gadget->ep_list, ep_list) { > -- > 2.43.0
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 09950ceeaed..66599ce8efa 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -247,8 +247,11 @@ struct usb_ep *usb_ep_autoconfig( return ep; } - if (gadget->ops->match_ep) + if (gadget->ops->match_ep) { ep = gadget->ops->match_ep(gadget, desc, NULL); + if (ep && ep_matches(gadget, ep, desc)) + return ep; + } /* Second, look at endpoints until an unclaimed one looks usable */ list_for_each_entry(ep, &gadget->ep_list, ep_list) {
If .match_ep() callback returns non-NULL endpoint, immediately check its usability and if the returned endpoint is usable, stop search and return the endpoint. Otherwise, continue with best effort search for usable endpoint. Currently the code would attempt the best effort search in any case, which may find another unexpected endpoint. It is likely that the intention of the original code was to stop the search early. Fixes: 77dcbdf3c1ce ("usb: gadget: Add match_ep() op to usb_gadget_ops") Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org> --- Cc: Alexander Sverdlin <alexander.sverdlin@siemens.com> Cc: Felipe Balbi <felipe.balbi@linux.intel.com> Cc: Lukasz Majewski <lukma@denx.de> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com> Cc: Nishanth Menon <nm@ti.com> Cc: Simon Glass <sjg@chromium.org> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com> Cc: Tom Rini <trini@konsulko.com> Cc: u-boot@lists.denx.de --- drivers/usb/gadget/epautoconf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)