RE: [PATCH 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

From: A.S. Dong
Date: Fri Jun 09 2017 - 10:20:56 EST


> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@xxxxxxxxx]
> Sent: Friday, June 09, 2017 5:26 PM
> To: A.S. Dong
> Cc: linux-serial@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm
> Mailing List; Greg Kroah-Hartman; Jiri Slaby; Andy Duan; Stefan Agner;
> Mingkai Hu; Y.B. Lu; Dong Aisheng
> Subject: Re: [PATCH 6/6] tty: serial: lpuart: add a more accurate baud
> rate calculation method
>
> On Fri, Jun 9, 2017 at 11:01 AM, A.S. Dong <aisheng.dong@xxxxxxx> wrote:
>
> >> >> > + u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> >> >> > + u32 clk = sport->port.uartclk;
> >> >> > +
> >> >> > + /*
> >> >> > + * The idea is to use the best OSR (over-sampling rate)
> >> possible.
> >> >> > + * Note, OSR is typically hard-set to 16 in other LPUART
> >> >> instantiations.
> >> >> > + * Loop to find the best OSR value possible, one that
> >> >> > + generates
> >> >> minimum
> >> >> > + * baud_diff iterate through the rest of the supported
> >> >> > + values of
> >> >> OSR.
> >> >> > + *
> >> >> > + * Calculation Formula:
> >> >> > + * Baud Rate = baud clock / ((OSR+1) Ã SBR)
> >> >> > + */
> >> >> > + baud_diff = baudrate;
> >> >> > + osr = 0;
> >> >> > + sbr = 0;
> >> >> > +
> >> >>
> >> >> > + for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> >>
> >> I missed one thing, what happened by default to OSR? What is the
> >> value in use?
> >>
> >
> > No valid default value. (osc/sbr are 0 by default) If no proper osc
> > and sbr calculated, a WARNING will show.
>
> Okay, so, it means the maximum supported speed is UART clock / 4. Correct?
>

Yes.

> >> So, the algo is the following:
> >>
> >> Assume the ranges like this:
> >> OSR = [4 ... 32]
> >> SBR = [2 ... 8192]
> >>
> >
> > Baud Rate = baud clock / ((OSR+1) Ã SBR)
> >
> > In HW:
> > OSR range : 3 â 31
> > SBR range: 1 â 8191
>
> I've read that, but think outside the box.
>
> >> Then:
> >>
> >> 1. Get ratio factor as
> >> ratio = CLK / desired baud rate 2. If ratio < 8192 * 9 / 2,
> >> just use (ratio / 4, 4) as (OSR, SBR) setting.
> >> (Needs clarification on OSR < 4)
> >
> > Sorry that I'm a bit mess here.
> > What is 8192 * 9 /2 meaning?
>
> I forgot the details...
>
> > And for (ratio / 4, 4) as (OSR,SBR), take 115200 as an example:
> > Assuming baud clock 24Mhz.
> >
> > Ratio = 24000000 / 115200 = 208
> > OSR = Ratio / 4 = 52
> > Then OSR is out of range which seems wrong.
>
> ...yes...
>
> >> 3. if ratio >= 8192 * 31, just use those two numbers (8192, 31). You
> >> can't do anything better there.
> >
> > This actually may not happen.
> > Even take a 9600 as example, the clk becomes:
> > 8191 * 31 * 9600 = 2.4GHz
> > Which is theoretically not exist.
> >
> >> 4. Otherwise, get a minimum required factor of OSR
> >> osr_min = ratio / 8192
> >> 5. Start your loop from osr_min + 1 to 31.
> >>
> >> 6 (optional). Of course you may not consider baud_diff > osr_min,
> >> it's I suppose obvious
> >>
> >> P.S. Note, all divisions by 2^n are just simple right shifts. Diffs
> >> are calculated as multiplication of OSR and SBR in comparison to
> >> ratio. One division so far.
>
> > I'm not quite understand the approach.
>
> ...lemme prepare a python script demonstrating it.
>

Great, thanks

> > How about you send a separate baud algorithm improvement patch later?
>
> Why not to do it right a way?
>

Because I thought that could be a separate patch which is doing algorithm
improvement, then we can have the full history and a clear comparison.

And also we are still not sure whether it works, we don't want to block on it
too long.

But if you're pretty sure about it, I would wait for some more time.

However, personally I would still rather keep them in two separate Patches
for clearer history and comparison.

> Just describe it in a comment if you afraid of reader can't understand
> from the code.
>

That is good.

Regards
Dong Aisheng

> --
> With Best Regards,
> Andy Shevchenko