Hi Fabrice, >On 3/15/2023 6:45 PM, Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx> wrote: >From: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx> >Sent: Wednesday, March 15, 2023 6:45 PM >To: Minas Harutyunyan <hminas@xxxxxxxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; >quentin.schulz@xxxxxxxxxxxxxxxxxxxxx >Cc: linux-usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-stm32@st- >md-mailman.stormreply.com; amelie.delaunay@xxxxxxxxxxx; >alexandre.torgue@xxxxxxxxxxx; fabrice.gasnier@xxxxxxxxxxx >Subject: [PATCH] usb: dwc2: fix a race, don't power off/on phy for dual-role >mode > >When in dual role mode (dr_mode == USB_DR_MODE_OTG), platform probe >successively basically calls: >- dwc2_gadget_init() >- dwc2_hcd_init() >- dwc2_lowlevel_hw_disable() since recent change [1] >- usb_add_gadget_udc() > >The PHYs (and so the clocks it may provide) shouldn't be disabled for all >SoCs, in OTG mode, as the HCD part has been initialized. > >On STM32 this creates some weird race condition upon boot, when: >- initially attached as a device, to a HOST >- and there is a gadget script invoked to setup the device part. >Below issue becomes systematic, as long as the gadget script isn't started >by userland: the hardware PHYs (and so the clocks provided by the >PHYs) remains disabled. >It ends up in having an endless interrupt storm, before the watchdog resets >the platform. > >[ 16.924163] dwc2 49000000.usb-otg: EPs: 9, dedicated fifos, 952 entries >in SPRAM >[ 16.962704] dwc2 49000000.usb-otg: DWC OTG Controller >[ 16.966488] dwc2 49000000.usb-otg: new USB bus registered, assigned bus >number 2 >[ 16.974051] dwc2 49000000.usb-otg: irq 77, io mem 0x49000000 >[ 17.032170] hub 2-0:1.0: USB hub found >[ 17.042299] hub 2-0:1.0: 1 port detected >[ 17.175408] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in >Host mode >[ 17.181741] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in >Host mode >[ 17.189303] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in >Host mode >... > >The host part is also not functional, until the gadget part is configured. > >The HW may only be disabled for peripheral mode (original init), e.g. >dr_mode == USB_DR_MODE_PERIPHERAL, until the gadget driver initializes. > >But when in USB_DR_MODE_OTG, the HW should remain enabled, as the HCD part >is able to run, while the gadget part isn't necessarily configured. > >I don't fully get the of purpose the original change, that claims disabling >the hardware is missing. It creates conditions on SOCs using the PHY >initialization to be completely non working in OTG mode. Original change [1] >should be reworked to be platform specific. > >[1] https://urldefense.com/v3/__https://lore.kernel.org/r/20221206-dwc2- >gadget-dual-role-v1-2-36515e1092cd@theobroma- >systems.com__;!!A4F2R9G_pg!Y21e8pRbIOVyLQTRP5HjdeDUHpSjbtiRQVFGVCOBBDu9yH32W >tdppqmP-8TLyGrBjyOBG5iI4qw6XMFEdfRJDhZ8HVc$ > >Fixes: ade23d7b7ec5 ("usb: dwc2: power on/off phy for peripheral mode in >dual-role mode") >Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx> Acked-by: Minas Harutyunyan <hminas@xxxxxxxxxxxx> >--- > drivers/usb/dwc2/gadget.c | 6 ++---- > drivers/usb/dwc2/platform.c | 3 +-- > 2 files changed, 3 insertions(+), 6 deletions(-) > >diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index >62fa6378d2d7..8b15742d9e8a 100644 >--- a/drivers/usb/dwc2/gadget.c >+++ b/drivers/usb/dwc2/gadget.c >@@ -4549,8 +4549,7 @@ static int dwc2_hsotg_udc_start(struct usb_gadget >*gadget, > hsotg->gadget.dev.of_node = hsotg->dev->of_node; > hsotg->gadget.speed = USB_SPEED_UNKNOWN; > >- if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL || >- (hsotg->dr_mode == USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg))) >{ >+ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) { > ret = dwc2_lowlevel_hw_enable(hsotg); > if (ret) > goto err; >@@ -4612,8 +4611,7 @@ static int dwc2_hsotg_udc_stop(struct usb_gadget >*gadget) > if (!IS_ERR_OR_NULL(hsotg->uphy)) > otg_set_peripheral(hsotg->uphy->otg, NULL); > >- if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL || >- (hsotg->dr_mode == USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg))) >+ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) > dwc2_lowlevel_hw_disable(hsotg); > > return 0; >diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index >23ef75996823..262c13b6362a 100644 >--- a/drivers/usb/dwc2/platform.c >+++ b/drivers/usb/dwc2/platform.c >@@ -576,8 +576,7 @@ static int dwc2_driver_probe(struct platform_device >*dev) > dwc2_debugfs_init(hsotg); > > /* Gadget code manages lowlevel hw on its own */ >- if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL || >- (hsotg->dr_mode == USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg))) >+ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) > dwc2_lowlevel_hw_disable(hsotg); > > #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \ >-- >2.25.1
