tty: serial: OMAP: work around broken IP block, driver

Prevent OMAP UARTs from going idle while they are still transferring data in PIO mode. ?This works around an oversight in the OMAP UART hardware present in OMAP34xx and earlier: an idle UART won't send a wakeup when the TX FIFO threshold is reached. ?This causes long delays during data transmission when the MPU powerdomain enters a low-power mode. ?The MPU interrupt controller is not able to respond to interrupts when it's in a low-power state, so the TX buffer is not refilled until another wakeup event occurs.

This fix changes the erratum i291 DMA idle workaround. ?Rather than toggling between force-idle and no-idle, it will toggle between smart-idle and no-idle. ?The important part of the workaround is the no-idle part, so this shouldn't result in any change in behavior.

This fix should work on all OMAP UARTs. ?Future patches intended for the 3.4 merge window will make this workaround conditional on a "feature" flag, and will use the OMAP36xx+ TX event wakeup support.

Thanks to Kevin Hilman <[email protected]> for mentioning the erratum i291 workaround, which led to the development of this approach.

Signed-off-by Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Govindraj.R <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Alan Cox <[email protected]>

Cc Tomi Valkeinen <[email protected]>

---
?arch/arm/mach-omap2/serial.c ? ? | ? ?8 ++++---- ?drivers/tty/serial/omap-serial.c | ? ?7 +++++++ ?2 files changed, 11 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/serial.c +2 -2

index 247d894..f590afc 100644
@@ -107,18 +107,18 @@ static void omap_uart_set_noidle(struct platform_device *pdev)
 ? ? ? ?omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_NO);
 ?}
 
-static void omap_uart_set_forceidle(struct platform_device *pdev)
+static void omap_uart_set_smartidle(struct platform_device *pdev)
 ?{
 ? ? ? ?struct omap_device *od = to_omap_device(pdev);
 
- ? ? ? omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_FORCE);
+ ? ? ? omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART);
 ?}
 

?#else
?static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) ?{}
?static void omap_uart_set_noidle(struct platform_device *pdev) {}

arch/arm/mach-omap2/serial.c +1 -1

-static void omap_uart_set_forceidle(struct platform_device *pdev) {}
+static void omap_uart_set_smartidle(struct platform_device *pdev) {}

?#endif /* CONFIG_PM */

?#ifdef CONFIG_OMAP_MUX

arch/arm/mach-omap2/serial.c +1 -1

@@ -349,7 +349,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 ? ? ? ?omap_up.uartclk = OMAP24XX_BASE_BAUD * 16;
 ? ? ? ?omap_up.flags = UPF_BOOT_AUTOCONF;
 ? ? ? ?omap_up.get_context_loss_count = omap_pm_get_dev_context_loss_count;
- ? ? ? omap_up.set_forceidle = omap_uart_set_forceidle;
+ ? ? ? omap_up.set_forceidle = omap_uart_set_smartidle;
 ? ? ? ?omap_up.set_noidle = omap_uart_set_noidle;
 ? ? ? ?omap_up.enable_wakeup = omap_uart_enable_wakeup;
 ? ? ? ?omap_up.dma_rx_buf_size = info->dma_rx_buf_size;

drivers/tty/serial/omap-serial.c +1 -0

index c9c9ba2..11fa156 100644
@@ -136,6 +136,7 @@ static void serial_omap_enable_ms(struct uart_port *port)
 ?static void serial_omap_stop_tx(struct uart_port *port)
 ?{
 ? ? ? ?struct uart_omap_port *up = (struct uart_omap_port *)port;
+ ? ? ? struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
 

? ? ? ?if (up->use_dma &&
? ? ? ? ? ? ? ?up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {

drivers/tty/serial/omap-serial.c +6 -1

@@ -158,6 +159,9 @@ static void serial_omap_stop_tx(struct uart_port *port)
 ? ? ? ? ? ? ? ?serial_out(up, UART_IER, up->ier);
 ? ? ? ?}
 
+ ? ? ? if (!up->use_dma && pdata->set_forceidle)
+ ? ? ? ? ? ? ? pdata->set_forceidle(up->pdev);
+
 ? ? ? ?pm_runtime_mark_last_busy(&up->pdev->dev);
 ? ? ? ?pm_runtime_put_autosuspend(&up->pdev->dev);
 ?}

@@ -286,6 +290,7 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
 ?static void serial_omap_start_tx(struct uart_port *port)
 ?{
 ? ? ? ?struct uart_omap_port *up = (struct uart_omap_port *)port;
+ ? ? ? struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
 ? ? ? ?struct circ_buf *xmit;
 ? ? ? ?unsigned int start;
 ? ? ? ?int ret = 0;

@@ -293,6 +298,8 @@ static void serial_omap_start_tx(struct uart_port *port)
 ? ? ? ?if (!up->use_dma) {
 ? ? ? ? ? ? ? ?pm_runtime_get_sync(&up->pdev->dev);
 ? ? ? ? ? ? ? ?serial_omap_enable_ier_thri(up);
+ ? ? ? ? ? ? ? if (pdata->set_noidle)
+ ? ? ? ? ? ? ? ? ? ? ? pdata->set_noidle(up->pdev);
 ? ? ? ? ? ? ? ?pm_runtime_mark_last_busy(&up->pdev->dev);
 ? ? ? ? ? ? ? ?pm_runtime_put_autosuspend(&up->pdev->dev);
 ? ? ? ? ? ? ? ?return;
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

From af9f18e15e0ef0e227b3efa42489b7bd8a20c2a9 Mon Sep 17 00:00:00 2001

Prevent OMAP UARTs from going idle while they are still transferring data in PIO mode. This works around an oversight in the OMAP UART hardware present in OMAP34xx and earlier: an idle UART won't send a wakeup when the TX FIFO threshold is reached. This causes long delays during data transmission when the MPU powerdomain enters a low-power mode. The MPU interrupt controller is not able to respond to interrupts when it's in a low-power state, so the TX buffer is not refilled until another wakeup event occurs.

This fix changes the erratum i291 DMA idle workaround. Rather than toggling between force-idle and no-idle, it will toggle between smart-idle and no-idle. The important part of the workaround is the no-idle part, so this shouldn't result in any change in behavior.

This fix should work on all OMAP UARTs. Future patches intended for the 3.4 merge window will make this workaround conditional on a "feature" flag, and will use the OMAP36xx+ TX event wakeup support.

Thanks to Kevin Hilman<[email protected]> for mentioning the erratum i291 workaround, which led to the development of this approach.

Signed-off-by Paul Walmsley<[email protected]>

Cc Kevin Hilman<[email protected]>

Cc Govindraj.R<[email protected]>

Cc Greg Kroah-Hartman<[email protected]>

Cc Alan Cox<[email protected]>

Cc Tomi Valkeinen<[email protected]>

---
  arch/arm/mach-omap2/serial.c     |    8 ++++----
  drivers/tty/serial/omap-serial.c |    7 +++++++
  2 files changed, 11 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/serial.c +4 -4

index 247d894..f590afc 100644
@@ -107,18 +107,18 @@ static void omap_uart_set_noidle(struct platform_device *pdev)
  	omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_NO);
  }
 
-static void omap_uart_set_forceidle(struct platform_device *pdev)
+static void omap_uart_set_smartidle(struct platform_device *pdev)
  {
  	struct omap_device *od = to_omap_device(pdev);
 
-	omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_FORCE);
+	omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART);
  }
 
  #else
  static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
  {}
  static void omap_uart_set_noidle(struct platform_device *pdev) {}
-static void omap_uart_set_forceidle(struct platform_device *pdev) {}
+static void omap_uart_set_smartidle(struct platform_device *pdev) {}
  #endif /* CONFIG_PM */
 
  #ifdef CONFIG_OMAP_MUX

@@ -349,7 +349,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
  	omap_up.uartclk = OMAP24XX_BASE_BAUD * 16;
  	omap_up.flags = UPF_BOOT_AUTOCONF;
  	omap_up.get_context_loss_count = omap_pm_get_dev_context_loss_count;
-	omap_up.set_forceidle = omap_uart_set_forceidle;
+	omap_up.set_forceidle = omap_uart_set_smartidle;
  	omap_up.set_noidle = omap_uart_set_noidle;
  	omap_up.enable_wakeup = omap_uart_enable_wakeup;
  	omap_up.dma_rx_buf_size = info->dma_rx_buf_size;

drivers/tty/serial/omap-serial.c +7 -1

index c9c9ba2..11fa156 100644
@@ -136,6 +136,7 @@ static void serial_omap_enable_ms(struct uart_port *port)
  static void serial_omap_stop_tx(struct uart_port *port)
  {
  	struct uart_omap_port *up = (struct uart_omap_port *)port;
+	struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
 
  	if (up->use_dma&&
  		up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {

@@ -158,6 +159,9 @@ static void serial_omap_stop_tx(struct uart_port *port)
  		serial_out(up, UART_IER, up->ier);
  	}
 
+	if (!up->use_dma&&  pdata->set_forceidle)
+		pdata->set_forceidle(up->pdev);
+
  	pm_runtime_mark_last_busy(&up->pdev->dev);
  	pm_runtime_put_autosuspend(&up->pdev->dev);
  }

@@ -286,6 +290,7 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
  static void serial_omap_start_tx(struct uart_port *port)
  {
  	struct uart_omap_port *up = (struct uart_omap_port *)port;
+	struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
  	struct circ_buf *xmit;
  	unsigned int start;
  	int ret = 0;

@@ -293,6 +298,8 @@ static void serial_omap_start_tx(struct uart_port *port)
  	if (!up->use_dma) {
  		pm_runtime_get_sync(&up->pdev->dev);
  		serial_omap_enable_ier_thri(up);
+		if (pdata->set_noidle)
+			pdata->set_noidle(up->pdev);
  		pm_runtime_mark_last_busy(&up->pdev->dev);
  		pm_runtime_put_autosuspend(&up->pdev->dev);
  		return;
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

3.3-rc4 is broken in the DT case because of the serial driver. And it looks like it is due to this fix. We cannot rely on pdata anymore in DT, and in that case it leads to an Oops due to NULL pdata.

And then we will have to add the support for all these OMAP custom hooks without pdata.

A basic fix (below) for the moment is to test for valid pdata inside the driver. I'll repost it properly if you are fine with it.

Salut Beno?t,

Quoted 3 lines · Benoit

3.3-rc4 is broken in the DT case because of the serial driver. And it looks like it is due to this fix. We cannot rely on pdata anymore in DT, and in that case it leads to an Oops due to NULL pdata.

Sorry about the breakage. I agree with the diagnosis. That code was just copied from the DMA errata part of the driver, so it will need to be fixed as well. Otherwise the same problem will happen when DMA is enabled.

Quoted 1 line

And then we will have to add the support for all these OMAP custom hooks without pdata.

That's really the key question for the medium- to long-term...

Quoted 2 lines

A basic fix (below) for the moment is to test for valid pdata inside the driver. I'll repost it properly if you are fine with it.

Looks fine to me, although I'd suggest fixing the DMA workaround path as well.

Reviewed-by Paul Walmsley<[email protected]>

Quoted 2 lines · Paul Walmsley

Looks fine to me, although I'd suggest fixing the DMA workaround path as well.

Mmm, I've tried to fix every instances of pdata in the code, you think I missed some?

I'll check again.

Quoted 1 line

Reviewed-by Paul Walmsley<[email protected]>

The following commit: be4b0281956c5cae4f63f31f11d07625a6988766 (tty: serial: OMAP: block idle while the UART is transferring data in PIO mode), is introducing an oops if OMAP is booted using device tree blob because the pdata will not be initialized.

Quoted 4 lines · Benoit

The following commit: be4b0281956c5cae4f63f31f11d07625a6988766 (tty: serial: OMAP: block idle while the UART is transferring data in PIO mode), is introducing an oops if OMAP is booted using device tree blob because the pdata will not be initialized.

Is that something that happens today on systems? Does this need to go in for 3.3-final, or can it wait for 3.4?

The receive FIFO wakeup latency estimate in the omap-serial driver is three orders of magnitude too small. ?This effectively prevents the MPU from going to a low-power state when CONFIG_CPU_IDLE=y. ?This is a major power management regression and masks some other FIFO-related bugs in the driver.

Fix by correcting the most egregious problem in the RX wakeup latency estimate. ?There are several other flaws in the estimator; these will be fixed by a separate patch series intended for 3.4.

The difference in low-power states with this patch can be observed via debugfs in pm_debug/count.

This estimate does not have any effect when CONFIG_CPU_IDLE=n.

Signed-off-by Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Govindraj.R <[email protected]>

Cc Tomi Valkeinen <[email protected]>

Cc Alan Cox <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---
?drivers/tty/serial/omap-serial.c | ? ?3 +--
?1 files changed, 1 insertions(+), 2 deletions(-)

drivers/tty/serial/omap-serial.c +0 -0

index 11fa156..72fa783 100644
@@ -740,8 +740,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? ?quot = serial_omap_get_divisor(port, baud);
 

? ? ? ?/* calculate wakeup latency constraint */
- ? ? ? up->calc_latency = (1000000 * up->port.fifosize) /
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (1000 * baud / 8);
+ ? ? ? up->calc_latency = (USEC_PER_SEC * up->port.fifosize) / (baud / 8); ? ? ? ?up->latency = up->calc_latency;
? ? ? ?schedule_work(&up->qos_work);

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

[ This series is targeted for merging during v3.3-rc ]

Hi

Here's an updated version of OMAP serial bugfix series against v3.3-rc1. This revision has: