| Message ID | 1644999135-13478-1-git-send-email-fabrice.gasnier@foss.st.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] usb: dwc2: drd: fix soft connect when gadget is unconfigured | expand |
On 2/16/2022 12:12 PM, Fabrice Gasnier wrote: > When the gadget driver hasn't been (yet) configured, and the cable is > connected to a HOST, the SFTDISCON gets cleared unconditionally, so the > HOST tries to enumerate it. > At the host side, this can result in a stuck USB port or worse. When > getting lucky, some dmesg can be observed at the host side: > new high-speed USB device number ... > device descriptor read/64, error -110 > > Fix it in drd, by checking the enabled flag before calling > dwc2_hsotg_core_connect(). It will be called later, once configured, > by the normal flow: > - udc_bind_to_driver > - usb_gadget_connect > - dwc2_hsotg_pullup > - dwc2_hsotg_core_connect > > Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support") > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com> Acked-by: Minas Harutyunyan <hminas@synopsys.com> > --- > Changes in v3: > - remove extra () in dwc2_is_device_enabled() macro. > Changes in v2: > - Fix build error: 'struct dwc2_hsotg' has no member named 'enabled'; > as reported by the kernel test robot. > https://urldefense.com/v3/__https://lore.kernel.org/all/202202112236.AwoOTtHO-lkp@intel.com/__;!!A4F2R9G_pg!KxYqv4z1NDZM9yu_eZO1eEo1ZARNbT10DdREGmrUOJvH3Jh9soyw9Hd5lckEWwA$ > Add dwc2_is_device_enabled() macro to handle this. > --- > drivers/usb/dwc2/core.h | 2 ++ > drivers/usb/dwc2/drd.c | 6 ++++-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 8a63da3..88c337b 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -1418,6 +1418,7 @@ void dwc2_hsotg_core_connect(struct dwc2_hsotg *hsotg); > void dwc2_hsotg_disconnect(struct dwc2_hsotg *dwc2); > int dwc2_hsotg_set_test_mode(struct dwc2_hsotg *hsotg, int testmode); > #define dwc2_is_device_connected(hsotg) (hsotg->connected) > +#define dwc2_is_device_enabled(hsotg) (hsotg->enabled) > int dwc2_backup_device_registers(struct dwc2_hsotg *hsotg); > int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup); > int dwc2_gadget_enter_hibernation(struct dwc2_hsotg *hsotg); > @@ -1454,6 +1455,7 @@ static inline int dwc2_hsotg_set_test_mode(struct dwc2_hsotg *hsotg, > int testmode) > { return 0; } > #define dwc2_is_device_connected(hsotg) (0) > +#define dwc2_is_device_enabled(hsotg) (0) > static inline int dwc2_backup_device_registers(struct dwc2_hsotg *hsotg) > { return 0; } > static inline int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, > diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c > index 1b39c47..d8d6493 100644 > --- a/drivers/usb/dwc2/drd.c > +++ b/drivers/usb/dwc2/drd.c > @@ -130,8 +130,10 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role) > already = dwc2_ovr_avalid(hsotg, true); > } else if (role == USB_ROLE_DEVICE) { > already = dwc2_ovr_bvalid(hsotg, true); > - /* This clear DCTL.SFTDISCON bit */ > - dwc2_hsotg_core_connect(hsotg); > + if (dwc2_is_device_enabled(hsotg)) { > + /* This clear DCTL.SFTDISCON bit */ > + dwc2_hsotg_core_connect(hsotg); > + } > } else { > if (dwc2_is_device_mode(hsotg)) { > if (!dwc2_ovr_bvalid(hsotg, false))
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 8a63da3..88c337b 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -1418,6 +1418,7 @@ void dwc2_hsotg_core_connect(struct dwc2_hsotg *hsotg); void dwc2_hsotg_disconnect(struct dwc2_hsotg *dwc2); int dwc2_hsotg_set_test_mode(struct dwc2_hsotg *hsotg, int testmode); #define dwc2_is_device_connected(hsotg) (hsotg->connected) +#define dwc2_is_device_enabled(hsotg) (hsotg->enabled) int dwc2_backup_device_registers(struct dwc2_hsotg *hsotg); int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup); int dwc2_gadget_enter_hibernation(struct dwc2_hsotg *hsotg); @@ -1454,6 +1455,7 @@ static inline int dwc2_hsotg_set_test_mode(struct dwc2_hsotg *hsotg, int testmode) { return 0; } #define dwc2_is_device_connected(hsotg) (0) +#define dwc2_is_device_enabled(hsotg) (0) static inline int dwc2_backup_device_registers(struct dwc2_hsotg *hsotg) { return 0; } static inline int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c index 1b39c47..d8d6493 100644 --- a/drivers/usb/dwc2/drd.c +++ b/drivers/usb/dwc2/drd.c @@ -130,8 +130,10 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role) already = dwc2_ovr_avalid(hsotg, true); } else if (role == USB_ROLE_DEVICE) { already = dwc2_ovr_bvalid(hsotg, true); - /* This clear DCTL.SFTDISCON bit */ - dwc2_hsotg_core_connect(hsotg); + if (dwc2_is_device_enabled(hsotg)) { + /* This clear DCTL.SFTDISCON bit */ + dwc2_hsotg_core_connect(hsotg); + } } else { if (dwc2_is_device_mode(hsotg)) { if (!dwc2_ovr_bvalid(hsotg, false))
When the gadget driver hasn't been (yet) configured, and the cable is connected to a HOST, the SFTDISCON gets cleared unconditionally, so the HOST tries to enumerate it. At the host side, this can result in a stuck USB port or worse. When getting lucky, some dmesg can be observed at the host side: new high-speed USB device number ... device descriptor read/64, error -110 Fix it in drd, by checking the enabled flag before calling dwc2_hsotg_core_connect(). It will be called later, once configured, by the normal flow: - udc_bind_to_driver - usb_gadget_connect - dwc2_hsotg_pullup - dwc2_hsotg_core_connect Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support") Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com> --- Changes in v3: - remove extra () in dwc2_is_device_enabled() macro. Changes in v2: - Fix build error: 'struct dwc2_hsotg' has no member named 'enabled'; as reported by the kernel test robot. https://lore.kernel.org/all/202202112236.AwoOTtHO-lkp@intel.com/ Add dwc2_is_device_enabled() macro to handle this. --- drivers/usb/dwc2/core.h | 2 ++ drivers/usb/dwc2/drd.c | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-)