Re: [PATCH] USB: serial: ch341: fix wrong baud rate setting calculation

[Johan Hovold <johan@kernel.org>]

[ Please keep everyone on CC, and wrap your messages at 78 columns or
  so; I've reflown the text below. ]

On Fri, Nov 01, 2019 at 10:25:53AM +0100, Michael Dreher wrote:
> Hi Johan
> 
> >> https://github.com/nospam2000/ch341-baudrate-calculation/blob/master/README.md
> >Interesting! From a very quick glance this looks very promising
> >(although he doesn't seem to handle rounding yet). I'll take a closer
> >look and compare with what I've done. 
> 
> That’s my code. I actually do rounding. It might not be obvious that
> the '(*2 + 1) / 2’ in  ((2UL * CH341_OSC_F) + / (prescaler *
> priv->baud_rate) + 1UL) / 2UL does a bankers rounding, that why I
> explained it in the README.MD:
> https://github.com/nospam2000/ch341-baudrate-calculation#rounding-issues

Yes, I noticed that today when taking closer look.

> > I also noticed that we should always pick the higher base clock if
> > the resulting divisor is odd (and within the valid range).
> 
> The general rule from the perspective of mathematics is to use the
> highest possible divider (and therefore the lowest possible prescaler)
> because that gives the lowest possible relative error in the division.
> As long as the result of divider*prescaler is the same it doesn’t
> matter which combination you choose. For example my algorithm prefers
> 1*208=208 over 2*104=208 of the other algorithms which doesn’t make a
> difference.

Indeed, I just mentioned this in reference to Jonathan's patch which
compared the two resulting rates with the third prescale register bit
set or cleared (effectively halving the rate). In this case, we can go
with either whenever the divisor is even. But generally, yes, a higher
divisor is better.

> At 110 baud there is the only difference between my algorithm and
> Jonathan’s code for the commonly used baud rates because I  pick
> 512*213 whereas he doesn’t use the 512 prescaler and therefore uses
> 1024*107 (both values are odd).

Yep, Jonathan only extended the current algorithm for the lowest
prescale value. I generalised it for all values, just like you did.

Note also that the third prescale bit affects divisor 2 through 8 for
all prescalers, not just when prescale bits 0 and 1 are zero
(prescaler=1 in your terminology) as your github doc claims.

It doesn't matter for your algorithm, just mentioning it for the sake
of completeness since I found this in my experiments.

Johan