Improve the accuracy of the baud rate generator by using round-to-closest instead of truncating when calculating baud rate divisors.

[Johan Hovold <johan@kernel.org>]

On Thu, Nov 15, 2018 at 03:16:04PM +0100, Nikolaj Fogh wrote:
> On 11/15/18 9:24 AM, Johan Hovold wrote:
> > On Tue, Nov 13, 2018 at 08:19:44PM +0100, Nikolaj Fogh wrote:
> >> On 11/12/18 10:54 AM, Johan Hovold wrote:
> >>> On Wed, Oct 31, 2018 at 09:16:48PM +0100, Nikolaj Fogh wrote:
> >>>> I have experienced that the ftdi_sio driver gives less-than-optimal
> >>>> baud rates as the driver truncates instead of rounds to nearest
> >>>> during baud rate divisor calculation.
> >>>> This patch improves on the baud rate generation. The generated baud
> >>>> rate corresponds to the optimal baud rate achievable with the chip.
> >>>> This is what the windows driver gives as well.
> >>> How did you verify this? Did you trace and compare the divisors
> >>> actually requested by the Windows driver, or did you measure the
> >>> resulting rates using a scope?
> >> I verified it by scope. Granted, I only verified it for one baud rate
> >> (961200). Whether it gives the same as the Windows driver in general,
> >> I'm not sure. However, I would think that rounding instead of flooring
> >> would always yield the most accurate result.
> > I'm not so sure in this case. The driver uses "sub-integer" divisors and
> > looks like it depends on truncation rather than rounding. Some
> > background here:
> >
> > 	https://www.ftdichip.com/Support/Knowledgebase/index.html?whatbaudratesareachieveabl.htm

> I have had a closer look at this (and the driver code), and it seemsthat
> each bit in the divisorcorresponds to 1/8th (0.125) in the calculation.
> 
> It is shuffled around a bit in the code (for legacy reasons I expect), and
> put in the higher order bits, but prior to that, I see no reason that
> rounding should not be used instead of truncating. I don't see how it
> "depends" on truncation.

As I mentioned in my follow-up mail, I agree with that; your proposed
change looks correct.

> > If you want to change these calculations you need to make a stronger
> > case for it and verify that we don't mess up some other rate
> > inadvertently.

> I have done a calculation which compares the error of the baud rate
> calculation going all the way from 1 to 3MBaud where it can be seen that
> the rounding (as expected) halves the maximum error. Whereas the old method
> went up to 12% baud rate error, the new method reaches 6%, so the range
> of baud rates where communication will be successful should increase. Also,
> the new method is always better or as-accurate as the old.

Excellent, thanks for confirming. Just mention something about that in
the commit message too.

> I guess that image attachments are not welcome in the mailing list, so
> I will refrain from attaching it. Let me know if I should send it to
> you.

Sure, if you want too that'd be great.

> I am using it in a system which uses a baud rate of 961200 (and not
> the standard 921600). Here the old calculation gave an error of 4.03%
> and the new gave 0.12% error.

Also good to have in the commit message.

> I will try to verify the numbers I have calculated with a logic analyzer to
> make sure that it corresponds with the real world. I can also try to compare
> it to the windows driver outputs.

That would be really good, at least for a few rates.

> As I only have a FT232RT (232bm) to test with, the patch should probably be
> limited to the changes in the ftdi_232bm_baud_base_to_divisor() function.

No, as long as you mention which device you used for testing, and the
numbers for the other other types looks similar, I think we can go ahead
and round those divisions too.

Thanks for doing this!

Johan