Messages in this thread | ![/](/images/icornerl.gif) | | From | Frank Li <> | Subject | RE: [EXT] Re: [PATCH 1/1] usb: gadget: call usb_gadget_check_config() to verify UDC capability | Date | Thu, 29 Jun 2023 03:40:18 +0000 |
| |
> -----Original Message----- > From: Roger Quadros <rogerq@kernel.org> > Sent: Wednesday, June 28, 2023 10:23 PM > To: Frank Li <frank.li@nxp.com>; r-gunasekaran@ti.com; imx@lists.linux.dev; > Jun Li <jun.li@nxp.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; Elson Roy Serrao > <quic_eserrao@quicinc.com>; Thinh Nguyen > <Thinh.Nguyen@synopsys.com>; Andy Shevchenko > <andriy.shevchenko@linux.intel.com>; Jó Ágila Bitsch <jgilab@gmail.com>; > Prashanth K <quic_prashk@quicinc.com>; Peter Chen > <peter.chen@kernel.org>; open list:USB SUBSYSTEM <linux- > usb@vger.kernel.org>; open list <linux-kernel@vger.kernel.org> > Subject: [EXT] Re: [PATCH 1/1] usb: gadget: call usb_gadget_check_config() > to verify UDC capability > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On 29/06/2023 03:54, Frank Li wrote: > > The legacy gadget driver omitted calling usb_gadget_check_config() > > to ensure that the USB device controller (UDC) has adequate resources, > > including sufficient endpoint numbers and types, to support the given > > configuration. > > > > Previously, usb_add_config() was solely invoked by the legacy gadget > > driver. Adds the necessary usb_gadget_check_config() after the bind() > > operation to fix the issue. > > You have only fixed composite.c. Not all gadget drivers use composite.c > so it will be still broken for them. > > Please also add default sane configuration in CDNS3 so it works even > if usb_gadget_check_config() is not invoked.
Which one was not call usb_add_config()? DWC3 also use .check_config = dwc3_gadget_check_config, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/dwc3/gadget.c
I think it is not correct by assume UDC can support specific config at gadget function driver. Add default value of CDNS3 actually hidden potential problem.
I don't suggest it before fixed hidden potential problem.
Frank
> > > > > Fixes: dce49449e04f ("usb: cdns3: allocate TX FIFO size according to > composite EP number") > > Reported-by: Ravi Gunasekaran <r-gunasekaran@ti.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/usb/gadget/composite.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/usb/gadget/composite.c > b/drivers/usb/gadget/composite.c > > index 1b3489149e5e..dd9b90481b4c 100644 > > --- a/drivers/usb/gadget/composite.c > > +++ b/drivers/usb/gadget/composite.c > > @@ -1125,6 +1125,10 @@ int usb_add_config(struct usb_composite_dev > *cdev, > > goto done; > > > > status = bind(config); > > + > > + if (status == 0) > > + status = usb_gadget_check_config(cdev->gadget); > > + > > if (status < 0) { > > while (!list_empty(&config->functions)) { > > struct usb_function *f; > > -- > cheers, > -roger
| ![\](/images/icornerr.gif) |