From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hector Palacios Subject: Re: [PATCH 1/3] serial: mxs-auart: implement flush_buffer hook Date: Fri, 29 Nov 2013 18:49:42 +0100 Message-ID: <5298D3B6.5050700@digi.com> References: <1385742927-11358-1-git-send-email-hector.palacios@digi.com> <1385742927-11358-2-git-send-email-hector.palacios@digi.com> <201311291750.22215.marex@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail1.bemta8.messagelabs.com ([216.82.243.193]:61302 "EHLO mail1.bemta8.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754617Ab3K2Rtv (ORCPT ); Fri, 29 Nov 2013 12:49:51 -0500 In-Reply-To: <201311291750.22215.marex@denx.de> Sender: linux-serial-owner@vger.kernel.org List-Id: linux-serial@vger.kernel.org To: Marek Vasut Cc: "linux-serial@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "gregkh@linuxfoundation.org" , "b32955@freescale.com" , "fabio.estevam@freescale.com" , "u.kleine-koenig@pengutronix.de" 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 >> --- >> 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. >> + } >> + /* 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); } Best regards, -- Hector Palacios From mboxrd@z Thu Jan 1 00:00:00 1970 From: hector.palacios@digi.com (Hector Palacios) Date: Fri, 29 Nov 2013 18:49:42 +0100 Subject: [PATCH 1/3] serial: mxs-auart: implement flush_buffer hook In-Reply-To: <201311291750.22215.marex@denx.de> References: <1385742927-11358-1-git-send-email-hector.palacios@digi.com> <1385742927-11358-2-git-send-email-hector.palacios@digi.com> <201311291750.22215.marex@denx.de> Message-ID: <5298D3B6.5050700@digi.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 >> --- >> 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. >> + } >> + /* 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); } Best regards, -- Hector Palacios