Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754454AbbHCQJp (ORCPT ); Mon, 3 Aug 2015 12:09:45 -0400 Received: from www.linutronix.de ([62.245.132.108]:33301 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754396AbbHCQJm (ORCPT ); Mon, 3 Aug 2015 12:09:42 -0400 Date: Mon, 3 Aug 2015 18:09:37 +0200 From: Sebastian Andrzej Siewior To: Peter Hurley Cc: John Ogness , linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Tony Lindgren , linux-omap@vger.kernel.org, nsekhar@ti.com, nm@ti.com, linux-serial@vger.kernel.org Subject: Re: [PATCH 3/3] serial: 8250: omap: restore registers on shutdown Message-ID: <20150803160937.GA26497@linutronix.de> References: <87egjp2r4a.fsf@linutronix.de> <55BAC67E.1010400@hurleysoftware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <55BAC67E.1010400@hurleysoftware.com> X-Key-Id: 2A8CF5D1 X-Key-Fingerprint: 6425 4695 FFF0 AA44 66CC 19E6 7B96 E816 2A8C F5D1 User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5105 Lines: 165 * Peter Hurley | 2015-07-30 20:51:10 [-0400]: >Hi John, Hi Peter, >I was never really a fan of the deferred set_termios(); >I think it's more appropriate to wait for tx dma to >complete in omap_8250_set_termios(). So you want something like this? This was only compile + boot tested (without triggering the corner case) and I know that 8250.h piece has to go in a separated patch (as requested in 2/3 of this series). Just checking if this is what you had in mind. diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index c43f74c53cd9..a407757dcecc 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -42,9 +42,9 @@ struct uart_8250_dma { size_t rx_size; size_t tx_size; - unsigned char tx_running:1; - unsigned char tx_err: 1; - unsigned char rx_running:1; + unsigned char tx_running; + unsigned char tx_err; + unsigned char rx_running; }; struct old_serial_port { diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index d9a37191a1ae..12249125a218 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -100,9 +100,9 @@ struct omap8250_priv { u8 wer; u8 xon; u8 xoff; - u8 delayed_restore; u16 quot; + wait_queue_head_t termios_wait; bool is_suspending; int wakeirq; int wakeups_enabled; @@ -256,18 +256,6 @@ static void omap8250_update_mdr1(struct uart_8250_port *up, static void omap8250_restore_regs(struct uart_8250_port *up) { struct omap8250_priv *priv = up->port.private_data; - struct uart_8250_dma *dma = up->dma; - - if (dma && dma->tx_running) { - /* - * TCSANOW requests the change to occur immediately however if - * we have a TX-DMA operation in progress then it has been - * observed that it might stall and never complete. Therefore we - * delay DMA completes to prevent this hang from happen. - */ - priv->delayed_restore = 1; - return; - } serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(up, UART_EFR, UART_EFR_ECB); @@ -309,6 +297,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up) up->port.ops->set_mctrl(&up->port, up->port.mctrl); } +static void omap_8250_dma_tx_complete(void *param); /* * OMAP can use "CLK / (16 or 13) / div" for baud rate. And then we have have * some differences in how we want to handle flow control. @@ -322,6 +311,7 @@ static void omap_8250_set_termios(struct uart_port *port, struct omap8250_priv *priv = up->port.private_data; unsigned char cval = 0; unsigned int baud; + unsigned int complete_dma = 0; switch (termios->c_cflag & CSIZE) { case CS5: @@ -473,6 +463,25 @@ static void omap_8250_set_termios(struct uart_port *port, if (termios->c_iflag & IXANY) up->mcr |= UART_MCR_XONANY; } + + if (up->dma && up->dma->tx_running) { + struct uart_8250_dma *dma = up->dma; + + /* + * TCSANOW requests the change to occur immediately however if + * we have a TX-DMA operation in progress then it has been + * observed that it might stall and never complete. Therefore we + * wait until DMA completes to prevent this hang from happen. + */ + + dma->tx_running = 2; + + spin_unlock_irq(&up->port.lock); + wait_event(priv->termios_wait, + dma->tx_running == 3); + spin_lock_irq(&up->port.lock); + complete_dma = 1; + } omap8250_restore_regs(up); spin_unlock_irq(&up->port.lock); @@ -488,6 +497,8 @@ static void omap_8250_set_termios(struct uart_port *port, /* Don't rewrite B0 */ if (tty_termios_baud_rate(termios)) tty_termios_encode_baud_rate(termios, baud, baud); + if (complete_dma) + omap_8250_dma_tx_complete(up); } /* same as 8250 except that we may have extra flow bits set in EFR */ @@ -869,17 +880,18 @@ static void omap_8250_dma_tx_complete(void *param) spin_lock_irqsave(&p->port.lock, flags); + if (dma->tx_running == 2) { + dma->tx_running = 3; + wake_up(&priv->termios_wait); + goto out; + } + dma->tx_running = 0; xmit->tail += dma->tx_size; xmit->tail &= UART_XMIT_SIZE - 1; p->port.icount.tx += dma->tx_size; - if (priv->delayed_restore) { - priv->delayed_restore = 0; - omap8250_restore_regs(p); - } - if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) uart_write_wakeup(&p->port); @@ -899,7 +911,7 @@ static void omap_8250_dma_tx_complete(void *param) p->ier |= UART_IER_THRI; serial_port_out(&p->port, UART_IER, p->ier); } - +out: spin_unlock_irqrestore(&p->port.lock, flags); } @@ -1216,6 +1228,7 @@ static int omap8250_probe(struct platform_device *pdev) priv->omap8250_dma.rx_size = RX_TRIGGER; priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER; priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER; + init_waitqueue_head(&priv->termios_wait); if (of_machine_is_compatible("ti,am33xx")) priv->habit |= OMAP_DMA_TX_KICK; Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/