On Mon, 9 Nov 2015 09:34:45 +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 separately).
>
> 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>
Florian,
if we apply both of your patches the flow will be like this:
1. txlen = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
2. if (txlen > SC16IS7XX_FIFO_SIZE) {
3. dev_err(...);
4. txlen = 0;
5. }
6. to_send = (to_send > txlen) ? txlen : to_send;
7. /* Prevent ... */
8. to_send = (to_send > SC16IS7XX_FIFO_SIZE) ?
9. SC16IS7XX_FIFO_SIZE : to_send;
If we replace '= 0' on line 4 with = SC16IS7XX_FIFO_SIZE, this
code can be rewritten as:
1. txlen = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
2. txlen = min(txlen, SC16IS7XX_FIFO_SIZE);
3. to_send = min(txlen, to_send);
4. to_send = min(to_send, SC16IS7XX_FIFO_SIZE);
Can you see now why I claim that your first patch is unnecessary?
It's enough to make sure txlen is not larger than FIFO, since to_send
cannot be larger than txlen it will never be larger than FIFO as well.
--
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
