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

[Johan Hovold <johan@kernel.org>]

On Wed, Jul 08, 2020 at 11:05:29PM +0200, Jerry wrote:
> Johan Hovold wrote on 7/6/20 3:59 PM:
> > On Mon, Jul 06, 2020 at 01:47:50PM +0200, Jerry wrote:
> >> Johan Hovold wrote on 7/3/20 5:01 PM:
> >>> Also, could try and see if your device detects breaks properly? Mine
> >>> just return a NUL char.

> I've done some experiments with CP2102 receiving a break.
> It seems that chip always receives 0x00 for the start of break (with 
> correct parity when even parity set, wrong for odd parity) and later 
> (probably after 250 ms) it also sets break flag in GET_COMM_STATUS.
> I don't see any indication of the break event in data. I tried to change 
> some things in your solution but without success.

Ok, thanks for testing! The SERIAL_BREAK_CHAR in ulFlowReplace probably
needs to be set for breaks to be reported in-band, but unfortunately
that doesn't seem to have any effect on CP2102.

> I also haven't ever seen Frame error (neither way). I tried several ways 
> (different tx/rx baudrate, receive a parity data without parity enabled, 
> generating shorter breaks) and I suppose that CP2102 can't indicate framing 
> error.
> 
> Luckily I haven't found any problem with parity checking.  :-)

That's good.

I've been giving this some more thought and decided that it's probably
best not to extend TIOCGICOUNT with a COMM_STATUS request after all.

TIOCGICOUNT is really only supposed to be used to retrieve the
modem-status interrupts in concert with TIOCMIWAIT, but the ioctl was
later amended with some error statistics as well. As I mentioned before,
the ioctl is linux-specific and the statistics counter are mostly
undocumented and the behaviour varies from driver to driver. And don't
think adding another device-specific implementation which essentially
polls for errors (rather than counts them) is a good idea.

Since you confirmed that the event based implementation works for your
use case I think we should stick to that as it's allows for the normal
POSIX mechanisms for detecting parity errors.

Johan