Re: [PATCH 4/6] usb: dwc3: Handle fractional reference clocks

[Sean Anderson]

On 1/14/22 8:09 PM, Robert Hancock wrote:
> On Fri, 2022-01-14 at 18:39 -0500, Sean Anderson wrote:
> > GUCTL.REFCLKPER can only account for clock frequencies with integer
> > periods. To address this, program REFCLK_FLADJ with the relative error
> > caused by period truncation. The formula given in the register reference
> > has been rearranged to allow calculation based on rate (instead of
> > period), and to allow for fixed-point arithmetic.
> >
> > Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
> > ---
> >
> >  drivers/usb/dwc3/core.c | 25 +++++++++++++++++++++++--
> >  drivers/usb/dwc3/core.h |  1 +
> >  2 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 5214daceda86..48bb3e42cdd0 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -348,7 +348,7 @@ static void dwc3_frame_length_adjustment(struct dwc3
> > *dwc)
> >  static void dwc3_ref_clk_period(struct dwc3 *dwc)
> >  {
> >  	u32 reg;
> > -	unsigned long rate, period;
> > +	unsigned long fladj, rate, period;
> >  
> >  	if (dwc->ref_clk) {
> >  		rate = clk_get_rate(dwc->ref_clk);
> > @@ -357,16 +357,37 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
> >  		period = NSEC_PER_SEC / rate;
> >  	} else if (dwc->ref_clk_per) {
> >  		period = dwc->ref_clk_per;
> > +		rate = NSEC_PER_SEC / period;
> >  	} else {
> >  		return;
> >  	}
> >  
> > +	/*
> > +	 * The calculation below is
> > +	 *
> > +	 * 125000 * (NSEC_PER_SEC / (rate * period) - 1)
> > +	 *
> > +	 * but rearranged for fixed-point arithmetic.
> > +	 *
> > +	 * Note that rate * period ~= NSEC_PER_SECOND, minus the number of
> > +	 * nanoseconds of error caused by the truncation which happened during
> > +	 * the division when calculating rate or period (whichever one was
> > +	 * derived from the other). We first calculate the relative error, then
> > +	 * scale it to units of 0.08%.
> > +	 */
> > +	fladj = div64_u64(125000ULL * NSEC_PER_SEC, (u64)rate * period);
> > +	fladj -= 125000;
> > +
> >  	reg = dwc3_readl(dwc->regs, DWC3_GUCTL);
> >  	reg &= ~DWC3_GUCTL_REFCLKPER_MASK;
> >  	reg |=  FIELD_PREP(DWC3_GUCTL_REFCLKPER_MASK, period);
> >  	dwc3_writel(dwc->regs, DWC3_GUCTL, reg);
> > -}
> >  
>
> dwc3_frame_length_adjustment which also writes to the DWC3_GFLADJ register has
> a conditional to skip it if DWC3_VER_IS_PRIOR(DWC3, 250A), not sure if it's
> needed for this field or not?

That's a good question...

The oldest version I found with it is 2.60a [1].

[1] https://www.mouser.com/pdfdocs/silver-celeron-datasheet-vol-2.pdf

--Sean