On Fri, Jul 03, 2020 at 08:45:45PM +0200, Jerry wrote: > Johan Hovold wrote on 7/3/20 9:45 AM: > > The problem here is that we're sort of violating the API and turning > > TIOCGICOUNT into a polling interface instead of just returning our error > > and interrupt counters. This means will always undercount any errors for > > a start. > > > > The chip appears to have a mechanism for reporting errors in-band, but > > that would amount to processing every character received to look for the > > escape char, which adds overhead. I'm guessing that interface would also > > insert an overrun error when returning the first character. > Yes, it is posible to use EMBED_EVENTS and chip will insert event codes > into stream of data bytes. But it will cost some processing power and if > transmitted data contains ESC char it will be escaped so it will cost some > additional bandwidth. In the worst case (all received data = ESC) it means > double bandwidth. Yeah, but sending a stream of ESC chars isn't a particularly interesting application anyway. ;) > I have found such solution here: > https://github.com/fortian/cp210x/blob/master/cp210x.c > but it is quite complex and I expected argument that it costs too much > (especially when using maximum baudrate) so I suggested simple way which > doesn't have an impact until ioctl is called. It will definitely have some overhead, but since it allows for proper posix behaviour and things like break detection I think it's warranted (but only when those features are requested of course). It may be possible to get the best of both worlds here if we poll for changes only if input parity checking is disabled. You get the statistics and could still use the Linux-specific TIOCGICOUNT ioctl to check for errors indirectly. > > But perhaps that it was we should be using as it allows parity the > > errors to be reported using the normal in-band methods. If we only > > enable it when parity checking is enabled then the overhead seems > > justified. > > > > I did a quick test here and the event insertion appears to work well for > > parity errors. Didn't manage to get it to report break events yet, and > > modem-status changes are being buffered and not reported until the next > > character. But in theory we could expand the implementation to provide > > more features later. > > > > I'll see if I can cook something up quickly. > > >> I suppose that GET_COMM_STATUS reads and CLEAR pending error flags (not > >> explained in datasheet but behaves that way). So if some errors are > >> reported during cp210x_tx_empty() it can be either counted immediately or > >> lost. I'm not sure if cp210x_tx_empty() is called ONLY during close but > >> seems be more consistent to count every detected error regardless who calls > >> GET_COMM_STATUS. > > tx_empty() is called when waiting for data to be drained for example > > during close but also on when changing terminal settings with TCSADRAIN > > or on tcdrain(). > But losing an error during tcdrain() is definitely wrong. It is common to > send (write) some request, then call tcdrain to be sure that whole request > was transmitted and then receive response. Calling tcdrain is necessary in > combination with GPIO manipulation but it can also be useful to measure > correct timeout because I need to know that data was already transmitted to > target (it can take long time for slow baudrate). There is no reason to > discard error flags during such waiting. Yes, you're right of course. Since comm-status request clears the flags they need to be accounted for whenever it's used. Johan
