Hi Sjoerd, On 12/01/2024 08:52, Sjoerd Simons wrote: > When dr_mode is "otg" the dwc3 is initially configured in _OTG mode; > However in this mode the gadget functionality doesn't work without > further configuration. To resolve that on gadget start switch to _DEVICE > mode globally and go back to _OTG on stop again. > > For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a > conflict with the same function exposed by xhci-dwc3
I think exporting dwc3_core_set_mode() is probably sensible here. But I'm not so sure on calling it from dwc3_gadget_start(), that's making assumptions about board specific implementation details - some boards might require additional configuration to switch into gadget mode. What about calling dwc3_core_set_mode() from within your board-specific board_usb_init() function? Obviously as you said the ideal solution here is that we can correctly traverse the type-c connector model from within U-Boot, but that's a whole other kettle of fish :P On Qualcomm platforms I'm currently handling this by overriding the dr_mode property in a <board>-u-boot.dtsi file, if you aren't using the same DT for U-Boot and Linux then maybe this would be acceptable for you too? Kind regards, > > Signed-off-by: Sjoerd Simons <sjo...@collabora.com> > > --- > > Changes in v4: > - New patch > > drivers/usb/dwc3/core.c | 10 +++++----- > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/gadget.c | 6 ++++++ > 3 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 4b4fcd8a22e..d22d4c4bb6a 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -42,7 +42,7 @@ > static LIST_HEAD(dwc3_list); > /* > -------------------------------------------------------------------------- */ > > -static void dwc3_set_mode(struct dwc3 *dwc, u32 mode) > +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode) > { > u32 reg; > > @@ -736,7 +736,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > > switch (dwc->dr_mode) { > case USB_DR_MODE_PERIPHERAL: > - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > + dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > ret = dwc3_gadget_init(dwc); > if (ret) { > dev_err(dwc->dev, "failed to initialize gadget\n"); > @@ -744,7 +744,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > } > break; > case USB_DR_MODE_HOST: > - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST); > + dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST); > ret = dwc3_host_init(dwc); > if (ret) { > dev_err(dwc->dev, "failed to initialize host\n"); > @@ -752,7 +752,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > } > break; > case USB_DR_MODE_OTG: > - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG); > + dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG); > ret = dwc3_host_init(dwc); > if (ret) { > dev_err(dwc->dev, "failed to initialize host\n"); > @@ -810,7 +810,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc) > * switch back to peripheral mode > * This enables the phy to enter idle and then, if enabled, suspend. > */ > - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > + dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > dwc3_gadget_run(dwc); > } > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 4162a682298..1e7eda89a34 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1057,6 +1057,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc); > void dwc3_of_parse(struct dwc3 *dwc); > int dwc3_init(struct dwc3 *dwc); > void dwc3_remove(struct dwc3 *dwc); > +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode); > > static inline int dwc3_host_init(struct dwc3 *dwc) > { return 0; } > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 406d36ceafe..69d9fe40e2f 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1468,6 +1468,9 @@ static int dwc3_gadget_start(struct usb_gadget *g, > > dwc->gadget_driver = driver; > > + if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG) > + dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > + > reg = dwc3_readl(dwc->regs, DWC3_DCFG); > reg &= ~(DWC3_DCFG_SPEED_MASK); > > @@ -1559,6 +1562,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g) > __dwc3_gadget_ep_disable(dwc->eps[0]); > __dwc3_gadget_ep_disable(dwc->eps[1]); > > + if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG) > + dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG); > + > dwc->gadget_driver = NULL; > > spin_unlock_irqrestore(&dwc->lock, flags); -- // Caleb (they/them)