drivers: usb: serial: ftdi_sio.c error flagging
Hi,
and sorry about the late reply.
Quoted 51 lines · On Fri, Mar 08, 2019 at 09:43:35AM -0700, [email protected] wrote:
[Resend with From address corrected. Sorry about that]
Johan,
Some of our customers are experiencing communication issues on RS485 that could be solved quite nicely by turning on termios.PARMRK. I'd be happy to go into details, but I don't think they're necessary for the discussion below.
The problem we encountered is that the error flagging produced by
ftdi_sio.cover-marks errors to the point that PARMRK becomes unusable (in our cases, everything ends up being flagged as errors, even the actual, good data).The issue in particular is that the driver marks *all* characters received in a single USB packet with an error flag based on whether a BREAK, frame error, or parity error condition is reported by that USB packet. In actuality, the FTDI chip's error condition seems to apply to the last byte in the received packet (I tested with an FT230X chip). Unfortunately, it's more complicated than that. I was unable to find any useful documentation on the USB packets the FTDI chips generate, but from trial and error, the BREAK condition handling seems to work something like this:
When a break condition is reported (
FTDI_RS_BIis set), the last byte received MAY be a BREAK. To confirm, wait for the next status packet. IF that status packet has no data ANDFTDI_RS_BIis still set, then the previous character was indeed a BREAK. Otherwise, the previous character was a normal data byte.This seems rather byzantine, but in my testing, it's been the only algorithm that was able to accurately identify BREAKs.
I attached some code that worked for the test cases I tried. Of course, I'm not overly confident that this will work in all cases.
As for parity errors: I have not been able to figure out how to mark ALL parity errors characters without also sometimes accidentally marking correctly received bytes as erroneous.
My sense is that the FTDI chips simply can't reliably flag all error characters (and only error characters) and we're probably not going to use PARMRK to try to handle the communications issue mentioned at the outset, so I'm on the fence whether is worthwhile to apply a patch along the lines of the below. However, I'd suggest that at least a comment be added in the driver making it clear that the error flagging is not accurate as implemented and that it will mark too many characters as erroneous, but that, perhaps, it's the best that can be done.
Quoted 1 line
Thoughts?
I'm aware that the current implementation flags all characters in the receive buffer when we detect an error, but I'm not sure we can do much better either.
Apparently, you get similar behaviour using their own drivers:
https://highfieldtales.wordpress.com/2014/09/27/lets-dig-into-an-issue-of-the-ft232-chip/
Having FTDI provide the required documentation would help (perhaps you can ask about this specific issue), otherwise it's down to tedious reverse engineering of behaviour which may not even be consistent between device types.
Johan