Hi Sjoerd, Caleb On ven., janv. 12, 2024 at 12:55, Caleb Connolly <caleb.conno...@linaro.org> wrote:
> 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. Indeed. As an example, take the Khadas VIM3 which has a glue driver to do the mode switching. > > What about calling dwc3_core_set_mode() from within your board-specific > board_usb_init() function? With DM_USB_GADGET, board_usb_init/cleanup() are no longer used. For VIM3, I solved this with: https://lore.kernel.org/all/20221024-meson-dm-usb-v1-1-2ab077a50...@baylibre.com/ Maybe that can help? > > 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? I would prefer a <board>-u-boot.dtsi change as well. > > 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)