usb: dwc2: Fix core reset and force mode delays
Subsystems (3)
v5 cover usb: dwc2: Fix core reset and force mode delays
This series accounts for the delay from the IDDIG debounce filter when switching modes. This delay is a function of the PHY clock speed and can range from 5-50 ms. This delay must be taken into account on core reset and force modes. A full explanation is provided in the patch commit log and code comments.
This revision of the series increases the IDDIG delay to 100 ms. Some rockchip platforms seem to timeout even with 50 ms so I have doubled this.
Appreciate any testing on RK3188 platforms.
v5:
* Added Stefan Wahren's tested-by
* Dropped RFT
v4:
* Increased the IDDIG delay to 110ms.
* Removed tested-by for patch 2 since I have changed the delays.
v3:
* Added tested-bys for patch 1-2
* Fixed an issue where a function was not returning a value
* Dropped patch 4
v2:
* Broke up the last patch of the original series
Regards,
John
John Youn (3):
usb: dwc2: gadget: Only initialize device if in device mode
usb: dwc2: Add delay to core soft reset
usb: dwc2: Properly account for the force mode delays
drivers/usb/dwc2/core.c | 126 +++++++++++++++++++++++++++++++++++++++------- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 7 ++- drivers/usb/dwc2/hw.h | 1 + 4 files changed, 116 insertions(+), 19 deletions(-)
Signature · 1 line
2.9.0
PATCH v5 1/3 usb: dwc2: gadget: Only initialize device if in device mode
In dwc2_hsotg_udc_start(), don't initialize the controller for device mode unless we are actually in device mode.
--- drivers/usb/dwc2/gadget.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/dwc2/gadget.c +5 -2
@@ -3466,8 +3466,11 @@ static int dwc2_hsotg_udc_start(struct usb_gadget *gadget,otg_set_peripheral (hsotg->uphy->otg, &hsotg->gadget);spin_lock_irqsave (&hsotg->lock, flags); -dwc2_hsotg_init (hsotg); -dwc2_hsotg_core_init_disconnected (hsotg, false); +if (dwc2_hw_is_device (hsotg)) { +dwc2_hsotg_init (hsotg); +dwc2_hsotg_core_init_disconnected (hsotg, false); + } + hsotg->enabled =0 ;spin_unlock_irqrestore (&hsotg->lock, flags);
Signature · 1 line
2.9.0
PATCH v5 2/3 usb: dwc2: Add delay to core soft reset
In reply to John YounAdd a delay to the core soft reset function to account for the IDDIG debounce filter.
If the current mode is host, either due to the force mode bit being set (which persists after core reset) or the connector id pin, a core soft reset will temporarily reset the mode to device and a delay from the IDDIG debounce filter will occur before going back to host mode.
--- drivers/usb/dwc2/core.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/hw.h | 1 + 3 files changed, 97 insertions(+)
drivers/usb/dwc2/core.c +95 -0
@@ -238,6 +238,77 @@ int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg)return ret; } +/** +* dwc2_wait_for_mode() - Waits for the controller mode. +* @hsotg: Programming view of the DWC_otg controller. +* @host_mode: If true, waits for host mode, otherwise device mode. +*/ +static void dwc2_wait_for_mode (struct dwc2_hsotg *hsotg, +bool host_mode) +{ +ktime_t start; +ktime_t end; +unsigned int timeout =110 ; + +dev_vdbg (hsotg->dev,"Waiting for %s mode\n" , + host_mode ?"host" :"device" ); + + start =ktime_get (); + +while (1 ) { +s64 ms; + +if (dwc2_is_host_mode (hsotg) == host_mode) { +dev_vdbg (hsotg->dev,"%s mode set\n" , + host_mode ?"Host" :"Device" ); +break ; + } + + end =ktime_get (); + ms =ktime_to_ms (ktime_sub (end, start)); + +if (ms >= (s64 )timeout) { +dev_warn (hsotg->dev,"%s: Couldn't set %s mode\n" , + __func__, host_mode ?"host" :"device" ); +break ; + } + +usleep_range (1000 ,2000 ); + } +} + +/** +* dwc2_iddig_filter_enabled() - Returns true if the IDDIG debounce +* filter is enabled. +*/ +static bool dwc2_iddig_filter_enabled (struct dwc2_hsotg *hsotg) +{ +u32 gsnpsid; +u32 ghwcfg4; + +if (!dwc2_hw_is_otg (hsotg)) +return false; + +/* Check if core configuration includes the IDDIG filter. */ + ghwcfg4 =dwc2_readl (hsotg->regs +GHWCFG4 ); +if (!(ghwcfg4 &GHWCFG4_IDDIG_FILT_EN )) +return false; + +/* +* Check if the IDDIG debounce filter is bypassed. Available +* in core version >= 3.10a. +*/ + gsnpsid =dwc2_readl (hsotg->regs +GSNPSID ); +if (gsnpsid >= DWC2_CORE_REV_3_10a) { +u32 gotgctl =dwc2_readl (hsotg->regs +GOTGCTL ); + +if (gotgctl &GOTGCTL_DBNCE_FLTR_BYPASS ) +return false; + } + +return true; +} +/* * Do core a soft reset of the core. Be careful with this because it * resets all the internal state machines of the core.
@@ -246,9 +317,30 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg) {u32 greset;int count =0 ; +bool wait_for_host_mode = false;dev_vdbg (hsotg->dev,"%s()\n" , __func__); +/* +* If the current mode is host, either due to the force mode +* bit being set (which persists after core reset) or the +* connector id pin, a core soft reset will temporarily reset +* the mode to device. A delay from the IDDIG debounce filter +* will occur before going back to host mode. +* +* Determine whether we will go back into host mode after a +* reset and account for this delay after the reset. +*/ +if (dwc2_iddig_filter_enabled (hsotg)) { +u32 gotgctl =dwc2_readl (hsotg->regs +GOTGCTL ); +u32 gusbcfg =dwc2_readl (hsotg->regs +GUSBCFG ); + +if (!(gotgctl &GOTGCTL_CONID_B ) || + (gusbcfg &GUSBCFG_FORCEHOSTMODE )) { + wait_for_host_mode = true; + } + } +/* Core Soft Reset */ greset =dwc2_readl (hsotg->regs +GRSTCTL ); greset |=GRSTCTL_CSFTRST ;
@@ -277,6 +369,9 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg) } }while (!(greset &GRSTCTL_AHBIDLE )); +if (wait_for_host_mode) +dwc2_wait_for_mode (hsotg, true); +return 0 ; }
drivers/usb/dwc2/core.h +1 -0
@@ -882,6 +882,7 @@ struct dwc2_hsotg {#define DWC2_CORE_REV_2_92a 0x4f54292a#define DWC2_CORE_REV_2_94a 0x4f54294a#define DWC2_CORE_REV_3_00a 0x4f54300a +#define DWC2_CORE_REV_3_10a 0x4f54310a#if IS_ENABLED (CONFIG_USB_DWC2_HOST ) ||IS_ENABLED (CONFIG_USB_DWC2_DUAL_ROLE ) union dwc2_hcd_internal_flags {
drivers/usb/dwc2/hw.h +1 -0
@@ -48,6 +48,7 @@#define GOTGCTL_ASESVLD (1 << 18)#define GOTGCTL_DBNC_SHORT (1 << 17)#define GOTGCTL_CONID_B (1 << 16) +#define GOTGCTL_DBNCE_FLTR_BYPASS (1 << 15)#define GOTGCTL_DEVHNPEN (1 << 11)#define GOTGCTL_HSTSETHNPEN (1 << 10)#define GOTGCTL_HNPREQ (1 << 9)
Signature · 1 line
2.9.0
PATCH v5 3/3 usb: dwc2: Properly account for the force mode delays
In reply to John YounWhen a force mode bit is set and the IDDIG debounce filter is enabled, there is a delay for the forced mode to take effect. This delay is due to the IDDIG debounce filter and is variable depending on the platform's PHY clock speed. To account for this delay we can poll for the expected mode.
On a clear force mode, since we don't know what mode to poll for, delay for a fixed 100 ms. This is the maximum delay based on the slowest PHY clock speed.
--- drivers/usb/dwc2/core.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-)
drivers/usb/dwc2/core.c +14 -17
@@ -395,9 +395,9 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)* Checks are done in this function to determine whether doing a force * would be valid or not. * -* If a force is done, it requires a 25ms delay to take effect. -* -* Returns true if the mode was forced. +* If a force is done, it requires a IDDIG debounce filter delay if +* the filter is configured and enabled. We poll the current mode of +* the controller to account for this delay. */ static bool dwc2_force_mode (struct dwc2_hsotg *hsotg,bool host) {
@@ -432,12 +432,18 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host) gusbcfg |= set;dwc2_writel (gusbcfg, hsotg->regs +GUSBCFG ); -msleep (25 ); +dwc2_wait_for_mode (hsotg, host);return true; } -/* -* Clears the force mode bits. +/** +* dwc2_clear_force_mode() - Clears the force mode bits. +* +* After clearing the bits, wait up to 100 ms to account for any +* potential IDDIG filter delay. We can't know if we expect this delay +* or not because the value of the connector ID status is affected by +* the force mode. We only need to call this once during probe if +* dr_mode == OTG. */ static void dwc2_clear_force_mode (struct dwc2_hsotg *hsotg) {
@@ -448,11 +454,8 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg) gusbcfg &= ~GUSBCFG_FORCEDEVMODE ;dwc2_writel (gusbcfg, hsotg->regs +GUSBCFG ); -/* -* NOTE: This long sleep is _very_ important, otherwise the core will -* not stay in host mode after a connector ID change! -*/ -msleep (25 ); +if (dwc2_iddig_filter_enabled (hsotg)) +usleep_range (100000 ,110000 ); }/*
@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode);break ; } - -/* -* NOTE: This is required for some rockchip soc based -* platforms. -*/ -msleep (50 ); }/*
Signature · 1 line
2.9.0
A message that is not in the archive (sent off-list or not yet received): b4aac4b197241b8199c5300656d245692eb45856.1473302184.git.johnyoun-HKixBCOQz3hWk0Htik3J/[email protected]
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Hi John,
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 13 lines
When a force mode bit is set and the IDDIG debounce filter is enabled, there is a delay for the forced mode to take effect. This delay is due to the IDDIG debounce filter and is variable depending on the platform's PHY clock speed. To account for this delay we can poll for the expected mode.
On a clear force mode, since we don't know what mode to poll for, delay for a fixed 100 ms. This is the maximum delay based on the slowest PHY clock speed.
Tested-by Stefan Wahren <[email protected]>---
[...]
drivers/usb/dwc2/core.c void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) · line 481
Quoted 11 lines
drivers/usb/dwc2/core.c +0
-6@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode);break ; } - -/* -* NOTE: This is required for some rockchip soc based -* platforms. -*/ -msleep (50 ); }
sorry for not finding the time to test your subsequent versions, but this still acts up on my Rockchip boards, as I'm still running into errors like
[ 4.875570] usb usb2-port1: connect-debounce failed
And it still requires the 50ms default sleep to work properly. But it seems I was able to find some interesting things when testing the individual parts of your patch. The port that is affected is a host-only port, so I can also get
[ 3.862440] dwc2 101c0000.usb: dwc2_force_dr_mode() to mode 1
{custom debug in dwc2_force_dr_mode}
[ 3.868223] dwc2 101c0000.usb: dwc2_force_mode() no OTG controller{custom debug in dwc2_force_mode at if (!dwc2_hw_is_otg) }
I remember that I also did my previous tests on the host-only ports (since the otg ones are often also used as power-supply) but sadly I only have remote access to my boards this week, so cannot change the cabling to actually try with a real otg dwc2.
Heiko
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Hi Heiko,
Quoted 38 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Hi John,
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 13 lines
When a force mode bit is set and the IDDIG debounce filter is enabled, there is a delay for the forced mode to take effect. This delay is due to the IDDIG debounce filter and is variable depending on the platform's PHY clock speed. To account for this delay we can poll for the expected mode.
On a clear force mode, since we don't know what mode to poll for, delay for a fixed 100 ms. This is the maximum delay based on the slowest PHY clock speed.
Tested-by Stefan Wahren <[email protected]>---
[...]
drivers/usb/dwc2/core.c void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) · line 481
Quoted 11 lines
drivers/usb/dwc2/core.c +0
-6@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode);break ; } - -/* -* NOTE: This is required for some rockchip soc based -* platforms. -*/ -msleep (50 ); }sorry for not finding the time to test your subsequent versions, but this still
acts up on my Rockchip boards, as I'm still running into errors like
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failed
could you please name the relevant DTS file of the affected boards?
Regards
Stefan
A message that is not in the archive (sent off-list or not yet received): 1336050364.27663.591ba3c4-a2b5-418f-9999-6b3cd631f440.open-xchange-7tX72C7vayboQLBSYMtkGA@public.gmane.org
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 40 lines
Quoted 40 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:Quoted 13 lines
When a force mode bit is set and the IDDIG debounce filter is enabled, there is a delay for the forced mode to take effect. This delay is due to the IDDIG debounce filter and is variable depending on the platform's PHY clock speed. To account for this delay we can poll for the expected mode.
On a clear force mode, since we don't know what mode to poll for, delay for a fixed 100 ms. This is the maximum delay based on the slowest PHY clock speed.
Tested-by Stefan Wahren <[email protected]>---
[...]
Quoted 6 lines
drivers/usb/dwc2/core.c +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode);break ;drivers/usb/dwc2/core.c void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg)
Quoted 10 lines
drivers/usb/dwc2/core.c +0
-6} - -/* -* NOTE: This is required for some rockchip soc based -* platforms. -*/ -msleep (50 ); }sorry for not finding the time to test your subsequent versions, but this still
acts up on my Rockchip boards, as I'm still running into errors like
Quoted 3 lines
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylin
both on host-only dwc2 controllers.
Checking my rk3288-veyron-pinky, I see my usb-ethernet coming up correctly and if I'm not mistaken (some sort of blindness when reading dmesg), there the adapter is connected to the real otg dwc2 on there.
So right now it really seems to be limited to host-only dwc2 controllers, but I guess I should double check that once I can switch cables around again.
Heiko
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Hi Heiko,
Quoted 49 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 42 lines
Quoted 41 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:Quoted 13 lines
When a force mode bit is set and the IDDIG debounce filter is enabled, there is a delay for the forced mode to take effect. This delay is due to the IDDIG debounce filter and is variable depending on the platform's PHY clock speed. To account for this delay we can poll for the expected mode.
On a clear force mode, since we don't know what mode to poll for, delay for a fixed 100 ms. This is the maximum delay based on the slowest PHY clock speed.
Tested-by Stefan Wahren <[email protected]>---
[...]
Quoted 6 lines
drivers/usb/dwc2/core.c +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode);break ;drivers/usb/dwc2/core.c void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg)
Quoted 10 lines
drivers/usb/dwc2/core.c +0
-6} - -/* -* NOTE: This is required for some rockchip soc based -* platforms. -*/ -msleep (50 ); }sorry for not finding the time to test your subsequent versions, but this still
acts up on my Rockchip boards, as I'm still running into errors like[ 4.875570] usb usb2-port1: connect-debounce failed
Quoted 8 lines
Quoted 1 line
could you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylinboth on host-only dwc2 controllers.
thanks, does patch 1 & 2 already have a negative effect on these controllers?
Stefan
Quoted 11 lines
Checking my rk3288-veyron-pinky, I see my usb-ethernet coming up correctly and
if I'm not mistaken (some sort of blindness when reading dmesg), there the adapter is connected to the real otg dwc2 on there.
So right now it really seems to be limited to host-only dwc2 controllers, but I guess I should double check that once I can switch cables around again.
Heiko
A message that is not in the archive (sent off-list or not yet received): 19473401.122600.97acfbc9-df96-4427-b533-911010ce5c3f.open-xchange-7tX72C7vayboQLBSYMtkGA@public.gmane.org
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 64 lines
Hi Heiko,
Quoted 62 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 49 lines
Quoted 49 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>---
[...]
Quoted 7 lines
(quoted diff) +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break;Review void dwc2_force_dr_mode(struct dwc2_hsotg
Quoted 10 lines
(quoted diff) +0
-6} - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50); }sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors likeQuoted 3 lines
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
Quoted 7 lines
Quoted 4 lines
rk3188-radxarock
rk3036-kylinboth on host-only dwc2 controllers.
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only vs. otg dwc2 controllers tomorrow as well.
Heiko
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Quoted 68 lines · Heiko Stübner
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 67 lines
Hi Heiko,
Quoted 65 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 49 lines
Quoted 49 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>Signed-off-by John Youn <[email protected]>---
[...]
Quoted 7 lines
(quoted diff) +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break;Review void dwc2_force_dr_mode(struct dwc2_hsotg
Quoted 10 lines
(quoted diff) +0
-6} - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50); }sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors likeQuoted 3 lines
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylin
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
both on host-only dwc2 controllers.
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only vs. otg dwc2 controllers tomorrow as well.
Ok, thanks for testing Heiko.
Given that, let's try to fix this in the next -rc (or before) because I think the Raspberry Pi needs the other parts of that last patch. We can just revert the msleep(50) if needed.
I'll also try to get my hands on a rockchip platform. I think that will help the testing along.
Regards,
John
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
A message that is not in the archive (sent off-list or not yet received): 97a3539d-7176-086f-71f9-371d8d4b016c-HKixBCOQz3hWk0Htik3J/[email protected]
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
Quoted 79 lines · Heiko Stübner
Quoted 70 lines
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 69 lines
Hi Heiko,
Quoted 66 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 49 lines
Quoted 49 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>---
[...]
Quoted 7 lines
(quoted diff) +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break;Review void dwc2_force_dr_mode(struct dwc2_hsotg
Quoted 10 lines
(quoted diff) +0
-6} - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50); }sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors likeQuoted 3 lines
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylinboth on host-only dwc2 controllers.
Quoted 7 lines
Quoted 2 lines
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only
vs. otg dwc2 controllers tomorrow as well.
Quoted 5 lines
Ok, thanks for testing Heiko.
Given that, let's try to fix this in the next -rc (or before) because I think the Raspberry Pi needs the other parts of that last patch. We can just revert the msleep(50) if needed.
I could nicely confirm, that only the host-only controller seems affected by this. Same kernel image that fails on the host-only one with the "connect- debounce failed" message seems to work once I switch that over to the otg controller and the system comes up nicely again.
So if we don't find the root cause in the short term, maybe we could at least limit the delay for host-only variants, like
---------------------- 8< -------------------
drivers/usb/dwc2/core.c +10 -1
@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)*/ void dwc2_force_dr_mode (struct dwc2_hsotg *hsotg) { +bool ret; +switch (hsotg->dr_mode) {case USB_DR_MODE_HOST : -dwc2_force_mode (hsotg, true); + ret =dwc2_force_mode (hsotg, true); +/* +* NOTE: This is required for some rockchip soc based +* platforms on their host-only dwc2. +*/ +if (!ret) +msleep (50 ); +break ;case USB_DR_MODE_PERIPHERAL :dwc2_force_mode (hsotg, false);
---------------------- 8< -------------------
I've been running with that change for some time now on both the rk3036-kylin and rk3188-radxarock. Plugged and unplugged different combinations of usb-devices on the usb otg and host ports, so far seems to work.
Heiko
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Quoted 121 lines · Heiko Stübner
Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
Quoted 79 lines · Heiko Stübner
Quoted 70 lines
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 69 lines
Hi Heiko,
Quoted 66 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 49 lines
Quoted 49 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>Signed-off-by John Youn <[email protected]>---
[...]
Quoted 7 lines
(quoted diff) +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break;Review void dwc2_force_dr_mode(struct dwc2_hsotg
Quoted 10 lines
(quoted diff) +0
-6} - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50); }sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors likeQuoted 3 lines
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylinboth on host-only dwc2 controllers.
Quoted 7 lines
Quoted 2 lines
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only
vs. otg dwc2 controllers tomorrow as well.Quoted 5 lines
Ok, thanks for testing Heiko.
Given that, let's try to fix this in the next -rc (or before) because I think the Raspberry Pi needs the other parts of that last patch. We can just revert the msleep(50) if needed.
I could nicely confirm, that only the host-only controller seems affected by this. Same kernel image that fails on the host-only one with the "connect- debounce failed" message seems to work once I switch that over to the otg controller and the system comes up nicely again.
So if we don't find the root cause in the short term, maybe we could at least limit the delay for host-only variants, like
---------------------- 8< -------------------
drivers/usb/dwc2/core.c +10
-2@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)*/ void dwc2_force_dr_mode (struct dwc2_hsotg *hsotg) { +bool ret; +switch (hsotg->dr_mode) {case USB_DR_MODE_HOST : -dwc2_force_mode (hsotg, true); + ret =dwc2_force_mode (hsotg, true); +/* +* NOTE: This is required for some rockchip soc based +* platforms on their host-only dwc2. +*/ +if (!ret) +msleep (50 ); +break ;case USB_DR_MODE_PERIPHERAL :dwc2_force_mode (hsotg, false); ---------------------- 8< -------------------
Quoted 5 lines
I've been running with that change for some time now on both the rk3036-kylin and rk3188-radxarock. Plugged and unplugged different combinations of usb-devices on the usb otg and host ports, so far seems to work.
Ok great. We'll use that for the -rc if we can't find the root cause.
Then hopefully we can apply the final dropped patch for 4.10.
Regards,
John
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
A message that is not in the archive (sent off-list or not yet received): 6b0c9143-63b3-c5c3-7122-bbbb0c9f610b-HKixBCOQz3hWk0Htik3J/[email protected]
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Hi John,
[add Felipe and Greg]
Am 14.09.2016 um 23:10 schrieb John Youn:
Quoted 114 lines · Heiko Stübner
Quoted 113 lines
Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
Quoted 72 lines · Heiko Stübner
Quoted 64 lines
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 63 lines
Hi Heiko,
Quoted 61 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 41 lines
Quoted 41 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>Signed-off-by John Youn <[email protected]>---
[...]
Quoted 16 lines
(quoted diff) +0
-6@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break; } - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50);Quoted 8 lines
Quoted 7 lines
Quoted 1 line
}sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors like[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylinQuoted 7 lines
Quoted 3 lines
Quoted 1 line
both on host-only dwc2 controllers.
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only
vs. otg dwc2 controllers tomorrow as well.Quoted 5 lines
Ok, thanks for testing Heiko.
Given that, let's try to fix this in the next -rc (or before) because I think the Raspberry Pi needs the other parts of that last patch. We can just revert the msleep(50) if needed.
I could nicely confirm, that only the host-only controller seems affected by this. Same kernel image that fails on the host-only one with the "connect- debounce failed" message seems to work once I switch that over to the otg controller and the system comes up nicely again.
So if we don't find the root cause in the short term, maybe we could at least limit the delay for host-only variants, like
---------------------- 8< -------------------
drivers/usb/dwc2/core.c +10
-2@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)*/ void dwc2_force_dr_mode (struct dwc2_hsotg *hsotg) { +bool ret; +switch (hsotg->dr_mode) {case USB_DR_MODE_HOST : -dwc2_force_mode (hsotg, true); + ret =dwc2_force_mode (hsotg, true); +/* +* NOTE: This is required for some rockchip soc based +* platforms on their host-only dwc2. +*/ +if (!ret) +msleep (50 ); +break ;case USB_DR_MODE_PERIPHERAL :dwc2_force_mode (hsotg, false); ---------------------- 8< -------------------
Quoted 8 lines
Quoted 5 lines
I've been running with that change for some time now on both the rk3036-kylin and rk3188-radxarock. Plugged and unplugged different combinations of usb-devices on the usb otg and host ports, so far seems to work.
Ok great. We'll use that for the -rc if we can't find the root cause.
Then hopefully we can apply the final dropped patch for 4.10.
unfortunately i noticed too late that Felipe and Greg didn't get the whole conversation. The series v5 has been merged in Greg's USB tree.
Stefan
Quoted 3 lines
Regards,
John
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
A message that is not in the archive (sent off-list or not yet received): ad02126f-6c0e-9216-f7ab-ec528eeda2e2-eS4NqCHxEME@public.gmane.org
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
Quoted 126 lines · Stefan Wahren
Hi John,
[add Felipe and Greg]
Am 14.09.2016 um 23:10 schrieb John Youn:
Quoted 121 lines · Heiko Stübner
Quoted 113 lines
Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
Quoted 72 lines · Heiko Stübner
Quoted 64 lines
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 63 lines
Hi Heiko,
Quoted 61 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 41 lines
Quoted 41 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>Signed-off-by John Youn <[email protected]>---
[...]
Quoted 16 lines
(quoted diff) +0
-6@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break; } - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50);Quoted 8 lines
Quoted 7 lines
Quoted 1 line
}sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors like[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylinQuoted 7 lines
Quoted 3 lines
Quoted 1 line
both on host-only dwc2 controllers.
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only
vs. otg dwc2 controllers tomorrow as well.Quoted 5 lines
Ok, thanks for testing Heiko.
Given that, let's try to fix this in the next -rc (or before) because I think the Raspberry Pi needs the other parts of that last patch. We can just revert the msleep(50) if needed.
I could nicely confirm, that only the host-only controller seems affected by this. Same kernel image that fails on the host-only one with the "connect- debounce failed" message seems to work once I switch that over to the otg controller and the system comes up nicely again.
So if we don't find the root cause in the short term, maybe we could at least limit the delay for host-only variants, like
---------------------- 8< -------------------
drivers/usb/dwc2/core.c +10
-2@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)*/ void dwc2_force_dr_mode (struct dwc2_hsotg *hsotg) { +bool ret; +switch (hsotg->dr_mode) {case USB_DR_MODE_HOST : -dwc2_force_mode (hsotg, true); + ret =dwc2_force_mode (hsotg, true); +/* +* NOTE: This is required for some rockchip soc based +* platforms on their host-only dwc2. +*/ +if (!ret) +msleep (50 ); +break ;case USB_DR_MODE_PERIPHERAL :dwc2_force_mode (hsotg, false); ---------------------- 8< -------------------Quoted 5 lines
I've been running with that change for some time now on both the rk3036-kylin and rk3188-radxarock. Plugged and unplugged different combinations of usb-devices on the usb otg and host ports, so far seems to work.
Ok great. We'll use that for the -rc if we can't find the root cause.
Quoted 5 lines
Quoted 1 line
Then hopefully we can apply the final dropped patch for 4.10.
unfortunately i noticed too late that Felipe and Greg didn't get the whole conversation. The series v5 has been merged in Greg's USB tree.
Yeah, we'll have to fix in -rc. Either revert the last patch or add the msleep.
Regards,
John
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays
In reply to Heiko StübnerQuoted 121 lines · Heiko Stübner
Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
Quoted 79 lines · Heiko Stübner
Quoted 70 lines
Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
Quoted 69 lines
Hi Heiko,
Quoted 66 lines
Heiko Stuebner <[email protected]> hat am 12. September 2016 um 13:05 geschrieben:
Hi Stefan,
Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
Quoted 49 lines
Quoted 49 lines
Heiko Stuebner <[email protected]> hat am 11. September 2016 um 23:19 geschrieben:
Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
Quoted 19 lines
When a force mode bit is set and the IDDIG debounce filter is enabled,
there is a delay for the forced mode to take effect. This delay is due
to the IDDIG debounce filter and is variable depending on the platform's
PHY clock speed. To account for this delay we can poll for the expected
mode.On a clear force mode, since we don't know what mode to poll for, delay
for a fixed 100 ms. This is the maximum delay based on the slowest PHY
clock speed.Tested-by Stefan Wahren <[email protected]>---
[...]
Quoted 7 lines
(quoted diff) +0
-0@@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) __func__, hsotg->dr_mode); break;Review void dwc2_force_dr_mode(struct dwc2_hsotg
Quoted 10 lines
(quoted diff) +0
-6} - - /* - * NOTE: This is required for some rockchip soc based - * platforms. - */ - msleep(50); }sorry for not finding the time to test your subsequent versions, but this
still
acts up on my Rockchip boards, as I'm still running into errors likeQuoted 3 lines
Quoted 1 line
[ 4.875570] usb usb2-port1: connect-debounce failedcould you please name the relevant DTS file of the affected boards?
So far I've been able to see that on
rk3188-radxarock
rk3036-kylinboth on host-only dwc2 controllers.
Quoted 7 lines
Quoted 2 lines
thanks, does patch 1 & 2 already have a negative effect on these controllers?
nope, patches 1+2 alone are ok and only the msleep(50) going away causes problems. I'm back home now, so I'll hopefully have time to check host-only
vs. otg dwc2 controllers tomorrow as well.Quoted 5 lines
Ok, thanks for testing Heiko.
Given that, let's try to fix this in the next -rc (or before) because I think the Raspberry Pi needs the other parts of that last patch. We can just revert the msleep(50) if needed.
I could nicely confirm, that only the host-only controller seems affected by this. Same kernel image that fails on the host-only one with the "connect- debounce failed" message seems to work once I switch that over to the otg controller and the system comes up nicely again.
So if we don't find the root cause in the short term, maybe we could at least limit the delay for host-only variants, like
---------------------- 8< -------------------
drivers/usb/dwc2/core.c +10
-2@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)*/ void dwc2_force_dr_mode (struct dwc2_hsotg *hsotg) { +bool ret; +switch (hsotg->dr_mode) {case USB_DR_MODE_HOST : -dwc2_force_mode (hsotg, true); + ret =dwc2_force_mode (hsotg, true); +/* +* NOTE: This is required for some rockchip soc based +* platforms on their host-only dwc2. +*/ +if (!ret) +msleep (50 ); +break ;case USB_DR_MODE_PERIPHERAL :dwc2_force_mode (hsotg, false); ---------------------- 8< -------------------
Quoted 6 lines
I've been running with that change for some time now on both the rk3036-kylin and rk3188-radxarock. Plugged and unplugged different combinations of usb-devices on the usb otg and host ports, so far seems to work.
Hi Heiko,
Let's add the delay just as you have it here.
I'm not sure how the delay comes into play in host-only hardware though. I will see if the RTL engineers know why that might be needed.
Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html