OMAP2+: Serial: Runtime adaptation + cleanup

Cleanup serial.c file in preparation to addition of runtime api's in omap-serial file. Remove all clock handling mechanism as this will be taken care with pm runtime api's in omap-serial.c file itself.

1.) Remove omap-device enable and disable. We can can use get_sync/put_sync api's 2.) Remove context save/restore can be done with runtime_resume callback for
get_sync call. No need to save context as all reg details available in uart_port structure can be used for restore, so add missing regs in uart port struct.
3.) Add func to identify console uart.
4.) Erratum handling informed as flag to driver and func to handle erratum
can be moved to omap-serial driver itself.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

arch/arm/mach-omap2/serial.c +0 -6

@@ -47,65 +40,9 @@
 #include "control.h"
 #include "mux.h"
 
-#define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV	0x52
-#define UART_OMAP_WER		0x17	/* Wake-up enable register */
-
-#define UART_ERRATA_FIFO_FULL_ABORT	(0x1 << 0)
-#define UART_ERRATA_i202_MDR1_ACCESS	(0x1 << 1)
-

Quoted 15 lines · Govindraj.R

Cleanup serial.c file in preparation to addition of runtime api's in omap-serial file. Remove all clock handling mechanism as this will be taken care with pm runtime api's in omap-serial.c file itself.

1.) Remove omap-device enable and disable. We can can use get_sync/put_sync api's 2.) Remove context save/restore can be done with runtime_resume callback for ? ? get_sync call. No need to save context as all reg details available in ? ? uart_port structure can be used for restore, so add missing regs in ? ? uart port struct.
3.) Add func to identify console uart.
4.) Erratum handling informed as flag to driver and func to handle erratum ? ? can be moved to omap-serial driver itself.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

[...]

Quoted 14 lines

(quoted diff) +0 -0

@@ -47,65 +40,9 @@

?#include "control.h"
?#include "mux.h"

arch/arm/mach-omap2/serial.c +0 -10

-#define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV ?0x52
-#define UART_OMAP_WER ? ? ? ? ? ? ? ?0x17 ? ?/* Wake-up enable register */
-
-#define UART_ERRATA_FIFO_FULL_ABORT ?(0x1 << 0)
-#define UART_ERRATA_i202_MDR1_ACCESS (0x1 << 1)
-
-/*
- * NOTE: By default the serial timeout is disabled as it causes lost characters
- * over the serial ports. This means that the UART clocks will stay on until
- * disabled via sysfs. This also causes that any deeper omap sleep states are

arch/arm/mach-omap2/serial.c

Quoted 6 lines

- * blocked.
- */

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

-#define DEFAULT_TIMEOUT 0
-
-#define MAX_UART_HWMOD_NAME_LEN ? ? ? ? ? ? ?16
+#define MAX_UART_HWMOD_NAME_LEN ? ? ? ? ? ? ? ?16

Minor: only whitespace change here, you converted tabs to spaces.

Rest of this looks fine.

Kevin
--
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

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

+
+#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */

Hi Govindraj,

On 6/8/2011 6:23 AM, Govindraj.R wrote:

[snip]

Quoted 2 lines

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

+
+#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */

[snip]

Review static int serial_omap_probe(struct · line 1387

Quoted 8 lines

(quoted diff) +3 -0

@@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct
 platform_device *pdev)
 ? ? ? ? ? ? ? ?up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 ? ? ? ?}
 
+ ? ? ? pm_runtime_use_autosuspend(&pdev->dev);
+ ? ? ? pm_runtime_set_autosuspend_delay(&pdev->dev,
+ ? ? ? ? ? ? ? ? ? ? ? OMAP_UART_AUTOSUSPEND_DELAY);

Something is weird here...DEFAULT_AUTOSUSPEND_DELAY is defined as (30*HZ) which would appear to be jiffies (ticks per second) and NOT msecs. However, pm_runtime_set_autosuspend is clearly expecting msecs. So this seems to conflict. By default 30*HZ for omap would be 30*128 = 3840ms so not quite 4 seconds.

What were you intending here?

Cheers
Jon
--
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

Quoted 26 lines · Jon Hunter

Hi Govindraj,

On 6/8/2011 6:23 AM, Govindraj.R wrote:

[snip]

Quoted 2 lines

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

+
+#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */

[snip]

drivers/tty/serial/omap-serial.c static int serial_omap_probe(struct · line 1387

Quoted 8 lines

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

@@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct
 platform_device *pdev)
                up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
        }
 
+       pm_runtime_use_autosuspend(&pdev->dev);
+       pm_runtime_set_autosuspend_delay(&pdev->dev,
+                       OMAP_UART_AUTOSUSPEND_DELAY);

Something is weird here...DEFAULT_AUTOSUSPEND_DELAY is defined as (30*HZ) which would appear to be jiffies (ticks per second) and NOT msecs. However, pm_runtime_set_autosuspend is clearly expecting msecs. So this seems to conflict. By default 30*HZ for omap would be 30*128 = 3840ms so not quite 4 seconds.

Quoted 1 line

What were you intending here?

Intention is to get approx 3 secs timeout for autosuspend.

Hi Govindraj

Quoted 30 lines · Govindraj

Quoted 29 lines · Jon Hunter

Hi Govindraj,

On 6/8/2011 6:23 AM, Govindraj.R wrote:

[snip]

Quoted 2 lines

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

+
+#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */

[snip]

drivers/tty/serial/omap-serial.c static int serial_omap_probe(struct · line 1387

Quoted 8 lines

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

@@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct
 platform_device *pdev)
 up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 }
 
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev,
+ OMAP_UART_AUTOSUSPEND_DELAY);

Something is weird here...DEFAULT_AUTOSUSPEND_DELAY is defined as (30*HZ)
which would appear to be jiffies (ticks per second) and NOT msecs. However,
pm_runtime_set_autosuspend is clearly expecting msecs. So this seems to conflict. By default 30*HZ for omap would be 30*128 = 3840ms so not quite 4
seconds.

Quoted 3 lines

Quoted 1 line

What were you intending here?

Intention is to get approx 3 secs timeout for autosuspend.

In that case you should just define DEFAULT_AUTOSUSPEND_DELAY as 30000. The above is just confusing as you are mixing time types and hence, it is not clear what you intend the default timeout to be.

Adapts omap-serial driver to use pm_runtime api's.

1.) Populate reg values to uart port which can be used for context restore.

2.) Moving context_restore func to driver from serial.c
3.) Adding port_enable/disable func to enable/disable given uart port.
enable port using get_sync and disable using autosuspend.
4.) using runtime irq safe api to make get_sync be called from irq context.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

---
 arch/arm/mach-omap2/serial.c                  |   22 +++
 arch/arm/plat-omap/include/plat/omap-serial.h |    2 +
 drivers/tty/serial/omap-serial.c              |  212 ++++++++++++++++++++++---
 3 files changed, 210 insertions(+), 26 deletions(-)

arch/arm/mach-omap2/serial.c +3 -0

index 8c1a4c7..1651c2c 100644
@@ -189,6 +189,27 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
 	}
 }
 
+static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
+{
+	struct omap_uart_port_info *up = pdev->dev.platform_data;

arch/arm/mach-omap2/serial.c +8 -0

+	/* Enable or clear io-pad wakeup */
+	if (enable)
+		omap_device_enable_ioring_wakeup(pdev);
+	else
+		omap_device_disable_ioring_wakeup(pdev);
+}
+
 static void omap_uart_idle_init(struct omap_uart_port_info *uart,
 				unsigned short num)
 {

@@ -332,6 +353,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 
 	pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
 	pdata->flags = UPF_BOOT_AUTOCONF;
+	pdata->enable_wakeup = omap_uart_wakeup_enable;
 	if (bdata->id == omap_uart_con_id)
 		pdata->console_uart = true;
 

arch/arm/plat-omap/include/plat/omap-serial.h +2 -0

index 2ca885b..ac30de8 100644
@@ -65,6 +65,7 @@ struct omap_uart_port_info {
 	unsigned int		errata;
 	unsigned int		console_uart;
 
+	void (*enable_wakeup)(struct platform_device *, bool);
 	void __iomem *wk_st;
 	void __iomem *wk_en;
 	u32 wk_mask;

@@ -120,6 +121,7 @@ struct uart_omap_port {
 	char			name[20];
 	unsigned long		port_activity;
 	unsigned int		errata;
+	void (*enable_wakeup)(struct platform_device *, bool);
 };
 
 #endif /* __OMAP_SERIAL_H__ */

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

index 47cadf4..897416f 100644
@@ -37,10 +37,14 @@
 #include <linux/clk.h>

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

 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 

@@ -94,6 +98,17 @@ serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
 	return port->uartclk/(baud * divisor);
 }
 

drivers/tty/serial/omap-serial.c +43 -8

 }
 
 static void serial_omap_stop_tx(struct uart_port *port)

@@ -131,21 +149,26 @@ static void serial_omap_stop_tx(struct uart_port *port)
 		up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
 	}
 
+	serial_omap_port_enable(up);
 	if (up->ier & UART_IER_THRI) {
 		up->ier &= ~UART_IER_THRI;
 		serial_out(up, UART_IER, up->ier);
 	}
+
+	serial_omap_port_disable(up);
 }
 
 static void serial_omap_stop_rx(struct uart_port *port)
 {
 	struct uart_omap_port *up = (struct uart_omap_port *)port;
 
+	serial_omap_port_enable(up);
 	if (up->use_dma)
 		serial_omap_stop_rxdma(up);
 	up->ier &= ~UART_IER_RLSI;
 	up->port.read_status_mask &= ~UART_LSR_DR;
 	serial_out(up, UART_IER, up->ier);
+	serial_omap_port_disable(up);
 }
 
 static inline void receive_chars(struct uart_omap_port *up, int *status)

@@ -261,8 +284,10 @@ static void serial_omap_start_tx(struct uart_port *port)
 	unsigned int start;
 	int ret = 0;
 
+	serial_omap_port_enable(up);
 	if (!up->use_dma) {
 		serial_omap_enable_ier_thri(up);
+		serial_omap_port_disable(up);
 		return;
 	}
 

@@ -354,9 +379,12 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 	unsigned int iir, lsr;
 	unsigned long flags;
 
+	serial_omap_port_enable(up);
 	iir = serial_in(up, UART_IIR);
-	if (iir & UART_IIR_NO_INT)
+	if (iir & UART_IIR_NO_INT) {
+		serial_omap_port_disable(up);
 		return IRQ_NONE;
+	}
 
 	spin_lock_irqsave(&up->port.lock, flags);
 	lsr = serial_in(up, UART_LSR);

@@ -378,6 +406,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 		transmit_chars(up);
 
 	spin_unlock_irqrestore(&up->port.lock, flags);
+	serial_omap_port_disable(up);
+
 	up->port_activity = jiffies;
 	return IRQ_HANDLED;
 }

@@ -388,11 +418,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
 	unsigned long flags = 0;
 	unsigned int ret = 0;
 
+	serial_omap_port_enable(up);
 	dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->pdev->id);
 	spin_lock_irqsave(&up->port.lock, flags);
 	ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
 	spin_unlock_irqrestore(&up->port.lock, flags);
-
+	serial_omap_port_disable(up);
 	return ret;
 }
 

@@ -402,7 +433,10 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
 	unsigned char status;
 	unsigned int ret = 0;
 
+	serial_omap_port_enable(up);
 	status = check_modem_status(up);
+	serial_omap_port_disable(up);
+
 	dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
 
 	if (status & UART_MSR_DCD)

@@ -434,7 +468,9 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 		mcr |= UART_MCR_LOOP;
 
 	mcr |= up->mcr;
+	serial_omap_port_enable(up);
 	serial_out(up, UART_MCR, mcr);
+	serial_omap_port_disable(up);
 }
 
 static void serial_omap_break_ctl(struct uart_port *port, int break_state)

@@ -443,6 +479,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
 	unsigned long flags = 0;
 
 	dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->pdev->id);
+	serial_omap_port_enable(up);
 	spin_lock_irqsave(&up->port.lock, flags);
 	if (break_state == -1)
 		up->lcr |= UART_LCR_SBC;

@@ -450,6 +487,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
 		up->lcr &= ~UART_LCR_SBC;
 	serial_out(up, UART_LCR, up->lcr);
 	spin_unlock_irqrestore(&up->port.lock, flags);
+	serial_omap_port_disable(up);
 }
 
 static int serial_omap_startup(struct uart_port *port)

@@ -468,6 +506,7 @@ static int serial_omap_startup(struct uart_port *port)
 
 	dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
 
+	serial_omap_port_enable(up);
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())

@@ -523,6 +562,7 @@ static int serial_omap_startup(struct uart_port *port)
 	/* Enable module level wake up */
 	serial_out(up, UART_OMAP_WER, OMAP_UART_WER_MOD_WKUP);
 
+	serial_omap_port_disable(up);
 	up->port_activity = jiffies;
 	return 0;
 }

@@ -533,6 +573,8 @@ static void serial_omap_shutdown(struct uart_port *port)
 	unsigned long flags = 0;
 
 	dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->pdev->id);
+
+	serial_omap_port_enable(up);
 	/*
 	 * Disable interrupts from this port
 	 */

@@ -566,6 +608,7 @@ static void serial_omap_shutdown(struct uart_port *port)
 			up->uart_dma.rx_buf_dma_phys);
 		up->uart_dma.rx_buf = NULL;
 	}
+	serial_omap_port_disable(up);
 	free_irq(up->port.irq, up);
 }
 

@@ -671,6 +714,10 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/13);
 	quot = serial_omap_get_divisor(port, baud);
 
+	up->dll = quot & 0xff;
+	up->dlh = quot >> 8;
+	up->mdr1 = UART_OMAP_MDR1_DISABLE;
+
 	up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 |
 			UART_FCR_ENABLE_FIFO;
 	if (up->use_dma)

@@ -680,6 +727,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	 * Ok, we're now changing the port state. Do it with
 	 * interrupts disabled.
 	 */
+	serial_omap_port_enable(up);
 	spin_lock_irqsave(&up->port.lock, flags);
 
 	/*

@@ -723,6 +771,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 		up->ier |= UART_IER_MSI;
 	serial_out(up, UART_IER, up->ier);
 	serial_out(up, UART_LCR, cval);		/* reset DLAB */
+	up->lcr = cval;
 
 	/* FIFOs and DMA Settings */
 

@@ -758,8 +807,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial_out(up, UART_MCR, up->mcr);
 
 	/* Protocol, Baud Rate, and Interrupt Settings */
-
-	serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
+	serial_out(up, UART_OMAP_MDR1, up->mdr1);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
 	up->efr = serial_in(up, UART_EFR);

@@ -769,8 +817,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial_out(up, UART_IER, 0);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
-	serial_out(up, UART_DLL, quot & 0xff);          /* LS of divisor */
-	serial_out(up, UART_DLM, quot >> 8);            /* MS of divisor */
+	serial_out(up, UART_DLL, up->dll);	/* LS of divisor */
+	serial_out(up, UART_DLM, up->dlh);	/* MS of divisor */
 
 	serial_out(up, UART_LCR, 0);
 	serial_out(up, UART_IER, up->ier);

@@ -780,9 +828,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial_out(up, UART_LCR, cval);
 
 	if (baud > 230400 && baud != 3000000)
-		serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_13X_MODE);
+		up->mdr1 = UART_OMAP_MDR1_13X_MODE;
 	else
-		serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_16X_MODE);
+		up->mdr1 = UART_OMAP_MDR1_16X_MODE;
+
+	serial_out(up, UART_OMAP_MDR1, up->mdr1)
 
 	/* Hardware Flow Control Configuration */
 

@@ -810,6 +860,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_omap_configure_xonxoff(up, termios);
 
 	spin_unlock_irqrestore(&up->port.lock, flags);
+	serial_omap_port_disable(up);
 	dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
 }
 

@@ -821,6 +872,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
 	unsigned char efr;
 
 	dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->pdev->id);
+
+	serial_omap_port_enable(up);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 	efr = serial_in(up, UART_EFR);
 	serial_out(up, UART_EFR, efr | UART_EFR_ECB);

drivers/tty/serial/omap-serial.c +12 -4

 }
 
 static void serial_omap_release_port(struct uart_port *port)

@@ -907,25 +964,31 @@ static inline void wait_for_xmitr(struct uart_omap_port *up)
 static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
 {
 	struct uart_omap_port *up = (struct uart_omap_port *)port;
+
+	serial_omap_port_enable(up);
 	wait_for_xmitr(up);
 	serial_out(up, UART_TX, ch);
+	serial_omap_port_disable(up);
 }
 
 static int serial_omap_poll_get_char(struct uart_port *port)
 {
 	struct uart_omap_port *up = (struct uart_omap_port *)port;
-	unsigned int status = serial_in(up, UART_LSR);
+	unsigned int status;
 
+	serial_omap_port_enable(up);
+	status = serial_in(up, UART_LSR);
 	if (!(status & UART_LSR_DR))
 		return NO_POLL_CHAR;
 
-	return serial_in(up, UART_RX);
+	status = serial_in(up, UART_RX);
+	serial_omap_port_disable(up);
+	return status;
 }
 
 #endif /* CONFIG_CONSOLE_POLL */
 
 #ifdef CONFIG_SERIAL_OMAP_CONSOLE
-
 static struct uart_omap_port *serial_omap_console_ports[4];
 
 static struct uart_driver serial_omap_reg;

@@ -955,6 +1018,8 @@ serial_omap_console_write(struct console *co, const char *s,
 	else
 		spin_lock(&up->port.lock);
 
+	serial_omap_port_enable(up);
+
 	/*
 	 * First save the IER then disable the interrupts
 	 */

@@ -979,6 +1044,7 @@ serial_omap_console_write(struct console *co, const char *s,
 	if (up->msr_saved_flags)
 		check_modem_status(up);
 
+	serial_omap_port_disable(up);
 	if (locked)
 		spin_unlock(&up->port.lock);
 	local_irq_restore(flags);

@@ -1061,22 +1127,27 @@ static struct uart_driver serial_omap_reg = {
 	.cons		= OMAP_CONSOLE,
 };
 
-static int

drivers/tty/serial/omap-serial.c +67 -5

+	}
+
 	return 0;
 }
 

@@ -1097,6 +1168,7 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 			serial_omap_stop_rxdma(up);
 			up->ier |= (UART_IER_RDI | UART_IER_RLSI);
 			serial_out(up, UART_IER, up->ier);
+			serial_omap_port_disable(up);
 		}
 		return;
 	}

@@ -1128,6 +1200,9 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 
 static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
 {
+	struct uart_omap_port *up = (struct uart_omap_port *)data;
+
+	serial_omap_port_disable(up);
 	return;
 }
 

@@ -1135,6 +1210,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
 {
 	int ret = 0;
 
+	serial_omap_port_enable(up);
 	if (up->uart_dma.rx_dma_channel == -1) {
 		ret = omap_request_dma(up->uart_dma.uart_dma_rx,
 				"UART Rx DMA",

@@ -1214,6 +1290,7 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
 		serial_omap_stop_tx(&up->port);
 		up->uart_dma.tx_dma_used = false;
 		spin_unlock(&(up->uart_dma.tx_lock));
+		serial_omap_port_disable(up);
 	} else {
 		omap_stop_dma(up->uart_dma.tx_dma_channel);
 		serial_omap_continue_tx(up);

@@ -1224,9 +1301,10 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
 
 static int serial_omap_probe(struct platform_device *pdev)
 {
-	struct uart_omap_port	*up;
+	struct uart_omap_port	*up = NULL;
 	struct resource		*mem, *irq, *dma_tx, *dma_rx;
 	struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
+	struct omap_device *od;
 	int ret = -ENOSPC;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1276,12 +1354,20 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->port.ops = &serial_omap_pops;
 	up->port.line = pdev->id;
 
-	up->port.membase = omap_up_info->membase;
-	up->port.mapbase = omap_up_info->mapbase;
+	up->port.mapbase = mem->start;
+	up->port.membase = ioremap(mem->start, mem->end - mem->start);
+
+	if (!up->port.membase) {
+		dev_err(&pdev->dev, "can't ioremap UART\n");
+		ret = -ENOMEM;
+		goto err1;
+	}
+
 	up->port.flags = omap_up_info->flags;
-	up->port.irqflags = omap_up_info->irqflags;
 	up->port.uartclk = omap_up_info->uartclk;
 	up->uart_dma.uart_base = mem->start;
+	up->errata = omap_up_info->errata;
+	up->enable_wakeup = omap_up_info->enable_wakeup;
 
 	if (omap_up_info->dma_enabled) {
 		up->uart_dma.uart_dma_tx = dma_tx->start;

@@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct platform_device *pdev)
 		up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 	}
 
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev,
+			OMAP_UART_AUTOSUSPEND_DELAY);
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_irq_safe(&pdev->dev);
+
+	if (omap_up_info->console_uart) {
+		od = to_omap_device(up->pdev);
+		omap_hwmod_idle(od->hwmods[0]);
+		serial_omap_port_enable(up);
+		serial_omap_port_disable(up);
+	}
+
 	ui[pdev->id] = up;
 	serial_omap_add_console_port(up);
 
 	ret = uart_add_one_port(&serial_omap_reg, &up->port);
 	if (ret != 0)
-		goto do_release_region;
+		goto err1;
 
+	dev_set_drvdata(&pdev->dev, up);
 	platform_set_drvdata(pdev, up);
+
 	return 0;
 err:
 	dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
 				pdev->id, __func__, ret);
+err1:
+	kfree(up);
 do_release_region:
 	release_mem_region(mem->start, (mem->end - mem->start) + 1);
 	return ret;

@@ -1318,20 +1422,76 @@ static int serial_omap_remove(struct platform_device *dev)
 
 	platform_set_drvdata(dev, NULL);
 	if (up) {
+		pm_runtime_disable(&up->pdev->dev);
 		uart_remove_one_port(&serial_omap_reg, &up->port);
 		kfree(up);
 	}
 	return 0;
 }
 
+static void omap_uart_restore_context(struct uart_omap_port *up)
+{
+	u16 efr = 0;
+
+	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+	serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+	efr = serial_in(up, UART_EFR);
+	serial_out(up, UART_EFR, UART_EFR_ECB);
+	serial_out(up, UART_LCR, 0x0); /* Operational mode */
+	serial_out(up, UART_IER, 0x0);
+	serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+	serial_out(up, UART_DLL, up->dll);
+	serial_out(up, UART_DLM, up->dlh);
+	serial_out(up, UART_LCR, 0x0); /* Operational mode */
+	serial_out(up, UART_IER, up->ier);
+	serial_out(up, UART_FCR, up->fcr);
+	serial_out(up, UART_LCR, 0x80);
+	serial_out(up, UART_MCR, up->mcr);
+	serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+	serial_out(up, UART_EFR, efr);
+	serial_out(up, UART_LCR, up->lcr);
+	/* UART 16x mode */
+	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+}
+
+static int omap_serial_runtime_suspend(struct device *dev)

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

+	return 0;
+}
+
+static const struct dev_pm_ops omap_serial_dev_pm_ops = {
+	.suspend = serial_omap_suspend,
+	.resume	= serial_omap_resume,
+	.runtime_suspend = omap_serial_runtime_suspend,
+	.runtime_resume = omap_serial_runtime_resume,
+};
+
 static struct platform_driver serial_omap_driver = {

Quoted 3 lines · Govindraj.R

Adapts omap-serial driver to use pm_runtime api's.

1.) Populate reg values to uart port which can be used for context restore.

Please make this part a separate patch.

Quoted 8 lines

2.) Moving context_restore func to driver from serial.c
3.) Adding port_enable/disable func to enable/disable given uart port. ? ? enable port using get_sync and disable using autosuspend. 4.) using runtime irq safe api to make get_sync be called from irq context.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

This is great! ?we're almost there. ? Some minor comments below...

Quoted 23 lines

---

?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 22 +++
?arch/arm/plat-omap/include/plat/omap-serial.h | ? ?2 +
?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?212 ++++++++++++++++++++++---

?3 files changed, 210 insertions(+), 26 deletions(-)

arch/arm/mach-omap2/serial.c +9 -0

index 8c1a4c7..1651c2c 100644
@@ -189,6 +189,27 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
 ? ? ? }
 ?}
 
+static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
+{
+ ? ? struct omap_uart_port_info *up = pdev->dev.platform_data;
+
+ ? ? /* Set or clear wake-enable bit */
+ ? ? if (up->wk_en && up->wk_mask) {
+ ? ? ? ? ? ? u32 v = __raw_readl(up->wk_en);
+ ? ? ? ? ? ? if (enable)
+ ? ? ? ? ? ? ? ? ? ? v |= up->wk_mask;

arch/arm/mach-omap2/serial.c static void omap_serial_fill_default_pads(struct omap_board_data *bdata) · line 204

Quoted 4 lines

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

+ ? ? ? ? ? ? else
+ ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
+ ? ? ? ? ? ? __raw_writel(v, up->wk_en);
+ ? ? }

I think I asked this in previous series, but can't remember the answer now... ?can't we enable bits in the WKEN registers once at init time, and then just use omap_hwmod_[enable|disable]_wakeup() here?

Quoted 48 lines

arch/arm/mach-omap2/serial.c +8 -0

+ ? ? /* Enable or clear io-pad wakeup */
+ ? ? if (enable)
+ ? ? ? ? ? ? omap_device_enable_ioring_wakeup(pdev);
+ ? ? else
+ ? ? ? ? ? ? omap_device_disable_ioring_wakeup(pdev);
+}
+
 ?static void omap_uart_idle_init(struct omap_uart_port_info *uart,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned short num)
 ?{

@@ -332,6 +353,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 
 ? ? ? pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
 ? ? ? pdata->flags = UPF_BOOT_AUTOCONF;
+ ? ? pdata->enable_wakeup = omap_uart_wakeup_enable;
 ? ? ? if (bdata->id == omap_uart_con_id)
 ? ? ? ? ? ? ? pdata->console_uart = true;
 

arch/arm/plat-omap/include/plat/omap-serial.h +2 -0

index 2ca885b..ac30de8 100644
@@ -65,6 +65,7 @@ struct omap_uart_port_info {
 ? ? ? unsigned int ? ? ? ? ? ?errata;
 ? ? ? unsigned int ? ? ? ? ? ?console_uart;
 
+ ? ? void (*enable_wakeup)(struct platform_device *, bool);
 ? ? ? void __iomem *wk_st;
 ? ? ? void __iomem *wk_en;
 ? ? ? u32 wk_mask;

@@ -120,6 +121,7 @@ struct uart_omap_port {
 ? ? ? char ? ? ? ? ? ? ? ? ? ?name[20];
 ? ? ? unsigned long ? ? ? ? ? port_activity;
 ? ? ? unsigned int ? ? ? ? ? ?errata;
+ ? ? void (*enable_wakeup)(struct platform_device *, bool);
 ?};
 

?#endif /* __OMAP_SERIAL_H__ */

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

index 47cadf4..897416f 100644
@@ -37,10 +37,14 @@

?#include <linux/clk.h>
?#include <linux/serial_core.h>
?#include <linux/irq.h>
+#include <linux/pm_runtime.h>

drivers/tty/serial/omap-serial.c line 47

Quoted 6 lines

?#include <plat/dma.h>
?#include <plat/dmtimer.h>
?#include <plat/omap-serial.h>

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

+#include <plat/omap_device.h>
+
+#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */

As Jon already pointed out, the HZ here is wrong. ?Please define this value in msecs.

Quoted 6 lines

?static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];

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

@@ -94,6 +98,17 @@ serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
 ? ? ? return port->uartclk/(baud * divisor);
 ?}
 

drivers/tty/serial/omap-serial.c serial_omap_get_divisor(struct uart_port *port, unsigned int baud) · line 110

Quoted 10 lines

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

+static inline void serial_omap_port_disable(struct uart_omap_port *up)
+{
+ ? ? pm_runtime_mark_last_busy(&up->pdev->dev);
+ ? ? pm_runtime_put_autosuspend(&up->pdev->dev);
+}
+
+static inline void serial_omap_port_enable(struct uart_omap_port *up)
+{
+ ? ? pm_runtime_get_sync(&up->pdev->dev);
+}

These inlines are not needed. ?Please use runtime PM calls directly in the code. ?For _enable(), this is straight forward. ?For _disable() though, I don't think you want (or need) to _mark_last_busy() for every instance of omap_port_disable(). ?You only need to do this for ones TX/RX transactions...

drivers/tty/serial/omap-serial.c static void serial_omap_enable_ms(struct uart_port *port) · line 132

Quoted 12 lines

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

 ?static void serial_omap_stop_rxdma(struct uart_omap_port *up)
 ?{
 ? ? ? if (up->uart_dma.rx_dma_used) {

@@ -110,8 +125,11 @@ static void serial_omap_enable_ms(struct uart_port *port)
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->pdev->id);
+
+ ? ? serial_omap_port_enable(up);
 ? ? ? up->ier |= UART_IER_MSI;
 ? ? ? serial_out(up, UART_IER, up->ier);
+ ? ? serial_omap_port_disable(up);

...for example, this one is not really activity based, so should probably just be pm_runtime_get_sync(), write the register, then pm_runtime_put() (async version.)

I didn't look at all the others below, but they should be looked at individually.

Quoted 231 lines

drivers/tty/serial/omap-serial.c +23 -2

 ?}
 
 ?static void serial_omap_stop_tx(struct uart_port *port)

@@ -131,21 +149,26 @@ static void serial_omap_stop_tx(struct uart_port *port)
 ? ? ? ? ? ? ? up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
 ? ? ? }
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (up->ier & UART_IER_THRI) {
 ? ? ? ? ? ? ? up->ier &= ~UART_IER_THRI;
 ? ? ? ? ? ? ? serial_out(up, UART_IER, up->ier);
 ? ? ? }
+
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static void serial_omap_stop_rx(struct uart_port *port)
 ?{
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (up->use_dma)
 ? ? ? ? ? ? ? serial_omap_stop_rxdma(up);
 ? ? ? up->ier &= ~UART_IER_RLSI;
 ? ? ? up->port.read_status_mask &= ~UART_LSR_DR;
 ? ? ? serial_out(up, UART_IER, up->ier);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static inline void receive_chars(struct uart_omap_port *up, int *status)

@@ -261,8 +284,10 @@ static void serial_omap_start_tx(struct uart_port *port)
 ? ? ? unsigned int start;
 ? ? ? int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (!up->use_dma) {
 ? ? ? ? ? ? ? serial_omap_enable_ier_thri(up);
+ ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? ? ? ? ? return;
 ? ? ? }
 

@@ -354,9 +379,12 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 ? ? ? unsigned int iir, lsr;
 ? ? ? unsigned long flags;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? iir = serial_in(up, UART_IIR);
- ? ? if (iir & UART_IIR_NO_INT)
+ ? ? if (iir & UART_IIR_NO_INT) {
+ ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? ? ? ? ? return IRQ_NONE;
+ ? ? }
 
 ? ? ? spin_lock_irqsave(&up->port.lock, flags);
 ? ? ? lsr = serial_in(up, UART_LSR);

@@ -378,6 +406,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 ? ? ? ? ? ? ? transmit_chars(up);
 
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
+ ? ? serial_omap_port_disable(up);
+
 ? ? ? up->port_activity = jiffies;
 ? ? ? return IRQ_HANDLED;
 ?}

@@ -388,11 +418,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
 ? ? ? unsigned long flags = 0;
 ? ? ? unsigned int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->pdev->id);
 ? ? ? spin_lock_irqsave(&up->port.lock, flags);
 ? ? ? ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
-
+ ? ? serial_omap_port_disable(up);
 ? ? ? return ret;
 ?}
 

@@ -402,7 +433,10 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
 ? ? ? unsigned char status;
 ? ? ? unsigned int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? status = check_modem_status(up);
+ ? ? serial_omap_port_disable(up);
+
 ? ? ? dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
 
 ? ? ? if (status & UART_MSR_DCD)

@@ -434,7 +468,9 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 ? ? ? ? ? ? ? mcr |= UART_MCR_LOOP;
 
 ? ? ? mcr |= up->mcr;
+ ? ? serial_omap_port_enable(up);
 ? ? ? serial_out(up, UART_MCR, mcr);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static void serial_omap_break_ctl(struct uart_port *port, int break_state)

@@ -443,6 +479,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
 ? ? ? unsigned long flags = 0;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->pdev->id);
+ ? ? serial_omap_port_enable(up);
 ? ? ? spin_lock_irqsave(&up->port.lock, flags);
 ? ? ? if (break_state == -1)
 ? ? ? ? ? ? ? up->lcr |= UART_LCR_SBC;

@@ -450,6 +487,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
 ? ? ? ? ? ? ? up->lcr &= ~UART_LCR_SBC;
 ? ? ? serial_out(up, UART_LCR, up->lcr);
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static int serial_omap_startup(struct uart_port *port)

@@ -468,6 +506,7 @@ static int serial_omap_startup(struct uart_port *port)
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
 
+ ? ? serial_omap_port_enable(up);

? ? ? /*
? ? ? ?* Clear the FIFO buffers and disable them. ? ? ? ?* (they will be reenabled in set_termios())

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

@@ -523,6 +562,7 @@ static int serial_omap_startup(struct uart_port *port)

? ? ? /* Enable module level wake up */
? ? ? serial_out(up, UART_OMAP_WER, OMAP_UART_WER_MOD_WKUP);

+ ? ? serial_omap_port_disable(up);
? ? ? up->port_activity = jiffies;
? ? ? return 0;
?}

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

@@ -533,6 +573,8 @@ static void serial_omap_shutdown(struct uart_port *port)
 ? ? ? unsigned long flags = 0;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->pdev->id);
+
+ ? ? serial_omap_port_enable(up);

? ? ? /*
? ? ? ?* Disable interrupts from this port
? ? ? ?*/

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

@@ -566,6 +608,7 @@ static void serial_omap_shutdown(struct uart_port *port)
 ? ? ? ? ? ? ? ? ? ? ? up->uart_dma.rx_buf_dma_phys);
 ? ? ? ? ? ? ? up->uart_dma.rx_buf = NULL;
 ? ? ? }
+ ? ? serial_omap_port_disable(up);
 ? ? ? free_irq(up->port.irq, up);
 ?}
 

@@ -671,6 +714,10 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/13);
 ? ? ? quot = serial_omap_get_divisor(port, baud);
 
+ ? ? up->dll = quot & 0xff;
+ ? ? up->dlh = quot >> 8;
+ ? ? up->mdr1 = UART_OMAP_MDR1_DISABLE;
+

? ? ? up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 | ? ? ? ? ? ? ? ? ? ? ? UART_FCR_ENABLE_FIFO;
? ? ? if (up->use_dma)

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

@@ -680,6 +727,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,

? ? ? ?* Ok, we're now changing the port state. Do it with ? ? ? ?* interrupts disabled.
? ? ? ?*/
+ ? ? serial_omap_port_enable(up);
? ? ? spin_lock_irqsave(&up->port.lock, flags);

? ? ? /*

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

@@ -723,6 +771,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? ? ? ? ? up->ier |= UART_IER_MSI;
 ? ? ? serial_out(up, UART_IER, up->ier);

? ? ? serial_out(up, UART_LCR, cval); ? ? ? ? /* reset DLAB */
+ ? ? up->lcr = cval;

? ? ? /* FIFOs and DMA Settings */

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

@@ -758,8 +807,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? serial_out(up, UART_MCR, up->mcr);
 

? ? ? /* Protocol, Baud Rate, and Interrupt Settings */ -
- ? ? serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1);
? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);

? ? ? up->efr = serial_in(up, UART_EFR);

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

@@ -769,8 +817,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? serial_out(up, UART_IER, 0);
 ? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
- ? ? serial_out(up, UART_DLL, quot & 0xff); ? ? ? ? ?/* LS of divisor */
- ? ? serial_out(up, UART_DLM, quot >> 8); ? ? ? ? ? ?/* MS of divisor */
+ ? ? serial_out(up, UART_DLL, up->dll); ? ? ?/* LS of divisor */
+ ? ? serial_out(up, UART_DLM, up->dlh); ? ? ?/* MS of divisor */
 
 ? ? ? serial_out(up, UART_LCR, 0);
 ? ? ? serial_out(up, UART_IER, up->ier);

@@ -780,9 +828,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? serial_out(up, UART_LCR, cval);
 
 ? ? ? if (baud > 230400 && baud != 3000000)
- ? ? ? ? ? ? serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_13X_MODE);
+ ? ? ? ? ? ? up->mdr1 = UART_OMAP_MDR1_13X_MODE;

? ? ? else
- ? ? ? ? ? ? serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_16X_MODE);
+ ? ? ? ? ? ? up->mdr1 = UART_OMAP_MDR1_16X_MODE;
+
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1)

? ? ? /* Hardware Flow Control Configuration */

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

@@ -810,6 +860,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? ? ? ? ? serial_omap_configure_xonxoff(up, termios);
 
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
+ ? ? serial_omap_port_disable(up);
 ? ? ? dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
 ?}
 

@@ -821,6 +872,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
 ? ? ? unsigned char efr;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->pdev->id);
+
+ ? ? serial_omap_port_enable(up);
 ? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 ? ? ? efr = serial_in(up, UART_EFR);
 ? ? ? serial_out(up, UART_EFR, efr | UART_EFR_ECB);

@@ -830,6 +883,10 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
 ? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 ? ? ? serial_out(up, UART_EFR, efr);
 ? ? ? serial_out(up, UART_LCR, 0);

drivers/tty/serial/omap-serial.c serial_omap_pm(struct uart_port *port, unsigned int state, · line 889

Quoted 4 lines

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

+ ? ? if (state)
+ ? ? ? ? ? ? pm_runtime_put_sync(&up->pdev->dev);
+ ? ? else
+ ? ? ? ? ? ? serial_omap_port_disable(up);

OK, so this boils down to either a _put_sync() or a _mark_last_busy + _put_autosuspend(), depending on whether this is suspending or resuming, right?

Why the difference?

Quoted 67 lines

drivers/tty/serial/omap-serial.c +9 -2

 ?}
 
 ?static void serial_omap_release_port(struct uart_port *port)

@@ -907,25 +964,31 @@ static inline void wait_for_xmitr(struct uart_omap_port *up)
 ?static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
 ?{
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
+
+ ? ? serial_omap_port_enable(up);
 ? ? ? wait_for_xmitr(up);
 ? ? ? serial_out(up, UART_TX, ch);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static int serial_omap_poll_get_char(struct uart_port *port)
 ?{
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
- ? ? unsigned int status = serial_in(up, UART_LSR);
+ ? ? unsigned int status;
 
+ ? ? serial_omap_port_enable(up);
+ ? ? status = serial_in(up, UART_LSR);
 ? ? ? if (!(status & UART_LSR_DR))
 ? ? ? ? ? ? ? return NO_POLL_CHAR;
 
- ? ? return serial_in(up, UART_RX);
+ ? ? status = serial_in(up, UART_RX);
+ ? ? serial_omap_port_disable(up);
+ ? ? return status;
 ?}
 

?#endif /* CONFIG_CONSOLE_POLL */

?#ifdef CONFIG_SERIAL_OMAP_CONSOLE
-
?static struct uart_omap_port *serial_omap_console_ports[4];

?static struct uart_driver serial_omap_reg;

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

@@ -955,6 +1018,8 @@ serial_omap_console_write(struct console *co, const char *s,

? ? ? else
? ? ? ? ? ? ? spin_lock(&up->port.lock);

+ ? ? serial_omap_port_enable(up);
+
? ? ? /*
? ? ? ?* First save the IER then disable the interrupts ? ? ? ?*/

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

@@ -979,6 +1044,7 @@ serial_omap_console_write(struct console *co, const char *s,
 ? ? ? if (up->msr_saved_flags)
 ? ? ? ? ? ? ? check_modem_status(up);
 
+ ? ? serial_omap_port_disable(up);
 ? ? ? if (locked)
 ? ? ? ? ? ? ? spin_unlock(&up->port.lock);
 ? ? ? local_irq_restore(flags);

@@ -1061,22 +1127,27 @@ static struct uart_driver serial_omap_reg = {
 ? ? ? .cons ? ? ? ? ? = OMAP_CONSOLE,
 ?};
 
-static int
-serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
+static int serial_omap_suspend(struct device *dev)
 ?{
- ? ? struct uart_omap_port *up = platform_get_drvdata(pdev);
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
 

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1136

Quoted 4 lines

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

- ? ? if (up)
+ ? ? if (up) {
 ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
+ ? ? ? ? ? ? console_trylock();

This locking needs to be explained. ?Why it is needed, but very importantly, why you are not checking the return value of the _trylock()

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1137

Quoted 1 line

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

+ ? ? ? ? ? ? serial_omap_pm(&up->port, 3, 0);

Why is this call needed?

uart_suspend_port() calls uart_change_pm() which should call the ->pm method of struct uart_ops (which is serial_omap_pm). ?What am I missing?

Also notice you're not calling serial_omap_pm() in the resume method below to change it back.

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1140

Quoted 3 lines

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

+ ? ? }
 ? ? ? return 0;
 ?}

Also, device wakeup capability should be checked and enabled/disabled in the suspend path (not only the runtime suspend path.)

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1147

Quoted 10 lines

drivers/tty/serial/omap-serial.c +4 -3

-static int serial_omap_resume(struct platform_device *dev)
+static int serial_omap_resume(struct device *dev)
 ?{
- ? ? struct uart_omap_port *up = platform_get_drvdata(dev);
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
 
- ? ? if (up)
+ ? ? if (up) {
 ? ? ? ? ? ? ? uart_resume_port(&serial_omap_reg, &up->port);
+ ? ? ? ? ? ? console_unlock();

Again, please describe locking in comments.

Also, what happens here if the console_trylock() in your suspend method failed?

Quoted 157 lines

drivers/tty/serial/omap-serial.c +38 -5

+ ? ? }
+
 ? ? ? return 0;
 ?}
 

@@ -1097,6 +1168,7 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 ? ? ? ? ? ? ? ? ? ? ? serial_omap_stop_rxdma(up);
 ? ? ? ? ? ? ? ? ? ? ? up->ier |= (UART_IER_RDI | UART_IER_RLSI);
 ? ? ? ? ? ? ? ? ? ? ? serial_out(up, UART_IER, up->ier);
+ ? ? ? ? ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? ? ? ? ? }
 ? ? ? ? ? ? ? return;
 ? ? ? }

@@ -1128,6 +1200,9 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 
 ?static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
 ?{
+ ? ? struct uart_omap_port *up = (struct uart_omap_port *)data;
+
+ ? ? serial_omap_port_disable(up);
 ? ? ? return;
 ?}
 

@@ -1135,6 +1210,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
 ?{
 ? ? ? int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (up->uart_dma.rx_dma_channel == -1) {
 ? ? ? ? ? ? ? ret = omap_request_dma(up->uart_dma.uart_dma_rx,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "UART Rx DMA",

@@ -1214,6 +1290,7 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
 ? ? ? ? ? ? ? serial_omap_stop_tx(&up->port);
 ? ? ? ? ? ? ? up->uart_dma.tx_dma_used = false;
 ? ? ? ? ? ? ? spin_unlock(&(up->uart_dma.tx_lock));
+ ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? } else {
 ? ? ? ? ? ? ? omap_stop_dma(up->uart_dma.tx_dma_channel);
 ? ? ? ? ? ? ? serial_omap_continue_tx(up);

@@ -1224,9 +1301,10 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
 
 ?static int serial_omap_probe(struct platform_device *pdev)
 ?{
- ? ? struct uart_omap_port ? *up;
+ ? ? struct uart_omap_port ? *up = NULL;
 ? ? ? struct resource ? ? ? ? *mem, *irq, *dma_tx, *dma_rx;
 ? ? ? struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
+ ? ? struct omap_device *od;
 ? ? ? int ret = -ENOSPC;
 
 ? ? ? mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1276,12 +1354,20 @@ static int serial_omap_probe(struct platform_device *pdev)
 ? ? ? up->port.ops = &serial_omap_pops;
 ? ? ? up->port.line = pdev->id;
 
- ? ? up->port.membase = omap_up_info->membase;
- ? ? up->port.mapbase = omap_up_info->mapbase;
+ ? ? up->port.mapbase = mem->start;
+ ? ? up->port.membase = ioremap(mem->start, mem->end - mem->start);
+
+ ? ? if (!up->port.membase) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "can't ioremap UART\n");
+ ? ? ? ? ? ? ret = -ENOMEM;
+ ? ? ? ? ? ? goto err1;
+ ? ? }
+
 ? ? ? up->port.flags = omap_up_info->flags;
- ? ? up->port.irqflags = omap_up_info->irqflags;
 ? ? ? up->port.uartclk = omap_up_info->uartclk;
 ? ? ? up->uart_dma.uart_base = mem->start;
+ ? ? up->errata = omap_up_info->errata;
+ ? ? up->enable_wakeup = omap_up_info->enable_wakeup;
 
 ? ? ? if (omap_up_info->dma_enabled) {
 ? ? ? ? ? ? ? up->uart_dma.uart_dma_tx = dma_tx->start;

@@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct platform_device *pdev)
 ? ? ? ? ? ? ? up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 ? ? ? }
 
+ ? ? pm_runtime_use_autosuspend(&pdev->dev);
+ ? ? pm_runtime_set_autosuspend_delay(&pdev->dev,
+ ? ? ? ? ? ? ? ? ? ? OMAP_UART_AUTOSUSPEND_DELAY);
+
+ ? ? pm_runtime_enable(&pdev->dev);
+ ? ? pm_runtime_irq_safe(&pdev->dev);
+
+ ? ? if (omap_up_info->console_uart) {
+ ? ? ? ? ? ? od = to_omap_device(up->pdev);
+ ? ? ? ? ? ? omap_hwmod_idle(od->hwmods[0]);
+ ? ? ? ? ? ? serial_omap_port_enable(up);
+ ? ? ? ? ? ? serial_omap_port_disable(up);
+ ? ? }
+
 ? ? ? ui[pdev->id] = up;
 ? ? ? serial_omap_add_console_port(up);
 
 ? ? ? ret = uart_add_one_port(&serial_omap_reg, &up->port);
 ? ? ? if (ret != 0)
- ? ? ? ? ? ? goto do_release_region;
+ ? ? ? ? ? ? goto err1;
 
+ ? ? dev_set_drvdata(&pdev->dev, up);
 ? ? ? platform_set_drvdata(pdev, up);
+
 ? ? ? return 0;

?err:
? ? ? dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pdev->id, __func__, ret); +err1:
+ ? ? kfree(up);
?do_release_region:
? ? ? release_mem_region(mem->start, (mem->end - mem->start) + 1); ? ? ? return ret;

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

@@ -1318,20 +1422,76 @@ static int serial_omap_remove(struct platform_device *dev)
 
 ? ? ? platform_set_drvdata(dev, NULL);
 ? ? ? if (up) {
+ ? ? ? ? ? ? pm_runtime_disable(&up->pdev->dev);
 ? ? ? ? ? ? ? uart_remove_one_port(&serial_omap_reg, &up->port);
 ? ? ? ? ? ? ? kfree(up);
 ? ? ? }
 ? ? ? return 0;
 ?}
 
+static void omap_uart_restore_context(struct uart_omap_port *up)
+{
+ ? ? u16 efr = 0;
+
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1);
+ ? ? serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+ ? ? efr = serial_in(up, UART_EFR);
+ ? ? serial_out(up, UART_EFR, UART_EFR_ECB);
+ ? ? serial_out(up, UART_LCR, 0x0); /* Operational mode */
+ ? ? serial_out(up, UART_IER, 0x0);
+ ? ? serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+ ? ? serial_out(up, UART_DLL, up->dll);
+ ? ? serial_out(up, UART_DLM, up->dlh);
+ ? ? serial_out(up, UART_LCR, 0x0); /* Operational mode */
+ ? ? serial_out(up, UART_IER, up->ier);
+ ? ? serial_out(up, UART_FCR, up->fcr);
+ ? ? serial_out(up, UART_LCR, 0x80);
+ ? ? serial_out(up, UART_MCR, up->mcr);
+ ? ? serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+ ? ? serial_out(up, UART_EFR, efr);
+ ? ? serial_out(up, UART_LCR, up->lcr);
+ ? ? /* UART 16x mode */
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1);
+}
+
+static int omap_serial_runtime_suspend(struct device *dev)
+{
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
+
+ ? ? if (!up)
+ ? ? ? ? ? ? goto done;
+
+ ? ? if (device_may_wakeup(dev))

drivers/tty/serial/omap-serial.c static int serial_omap_remove(struct platform_device *dev) · line 1467

Quoted 3 lines

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

+ ? ? ? ? ? ? up->enable_wakeup(up->pdev, true);
+ ? ? else
+ ? ? ? ? ? ? up->enable_wakeup(up->pdev, false);

I know the earlier code was doing it this way too, but an optimization would be to have the driver keep state whether the wakeups are enabled or disabled, so you don't have to keep calling this function (which can be expensive with the PRCM reads/writes.

That state can also be used in the suspend path, which should also be managing wakeups.

Quoted 7 lines

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

+done:
+ ? ? return 0;
+}
+
+static int omap_serial_runtime_resume(struct device *dev)
+{
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);

drivers/tty/serial/omap-serial.c static int serial_omap_remove(struct platform_device *dev) · line 1477

Quoted 3 lines

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

+
+ ? ? if (up)
+ ? ? ? ? ? ? omap_uart_restore_context(up);

You only need to restore context when returning from off-mode. ?You should be using omap_device_get_context_loss_count (called via pdata function pointer) for this. ?See the MMC driver or DSS driver for how this is done.

Quoted 11 lines

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

+ ? ? return 0;
+}
+
+static const struct dev_pm_ops omap_serial_dev_pm_ops = {
+ ? ? .suspend = serial_omap_suspend,
+ ? ? .resume = serial_omap_resume,
+ ? ? .runtime_suspend = omap_serial_runtime_suspend,
+ ? ? .runtime_resume = omap_serial_runtime_resume,
+};
+
 ?static struct platform_driver serial_omap_driver = {

drivers/tty/serial/omap-serial.c static int serial_omap_remove(struct platform_device *dev) · line 1495

Quoted 10 lines

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

 ? ? ? .probe ? ? ? ? ?= serial_omap_probe,
 ? ? ? .remove ? ? ? ? = serial_omap_remove,
-
- ? ? .suspend ? ? ? ?= serial_omap_suspend,
- ? ? .resume ? ? ? ? = serial_omap_resume,
 ? ? ? .driver ? ? ? ? = {
 ? ? ? ? ? ? ? .name ? = DRIVER_NAME,
+ ? ? ? ? ? ? .pm = &omap_serial_dev_pm_ops,
 ? ? ? },
 ?};

Kevin
--
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

Quoted 42 lines · Kevin Hilman

Quoted 3 lines · Govindraj.R

Adapts omap-serial driver to use pm_runtime api's.

1.) Populate reg values to uart port which can be used for context restore.

Please make this part a separate patch.

Quoted 8 lines

2.) Moving context_restore func to driver from serial.c
3.) Adding port_enable/disable func to enable/disable given uart port. ? ? enable port using get_sync and disable using autosuspend. 4.) using runtime irq safe api to make get_sync be called from irq context.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

This is great! ?we're almost there. ? Some minor comments below...

Quoted 23 lines

---

?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 22 +++
?arch/arm/plat-omap/include/plat/omap-serial.h | ? ?2 +
?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?212 ++++++++++++++++++++++---

?3 files changed, 210 insertions(+), 26 deletions(-)

arch/arm/mach-omap2/serial.c +9 -0

index 8c1a4c7..1651c2c 100644
@@ -189,6 +189,27 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
 ? ? ? }
 ?}
 
+static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
+{
+ ? ? struct omap_uart_port_info *up = pdev->dev.platform_data;
+
+ ? ? /* Set or clear wake-enable bit */
+ ? ? if (up->wk_en && up->wk_mask) {
+ ? ? ? ? ? ? u32 v = __raw_readl(up->wk_en);
+ ? ? ? ? ? ? if (enable)
+ ? ? ? ? ? ? ? ? ? ? v |= up->wk_mask;

Quoted 9 lines

arch/arm/mach-omap2/serial.c static void omap_serial_fill_default_pads(struct omap_board_data *bdata) · line 204

Quoted 4 lines

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

+ ? ? ? ? ? ? else
+ ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
+ ? ? ? ? ? ? __raw_writel(v, up->wk_en);
+ ? ? }

I think I asked this in previous series, but can't remember the answer now... ?can't we enable bits in the WKEN registers once at init time, and then just use omap_hwmod_[enable|disable]_wakeup() here?

by default all bits are enabled in WKEN,

I will use omap_hwmod_[enable|disable]_wakeup() api's

if these API's take care of WKEN regs, then no issue using the same.

Quoted 48 lines

Quoted 48 lines

arch/arm/mach-omap2/serial.c +8 -0

+ ? ? /* Enable or clear io-pad wakeup */
+ ? ? if (enable)
+ ? ? ? ? ? ? omap_device_enable_ioring_wakeup(pdev);
+ ? ? else
+ ? ? ? ? ? ? omap_device_disable_ioring_wakeup(pdev);
+}
+
 ?static void omap_uart_idle_init(struct omap_uart_port_info *uart,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned short num)
 ?{

@@ -332,6 +353,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 
 ? ? ? pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
 ? ? ? pdata->flags = UPF_BOOT_AUTOCONF;
+ ? ? pdata->enable_wakeup = omap_uart_wakeup_enable;
 ? ? ? if (bdata->id == omap_uart_con_id)
 ? ? ? ? ? ? ? pdata->console_uart = true;
 

arch/arm/plat-omap/include/plat/omap-serial.h +2 -0

index 2ca885b..ac30de8 100644
@@ -65,6 +65,7 @@ struct omap_uart_port_info {
 ? ? ? unsigned int ? ? ? ? ? ?errata;
 ? ? ? unsigned int ? ? ? ? ? ?console_uart;
 
+ ? ? void (*enable_wakeup)(struct platform_device *, bool);
 ? ? ? void __iomem *wk_st;
 ? ? ? void __iomem *wk_en;
 ? ? ? u32 wk_mask;

@@ -120,6 +121,7 @@ struct uart_omap_port {
 ? ? ? char ? ? ? ? ? ? ? ? ? ?name[20];
 ? ? ? unsigned long ? ? ? ? ? port_activity;
 ? ? ? unsigned int ? ? ? ? ? ?errata;
+ ? ? void (*enable_wakeup)(struct platform_device *, bool);
 ?};
 

?#endif /* __OMAP_SERIAL_H__ */

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

index 47cadf4..897416f 100644
@@ -37,10 +37,14 @@

?#include <linux/clk.h>
?#include <linux/serial_core.h>
?#include <linux/irq.h>
+#include <linux/pm_runtime.h>

Quoted 10 lines

drivers/tty/serial/omap-serial.c line 47

Quoted 6 lines

?#include <plat/dma.h>
?#include <plat/dmtimer.h>
?#include <plat/omap-serial.h>

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

+#include <plat/omap_device.h>
+
+#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */

As Jon already pointed out, the HZ here is wrong. ?Please define this value in msecs.

corrected.

Quoted 6 lines

Quoted 6 lines

?static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];

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

@@ -94,6 +98,17 @@ serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
 ? ? ? return port->uartclk/(baud * divisor);
 ?}
 

Quoted 17 lines

drivers/tty/serial/omap-serial.c serial_omap_get_divisor(struct uart_port *port, unsigned int baud) · line 110

Quoted 10 lines

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

+static inline void serial_omap_port_disable(struct uart_omap_port *up)
+{
+ ? ? pm_runtime_mark_last_busy(&up->pdev->dev);
+ ? ? pm_runtime_put_autosuspend(&up->pdev->dev);
+}
+
+static inline void serial_omap_port_enable(struct uart_omap_port *up)
+{
+ ? ? pm_runtime_get_sync(&up->pdev->dev);
+}

These inlines are not needed. ?Please use runtime PM calls directly in the code. ?For _enable(), this is straight forward. ?For _disable() though, I don't think you want (or need) to _mark_last_busy() for every instance of omap_port_disable(). ?You only need to do this for ones TX/RX transactions...

Fine. will modify.

Quoted 17 lines

drivers/tty/serial/omap-serial.c static void serial_omap_enable_ms(struct uart_port *port) · line 132

Quoted 12 lines

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

 ?static void serial_omap_stop_rxdma(struct uart_omap_port *up)
 ?{
 ? ? ? if (up->uart_dma.rx_dma_used) {

@@ -110,8 +125,11 @@ static void serial_omap_enable_ms(struct uart_port *port)
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->pdev->id);
+
+ ? ? serial_omap_port_enable(up);
 ? ? ? up->ier |= UART_IER_MSI;
 ? ? ? serial_out(up, UART_IER, up->ier);
+ ? ? serial_omap_port_disable(up);

...for example, this one is not really activity based, so should probably just be pm_runtime_get_sync(), write the register, then pm_runtime_put() (async version.)

Quoted 3 lines

I didn't look at all the others below, but they should be looked at individually.

ok. I will check them.

Quoted 231 lines

Quoted 231 lines

drivers/tty/serial/omap-serial.c +23 -2

 ?}
 
 ?static void serial_omap_stop_tx(struct uart_port *port)

@@ -131,21 +149,26 @@ static void serial_omap_stop_tx(struct uart_port *port)
 ? ? ? ? ? ? ? up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
 ? ? ? }
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (up->ier & UART_IER_THRI) {
 ? ? ? ? ? ? ? up->ier &= ~UART_IER_THRI;
 ? ? ? ? ? ? ? serial_out(up, UART_IER, up->ier);
 ? ? ? }
+
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static void serial_omap_stop_rx(struct uart_port *port)
 ?{
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (up->use_dma)
 ? ? ? ? ? ? ? serial_omap_stop_rxdma(up);
 ? ? ? up->ier &= ~UART_IER_RLSI;
 ? ? ? up->port.read_status_mask &= ~UART_LSR_DR;
 ? ? ? serial_out(up, UART_IER, up->ier);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static inline void receive_chars(struct uart_omap_port *up, int *status)

@@ -261,8 +284,10 @@ static void serial_omap_start_tx(struct uart_port *port)
 ? ? ? unsigned int start;
 ? ? ? int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (!up->use_dma) {
 ? ? ? ? ? ? ? serial_omap_enable_ier_thri(up);
+ ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? ? ? ? ? return;
 ? ? ? }
 

@@ -354,9 +379,12 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 ? ? ? unsigned int iir, lsr;
 ? ? ? unsigned long flags;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? iir = serial_in(up, UART_IIR);
- ? ? if (iir & UART_IIR_NO_INT)
+ ? ? if (iir & UART_IIR_NO_INT) {
+ ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? ? ? ? ? return IRQ_NONE;
+ ? ? }
 
 ? ? ? spin_lock_irqsave(&up->port.lock, flags);
 ? ? ? lsr = serial_in(up, UART_LSR);

@@ -378,6 +406,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 ? ? ? ? ? ? ? transmit_chars(up);
 
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
+ ? ? serial_omap_port_disable(up);
+
 ? ? ? up->port_activity = jiffies;
 ? ? ? return IRQ_HANDLED;
 ?}

@@ -388,11 +418,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
 ? ? ? unsigned long flags = 0;
 ? ? ? unsigned int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->pdev->id);
 ? ? ? spin_lock_irqsave(&up->port.lock, flags);
 ? ? ? ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
-
+ ? ? serial_omap_port_disable(up);
 ? ? ? return ret;
 ?}
 

@@ -402,7 +433,10 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
 ? ? ? unsigned char status;
 ? ? ? unsigned int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? status = check_modem_status(up);
+ ? ? serial_omap_port_disable(up);
+
 ? ? ? dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
 
 ? ? ? if (status & UART_MSR_DCD)

@@ -434,7 +468,9 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 ? ? ? ? ? ? ? mcr |= UART_MCR_LOOP;
 
 ? ? ? mcr |= up->mcr;
+ ? ? serial_omap_port_enable(up);
 ? ? ? serial_out(up, UART_MCR, mcr);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static void serial_omap_break_ctl(struct uart_port *port, int break_state)

@@ -443,6 +479,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
 ? ? ? unsigned long flags = 0;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->pdev->id);
+ ? ? serial_omap_port_enable(up);
 ? ? ? spin_lock_irqsave(&up->port.lock, flags);
 ? ? ? if (break_state == -1)
 ? ? ? ? ? ? ? up->lcr |= UART_LCR_SBC;

@@ -450,6 +487,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
 ? ? ? ? ? ? ? up->lcr &= ~UART_LCR_SBC;
 ? ? ? serial_out(up, UART_LCR, up->lcr);
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static int serial_omap_startup(struct uart_port *port)

@@ -468,6 +506,7 @@ static int serial_omap_startup(struct uart_port *port)
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
 
+ ? ? serial_omap_port_enable(up);

? ? ? /*
? ? ? ?* Clear the FIFO buffers and disable them. ? ? ? ?* (they will be reenabled in set_termios())

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

@@ -523,6 +562,7 @@ static int serial_omap_startup(struct uart_port *port)

? ? ? /* Enable module level wake up */
? ? ? serial_out(up, UART_OMAP_WER, OMAP_UART_WER_MOD_WKUP);

+ ? ? serial_omap_port_disable(up);
? ? ? up->port_activity = jiffies;
? ? ? return 0;
?}

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

@@ -533,6 +573,8 @@ static void serial_omap_shutdown(struct uart_port *port)
 ? ? ? unsigned long flags = 0;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->pdev->id);
+
+ ? ? serial_omap_port_enable(up);

? ? ? /*
? ? ? ?* Disable interrupts from this port
? ? ? ?*/

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

@@ -566,6 +608,7 @@ static void serial_omap_shutdown(struct uart_port *port)
 ? ? ? ? ? ? ? ? ? ? ? up->uart_dma.rx_buf_dma_phys);
 ? ? ? ? ? ? ? up->uart_dma.rx_buf = NULL;
 ? ? ? }
+ ? ? serial_omap_port_disable(up);
 ? ? ? free_irq(up->port.irq, up);
 ?}
 

@@ -671,6 +714,10 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/13);
 ? ? ? quot = serial_omap_get_divisor(port, baud);
 
+ ? ? up->dll = quot & 0xff;
+ ? ? up->dlh = quot >> 8;
+ ? ? up->mdr1 = UART_OMAP_MDR1_DISABLE;
+

? ? ? up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 | ? ? ? ? ? ? ? ? ? ? ? UART_FCR_ENABLE_FIFO;
? ? ? if (up->use_dma)

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

@@ -680,6 +727,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,

? ? ? ?* Ok, we're now changing the port state. Do it with ? ? ? ?* interrupts disabled.
? ? ? ?*/
+ ? ? serial_omap_port_enable(up);
? ? ? spin_lock_irqsave(&up->port.lock, flags);

? ? ? /*

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

@@ -723,6 +771,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? ? ? ? ? up->ier |= UART_IER_MSI;
 ? ? ? serial_out(up, UART_IER, up->ier);

? ? ? serial_out(up, UART_LCR, cval); ? ? ? ? /* reset DLAB */
+ ? ? up->lcr = cval;

? ? ? /* FIFOs and DMA Settings */

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

@@ -758,8 +807,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? serial_out(up, UART_MCR, up->mcr);
 

? ? ? /* Protocol, Baud Rate, and Interrupt Settings */ -
- ? ? serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1);
? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);

? ? ? up->efr = serial_in(up, UART_EFR);

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

@@ -769,8 +817,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? serial_out(up, UART_IER, 0);
 ? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
- ? ? serial_out(up, UART_DLL, quot & 0xff); ? ? ? ? ?/* LS of divisor */
- ? ? serial_out(up, UART_DLM, quot >> 8); ? ? ? ? ? ?/* MS of divisor */
+ ? ? serial_out(up, UART_DLL, up->dll); ? ? ?/* LS of divisor */
+ ? ? serial_out(up, UART_DLM, up->dlh); ? ? ?/* MS of divisor */
 
 ? ? ? serial_out(up, UART_LCR, 0);
 ? ? ? serial_out(up, UART_IER, up->ier);

@@ -780,9 +828,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? serial_out(up, UART_LCR, cval);
 
 ? ? ? if (baud > 230400 && baud != 3000000)
- ? ? ? ? ? ? serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_13X_MODE);
+ ? ? ? ? ? ? up->mdr1 = UART_OMAP_MDR1_13X_MODE;

? ? ? else
- ? ? ? ? ? ? serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_16X_MODE);
+ ? ? ? ? ? ? up->mdr1 = UART_OMAP_MDR1_16X_MODE;
+
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1)

? ? ? /* Hardware Flow Control Configuration */

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

@@ -810,6 +860,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 ? ? ? ? ? ? ? serial_omap_configure_xonxoff(up, termios);
 
 ? ? ? spin_unlock_irqrestore(&up->port.lock, flags);
+ ? ? serial_omap_port_disable(up);
 ? ? ? dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
 ?}
 

@@ -821,6 +872,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
 ? ? ? unsigned char efr;
 
 ? ? ? dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->pdev->id);
+
+ ? ? serial_omap_port_enable(up);
 ? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 ? ? ? efr = serial_in(up, UART_EFR);
 ? ? ? serial_out(up, UART_EFR, efr | UART_EFR_ECB);

@@ -830,6 +883,10 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
 ? ? ? serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 ? ? ? serial_out(up, UART_EFR, efr);
 ? ? ? serial_out(up, UART_LCR, 0);

Quoted 9 lines

drivers/tty/serial/omap-serial.c serial_omap_pm(struct uart_port *port, unsigned int state, · line 889

Quoted 4 lines

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

+ ? ? if (state)
+ ? ? ? ? ? ? pm_runtime_put_sync(&up->pdev->dev);
+ ? ? else
+ ? ? ? ? ? ? serial_omap_port_disable(up);

OK, so this boils down to either a _put_sync() or a _mark_last_busy + _put_autosuspend(), depending on whether this is suspending or resuming, right?

yes also during bootup.
disable the ports immediately that are not used during bootup.

Quoted 1 line

Why the difference?

Need to put the ports down immediately during system wide suspend other wise autosuspend delay will prevent system to enter suspend state immediately.

Quoted 67 lines

Quoted 67 lines

drivers/tty/serial/omap-serial.c +9 -2

 ?}
 
 ?static void serial_omap_release_port(struct uart_port *port)

@@ -907,25 +964,31 @@ static inline void wait_for_xmitr(struct uart_omap_port *up)
 ?static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
 ?{
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
+
+ ? ? serial_omap_port_enable(up);
 ? ? ? wait_for_xmitr(up);
 ? ? ? serial_out(up, UART_TX, ch);
+ ? ? serial_omap_port_disable(up);
 ?}
 
 ?static int serial_omap_poll_get_char(struct uart_port *port)
 ?{
 ? ? ? struct uart_omap_port *up = (struct uart_omap_port *)port;
- ? ? unsigned int status = serial_in(up, UART_LSR);
+ ? ? unsigned int status;
 
+ ? ? serial_omap_port_enable(up);
+ ? ? status = serial_in(up, UART_LSR);
 ? ? ? if (!(status & UART_LSR_DR))
 ? ? ? ? ? ? ? return NO_POLL_CHAR;
 
- ? ? return serial_in(up, UART_RX);
+ ? ? status = serial_in(up, UART_RX);
+ ? ? serial_omap_port_disable(up);
+ ? ? return status;
 ?}
 

?#endif /* CONFIG_CONSOLE_POLL */

?#ifdef CONFIG_SERIAL_OMAP_CONSOLE
-
?static struct uart_omap_port *serial_omap_console_ports[4];

?static struct uart_driver serial_omap_reg;

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

@@ -955,6 +1018,8 @@ serial_omap_console_write(struct console *co, const char *s,

? ? ? else
? ? ? ? ? ? ? spin_lock(&up->port.lock);

+ ? ? serial_omap_port_enable(up);
+
? ? ? /*
? ? ? ?* First save the IER then disable the interrupts ? ? ? ?*/

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

@@ -979,6 +1044,7 @@ serial_omap_console_write(struct console *co, const char *s,
 ? ? ? if (up->msr_saved_flags)
 ? ? ? ? ? ? ? check_modem_status(up);
 
+ ? ? serial_omap_port_disable(up);
 ? ? ? if (locked)
 ? ? ? ? ? ? ? spin_unlock(&up->port.lock);
 ? ? ? local_irq_restore(flags);

@@ -1061,22 +1127,27 @@ static struct uart_driver serial_omap_reg = {
 ? ? ? .cons ? ? ? ? ? = OMAP_CONSOLE,
 ?};
 
-static int
-serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
+static int serial_omap_suspend(struct device *dev)
 ?{
- ? ? struct uart_omap_port *up = platform_get_drvdata(pdev);
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
 

Quoted 8 lines

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1136

Quoted 4 lines

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

- ? ? if (up)
+ ? ? if (up) {
 ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
+ ? ? ? ? ? ? console_trylock();

This locking needs to be explained. ?Why it is needed, but very importantly, why you are not checking the return value of the _trylock()

any print after this point can result in failure of immediate system suspending due to delayed autosuspend from omap_console_write.

log prints after uart suspend and print them during resume.

Quoted 4 lines

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1137

Quoted 1 line

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

+ ? ? ? ? ? ? serial_omap_pm(&up->port, 3, 0);

Why is this call needed?

Actually this is needed if no_console_suspend is used, for that case the console will remain active since serial_omap_pm is not getting called from serial_core and port is not suspended immediately with put_sync.

prints during system wide suspend and delayed autosuspend from console_write keeps system active in no_console_suspend case so put in forced suspend state and log all prints to be printed later.

probably needs to a condition check if (no_console_suspend)

Quoted 6 lines

uart_suspend_port() calls uart_change_pm() which should call the ->pm method of struct uart_ops (which is serial_omap_pm). ?What am I missing?

Also notice you're not calling serial_omap_pm() in the resume method below to change it back.

Quoted 7 lines

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1140

Quoted 3 lines

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

+ ? ? }
 ? ? ? return 0;
 ?}

Also, device wakeup capability should be checked and enabled/disabled in the suspend path (not only the runtime suspend path.)

ok.

Quoted 13 lines

drivers/tty/serial/omap-serial.c static struct uart_driver serial_omap_reg = { · line 1147

Quoted 10 lines

drivers/tty/serial/omap-serial.c +4 -3

-static int serial_omap_resume(struct platform_device *dev)
+static int serial_omap_resume(struct device *dev)
 ?{
- ? ? struct uart_omap_port *up = platform_get_drvdata(dev);
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
 
- ? ? if (up)
+ ? ? if (up) {
 ? ? ? ? ? ? ? uart_resume_port(&serial_omap_reg, &up->port);
+ ? ? ? ? ? ? console_unlock();

Again, please describe locking in comments.

Quoted 3 lines

Also, what happens here if the console_trylock() in your suspend method failed?

need to add a flag to check and unlock
from return status of trylock.

Quoted 157 lines

Quoted 157 lines

drivers/tty/serial/omap-serial.c +38 -5

+ ? ? }
+
 ? ? ? return 0;
 ?}
 

@@ -1097,6 +1168,7 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 ? ? ? ? ? ? ? ? ? ? ? serial_omap_stop_rxdma(up);
 ? ? ? ? ? ? ? ? ? ? ? up->ier |= (UART_IER_RDI | UART_IER_RLSI);
 ? ? ? ? ? ? ? ? ? ? ? serial_out(up, UART_IER, up->ier);
+ ? ? ? ? ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? ? ? ? ? }
 ? ? ? ? ? ? ? return;
 ? ? ? }

@@ -1128,6 +1200,9 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 
 ?static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
 ?{
+ ? ? struct uart_omap_port *up = (struct uart_omap_port *)data;
+
+ ? ? serial_omap_port_disable(up);
 ? ? ? return;
 ?}
 

@@ -1135,6 +1210,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
 ?{
 ? ? ? int ret = 0;
 
+ ? ? serial_omap_port_enable(up);
 ? ? ? if (up->uart_dma.rx_dma_channel == -1) {
 ? ? ? ? ? ? ? ret = omap_request_dma(up->uart_dma.uart_dma_rx,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "UART Rx DMA",

@@ -1214,6 +1290,7 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
 ? ? ? ? ? ? ? serial_omap_stop_tx(&up->port);
 ? ? ? ? ? ? ? up->uart_dma.tx_dma_used = false;
 ? ? ? ? ? ? ? spin_unlock(&(up->uart_dma.tx_lock));
+ ? ? ? ? ? ? serial_omap_port_disable(up);
 ? ? ? } else {
 ? ? ? ? ? ? ? omap_stop_dma(up->uart_dma.tx_dma_channel);
 ? ? ? ? ? ? ? serial_omap_continue_tx(up);

@@ -1224,9 +1301,10 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
 
 ?static int serial_omap_probe(struct platform_device *pdev)
 ?{
- ? ? struct uart_omap_port ? *up;
+ ? ? struct uart_omap_port ? *up = NULL;
 ? ? ? struct resource ? ? ? ? *mem, *irq, *dma_tx, *dma_rx;
 ? ? ? struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
+ ? ? struct omap_device *od;
 ? ? ? int ret = -ENOSPC;
 
 ? ? ? mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1276,12 +1354,20 @@ static int serial_omap_probe(struct platform_device *pdev)
 ? ? ? up->port.ops = &serial_omap_pops;
 ? ? ? up->port.line = pdev->id;
 
- ? ? up->port.membase = omap_up_info->membase;
- ? ? up->port.mapbase = omap_up_info->mapbase;
+ ? ? up->port.mapbase = mem->start;
+ ? ? up->port.membase = ioremap(mem->start, mem->end - mem->start);
+
+ ? ? if (!up->port.membase) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "can't ioremap UART\n");
+ ? ? ? ? ? ? ret = -ENOMEM;
+ ? ? ? ? ? ? goto err1;
+ ? ? }
+
 ? ? ? up->port.flags = omap_up_info->flags;
- ? ? up->port.irqflags = omap_up_info->irqflags;
 ? ? ? up->port.uartclk = omap_up_info->uartclk;
 ? ? ? up->uart_dma.uart_base = mem->start;
+ ? ? up->errata = omap_up_info->errata;
+ ? ? up->enable_wakeup = omap_up_info->enable_wakeup;
 
 ? ? ? if (omap_up_info->dma_enabled) {
 ? ? ? ? ? ? ? up->uart_dma.uart_dma_tx = dma_tx->start;

@@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct platform_device *pdev)
 ? ? ? ? ? ? ? up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 ? ? ? }
 
+ ? ? pm_runtime_use_autosuspend(&pdev->dev);
+ ? ? pm_runtime_set_autosuspend_delay(&pdev->dev,
+ ? ? ? ? ? ? ? ? ? ? OMAP_UART_AUTOSUSPEND_DELAY);
+
+ ? ? pm_runtime_enable(&pdev->dev);
+ ? ? pm_runtime_irq_safe(&pdev->dev);
+
+ ? ? if (omap_up_info->console_uart) {
+ ? ? ? ? ? ? od = to_omap_device(up->pdev);
+ ? ? ? ? ? ? omap_hwmod_idle(od->hwmods[0]);
+ ? ? ? ? ? ? serial_omap_port_enable(up);
+ ? ? ? ? ? ? serial_omap_port_disable(up);
+ ? ? }
+
 ? ? ? ui[pdev->id] = up;
 ? ? ? serial_omap_add_console_port(up);
 
 ? ? ? ret = uart_add_one_port(&serial_omap_reg, &up->port);
 ? ? ? if (ret != 0)
- ? ? ? ? ? ? goto do_release_region;
+ ? ? ? ? ? ? goto err1;
 
+ ? ? dev_set_drvdata(&pdev->dev, up);
 ? ? ? platform_set_drvdata(pdev, up);
+
 ? ? ? return 0;

?err:
? ? ? dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pdev->id, __func__, ret); +err1:
+ ? ? kfree(up);
?do_release_region:
? ? ? release_mem_region(mem->start, (mem->end - mem->start) + 1); ? ? ? return ret;

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

@@ -1318,20 +1422,76 @@ static int serial_omap_remove(struct platform_device *dev)
 
 ? ? ? platform_set_drvdata(dev, NULL);
 ? ? ? if (up) {
+ ? ? ? ? ? ? pm_runtime_disable(&up->pdev->dev);
 ? ? ? ? ? ? ? uart_remove_one_port(&serial_omap_reg, &up->port);
 ? ? ? ? ? ? ? kfree(up);
 ? ? ? }
 ? ? ? return 0;
 ?}
 
+static void omap_uart_restore_context(struct uart_omap_port *up)
+{
+ ? ? u16 efr = 0;
+
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1);
+ ? ? serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+ ? ? efr = serial_in(up, UART_EFR);
+ ? ? serial_out(up, UART_EFR, UART_EFR_ECB);
+ ? ? serial_out(up, UART_LCR, 0x0); /* Operational mode */
+ ? ? serial_out(up, UART_IER, 0x0);
+ ? ? serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+ ? ? serial_out(up, UART_DLL, up->dll);
+ ? ? serial_out(up, UART_DLM, up->dlh);
+ ? ? serial_out(up, UART_LCR, 0x0); /* Operational mode */
+ ? ? serial_out(up, UART_IER, up->ier);
+ ? ? serial_out(up, UART_FCR, up->fcr);
+ ? ? serial_out(up, UART_LCR, 0x80);
+ ? ? serial_out(up, UART_MCR, up->mcr);
+ ? ? serial_out(up, UART_LCR, 0xBF); /* Config B mode */
+ ? ? serial_out(up, UART_EFR, efr);
+ ? ? serial_out(up, UART_LCR, up->lcr);
+ ? ? /* UART 16x mode */
+ ? ? serial_out(up, UART_OMAP_MDR1, up->mdr1);
+}
+
+static int omap_serial_runtime_suspend(struct device *dev)
+{
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
+
+ ? ? if (!up)
+ ? ? ? ? ? ? goto done;
+
+ ? ? if (device_may_wakeup(dev))

Quoted 9 lines

drivers/tty/serial/omap-serial.c static int serial_omap_remove(struct platform_device *dev) · line 1467

Quoted 3 lines

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

+ ? ? ? ? ? ? up->enable_wakeup(up->pdev, true);
+ ? ? else
+ ? ? ? ? ? ? up->enable_wakeup(up->pdev, false);

I know the earlier code was doing it this way too, but an optimization would be to have the driver keep state whether the wakeups are enabled or disabled, so you don't have to keep calling this function (which can be expensive with the PRCM reads/writes.

if dynamically disabled from user space from sys-fs interface. it may not reflect disable_wakup immediately if internal state machine of wakeup is maintained within uart driver.

also need to modify the internals of this func pointer to use hmwod API's as commented above.

Acquire console lock before enabling and writing to console-uart to avoid any recursive prints from console write.
for ex:

        --> printk
          --> uart_console_write
            --> get_sync
              --> printk from omap_device enable
                --> uart_console write

Use RPM_SUSPENDING check to avoid below scenario during bootup As during bootup console_lock is not available.

       --> uart_add_one_port
           --> console_register
               --> console_lock
                --> console_unlock
                     --> call_console_drivers (here yet console_lock is not released)
                          --> uart_console_write

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

---
 drivers/tty/serial/omap-serial.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

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

index 897416f..ee94291 100644