2023-11-22 18:01:24

by Hugo Villeneuve

Subject: [PATCH] serial: max310x: change confusing comment about Tx FIFO

From: Hugo Villeneuve <[email protected]>

The comment wording can be confusing, as txlen will return the number of
bytes available in the FIFO, which can be less than the maximum theoretical
Tx FIFO size.

Change the comment so that it is unambiguous.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 97e4965b73d4..f3a99daebdaa 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -780,7 +780,7 @@ static void max310x_handle_tx(struct uart_port *port)
to_send = uart_circ_chars_pending(xmit);
until_end = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
if (likely(to_send)) {
- /* Limit to size of TX FIFO */
+ /* Limit to space available in TX FIFO */
txlen = max310x_port_read(port, MAX310X_TXFIFOLVL_REG);
txlen = port->fifosize - txlen;
to_send = (to_send > txlen) ? txlen : to_send;

base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263
--
2.39.2

2023-12-01 16:04:47

by Jan Kundrát

Subject: Re: [PATCH] serial: max310x: change confusing comment about Tx FIFO

On středa 22. listopadu 2023 18:59:56 CET, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <[email protected]>
>
> The comment wording can be confusing, as txlen will return the number of
> bytes available in the FIFO, which can be less than the maximum theoretical
> Tx FIFO size.

This (commit) message is confusing, too, IMHO, because `txlen` is the
number of bytes that are currently waiting in the TX FIFO. So that number
is "available" for the HW UART to pick up and send, but it's not a number
of bytes that's "available" in the FIFO for host to push more bytes to. I
guess you might want to tweak that description here.

Cheers,
Jan

>
> Change the comment so that it is unambiguous.
>
> Signed-off-by: Hugo Villeneuve <[email protected]>
> ---
> drivers/tty/serial/max310x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 97e4965b73d4..f3a99daebdaa 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
> @@ -780,7 +780,7 @@ static void max310x_handle_tx(struct uart_port *port)
> to_send = uart_circ_chars_pending(xmit);
> until_end = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
> if (likely(to_send)) {
> - /* Limit to size of TX FIFO */
> + /* Limit to space available in TX FIFO */
> txlen = max310x_port_read(port, MAX310X_TXFIFOLVL_REG);
> txlen = port->fifosize - txlen;
> to_send = (to_send > txlen) ? txlen : to_send;
>
> base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263

2023-12-01 16:44:15

by Hugo Villeneuve

Subject: Re: [PATCH] serial: max310x: change confusing comment about Tx FIFO

On Fri, 01 Dec 2023 17:04:15 +0100
Jan Kundrát <[email protected]> wrote:

> On středa 22. listopadu 2023 18:59:56 CET, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <[email protected]>
> >
> > The comment wording can be confusing, as txlen will return the number of
> > bytes available in the FIFO, which can be less than the maximum theoretical
> > Tx FIFO size.
>
> This (commit) message is confusing, too, IMHO, because `txlen` is the
> number of bytes that are currently waiting in the TX FIFO. So that number
> is "available" for the HW UART to pick up and send, but it's not a number
> of bytes that's "available" in the FIFO for host to push more bytes to. I
> guess you might want to tweak that description here.

Hi Jan,
you are right, the commit message is confusing. I copied it from the
commit message of a similar patch for the sc16is7xx driver, and I
should have modified it for the max310x:

SC16IS7XX TXLVL: spaces available in TX FIFO
MAX310X TXFIFOLVL: current fill level in TX FIFO

The patch has already been applied to Greg's tty-next tree (my
understanding is that it cannot be rebased?), but at least
the comment in the code is still valid.

Thank you for the precision.

Hugo V.


> >
> > Change the comment so that it is unambiguous.
> >
> > Signed-off-by: Hugo Villeneuve <[email protected]>
> > ---
> > drivers/tty/serial/max310x.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> > index 97e4965b73d4..f3a99daebdaa 100644
> > --- a/drivers/tty/serial/max310x.c
> > +++ b/drivers/tty/serial/max310x.c
> > @@ -780,7 +780,7 @@ static void max310x_handle_tx(struct uart_port *port)
> > to_send = uart_circ_chars_pending(xmit);
> > until_end = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
> > if (likely(to_send)) {
> > - /* Limit to size of TX FIFO */
> > + /* Limit to space available in TX FIFO */
> > txlen = max310x_port_read(port, MAX310X_TXFIFOLVL_REG);
> > txlen = port->fifosize - txlen;
> > to_send = (to_send > txlen) ? txlen : to_send;
> >
> > base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263
>
>