Re: [PATCH v4] usbserial: cp210x - icount support for parity error checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Johan Hovold wrote on 7/3/20 9:45 AM:
Heh, yeah, it tends to be that way. :) But thanks for the great summary
of your findings!

I think it probably makes most sense to keep the error in as it's a
quirk of the device rather than risk missing an actual overrun.
OK
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.

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.
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.

Correct.

It's quite possible that that's missing elsewhere, but at least those
counters are updated in completion callbacks which do not run in
parallel unlike ioctls(), which are not serialised.

Johan

Jerry




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux