On Tue, 3 Nov 2015 15:20:56 +0100, Florian Achleitner wrote:
> We found that our sc16is7xx on spi reported a TX fifo free space value
> (TXLVL_REG) of 255 ocassionally, which is obviously wrong, with a
> 64 byte fifo (and caused a buffer overrun, which is fixed seperately).
>
> To trigger this, a large write to the tty is sufficient. The fifo fills,
> TXLVL_REG reads zero, but the handle_tx function does a zero-data-length
> write to the TX fifo anyways through sc16is7xx_fifo_write. The next
> TXLVL_REG read then yields 255, for unknown reasons. A subsequent read
> is ok.
>
> Prevent zero-data-length writes if the TX fifo is full, because they are
> pointless, and because they trigger wrong TXLVL read-outs.
>
> Signed-off-by: Florian Achleitner <achleitner.florian@xxxxxxxxxxx>
> ---
> drivers/tty/serial/sc16is7xx.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 107a099..b00fa25 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -389,6 +389,13 @@ static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
> const u8 line = sc16is7xx_line(port);
> u8 addr = (SC16IS7XX_THR_REG << SC16IS7XX_REG_SHIFT) | line;
>
> + /*
> + * Don't send zero-length data, at least on SPI it confuses the chip reading
> + * wrong TXLVL data.
> + */
> + if (unlikely(!to_send))
> + return;
> +
> regcache_cache_bypass(s->regmap, true);
> regmap_raw_write(s->regmap, addr, s->buf, to_send);
> regcache_cache_bypass(s->regmap, false);
> @@ -630,6 +637,11 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
> if (likely(to_send)) {
> /* Limit to size of TX FIFO */
> txlen = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
> + if (txlen > SC16IS7XX_FIFO_SIZE) {
> + dev_err_ratelimited(port->dev, "chip reports %d free bytes in TX "
> + "fifo, but it only has %d", txlen, SC16IS7XX_FIFO_SIZE);
> + txlen = 0;
> + }
> to_send = (to_send > txlen) ? txlen : to_send;
> /* prevent buffer overrun if reported txlen is flawed */
> to_send = (to_send > SC16IS7XX_FIFO_SIZE) ? SC16IS7XX_FIFO_SIZE : to_send;
Please read: https://www.kernel.org/doc/Documentation/SubmittingPatches
In particular make sure your commit message is wrapped properly and
that you run the checkpath.pl script with --strict option on your
patches. You have some lines > 80 chars and split string here.
Other than that change is looking good!
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
