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
