lkml.org 
[lkml]   [2023]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXT] Re: [PATCH 1/1] usb: gadget: call usb_gadget_check_config() to verify UDC capability
Date


> -----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
\
 
 \ /
  Last update: 2023-06-29 05:41    [W:1.079 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site