RE: [PATCH] serial: sh-sci: correct units in comment about DMA timeout

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Ulrich Hecht <uli+renesas@fpond.eu>
Cc: "wsa@kernel.org" <wsa@kernel.org>,
	"geert@linux-m68k.org" <geert@linux-m68k.org>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>
Subject: RE: [PATCH] serial: sh-sci: correct units in comment about DMA timeout
Date: Fri, 9 Apr 2021 12:16:13 +0000	[thread overview]
Message-ID: <OSAPR01MB3683F5855937833872F4FE23D8739@OSAPR01MB3683.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20210409082524.3480-1-uli+renesas@fpond.eu>

Hi Ulrich-san,

Thank you for the patch!

> From: Ulrich Hecht, Sent: Friday, April 9, 2021 5:25 PM
> 
> Since the transition to hrtimers, the calculation does not involve jiffies
> any longer, which has led to confusion. State the times in ms instead.

IIUC, the unit of rx_timeout was changed from milliseconds to microseconds
when hrtimer is used. So, almost all comments was not needed.

Also, I'm wondering if the following condition is not needed or not.
This is because this was "20ms". But, perhaps, the driver will not
set rx_timeout to "20us" or small.

        if (s->rx_timeout < 20)
                s->rx_timeout = 20;

Best regards,
Yoshihiro Shimoda

> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
> ---
>  drivers/tty/serial/sh-sci.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index e3af97a59856..c4ce4cd120ba 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2613,11 +2613,11 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
>  	 * Calculate delay for 2 DMA buffers (4 FIFO).
>  	 * See serial_core.c::uart_update_timeout().
>  	 * With 10 bits (CS8), 250Hz, 115200 baud and 64 bytes FIFO, the above
> -	 * function calculates 1 jiffie for the data plus 5 jiffies for the
> -	 * "slop(e)." Then below we calculate 5 jiffies (20ms) for 2 DMA
> -	 * buffers (4 FIFO sizes), but when performing a faster transfer, the
> -	 * value obtained by this formula is too small. Therefore, if the value
> -	 * is smaller than 20ms, use 20ms as the timeout value for DMA.
> +	 * function calculates 4ms for the data plus 20ms for the "slop(e)."
> +	 * Then below we calculate 20ms for 2 DMA buffers (4 FIFO sizes),
> +	 * but when performing a faster transfer, the value obtained by this
> +	 * formula is too small. Therefore, if the value is smaller than
> +	 * 20ms, use 20ms as the timeout value for DMA.
>  	 */
>  	s->rx_frame = (10000 * bits) / (baud / 100);
>  #ifdef CONFIG_SERIAL_SH_SCI_DMA
> --
> 2.20.1