[PATCH 1/3] serial: mxs-auart: implement flush_buffer hook

[l.stach@pengutronix.de (Lucas Stach)]

Am Freitag, den 29.11.2013, 19:02 +0100 schrieb Marek Vasut:
> Dear Hector Palacios,
> 
> > Hi Marek,
> > 
> > On 11/29/2013 05:50 PM, Marek Vasut wrote:
> > > Hello Hector,
> > > 
> > >> Terminate any DMA transfer and verify the TX FIFO is empty.
> > >> 
> > >> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> > >> ---
> > >> 
> > >>   drivers/tty/serial/mxs-auart.c | 23 +++++++++++++++++++++++
> > >>   1 file changed, 23 insertions(+)
> > >> 
> > >> diff --git a/drivers/tty/serial/mxs-auart.c
> > >> b/drivers/tty/serial/mxs-auart.c index 9f0461778fc1..d9bf6e103f65 100644
> > >> --- a/drivers/tty/serial/mxs-auart.c
> > >> +++ b/drivers/tty/serial/mxs-auart.c
> > >> @@ -782,6 +782,28 @@ static unsigned int mxs_auart_tx_empty(struct
> > >> uart_port *u) return 0;
> > >> 
> > >>   }
> > >> 
> > >> +/*
> > >> + * Flush the transmit buffer.
> > >> + * Locking: called with port lock held and IRQs disabled.
> > >> + */
> > >> +static void mxs_auart_flush_buffer(struct uart_port *u)
> > >> +{
> > >> +	struct mxs_auart_port *s = to_auart_port(u);
> > >> +	struct dma_chan *channel = s->tx_dma_chan;
> > >> +	unsigned int to;
> > >> +
> > >> +	if (auart_dma_enabled(s)) {
> > >> +		/* Avoid deadlock with the DMA engine callback */
> > >> +		spin_unlock(&s->port.lock);
> > >> +		dmaengine_terminate_all(channel);
> > >> +		spin_lock(&s->port.lock);
> > > 
> > > Can you not maybe just set some flag here to tell the DMA engine callback
> > > things are shutting down and to don't do anything funny anymore ?
> > 
> > I copied these spin_lock protections from the amba-pl011.c driver.
> > The callbacks in the mxs-auart driver are not taking the lock but I was not
> > sure if any of the serial core functions they call are, so I thought it
> > was a good idea to take it.
> 
> It might be worth investigating this then.
> 
> > >> +	}
> > >> +	/* Wait for the FIFO to flush */
> > >> +	to = u->timeout;
> > >> +	while (!mxs_auart_tx_empty(u) && to--)
> > >> +		mdelay(1);
> > > 
> > > Maybe you can put some cond_resched() into the loop to avoid wasting too
> > > many cycles ?
> > 
> > Do you mean like this? Ok
> > 
> > 	while (!mxs_auart_tx_empty(u) && to--) {
> > 		cond_resched();
> > 		mdelay(1);
> > 	}
> 
> Yes, like that.
> 
This path isn't timing critical. mdelay still keeps the cpu spinning,
even with a resched. Just use an absolute timeout and sleep instead of
delay.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |