Re: cp210x module broken in 5.12.5 and 5.12.6, works in 5.11.21 (with bisection)

[Johan Hovold <johan@kernel.org>]

On Fri, Jun 04, 2021 at 01:25:19PM -0500, Alex Villacís Lasso wrote:
> El 4/6/21 a las 10:42, Johan Hovold escribió:

> > I just ran a quick test here and and leaving the ixoff_limit at zero
> > essentially breaks software flow control since XOFF will be sent when
> > there are only 7 characters in the receive buffer.
> >
> > Since software flow control support was only recently added, we may have
> > to accept that for CP2102N to fix the regression, but I'd really like to
> > understand why your devices behave the way they do first and see if
> > there's some other way to work around this.
> >
> > Hopefully Silabs can provide some insight.
> >
> > Also, could you try setting those limits to some other values and see if
> > the SET_MHS (request 0x7) errors go away?
> >
> > Setting both to 513 is supposed to give us 192/64 according to the
> > datasheet which would be good enough, for example. Seems to work as
> > documented here (at least for XOFF).

> I am starting to suspect that the root cause is that the 0x07 command 
> (CP210X_SET_MHS macro in the code) is invalid to send, if the device has 
> been previously programmed with nonzero ulXonLimit/ulXoffLimit. When the 
> patch programs both limits back to 0, the command succeeds.

Right, that's what the bisection and logs seem to suggest.

> I am attaching the patch I used, which is the combination of both debug 
> patches, plus this change:
> 
> @@ -1195,11 +1201,14 @@
>          else
>                  flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
> 
> -       flow_ctl.ulXonLimit = cpu_to_le32(128);
> -       flow_ctl.ulXoffLimit = cpu_to_le32(128);
> +       flow_ctl.ulXonLimit = (I_IXON(tty)) ? cpu_to_le32(128) : 
> cpu_to_le32(0);
> +       flow_ctl.ulXoffLimit = (I_IXOFF(tty)) ? cpu_to_le32(128) : 
> cpu_to_le32(0);

These are both only needed when IXOFF (input flow control) is used (IXON
is for output flow control and does not use these limits).

And the fact that they cause a mostly unrelated error when set still
indicates a firmware bug. That doesn't mean it may be possible to work
around it somehow of course.

> With this patch, the miniterm.py program sort of keeps running and shows 
> output. Not a perfect patch by any means, since some failures still happen:

> $ miniterm.py /dev/ttyUSB0 115200
> <program waits for input>
> 
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_change_speed - setting baud rate to 9600
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - BEFORE: ctrl = 0x00, flow = 0x00
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - BEFORE: xon_limit = 0, xoff_limit = 0
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - AFTER: ctrl = 0x00, flow = 0x01
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - AFTER: xon_limit = 128, xoff_limit = 0

Another data point: just setting the XON limit is enough to trigger the
bug.

> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_tiocmset_port - control = 0x0303
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: failed set request 
> 0x7 status: -32

As here SET_MHS fails.

> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_change_speed - setting baud rate to 115384
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - BEFORE: ctrl = 0x00, flow = 0x01
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - BEFORE: xon_limit = 128, xoff_limit = 0
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - AFTER: ctrl = 0x01, flow = 0x40
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_set_flow_control - AFTER: xon_limit = 0, xoff_limit = 0
> jun 04 13:05:12 karlalex-asus kernel: cp210x ttyUSB0: 
> cp210x_tiocmset_port - control = 0x0101

And when XON is reset, settings RTS again works.

For completeness you could try setting only the XOFF limit and see if
that alone is sufficient to trigger the issue.

But please also try hardcoding both limits to 513 as I mentioned
above and see if that makes any difference.

Johan