diff mbox

usb: dwc2: host: Properly set the HFIR

Message ID 1453506971-31445-1-git-send-email-dianders@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Doug Anderson Jan. 22, 2016, 11:56 p.m. UTC
According to the most up to date version of the dwc2 databook, the FRINT
field of the HFIR register should be programmed to:
* 125 us * (PHY clock freq for HS) - 1
* 1000 us * (PHY clock freq for FS/LS) - 1

This is opposed to older versions of the doc that claimed it should be:
* 125 us * (PHY clock freq for HS)
* 1000 us * (PHY clock freq for FS/LS)

In case you didn't spot it, the difference is the "- 1".

Let's add the "- 1" to match the newest user manual.  It's presumed that
the "- 1" should have always been there and that this was always a
documentation error.  If some hardware needs the "- 1" and other
hardware doesn't, we'll have to add a configuration parameter for it in
the future.

I checked things before and after this patch on rk3288 using a Total
Phase Beagle 5000 analyzer.

Before this patch, a low speed mouse shows constant Frame Timing Jitter
errors.  After this patch errors have gone away.

Before this patch SOF packets move forward about 1 us per 4 ms.  After
this patch the SOF packets move backward about 1 us per 255 ms.  Some
specific SOF timestamps from the analyzer are below.

Before:
  6.603.790
  6.603.916
  6.604.041
  6.604.166
  ...
  6.607.541
  6.607.667
  6.607.792
  6.607.917
  ...
  6.611.417
  6.611.543
  6.611.668
  6.611.793

After:
  6.215.159
  6.215.284
  6.215.408
  6.215.533
  6.215.658
  ...
  6.470.658
  6.470.783
  6.470.907
  ...
  6.726.032
  6.726.157
  6.725.281
  6.725.406

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Heiko Stuebner Jan. 23, 2016, 5:53 p.m. UTC | #1
Am Freitag, 22. Januar 2016, 15:56:10 schrieb Douglas Anderson:
> According to the most up to date version of the dwc2 databook, the FRINT
> field of the HFIR register should be programmed to:
> * 125 us * (PHY clock freq for HS) - 1
> * 1000 us * (PHY clock freq for FS/LS) - 1
> 
> This is opposed to older versions of the doc that claimed it should be:
> * 125 us * (PHY clock freq for HS)
> * 1000 us * (PHY clock freq for FS/LS)
> 
> In case you didn't spot it, the difference is the "- 1".
> 
> Let's add the "- 1" to match the newest user manual.  It's presumed that
> the "- 1" should have always been there and that this was always a
> documentation error.  If some hardware needs the "- 1" and other
> hardware doesn't, we'll have to add a configuration parameter for it in
> the future.

Same setup and description as in Doug's 21-patch series, so on a
veyron-jerry:

Tested-by: Heiko Stuebner <heiko@sntech.de>


Heiko
diff mbox

Patch

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 39a0fa8a4c0a..c7798476e25b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -2251,10 +2251,10 @@  u32 dwc2_calc_frame_interval(struct dwc2_hsotg *hsotg)
 
 	if ((hprt0 & HPRT0_SPD_MASK) >> HPRT0_SPD_SHIFT == HPRT0_SPD_HIGH_SPEED)
 		/* High speed case */
-		return 125 * clock;
+		return 125 * clock - 1;
 	else
 		/* FS/LS case */
-		return 1000 * clock;
+		return 1000 * clock - 1;
 }
 
 /**