| Message ID | 20230328160756.30520-3-quic_kriskura@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Handle core soft reset failure in pullup | expand |
Hi Alan, Geert, Can you help review and provide comments/approval on the following patch. Regards, Krishna, On 3/28/2023 9:37 PM, Krishna Kurapati wrote: > In the event, gadget_connect call (which invokes pullup) fails, > propagate the error to udc bind operation which inturn sends the > error to configfs. The userspace can then retry enumeartion if > it chooses to. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > drivers/usb/gadget/udc/core.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index 23b0629a8774..975205a1853f 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -1051,12 +1051,16 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state); > > /* ------------------------------------------------------------------------- */ > > -static void usb_udc_connect_control(struct usb_udc *udc) > +static int usb_udc_connect_control(struct usb_udc *udc) > { > + int ret; > + > if (udc->vbus) > - usb_gadget_connect(udc->gadget); > + ret = usb_gadget_connect(udc->gadget); > else > - usb_gadget_disconnect(udc->gadget); > + ret = usb_gadget_disconnect(udc->gadget); > + > + return ret; > } > > /** > @@ -1500,11 +1504,19 @@ static int gadget_bind_driver(struct device *dev) > if (ret) > goto err_start; > usb_gadget_enable_async_callbacks(udc); > - usb_udc_connect_control(udc); > + ret = usb_udc_connect_control(udc); > + if (ret) > + goto err_connect_control; > > kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); > return 0; > > + err_connect_control: > + usb_gadget_disable_async_callbacks(udc); > + if (gadget->irq) > + synchronize_irq(gadget->irq); > + usb_gadget_udc_stop(udc); > + > err_start: > driver->unbind(udc->gadget); >
Hi Krishna, On Wed, Apr 26, 2023 at 3:17 AM Krishna Kurapati PSSNV <quic_kriskura@quicinc.com> wrote: > Hi Alan, Geert, > > Can you help review and provide comments/approval on the following patch. I don't know why you are addressing me, as I never touched the affected file, am not listed as its maintainer, and don't know much about USB UDC. Gr{oetje,eeting}s, Geert
On Wed, Apr 26, 2023 at 06:47:13AM +0530, Krishna Kurapati PSSNV wrote: > Hi Alan, Geert, > > Can you help review and provide comments/approval on the following patch. > > Regards, > Krishna, Sorry this took so long to review; I've been quite busy. The patch is good except for one stylistic thing... > On 3/28/2023 9:37 PM, Krishna Kurapati wrote: > > In the event, gadget_connect call (which invokes pullup) fails, > > propagate the error to udc bind operation which inturn sends the > > error to configfs. The userspace can then retry enumeartion if > > it chooses to. > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > --- > > drivers/usb/gadget/udc/core.c | 20 ++++++++++++++++---- > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > index 23b0629a8774..975205a1853f 100644 > > --- a/drivers/usb/gadget/udc/core.c > > +++ b/drivers/usb/gadget/udc/core.c > > @@ -1051,12 +1051,16 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state); > > /* ------------------------------------------------------------------------- */ > > -static void usb_udc_connect_control(struct usb_udc *udc) > > +static int usb_udc_connect_control(struct usb_udc *udc) > > { > > + int ret; > > + > > if (udc->vbus) > > - usb_gadget_connect(udc->gadget); > > + ret = usb_gadget_connect(udc->gadget); > > else > > - usb_gadget_disconnect(udc->gadget); > > + ret = usb_gadget_disconnect(udc->gadget); > > + > > + return ret; > > } > > /** > > @@ -1500,11 +1504,19 @@ static int gadget_bind_driver(struct device *dev) > > if (ret) > > goto err_start; > > usb_gadget_enable_async_callbacks(udc); > > - usb_udc_connect_control(udc); > > + ret = usb_udc_connect_control(udc); > > + if (ret) > > + goto err_connect_control; > > kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); > > return 0; > > + err_connect_control: For consistency with the other error-handling statement labels in this routine, there should be a blank line immediately before this label. > > + usb_gadget_disable_async_callbacks(udc); > > + if (gadget->irq) > > + synchronize_irq(gadget->irq); > > + usb_gadget_udc_stop(udc); > > + > > err_start: > > driver->unbind(udc->gadget); Everything else looks okay. Acked-by: Alan Stern <stern@rowland.harvard.edu> Alan Stern
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 23b0629a8774..975205a1853f 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -1051,12 +1051,16 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state); /* ------------------------------------------------------------------------- */ -static void usb_udc_connect_control(struct usb_udc *udc) +static int usb_udc_connect_control(struct usb_udc *udc) { + int ret; + if (udc->vbus) - usb_gadget_connect(udc->gadget); + ret = usb_gadget_connect(udc->gadget); else - usb_gadget_disconnect(udc->gadget); + ret = usb_gadget_disconnect(udc->gadget); + + return ret; } /** @@ -1500,11 +1504,19 @@ static int gadget_bind_driver(struct device *dev) if (ret) goto err_start; usb_gadget_enable_async_callbacks(udc); - usb_udc_connect_control(udc); + ret = usb_udc_connect_control(udc); + if (ret) + goto err_connect_control; kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); return 0; + err_connect_control: + usb_gadget_disable_async_callbacks(udc); + if (gadget->irq) + synchronize_irq(gadget->irq); + usb_gadget_udc_stop(udc); + err_start: driver->unbind(udc->gadget);
In the event, gadget_connect call (which invokes pullup) fails, propagate the error to udc bind operation which inturn sends the error to configfs. The userspace can then retry enumeartion if it chooses to. Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> --- drivers/usb/gadget/udc/core.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)