On Sun, 12 Apr 2015 09:46:52 +0200, Michel Vos wrote:
> Hi, I am working on a board with a nxp sc16is740 and am using the
> driver for this. I noticed that when closing a connection the driver
> would query the tx level repeatedly and this was caused by this
> procedure:
>
> static unsigned int sc16is7xx_tx_empty(struct uart_port *port)
> {
> unsigned int lvl, lsr;
>
> lvl = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
> lsr = sc16is7xx_port_read(port, SC16IS7XX_LSR_REG);
>
> return ((lsr & SC16IS7XX_LSR_THRE_BIT) && !lvl) ? TIOCSER_TEMT : 0;
> }
>
> The lvl variable seems to contain the number of bytes in the FIFO but
> in reality it contains the number of free places in the FIFO.
Indeed. It seems TXLVL is interpreted correctly on TX path, only
.tx_empty() is broken.
> So the fix I want to propose is this:
"This is not a correct way to submit a patch" & "your mail client
mangled the whitespace". Please read:
https://www.kernel.org/doc/Documentation/SubmittingPatches
also linux-wireless guys have a nice git guide:
https://wireless.wiki.kernel.org/en/developers/documentation/git-guide
Note that it assumes linux-wireless development, so the git commands are
useful but you probably don't want to send your serial patch to l-w
developers ;)
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index df9a384..0e59860 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -699,7 +699,7 @@ static unsigned int sc16is7xx_tx_empty(struct
> uart_port *port)
> {
> unsigned int lvl, lsr;
>
> - lvl = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
> + lvl = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG) - SC16IS7XX_FIFO_SIZE;
This line is > 80 chars, please use checkpatch.pl script before
submitting next version.
> lsr = sc16is7xx_port_read(port, SC16IS7XX_LSR_REG);
>
> return ((lsr & SC16IS7XX_LSR_THRE_BIT) && !lvl) ? TIOCSER_TEMT : 0;
lsr test also looks wrong - we should test the TEMT bit. Furthermore
according to datasheet the THRE/TEMT implies empty FIFO, so TXLVL
should always read 64 if either of them is set.
Could you confirm with empirical tests that when TEMT is set then
following condition always hold:
- SC16IS7XX_TXLVL_REG is 64;
- THRE is set;
and send appropriate patch?
--
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
