From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Hector Palacios <hector.palacios@digi.com>
Cc: "fabio.estevam@freescale.com" <fabio.estevam@freescale.com>,
"marex@denx.de" <marex@denx.de>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"b32955@freescale.com" <b32955@freescale.com>,
"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
"shawn.guo@linaro.org" <shawn.guo@linaro.org>,
Jiri Slaby <jslaby@suse.cz>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] serial: mxs-auart: wait for DMA buffer to flush before shutdown
Date: Wed, 2 Oct 2013 10:30:14 +0200 [thread overview]
Message-ID: <20131002083014.GN2548@pengutronix.de> (raw)
In-Reply-To: <524BD4A6.5060208@digi.com>
Hello Hector,
On Wed, Oct 02, 2013 at 10:09:10AM +0200, Hector Palacios wrote:
> On 10/02/2013 09:44 AM, Uwe Kleine-König wrote:
> >On Wed, Oct 02, 2013 at 09:31:16AM +0200, Hector Palacios wrote:
> >>On 10/01/2013 09:48 PM, Uwe Kleine-König wrote:
> >>>On Tue, Oct 01, 2013 at 06:18:35PM +0200, Hector Palacios wrote:
> >>>>+ /* Wait enough time to flush DMA buffer completely */
> >>>>+ to = u->timeout * UART_XMIT_SIZE / u->fifosize;
> >>>u->timeout is the time needed to send one char, right? UART_XMIT_SIZE is
> >>>the size of the circular buffer, fifosize is the size of the fifo. I
> >>>don't get what you get by dividing by the fifosize. I would have
> >>>expected something like:
> >>>
> >>> u->timeout * min(UART_XMIT_SIZE, u->fifosize)
> >>>
> >>>Can you explain, maybe in a comment along the code for the next person
> >>>not understanding?
> >>
> >>u->timeout is *not* the time needed to send one char but the time
> >>needed to flush the complete port fifo (see uart_update_timeout() in
> >>serial_core.c where it is set).
> >Ah, right. (BTW, uart_update_timeout should better round up instead of
> >round down, i.e.
> >- port->timeout = (HZ * bits) / baud + HZ/50;
> >+ port->timeout = DIV_ROUND_UP(HZ * bits, baud) + HZ/50;
> >)
> >
> >>UART_XMIT_SIZE is the number of bytes in the DMA buffer, so I divide
> >>the max number of bytes in the FIFO by the FIFO size and multiply by
> >>the FIFO timeout.
> >ditto here, better round up. Although I think using a completion could
> >shorten the timeout considerably because there are often less than
> >UART_XMIT_SIZE chars in the buffer?!
>
> I'm not really waiting the full timeout. I'm checking for TX fifo
> empty every 1ms and for a maximum of 'to' ms.
Correct. Still using DIV_ROUND_UP would be better, right?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: u.kleine-koenig@pengutronix.de (Uwe Kleine-König)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] serial: mxs-auart: wait for DMA buffer to flush before shutdown
Date: Wed, 2 Oct 2013 10:30:14 +0200 [thread overview]
Message-ID: <20131002083014.GN2548@pengutronix.de> (raw)
In-Reply-To: <524BD4A6.5060208@digi.com>
Hello Hector,
On Wed, Oct 02, 2013 at 10:09:10AM +0200, Hector Palacios wrote:
> On 10/02/2013 09:44 AM, Uwe Kleine-K?nig wrote:
> >On Wed, Oct 02, 2013 at 09:31:16AM +0200, Hector Palacios wrote:
> >>On 10/01/2013 09:48 PM, Uwe Kleine-K?nig wrote:
> >>>On Tue, Oct 01, 2013 at 06:18:35PM +0200, Hector Palacios wrote:
> >>>>+ /* Wait enough time to flush DMA buffer completely */
> >>>>+ to = u->timeout * UART_XMIT_SIZE / u->fifosize;
> >>>u->timeout is the time needed to send one char, right? UART_XMIT_SIZE is
> >>>the size of the circular buffer, fifosize is the size of the fifo. I
> >>>don't get what you get by dividing by the fifosize. I would have
> >>>expected something like:
> >>>
> >>> u->timeout * min(UART_XMIT_SIZE, u->fifosize)
> >>>
> >>>Can you explain, maybe in a comment along the code for the next person
> >>>not understanding?
> >>
> >>u->timeout is *not* the time needed to send one char but the time
> >>needed to flush the complete port fifo (see uart_update_timeout() in
> >>serial_core.c where it is set).
> >Ah, right. (BTW, uart_update_timeout should better round up instead of
> >round down, i.e.
> >- port->timeout = (HZ * bits) / baud + HZ/50;
> >+ port->timeout = DIV_ROUND_UP(HZ * bits, baud) + HZ/50;
> >)
> >
> >>UART_XMIT_SIZE is the number of bytes in the DMA buffer, so I divide
> >>the max number of bytes in the FIFO by the FIFO size and multiply by
> >>the FIFO timeout.
> >ditto here, better round up. Although I think using a completion could
> >shorten the timeout considerably because there are often less than
> >UART_XMIT_SIZE chars in the buffer?!
>
> I'm not really waiting the full timeout. I'm checking for TX fifo
> empty every 1ms and for a maximum of 'to' ms.
Correct. Still using DIV_ROUND_UP would be better, right?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
next prev parent reply other threads:[~2013-10-02 8:30 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-01 16:18 [PATCH] serial: mxs-auart: wait for DMA buffer to flush before shutdown Hector Palacios
2013-10-01 19:48 ` Uwe Kleine-König
2013-10-02 7:31 ` Hector Palacios
2013-10-02 7:44 ` Uwe Kleine-König
2013-10-02 7:44 ` Uwe Kleine-König
2013-10-02 8:09 ` Hector Palacios
2013-10-02 8:09 ` Hector Palacios
2013-10-02 8:30 ` Uwe Kleine-König [this message]
2013-10-02 8:30 ` Uwe Kleine-König
2013-10-02 8:46 ` Hector Palacios
2013-10-02 8:46 ` Hector Palacios
2013-10-02 9:59 ` Uwe Kleine-König
2013-10-02 9:59 ` Uwe Kleine-König
2013-10-02 8:36 ` Uwe Kleine-König
2013-10-02 8:53 ` Hector Palacios
2013-10-02 8:53 ` Hector Palacios
2013-10-01 19:56 ` Marek Vasut
2013-10-02 8:24 ` Hector Palacios
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131002083014.GN2548@pengutronix.de \
--to=u.kleine-koenig@pengutronix.de \
--cc=b32955@freescale.com \
--cc=fabio.estevam@freescale.com \
--cc=gregkh@linuxfoundation.org \
--cc=hector.palacios@digi.com \
--cc=jslaby@suse.cz \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-serial@vger.kernel.org \
--cc=marex@denx.de \
--cc=shawn.guo@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.