> From: Ido Shayevitz [mailto:idos@xxxxxxxxxxxxxx] > Sent: Monday, July 30, 2012 3:15 AM > > On Thu, July 26, 2012 4:38 pm, Paul Zimmerman wrote: > >> From: linux-usb-owner@xxxxxxxxxxxxxxx > >> [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Ido Shayevitz > >> Sent: Wednesday, July 25, 2012 5:46 AM > >> --- > >> drivers/usb/dwc3/Kconfig | 6 +- > >> drivers/usb/dwc3/Makefile | 2 + > >> drivers/usb/dwc3/core.c | 15 +- > >> drivers/usb/dwc3/core.h | 54 +++++- > >> drivers/usb/dwc3/dwc3_otg.c | 512 > >> ++++++++++++++++++++++++++++++++++++++++++ > >> drivers/usb/dwc3/dwc3_otg.h | 38 +++ > >> drivers/usb/dwc3/gadget.c | 63 +++++ > >> drivers/usb/host/xhci-plat.c | 21 ++ > >> drivers/usb/host/xhci.c | 13 +- > >> 9 files changed, 711 insertions(+), 13 deletions(-) > >> create mode 100644 drivers/usb/dwc3/dwc3_otg.c > >> create mode 100644 drivers/usb/dwc3/dwc3_otg.h > >> > >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > >> index d13c60f..0cc108d 100644 > >> --- a/drivers/usb/dwc3/Kconfig > >> +++ b/drivers/usb/dwc3/Kconfig > >> @@ -1,9 +1,9 @@ > >> config USB_DWC3 > >> tristate "DesignWare USB3 DRD Core Support" > >> - depends on (USB && USB_GADGET) > >> + depends on (USB || USB_GADGET) > > > > Are you sure this is correct? How can a dual-role device work unless both > > USB and USB_GADGET are set? > > If the USB_DWC3 is not selected then even dwc3/core.c is not being built > and core.c supports also non DRD cores (depends on DWC3_MODE), so we want > to build the DWC3 if there is at least host support or a gadget support. > In case that the DWC_MODE is DRD but USB is not on, then dwc3/host.c will > add the xHCI device, but will be no xHCI driver (xhci-plat) that will > probe on this device. I just tried compiling this without USB_GADGET defined, and I get these errors: HOSTCC arch/x86/boot/compressed/mkpiggy ERROR: "usb_gadget_map_request" [drivers/usb/dwc3/dwc3.ko] undefined! ERROR: "usb_del_gadget_udc" [drivers/usb/dwc3/dwc3.ko] undefined! ERROR: "usb_gadget_unmap_request" [drivers/usb/dwc3/dwc3.ko] undefined! ERROR: "usb_add_gadget_udc" [drivers/usb/dwc3/dwc3.ko] undefined! > >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > >> index c34452a..5343e39 100644 > >> --- a/drivers/usb/dwc3/core.c > >> +++ b/drivers/usb/dwc3/core.c > >> @@ -517,15 +517,24 @@ static int __devinit dwc3_probe(struct > >> platform_device *pdev) > >> break; > >> case DWC3_MODE_DRD: > >> dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG); > >> + ret = dwc3_otg_init(dwc); > >> + if (ret) { > >> + dev_err(dev, "failed to initialize otg\n"); > >> + goto err1; > >> + } > >> + > > > > You should follow the existing pattern for error handling in this > > function. If there is an error, jump to the error exit, and put your > > exit function there, instead of here. > > I think I followed the pattern for error handling in this function. > Please see that it is the same as "case DWC3_MODE_HOST" or "case > DWC3_MODE_DEVICE" > > >> ret = dwc3_host_init(dwc); > >> if (ret) { > >> dev_err(dev, "failed to initialize host\n"); > >> + dwc3_otg_exit(dwc); > > > > Same here. > > Here I was thinking of moving "dwc3_otg_exit()" to the error handling > section, but I think it will make the error handling section too clumsy > since eventhough the mode is DWC3_MODE_DRD, in this point I want to exit > only the otg and not the host or gadget. So this should have a new error > label that will be located between err1 and err2. On the other hand if the > error will happen later, as the "goto err2" statement below in this > function, then if the mode is DRD I will want to activate the exit > function of also the host and gadget but not doing the otg exit again (the > new label) and then do err1... So it becomes clumsy. Yes, I see now that the error handling for DRD in that function was broken before your patch, and your patch fixes it. So it's up to Felipe to decide if the way you did it is acceptable. -- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
