[PATCH v4 00/11] OMAP2+: Serial: Runtime adaptation + cleanup

[PATCH v4 00/11] OMAP2+: Serial: Runtime adaptation + cleanup

[Govindraj.R]

Converting uart driver to adapt to pm runtime API's.
Code re-org + cleanup.
Moving some functionality from serial.c to omap-serial.c

Changes involves:
================
1.) Cleaning up certain uart calls from sram_idle func.
2.) Removed all types of uart clock handling code from serial.c
3.) Using hwmod_mux API enable wakeup capability for uart pad during
   hwmod_idle state i.e., when uart clocks are disabled we can enable
   io-pad wakeup capability for uart if mux_data is available for
   given uart. Also during during resume from idle call to uart we need
   to enable clocks back conditionally and this can be done only when io-pad
   wakeup event bit is set for uart_rx pad. So we need a hwmod API
   which can probe the uart pad and let us know whether a uart wakeup
   happened. So omap_hmwod_pad_wakeup_status API is added to meet this
   requirement.
3.) Adapted omap-serial driver to use runtime API's.
4.) Modify serial_init calls to accept certain uart parameters from board file.
5.) using prepare and resume_call to disable/enable uart_port.
    Reference to discussion why we need to use prepare and resume hooks.
	http://www.mail-archive.com/linux-omap at vger.kernel.org/msg52707.html
	http://www.mail-archive.com/linux-omap at vger.kernel.org/msg53209.html
	
Patch series is based on 3.1.0-rc4 + omap_device fixes.

[PATCH v4 01/11] OMAP2+: hwmod: Add API to enable IO ring wakeup.

[Govindraj.R]

Add API to enable IO pad wakeup capability based on mux dynamic pad and
wake_up enable flag available from hwmod_mux initialization.

Use the wakeup_enable flag and enable wakeup capability
for the given pads. Wakeup capability will be enabled/disabled
during hwmod idle transition based on whether wakeup_flag is
set or cleared.

Map the enable/disable pad wakeup API's to hwmod_wakeup_enable/disable.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   59 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 84cc0bd..e751dd9 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2062,6 +2062,34 @@ static int __init omap_hwmod_setup_all(void)
 core_initcall(omap_hwmod_setup_all);
 
 /**
+ * omap_hwmod_set_ioring_wakeup - enable io pad wakeup flag.
+ * @oh: struct omap_hwmod *
+ * @set: bool value indicating to set or clear wakeup status.
+ *
+ * Set or Clear wakeup flag for the io_pad.
+ */
+static int omap_hwmod_set_ioring_wakeup(struct omap_hwmod *oh, bool set_wake)
+{
+	struct omap_device_pad *pad;
+	int ret = -EINVAL, j;
+
+	if (oh->mux && oh->mux->enabled) {
+		for (j = 0; j < oh->mux->nr_pads_dynamic; j++) {
+			pad = oh->mux->pads_dynamic[j];
+			if (pad->flags & OMAP_DEVICE_PAD_WAKEUP) {
+				if (set_wake)
+					pad->idle |= OMAP_WAKEUP_EN;
+				else
+					pad->idle &= ~OMAP_WAKEUP_EN;
+				ret = 0;
+			}
+		}
+	}
+
+	return ret;
+}
+
+/**
  * omap_hwmod_enable - enable an omap_hwmod
  * @oh: struct omap_hwmod *
  *
@@ -2393,6 +2421,35 @@ int omap_hwmod_del_initiator_dep(struct omap_hwmod *oh,
 {
 	return _del_initiator_dep(oh, init_oh);
 }
+/**
+ * omap_hwmod_enable_ioring_wakeup - Set wakeup flag for iopad.
+ * @oh: struct omap_hwmod *
+ *
+ * Traverse through dynamic pads, if pad is enabled then
+ * set wakeup enable bit flag for the mux pin. Wakeup pad bit
+ * will be set during hwmod idle transistion.
+ * Return error if pads are not enabled or not available.
+ */
+int omap_hwmod_enable_ioring_wakeup(struct omap_hwmod *oh)
+{
+	/* Enable pad wake-up capability */
+	return omap_hwmod_set_ioring_wakeup(oh, true);
+}
+
+/**
+ * omap_hwmod_disable_ioring_wakeup - Clear wakeup flag for iopad.
+ * @oh: struct omap_hwmod *
+ *
+ * Traverse through dynamic pads, if pad is enabled then
+ * clear wakeup enable bit flag for the mux pin. Wakeup pad bit
+ * will be set during hwmod idle transistion.
+ * Return error if pads are not enabled or not available.
+ */
+int omap_hwmod_disable_ioring_wakeup(struct omap_hwmod *oh)
+{
+	/* Disable pad wakeup capability */
+	return omap_hwmod_set_ioring_wakeup(oh, false);
+}
 
 /**
  * omap_hwmod_enable_wakeup - allow device to wake up the system
@@ -2419,6 +2476,7 @@ int omap_hwmod_enable_wakeup(struct omap_hwmod *oh)
 	v = oh->_sysc_cache;
 	_enable_wakeup(oh, &v);
 	_write_sysconfig(v, oh);
+	omap_hwmod_enable_ioring_wakeup(oh);
 	spin_unlock_irqrestore(&oh->_lock, flags);
 
 	return 0;
@@ -2449,6 +2507,7 @@ int omap_hwmod_disable_wakeup(struct omap_hwmod *oh)
 	v = oh->_sysc_cache;
 	_disable_wakeup(oh, &v);
 	_write_sysconfig(v, oh);
+	omap_hwmod_disable_ioring_wakeup(oh);
 	spin_unlock_irqrestore(&oh->_lock, flags);
 
 	return 0;
-- 
1.7.4.1

[PATCH v4 02/11] OMAP2+: hwmod: Add API to check IO PAD wakeup status

[Govindraj.R]

Add API to determine IO-PAD wakeup event status for a given
hwmod dynamic_mux pad.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/mux.c                    |   30 ++++++++++++++++++++++++++
 arch/arm/mach-omap2/mux.h                    |   13 +++++++++++
 arch/arm/mach-omap2/omap_hwmod.c             |    7 ++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
 4 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 655e948..5f15ab8 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -351,6 +351,36 @@ err1:
 	return NULL;
 }
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux:		Pads for a hwmod
+ *
+ * Gets the wakeup status of given pad from omap-hwmod.
+ * Returns true if wakeup event is set for pad else false
+ * if wakeup is not occured or pads are not avialable.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+	int i;
+	unsigned int val;
+	u8 ret = false;
+
+	for (i = 0; i < hmux->nr_pads; i++) {
+		struct omap_device_pad *pad = &hmux->pads[i];
+
+		if (pad->flags & OMAP_DEVICE_PAD_WAKEUP) {
+			val = omap_mux_read(pad->partition,
+					pad->mux->reg_offset);
+			if (val & OMAP_WAKEUP_EVENT) {
+				ret = true;
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
 /* Assumes the calling function takes care of locking */
 void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 {
diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
index 2132308..8b2150a 100644
--- a/arch/arm/mach-omap2/mux.h
+++ b/arch/arm/mach-omap2/mux.h
@@ -225,8 +225,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
  */
 void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux:		Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux);
 #else
 
+static inline bool
+omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+	return 0;
+}
+
 static inline int omap_mux_init_gpio(int gpio, int val)
 {
 	return 0;
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e751dd9..a8b24d7 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2724,3 +2724,10 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod *oh)
 
 	return 0;
 }
+
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh)
+{
+	if (oh && oh->mux)
+		return omap_hwmod_mux_get_wake_status(oh->mux);
+	return -EINVAL;
+}
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 0e329ca..9a6195c 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -607,6 +607,7 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
 
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh);
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
  * to use initcalls once the initial boot ordering is straightened out
-- 
1.7.4.1

[PATCH v4 03/11] OMAP2+: UART: cleanup + remove certain uart calls from pm code

[Govindraj.R]

In preparation to UART runtime conversion. Remove certain uart specific calls
from pm24xx/34xx files. Remove uart pads from 3430 board file.
These func calls will no more be used with upcoming uart runtime design.

1.) omap_uart_can_sleep :- not needed driver can autosuspend based on usage_count
    and autosuspend delay.
2.) omap_uart_prepare_suspend :- can be taken care with driver.
3.) The pad values in 3430 board file are same as the default pad values to be
    updated in serial.c file with uart runtime conversion changes.
    Avoid structure duplication and use default pads.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/board-3430sdp.c      |  100 +-----------------------------
 arch/arm/mach-omap2/pm24xx.c             |    3 -
 arch/arm/mach-omap2/pm34xx.c             |    5 --
 arch/arm/plat-omap/include/plat/serial.h |    4 -
 4 files changed, 1 insertions(+), 111 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index bd600cf..b12b38f 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -482,106 +482,8 @@ static const struct usbhs_omap_board_data usbhs_bdata __initconst = {
 static struct omap_board_mux board_mux[] __initdata = {
 	{ .reg_offset = OMAP_MUX_TERMINATOR },
 };
-
-static struct omap_device_pad serial1_pads[] __initdata = {
-	/*
-	 * Note that off output enable is an active low
-	 * signal. So setting this means pin is a
-	 * input enabled in off mode
-	 */
-	OMAP_MUX_STATIC("uart1_cts.uart1_cts",
-			 OMAP_PIN_INPUT |
-			 OMAP_PIN_OFF_INPUT_PULLDOWN |
-			 OMAP_OFFOUT_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart1_rts.uart1_rts",
-			 OMAP_PIN_OUTPUT |
-			 OMAP_OFF_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart1_rx.uart1_rx",
-			 OMAP_PIN_INPUT |
-			 OMAP_PIN_OFF_INPUT_PULLDOWN |
-			 OMAP_OFFOUT_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart1_tx.uart1_tx",
-			 OMAP_PIN_OUTPUT |
-			 OMAP_OFF_EN |
-			 OMAP_MUX_MODE0),
-};
-
-static struct omap_device_pad serial2_pads[] __initdata = {
-	OMAP_MUX_STATIC("uart2_cts.uart2_cts",
-			 OMAP_PIN_INPUT_PULLUP |
-			 OMAP_PIN_OFF_INPUT_PULLDOWN |
-			 OMAP_OFFOUT_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart2_rts.uart2_rts",
-			 OMAP_PIN_OUTPUT |
-			 OMAP_OFF_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart2_rx.uart2_rx",
-			 OMAP_PIN_INPUT |
-			 OMAP_PIN_OFF_INPUT_PULLDOWN |
-			 OMAP_OFFOUT_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart2_tx.uart2_tx",
-			 OMAP_PIN_OUTPUT |
-			 OMAP_OFF_EN |
-			 OMAP_MUX_MODE0),
-};
-
-static struct omap_device_pad serial3_pads[] __initdata = {
-	OMAP_MUX_STATIC("uart3_cts_rctx.uart3_cts_rctx",
-			 OMAP_PIN_INPUT_PULLDOWN |
-			 OMAP_PIN_OFF_INPUT_PULLDOWN |
-			 OMAP_OFFOUT_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart3_rts_sd.uart3_rts_sd",
-			 OMAP_PIN_OUTPUT |
-			 OMAP_OFF_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart3_rx_irrx.uart3_rx_irrx",
-			 OMAP_PIN_INPUT |
-			 OMAP_PIN_OFF_INPUT_PULLDOWN |
-			 OMAP_OFFOUT_EN |
-			 OMAP_MUX_MODE0),
-	OMAP_MUX_STATIC("uart3_tx_irtx.uart3_tx_irtx",
-			 OMAP_PIN_OUTPUT |
-			 OMAP_OFF_EN |
-			 OMAP_MUX_MODE0),
-};
-
-static struct omap_board_data serial1_data __initdata = {
-	.id		= 0,
-	.pads		= serial1_pads,
-	.pads_cnt	= ARRAY_SIZE(serial1_pads),
-};
-
-static struct omap_board_data serial2_data __initdata = {
-	.id		= 1,
-	.pads		= serial2_pads,
-	.pads_cnt	= ARRAY_SIZE(serial2_pads),
-};
-
-static struct omap_board_data serial3_data __initdata = {
-	.id		= 2,
-	.pads		= serial3_pads,
-	.pads_cnt	= ARRAY_SIZE(serial3_pads),
-};
-
-static inline void board_serial_init(void)
-{
-	omap_serial_init_port(&serial1_data);
-	omap_serial_init_port(&serial2_data);
-	omap_serial_init_port(&serial3_data);
-}
 #else
 #define board_mux	NULL
-
-static inline void board_serial_init(void)
-{
-	omap_serial_init();
-}
 #endif
 
 /*
@@ -718,7 +620,7 @@ static void __init omap_3430sdp_init(void)
 	else
 		gpio_pendown = SDP3430_TS_GPIO_IRQ_SDPV1;
 	omap_ads7846_init(1, gpio_pendown, 310, NULL);
-	board_serial_init();
+	omap_serial_init();
 	usb_musb_init(NULL);
 	board_smc91x_init();
 	board_flash_init(sdp_flash_partitions, chip_sel_3430, 0);
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
index bf089e7..39f26af 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -265,8 +265,6 @@ static int omap2_can_sleep(void)
 {
 	if (omap2_fclks_active())
 		return 0;
-	if (!omap_uart_can_sleep())
-		return 0;
 	if (osc_ck->usecount > 1)
 		return 0;
 	if (omap_dma_running())
@@ -317,7 +315,6 @@ static int omap2_pm_suspend(void)
 	mir1 = omap_readl(0x480fe0a4);
 	omap_writel(1 << 5, 0x480fe0ac);
 
-	omap_uart_prepare_suspend();
 	omap2_enter_full_retention();
 
 	omap_writel(mir1, 0x480fe0a4);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 7255d9b..9f3bf2c 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -485,8 +485,6 @@ console_still_active:
 
 int omap3_can_sleep(void)
 {
-	if (!omap_uart_can_sleep())
-		return 0;
 	return 1;
 }
 
@@ -531,7 +529,6 @@ static int omap3_pm_suspend(void)
 			goto restore;
 	}
 
-	omap_uart_prepare_suspend();
 	omap3_intc_suspend();
 
 	omap_sram_idle();
@@ -578,14 +575,12 @@ static int omap3_pm_begin(suspend_state_t state)
 {
 	disable_hlt();
 	suspend_state = state;
-	omap_uart_enable_irqs(0);
 	return 0;
 }
 
 static void omap3_pm_end(void)
 {
 	suspend_state = PM_SUSPEND_ON;
-	omap_uart_enable_irqs(1);
 	enable_hlt();
 	return;
 }
diff --git a/arch/arm/plat-omap/include/plat/serial.h b/arch/arm/plat-omap/include/plat/serial.h
index de3b10c..bfd73b7 100644
--- a/arch/arm/plat-omap/include/plat/serial.h
+++ b/arch/arm/plat-omap/include/plat/serial.h
@@ -109,12 +109,8 @@ struct omap_board_data;
 
 extern void omap_serial_init(void);
 extern void omap_serial_init_port(struct omap_board_data *bdata);
-extern int omap_uart_can_sleep(void);
-extern void omap_uart_check_wakeup(void);
-extern void omap_uart_prepare_suspend(void);
 extern void omap_uart_prepare_idle(int num);
 extern void omap_uart_resume_idle(int num);
-extern void omap_uart_enable_irqs(int enable);
 #endif
 
 #endif
-- 
1.7.4.1

[PATCH v4 04/11] OMAP2+: UART: Remove uart clock handling code from serial.c

[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 <alan@linux.intel.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/serial.c                  |  742 ++-----------------------
 arch/arm/plat-omap/include/plat/omap-serial.h |   11 +-
 2 files changed, 53 insertions(+), 700 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 3d1c1d3..e732d6c 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -19,25 +19,16 @@
  */
 #include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/serial_reg.h>
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/serial_8250.h>
 #include <linux/pm_runtime.h>
-#include <linux/console.h>
 
-#ifdef CONFIG_SERIAL_OMAP
 #include <plat/omap-serial.h>
-#endif
-
 #include <plat/common.h>
 #include <plat/board.h>
-#include <plat/clock.h>
-#include <plat/dma.h>
-#include <plat/omap_hwmod.h>
 #include <plat/omap_device.h>
 
 #include "prm2xxx_3xxx.h"
@@ -47,66 +38,8 @@
 #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)
-
-/*
- * 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
- * blocked. 
- */
-#define DEFAULT_TIMEOUT 0
-
 #define MAX_UART_HWMOD_NAME_LEN		16
 
-struct omap_uart_state {
-	int num;
-	int can_sleep;
-	struct timer_list timer;
-	u32 timeout;
-
-	void __iomem *wk_st;
-	void __iomem *wk_en;
-	u32 wk_mask;
-	u32 padconf;
-	u32 dma_enabled;
-
-	struct clk *ick;
-	struct clk *fck;
-	int clocked;
-
-	int irq;
-	int regshift;
-	int irqflags;
-	void __iomem *membase;
-	resource_size_t mapbase;
-
-	struct list_head node;
-	struct omap_hwmod *oh;
-	struct platform_device *pdev;
-
-	u32 errata;
-#if defined(CONFIG_ARCH_OMAP3) && defined(CONFIG_PM)
-	int context_valid;
-
-	/* Registers to be saved/restored for OFF-mode */
-	u16 dll;
-	u16 dlh;
-	u16 ier;
-	u16 sysc;
-	u16 scr;
-	u16 wer;
-	u16 mcr;
-#endif
-};
-
-static LIST_HEAD(uart_list);
-static u8 num_uarts;
-
 static int uart_idle_hwmod(struct omap_device *od)
 {
 	omap_hwmod_idle(od->hwmods[0]);
@@ -129,396 +62,35 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
 	},
 };
 
-static inline unsigned int __serial_read_reg(struct uart_port *up,
-					     int offset)
-{
-	offset <<= up->regshift;
-	return (unsigned int)__raw_readb(up->membase + offset);
-}
-
-static inline unsigned int serial_read_reg(struct omap_uart_state *uart,
-					   int offset)
-{
-	offset <<= uart->regshift;
-	return (unsigned int)__raw_readb(uart->membase + offset);
-}
-
-static inline void __serial_write_reg(struct uart_port *up, int offset,
-		int value)
-{
-	offset <<= up->regshift;
-	__raw_writeb(value, up->membase + offset);
-}
-
-static inline void serial_write_reg(struct omap_uart_state *uart, int offset,
-				    int value)
-{
-	offset <<= uart->regshift;
-	__raw_writeb(value, uart->membase + offset);
-}
-
-/*
- * Internal UARTs need to be initialized for the 8250 autoconfig to work
- * properly. Note that the TX watermark initialization may not be needed
- * once the 8250.c watermark handling code is merged.
- */
-
-static inline void __init omap_uart_reset(struct omap_uart_state *uart)
-{
-	serial_write_reg(uart, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
-	serial_write_reg(uart, UART_OMAP_SCR, 0x08);
-	serial_write_reg(uart, UART_OMAP_MDR1, UART_OMAP_MDR1_16X_MODE);
-}
-
-#if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3)
-
-/*
- * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)
- * The access to uart register after MDR1 Access
- * causes UART to corrupt data.
- *
- * Need a delay =
- * 5 L4 clock cycles + 5 UART functional clock cycle (@48MHz = ~0.2uS)
- * give 10 times as much
- */
-static void omap_uart_mdr1_errataset(struct omap_uart_state *uart, u8 mdr1_val,
-		u8 fcr_val)
-{
-	u8 timeout = 255;
-
-	serial_write_reg(uart, UART_OMAP_MDR1, mdr1_val);
-	udelay(2);
-	serial_write_reg(uart, UART_FCR, fcr_val | UART_FCR_CLEAR_XMIT |
-			UART_FCR_CLEAR_RCVR);
-	/*
-	 * Wait for FIFO to empty: when empty, RX_FIFO_E bit is 0 and
-	 * TX_FIFO_E bit is 1.
-	 */
-	while (UART_LSR_THRE != (serial_read_reg(uart, UART_LSR) &
-				(UART_LSR_THRE | UART_LSR_DR))) {
-		timeout--;
-		if (!timeout) {
-			/* Should *never* happen. we warn and carry on */
-			dev_crit(&uart->pdev->dev, "Errata i202: timedout %x\n",
-			serial_read_reg(uart, UART_LSR));
-			break;
-		}
-		udelay(1);
-	}
-}
-
-static void omap_uart_save_context(struct omap_uart_state *uart)
-{
-	u16 lcr = 0;
-
-	if (!enable_off_mode)
-		return;
-
-	lcr = serial_read_reg(uart, UART_LCR);
-	serial_write_reg(uart, UART_LCR, UART_LCR_CONF_MODE_B);
-	uart->dll = serial_read_reg(uart, UART_DLL);
-	uart->dlh = serial_read_reg(uart, UART_DLM);
-	serial_write_reg(uart, UART_LCR, lcr);
-	uart->ier = serial_read_reg(uart, UART_IER);
-	uart->sysc = serial_read_reg(uart, UART_OMAP_SYSC);
-	uart->scr = serial_read_reg(uart, UART_OMAP_SCR);
-	uart->wer = serial_read_reg(uart, UART_OMAP_WER);
-	serial_write_reg(uart, UART_LCR, UART_LCR_CONF_MODE_A);
-	uart->mcr = serial_read_reg(uart, UART_MCR);
-	serial_write_reg(uart, UART_LCR, lcr);
-
-	uart->context_valid = 1;
-}
-
-static void omap_uart_restore_context(struct omap_uart_state *uart)
-{
-	u16 efr = 0;
-
-	if (!enable_off_mode)
-		return;
-
-	if (!uart->context_valid)
-		return;
-
-	uart->context_valid = 0;
-
-	if (uart->errata & UART_ERRATA_i202_MDR1_ACCESS)
-		omap_uart_mdr1_errataset(uart, UART_OMAP_MDR1_DISABLE, 0xA0);
-	else
-		serial_write_reg(uart, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
-
-	serial_write_reg(uart, UART_LCR, UART_LCR_CONF_MODE_B);
-	efr = serial_read_reg(uart, UART_EFR);
-	serial_write_reg(uart, UART_EFR, UART_EFR_ECB);
-	serial_write_reg(uart, UART_LCR, 0x0); /* Operational mode */
-	serial_write_reg(uart, UART_IER, 0x0);
-	serial_write_reg(uart, UART_LCR, UART_LCR_CONF_MODE_B);
-	serial_write_reg(uart, UART_DLL, uart->dll);
-	serial_write_reg(uart, UART_DLM, uart->dlh);
-	serial_write_reg(uart, UART_LCR, 0x0); /* Operational mode */
-	serial_write_reg(uart, UART_IER, uart->ier);
-	serial_write_reg(uart, UART_LCR, UART_LCR_CONF_MODE_A);
-	serial_write_reg(uart, UART_MCR, uart->mcr);
-	serial_write_reg(uart, UART_LCR, UART_LCR_CONF_MODE_B);
-	serial_write_reg(uart, UART_EFR, efr);
-	serial_write_reg(uart, UART_LCR, UART_LCR_WLEN8);
-	serial_write_reg(uart, UART_OMAP_SCR, uart->scr);
-	serial_write_reg(uart, UART_OMAP_WER, uart->wer);
-	serial_write_reg(uart, UART_OMAP_SYSC, uart->sysc);
-
-	if (uart->errata & UART_ERRATA_i202_MDR1_ACCESS)
-		omap_uart_mdr1_errataset(uart, UART_OMAP_MDR1_16X_MODE, 0xA1);
-	else
-		/* UART 16x mode */
-		serial_write_reg(uart, UART_OMAP_MDR1,
-				UART_OMAP_MDR1_16X_MODE);
-}
-#else
-static inline void omap_uart_save_context(struct omap_uart_state *uart) {}
-static inline void omap_uart_restore_context(struct omap_uart_state *uart) {}
-#endif /* CONFIG_PM && CONFIG_ARCH_OMAP3 */
-
-static inline void omap_uart_enable_clocks(struct omap_uart_state *uart)
-{
-	if (uart->clocked)
-		return;
-
-	omap_device_enable(uart->pdev);
-	uart->clocked = 1;
-	omap_uart_restore_context(uart);
-}
-
-#ifdef CONFIG_PM
-
-static inline void omap_uart_disable_clocks(struct omap_uart_state *uart)
-{
-	if (!uart->clocked)
-		return;
-
-	omap_uart_save_context(uart);
-	uart->clocked = 0;
-	omap_device_idle(uart->pdev);
-}
-
-static void omap_uart_enable_wakeup(struct omap_uart_state *uart)
-{
-	/* Set wake-enable bit */
-	if (uart->wk_en && uart->wk_mask) {
-		u32 v = __raw_readl(uart->wk_en);
-		v |= uart->wk_mask;
-		__raw_writel(v, uart->wk_en);
-	}
-
-	/* Ensure IOPAD wake-enables are set */
-	if (cpu_is_omap34xx() && uart->padconf) {
-		u16 v = omap_ctrl_readw(uart->padconf);
-		v |= OMAP3_PADCONF_WAKEUPENABLE0;
-		omap_ctrl_writew(v, uart->padconf);
-	}
-}
-
-static void omap_uart_disable_wakeup(struct omap_uart_state *uart)
-{
-	/* Clear wake-enable bit */
-	if (uart->wk_en && uart->wk_mask) {
-		u32 v = __raw_readl(uart->wk_en);
-		v &= ~uart->wk_mask;
-		__raw_writel(v, uart->wk_en);
-	}
-
-	/* Ensure IOPAD wake-enables are cleared */
-	if (cpu_is_omap34xx() && uart->padconf) {
-		u16 v = omap_ctrl_readw(uart->padconf);
-		v &= ~OMAP3_PADCONF_WAKEUPENABLE0;
-		omap_ctrl_writew(v, uart->padconf);
-	}
-}
-
-static void omap_uart_smart_idle_enable(struct omap_uart_state *uart,
-					       int enable)
-{
-	u8 idlemode;
-
-	if (enable) {
-		/**
-		 * Errata 2.15: [UART]:Cannot Acknowledge Idle Requests
-		 * in Smartidle Mode When Configured for DMA Operations.
-		 */
-		if (uart->dma_enabled)
-			idlemode = HWMOD_IDLEMODE_FORCE;
-		else
-			idlemode = HWMOD_IDLEMODE_SMART;
-	} else {
-		idlemode = HWMOD_IDLEMODE_NO;
-	}
-
-	omap_hwmod_set_slave_idlemode(uart->oh, idlemode);
-}
-
-static void omap_uart_block_sleep(struct omap_uart_state *uart)
-{
-	omap_uart_enable_clocks(uart);
-
-	omap_uart_smart_idle_enable(uart, 0);
-	uart->can_sleep = 0;
-	if (uart->timeout)
-		mod_timer(&uart->timer, jiffies + uart->timeout);
-	else
-		del_timer(&uart->timer);
-}
-
-static void omap_uart_allow_sleep(struct omap_uart_state *uart)
-{
-	if (device_may_wakeup(&uart->pdev->dev))
-		omap_uart_enable_wakeup(uart);
-	else
-		omap_uart_disable_wakeup(uart);
-
-	if (!uart->clocked)
-		return;
-
-	omap_uart_smart_idle_enable(uart, 1);
-	uart->can_sleep = 1;
-	del_timer(&uart->timer);
-}
-
-static void omap_uart_idle_timer(unsigned long data)
-{
-	struct omap_uart_state *uart = (struct omap_uart_state *)data;
-
-	omap_uart_allow_sleep(uart);
-}
-
-void omap_uart_prepare_idle(int num)
-{
-	struct omap_uart_state *uart;
-
-	list_for_each_entry(uart, &uart_list, node) {
-		if (num == uart->num && uart->can_sleep) {
-			omap_uart_disable_clocks(uart);
-			return;
-		}
-	}
-}
-
-void omap_uart_resume_idle(int num)
+static void omap_uart_idle_init(struct omap_uart_port_info *uart,
+				unsigned short num)
 {
-	struct omap_uart_state *uart;
-
-	list_for_each_entry(uart, &uart_list, node) {
-		if (num == uart->num && uart->can_sleep) {
-			omap_uart_enable_clocks(uart);
-
-			/* Check for IO pad wakeup */
-			if (cpu_is_omap34xx() && uart->padconf) {
-				u16 p = omap_ctrl_readw(uart->padconf);
-
-				if (p & OMAP3_PADCONF_WAKEUPEVENT0)
-					omap_uart_block_sleep(uart);
-			}
-
-			/* Check for normal UART wakeup */
-			if (__raw_readl(uart->wk_st) & uart->wk_mask)
-				omap_uart_block_sleep(uart);
-			return;
-		}
-	}
-}
-
-void omap_uart_prepare_suspend(void)
-{
-	struct omap_uart_state *uart;
-
-	list_for_each_entry(uart, &uart_list, node) {
-		omap_uart_allow_sleep(uart);
-	}
-}
-
-int omap_uart_can_sleep(void)
-{
-	struct omap_uart_state *uart;
-	int can_sleep = 1;
-
-	list_for_each_entry(uart, &uart_list, node) {
-		if (!uart->clocked)
-			continue;
-
-		if (!uart->can_sleep) {
-			can_sleep = 0;
-			continue;
-		}
-
-		/* This UART can now safely sleep. */
-		omap_uart_allow_sleep(uart);
-	}
-
-	return can_sleep;
-}
-
-/**
- * omap_uart_interrupt()
- *
- * This handler is used only to detect that *any* UART interrupt has
- * occurred.  It does _nothing_ to handle the interrupt.  Rather,
- * any UART interrupt will trigger the inactivity timer so the
- * UART will not idle or sleep for its timeout period.
- *
- **/
-/* static int first_interrupt; */
-static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
-{
-	struct omap_uart_state *uart = dev_id;
-
-	omap_uart_block_sleep(uart);
-
-	return IRQ_NONE;
-}
-
-static void omap_uart_idle_init(struct omap_uart_state *uart)
-{
-	int ret;
-
-	uart->can_sleep = 0;
-	uart->timeout = DEFAULT_TIMEOUT;
-	setup_timer(&uart->timer, omap_uart_idle_timer,
-		    (unsigned long) uart);
-	if (uart->timeout)
-		mod_timer(&uart->timer, jiffies + uart->timeout);
-	omap_uart_smart_idle_enable(uart, 0);
-
-	if (cpu_is_omap34xx() && !cpu_is_ti816x()) {
-		u32 mod = (uart->num > 1) ? OMAP3430_PER_MOD : CORE_MOD;
+	if (cpu_is_omap34xx()) {
+		u32 mod = num > 1 ? OMAP3430_PER_MOD : CORE_MOD;
 		u32 wk_mask = 0;
-		u32 padconf = 0;
 
-		/* XXX These PRM accesses do not belong here */
 		uart->wk_en = OMAP34XX_PRM_REGADDR(mod, PM_WKEN1);
 		uart->wk_st = OMAP34XX_PRM_REGADDR(mod, PM_WKST1);
-		switch (uart->num) {
+		switch (num) {
 		case 0:
 			wk_mask = OMAP3430_ST_UART1_MASK;
-			padconf = 0x182;
 			break;
 		case 1:
 			wk_mask = OMAP3430_ST_UART2_MASK;
-			padconf = 0x17a;
 			break;
 		case 2:
 			wk_mask = OMAP3430_ST_UART3_MASK;
-			padconf = 0x19e;
 			break;
 		case 3:
 			wk_mask = OMAP3630_ST_UART4_MASK;
-			padconf = 0x0d2;
 			break;
 		}
 		uart->wk_mask = wk_mask;
-		uart->padconf = padconf;
 	} else if (cpu_is_omap24xx()) {
 		u32 wk_mask = 0;
 		u32 wk_en = PM_WKEN1, wk_st = PM_WKST1;
 
-		switch (uart->num) {
+		switch (num) {
 		case 0:
 			wk_mask = OMAP24XX_ST_UART1_MASK;
 			break;
@@ -543,155 +115,33 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 		uart->wk_en = NULL;
 		uart->wk_st = NULL;
 		uart->wk_mask = 0;
-		uart->padconf = 0;
-	}
-
-	uart->irqflags |= IRQF_SHARED;
-	ret = request_threaded_irq(uart->irq, NULL, omap_uart_interrupt,
-				   IRQF_SHARED, "serial idle", (void *)uart);
-	WARN_ON(ret);
-}
-
-void omap_uart_enable_irqs(int enable)
-{
-	int ret;
-	struct omap_uart_state *uart;
-
-	list_for_each_entry(uart, &uart_list, node) {
-		if (enable) {
-			pm_runtime_put_sync(&uart->pdev->dev);
-			ret = request_threaded_irq(uart->irq, NULL,
-						   omap_uart_interrupt,
-						   IRQF_SHARED,
-						   "serial idle",
-						   (void *)uart);
-		} else {
-			pm_runtime_get_noresume(&uart->pdev->dev);
-			free_irq(uart->irq, (void *)uart);
-		}
-	}
-}
-
-static ssize_t sleep_timeout_show(struct device *dev,
-				  struct device_attribute *attr,
-				  char *buf)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct omap_device *odev = to_omap_device(pdev);
-	struct omap_uart_state *uart = odev->hwmods[0]->dev_attr;
-
-	return sprintf(buf, "%u\n", uart->timeout / HZ);
-}
-
-static ssize_t sleep_timeout_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t n)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct omap_device *odev = to_omap_device(pdev);
-	struct omap_uart_state *uart = odev->hwmods[0]->dev_attr;
-	unsigned int value;
-
-	if (sscanf(buf, "%u", &value) != 1) {
-		dev_err(dev, "sleep_timeout_store: Invalid value\n");
-		return -EINVAL;
 	}
-
-	uart->timeout = value * HZ;
-	if (uart->timeout)
-		mod_timer(&uart->timer, jiffies + uart->timeout);
-	else
-		/* A zero value means disable timeout feature */
-		omap_uart_block_sleep(uart);
-
-	return n;
 }
 
-static DEVICE_ATTR(sleep_timeout, 0644, sleep_timeout_show,
-		sleep_timeout_store);
-#define DEV_CREATE_FILE(dev, attr) WARN_ON(device_create_file(dev, attr))
-#else
-static inline void omap_uart_idle_init(struct omap_uart_state *uart) {}
-static void omap_uart_block_sleep(struct omap_uart_state *uart)
+struct omap_hwmod *omap_uart_hwmod_lookup(int num)
 {
-	/* Needed to enable UART clocks when built without CONFIG_PM */
-	omap_uart_enable_clocks(uart);
-}
-#define DEV_CREATE_FILE(dev, attr)
-#endif /* CONFIG_PM */
-
-#ifndef CONFIG_SERIAL_OMAP
-/*
- * Override the default 8250 read handler: mem_serial_in()
- * Empty RX fifo read causes an abort on omap3630 and omap4
- * This function makes sure that an empty rx fifo is not read on these silicons
- * (OMAP1/2/3430 are not affected)
- */
-static unsigned int serial_in_override(struct uart_port *up, int offset)
-{
-	if (UART_RX == offset) {
-		unsigned int lsr;
-		lsr = __serial_read_reg(up, UART_LSR);
-		if (!(lsr & UART_LSR_DR))
-			return -EPERM;
-	}
-
-	return __serial_read_reg(up, offset);
-}
-
-static void serial_out_override(struct uart_port *up, int offset, int value)
-{
-	unsigned int status, tmout = 10000;
+	struct omap_hwmod *oh;
+	char oh_name[MAX_UART_HWMOD_NAME_LEN];
 
-	status = __serial_read_reg(up, UART_LSR);
-	while (!(status & UART_LSR_THRE)) {
-		/* Wait up to 10ms for the character(s) to be sent. */
-		if (--tmout == 0)
-			break;
-		udelay(1);
-		status = __serial_read_reg(up, UART_LSR);
-	}
-	__serial_write_reg(up, offset, value);
+	snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN, "uart%d", num + 1);
+	oh = omap_hwmod_lookup(oh_name);
+	WARN(IS_ERR(oh), "Could not lookup hmwod info for %s\n",
+					oh_name);
+	return oh;
 }
-#endif
 
 static int __init omap_serial_early_init(void)
 {
 	int i = 0;
+	struct omap_hwmod *oh;
 
-	do {
-		char oh_name[MAX_UART_HWMOD_NAME_LEN];
-		struct omap_hwmod *oh;
-		struct omap_uart_state *uart;
-
-		snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN,
-			 "uart%d", i + 1);
-		oh = omap_hwmod_lookup(oh_name);
+	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
+		oh = omap_uart_hwmod_lookup(i);
 		if (!oh)
-			break;
-
-		uart = kzalloc(sizeof(struct omap_uart_state), GFP_KERNEL);
-		if (WARN_ON(!uart))
-			return -ENODEV;
-
-		uart->oh = oh;
-		uart->num = i++;
-		list_add_tail(&uart->node, &uart_list);
-		num_uarts++;
-
-		/*
-		 * NOTE: omap_hwmod_setup*() has not yet been called,
-		 *       so no hwmod functions will work yet.
-		 */
-
-		/*
-		 * During UART early init, device need to be probed
-		 * to determine SoC specific init before omap_device
-		 * is ready.  Therefore, don't allow idle here
-		 */
-		uart->oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
-	} while (1);
+			continue;
 
+		oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
+	}
 	return 0;
 }
 core_initcall(omap_serial_early_init);
@@ -709,150 +159,49 @@ core_initcall(omap_serial_early_init);
  */
 void __init omap_serial_init_port(struct omap_board_data *bdata)
 {
-	struct omap_uart_state *uart;
 	struct omap_hwmod *oh;
 	struct platform_device *pdev;
-	void *pdata = NULL;
-	u32 pdata_size = 0;
-	char *name;
-#ifndef CONFIG_SERIAL_OMAP
-	struct plat_serial8250_port ports[2] = {
-		{},
-		{.flags = 0},
-	};
-	struct plat_serial8250_port *p = &ports[0];
-#else
-	struct omap_uart_port_info omap_up;
-#endif
+	struct omap_uart_port_info *pdata;
+	char *name = DRIVER_NAME;
 
 	if (WARN_ON(!bdata))
 		return;
 	if (WARN_ON(bdata->id < 0))
 		return;
-	if (WARN_ON(bdata->id >= num_uarts))
+	if (WARN_ON(bdata->id >= OMAP_MAX_HSUART_PORTS))
 		return;
 
-	list_for_each_entry(uart, &uart_list, node)
-		if (bdata->id == uart->num)
-			break;
-
-	oh = uart->oh;
-	uart->dma_enabled = 0;
-#ifndef CONFIG_SERIAL_OMAP
-	name = "serial8250";
-
-	/*
-	 * !! 8250 driver does not use standard IORESOURCE* It
-	 * has it's own custom pdata that can be taken from
-	 * the hwmod resource data.  But, this needs to be
-	 * done after the build.
-	 *
-	 * ?? does it have to be done before the register ??
-	 * YES, because platform_device_data_add() copies
-	 * pdata, it does not use a pointer.
-	 */
-	p->flags = UPF_BOOT_AUTOCONF;
-	p->iotype = UPIO_MEM;
-	p->regshift = 2;
-	p->uartclk = OMAP24XX_BASE_BAUD * 16;
-	p->irq = oh->mpu_irqs[0].irq;
-	p->mapbase = oh->slaves[0]->addr->pa_start;
-	p->membase = omap_hwmod_get_mpu_rt_va(oh);
-	p->irqflags = IRQF_SHARED;
-	p->private_data = uart;
-
-	/*
-	 * omap44xx, ti816x: Never read empty UART fifo
-	 * omap3xxx: Never read empty UART fifo on UARTs
-	 * with IP rev >=0x52
-	 */
-	uart->regshift = p->regshift;
-	uart->membase = p->membase;
-	if (cpu_is_omap44xx() || cpu_is_ti816x())
-		uart->errata |= UART_ERRATA_FIFO_FULL_ABORT;
-	else if ((serial_read_reg(uart, UART_OMAP_MVER) & 0xFF)
-			>= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
-		uart->errata |= UART_ERRATA_FIFO_FULL_ABORT;
+	oh = omap_uart_hwmod_lookup(bdata->id);
+	if (!oh)
+		return;
 
-	if (uart->errata & UART_ERRATA_FIFO_FULL_ABORT) {
-		p->serial_in = serial_in_override;
-		p->serial_out = serial_out_override;
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		pr_err("Memory allocation for UART pdata failed\n");
+		return;
 	}
 
-	pdata = &ports[0];
-	pdata_size = 2 * sizeof(struct plat_serial8250_port);
-#else
-
-	name = DRIVER_NAME;
+	omap_uart_idle_init(pdata, bdata->id);
 
-	omap_up.dma_enabled = uart->dma_enabled;
-	omap_up.uartclk = OMAP24XX_BASE_BAUD * 16;
-	omap_up.mapbase = oh->slaves[0]->addr->pa_start;
-	omap_up.membase = omap_hwmod_get_mpu_rt_va(oh);
-	omap_up.irqflags = IRQF_SHARED;
-	omap_up.flags = UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
-
-	pdata = &omap_up;
-	pdata_size = sizeof(struct omap_uart_port_info);
-#endif
+	/* Enable the MDR1 errata for OMAP3 */
+	if (cpu_is_omap34xx() && !cpu_is_ti816x())
+		pdata->errata |= UART_ERRATA_i202_MDR1_ACCESS;
 
-	if (WARN_ON(!oh))
-		return;
+	pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
+	pdata->flags = UPF_BOOT_AUTOCONF;
 
-	pdev = omap_device_build(name, uart->num, oh, pdata, pdata_size,
-			       omap_uart_latency,
-			       ARRAY_SIZE(omap_uart_latency), false);
+	pdev = omap_device_build(name, bdata->id, oh, pdata,
+				sizeof(*pdata),	omap_uart_latency,
+				ARRAY_SIZE(omap_uart_latency), false);
 	WARN(IS_ERR(pdev), "Could not build omap_device for %s: %s.\n",
 	     name, oh->name);
 
-	omap_device_disable_idle_on_suspend(pdev);
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
-
-	uart->irq = oh->mpu_irqs[0].irq;
-	uart->regshift = 2;
-	uart->mapbase = oh->slaves[0]->addr->pa_start;
-	uart->membase = omap_hwmod_get_mpu_rt_va(oh);
-	uart->pdev = pdev;
-
-	oh->dev_attr = uart;
-
-	console_lock(); /* in case the earlycon is on the UART */
-
-	/*
-	 * Because of early UART probing, UART did not get idled
-	 * on init.  Now that omap_device is ready, ensure full idle
-	 * before doing omap_device_enable().
-	 */
-	omap_hwmod_idle(uart->oh);
-
-	omap_device_enable(uart->pdev);
-	omap_uart_idle_init(uart);
-	omap_uart_reset(uart);
-	omap_hwmod_enable_wakeup(uart->oh);
-	omap_device_idle(uart->pdev);
-
-	/*
-	 * Need to block sleep long enough for interrupt driven
-	 * driver to start.  Console driver is in polling mode
-	 * so device needs to be kept enabled while polling driver
-	 * is in use.
-	 */
-	if (uart->timeout)
-		uart->timeout = (30 * HZ);
-	omap_uart_block_sleep(uart);
-	uart->timeout = DEFAULT_TIMEOUT;
-
-	console_unlock();
-
-	if ((cpu_is_omap34xx() && uart->padconf) ||
-	    (uart->wk_en && uart->wk_mask)) {
+	if ((cpu_is_omap34xx() && bdata->pads) ||
+			(pdata->wk_en && pdata->wk_mask))
 		device_init_wakeup(&pdev->dev, true);
-		DEV_CREATE_FILE(&pdev->dev, &dev_attr_sleep_timeout);
-	}
 
-	/* Enable the MDR1 errata for OMAP3 */
-	if (cpu_is_omap34xx() && !cpu_is_ti816x())
-		uart->errata |= UART_ERRATA_i202_MDR1_ACCESS;
+	kfree(pdata);
 }
 
 /**
@@ -864,15 +213,14 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
  */
 void __init omap_serial_init(void)
 {
-	struct omap_uart_state *uart;
 	struct omap_board_data bdata;
+	u8 i;
 
-	list_for_each_entry(uart, &uart_list, node) {
-		bdata.id = uart->num;
+	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
+		bdata.id = i;
 		bdata.flags = 0;
 		bdata.pads = NULL;
 		bdata.pads_cnt = 0;
 		omap_serial_init_port(&bdata);
-
 	}
 }
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 2682043..f79a76f 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -56,13 +56,17 @@
 
 #define MSR_SAVE_FLAGS		UART_MSR_ANY_DELTA
 
+#define UART_ERRATA_i202_MDR1_ACCESS	BIT(0)
+
 struct omap_uart_port_info {
 	bool			dma_enabled;	/* To specify DMA Mode */
 	unsigned int		uartclk;	/* UART clock rate */
-	void __iomem		*membase;	/* ioremap cookie or NULL */
-	resource_size_t		mapbase;	/* resource base */
-	unsigned long		irqflags;	/* request_irq flags */
 	upf_t			flags;		/* UPF_* flags */
+	unsigned int		errata;
+
+	void __iomem *wk_st;
+	void __iomem *wk_en;
+	u32 wk_mask;
 };
 
 struct uart_omap_dma {
@@ -111,6 +115,7 @@ struct uart_omap_port {
 	unsigned char		msr_saved_flags;
 	char			name[20];
 	unsigned long		port_activity;
+	unsigned int		errata;
 };
 
 #endif /* __OMAP_SERIAL_H__ */
-- 
1.7.4.1

[PATCH v4 05/11] OMAP2+: Serial: Add default mux for all uarts.

[Govindraj.R]

Add default mux data for all uart's if mux info is not passed from
board file to avoid breaking any board support.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/serial.c |  132 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 132 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index e732d6c..2e10357 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -62,6 +62,134 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
 	},
 };
 
+#ifdef CONFIG_OMAP_MUX
+static struct omap_device_pad default_uart1_pads[] __initdata = {
+	{
+		.name	= "uart1_cts.uart1_cts",
+		.enable	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart1_rts.uart1_rts",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart1_tx.uart1_tx",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart1_rx.uart1_rx",
+		.flags	= OMAP_DEVICE_PAD_REMUX | OMAP_DEVICE_PAD_WAKEUP,
+		.enable	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+		.idle	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+	},
+};
+
+static struct omap_device_pad default_uart2_pads[] __initdata = {
+	{
+		.name	= "uart2_cts.uart2_cts",
+		.enable	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart2_rts.uart2_rts",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart2_tx.uart2_tx",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart2_rx.uart2_rx",
+		.flags	= OMAP_DEVICE_PAD_REMUX | OMAP_DEVICE_PAD_WAKEUP,
+		.enable	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+		.idle	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+	},
+};
+
+static struct omap_device_pad default_uart3_pads[] __initdata = {
+	{
+		.name	= "uart3_cts_rctx.uart3_cts_rctx",
+		.enable	= OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart3_rts_sd.uart3_rts_sd",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart3_tx_irtx.uart3_tx_irtx",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart3_rx_irrx.uart3_rx_irrx",
+		.flags	= OMAP_DEVICE_PAD_REMUX | OMAP_DEVICE_PAD_WAKEUP,
+		.enable	= OMAP_PIN_INPUT | OMAP_MUX_MODE0,
+		.idle	= OMAP_PIN_INPUT | OMAP_MUX_MODE0,
+	},
+};
+
+static struct omap_device_pad default_omap36xx_uart4_pads[] __initdata = {
+	{
+		.name   = "gpmc_wait2.uart4_tx",
+		.enable = OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "gpmc_wait3.uart4_rx",
+		.flags	= OMAP_DEVICE_PAD_REMUX | OMAP_DEVICE_PAD_WAKEUP,
+		.enable	= OMAP_PIN_INPUT | OMAP_MUX_MODE2,
+		.idle	= OMAP_PIN_INPUT | OMAP_MUX_MODE2,
+	},
+};
+
+static struct omap_device_pad default_omap4_uart4_pads[] __initdata = {
+	{
+		.name	= "uart4_tx.uart4_tx",
+		.enable	= OMAP_PIN_OUTPUT | OMAP_MUX_MODE0,
+	},
+	{
+		.name	= "uart4_rx.uart4_rx",
+		.flags	= OMAP_DEVICE_PAD_REMUX | OMAP_DEVICE_PAD_WAKEUP,
+		.enable	= OMAP_PIN_INPUT | OMAP_MUX_MODE0,
+		.idle	= OMAP_PIN_INPUT | OMAP_MUX_MODE0,
+	},
+};
+#else
+static struct omap_device_pad default_uart1_pads[] __initdata = {};
+static struct omap_device_pad default_uart2_pads[] __initdata = {};
+static struct omap_device_pad default_uart3_pads[] __initdata = {};
+static struct omap_device_pad default_omap36xx_uart4_pads[] __initdata = {};
+static struct omap_device_pad default_omap4_uart4_pads[] __initdata = {};
+#endif
+
+static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
+{
+	switch (bdata->id) {
+	case 0:
+		bdata->pads = default_uart1_pads;
+		bdata->pads_cnt = ARRAY_SIZE(default_uart1_pads);
+		break;
+	case 1:
+		bdata->pads = default_uart2_pads;
+		bdata->pads_cnt = ARRAY_SIZE(default_uart2_pads);
+		break;
+	case 2:
+		bdata->pads = default_uart3_pads;
+		bdata->pads_cnt = ARRAY_SIZE(default_uart3_pads);
+		break;
+	case 3:
+		if (cpu_is_omap44xx()) {
+			bdata->pads = default_omap4_uart4_pads;
+			bdata->pads_cnt =
+				ARRAY_SIZE(default_omap4_uart4_pads);
+		} else {
+			bdata->pads = default_omap36xx_uart4_pads;
+			bdata->pads_cnt =
+				ARRAY_SIZE(default_omap36xx_uart4_pads);
+		}
+		break;
+	default:
+		break;
+	}
+}
+
 static void omap_uart_idle_init(struct omap_uart_port_info *uart,
 				unsigned short num)
 {
@@ -221,6 +349,10 @@ void __init omap_serial_init(void)
 		bdata.flags = 0;
 		bdata.pads = NULL;
 		bdata.pads_cnt = 0;
+
+		if (cpu_is_omap44xx() || cpu_is_omap34xx())
+			omap_serial_fill_default_pads(&bdata);
+
 		omap_serial_init_port(&bdata);
 	}
 }
-- 
1.7.4.1

[PATCH v4 06/11] Serial: OMAP: Store certain reg values to port structure

[Govindraj.R]

In preparation to runtime conversion add missing uart regs to
port structure which can be used in context restore.
Also ensuring all uart reg info's are part of port structure.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/plat-omap/include/plat/omap-serial.h |    3 ++
 drivers/tty/serial/omap-serial.c              |   33 ++++++++++++++----------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index f79a76f..06e3aa2 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -104,6 +104,9 @@ struct uart_omap_port {
 	unsigned char		mcr;
 	unsigned char		fcr;
 	unsigned char		efr;
+	unsigned char		dll;
+	unsigned char		dlh;
+	unsigned char		mdr1;
 
 	int			use_dma;
 	/*
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 5e713d3..6ac0ade 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -433,8 +433,9 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	if (mctrl & TIOCM_LOOP)
 		mcr |= UART_MCR_LOOP;
 
-	mcr |= up->mcr;
-	serial_out(up, UART_MCR, mcr);
+	up->mcr = serial_in(up, UART_MCR);
+	up->mcr |= mcr;
+	serial_out(up, UART_MCR, up->mcr);
 }
 
 static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -573,8 +574,6 @@ static inline void
 serial_omap_configure_xonxoff
 		(struct uart_omap_port *up, struct ktermios *termios)
 {
-	unsigned char efr = 0;
-
 	up->lcr = serial_in(up, UART_LCR);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 	up->efr = serial_in(up, UART_EFR);
@@ -584,8 +583,7 @@ serial_omap_configure_xonxoff
 	serial_out(up, UART_XOFF1, termios->c_cc[VSTOP]);
 
 	/* clear SW control mode bits */
-	efr = up->efr;
-	efr &= OMAP_UART_SW_CLR;
+	up->efr &= OMAP_UART_SW_CLR;
 
 	/*
 	 * IXON Flag:
@@ -593,7 +591,7 @@ serial_omap_configure_xonxoff
 	 * Transmit XON1, XOFF1
 	 */
 	if (termios->c_iflag & IXON)
-		efr |= OMAP_UART_SW_TX;
+		up->efr |= OMAP_UART_SW_TX;
 
 	/*
 	 * IXOFF Flag:
@@ -601,7 +599,7 @@ serial_omap_configure_xonxoff
 	 * Receiver compares XON1, XOFF1.
 	 */
 	if (termios->c_iflag & IXOFF)
-		efr |= OMAP_UART_SW_RX;
+		up->efr |= OMAP_UART_SW_RX;
 
 	serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
@@ -624,7 +622,7 @@ serial_omap_configure_xonxoff
 	 * load the new software flow control mode IXON or IXOFF
 	 * and restore the UARTi.EFR_REG[4] ENHANCED_EN value.
 	 */
-	serial_out(up, UART_EFR, efr | UART_EFR_SCD);
+	serial_out(up, UART_EFR, up->efr | UART_EFR_SCD);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 
 	serial_out(up, UART_MCR, up->mcr & ~UART_MCR_TCRTLR);
@@ -671,6 +669,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)
@@ -723,6 +725,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 */
 
@@ -759,7 +762,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* 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 +772,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 +783,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 */
 
-- 
1.7.4.1

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Govindraj.R]

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

1.) Moving context_restore func to driver from serial.c
2.) Use runtime irq safe to use get_sync from irq context.
3.) autosuspend for port specific activities and put for reg access.
4.) for earlyprintk usage the debug macro will write to console uart
    so keep uart clocks active from boot, idle using hwmod API's re-enable back
    using runtime API's.
5.) Moving context restore to runtime suspend and using reg values from port
    structure to restore the uart port context based on context_loss_count.
    Maintain internal state machine for avoiding repeated enable/disable of
    uart port wakeup mechanism.
6.) Add API to enable wakeup and check wakeup status.

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/serial.c                  |   49 ++++++
 arch/arm/plat-omap/include/plat/omap-serial.h |   10 ++
 drivers/tty/serial/omap-serial.c              |  211 ++++++++++++++++++++++---
 3 files changed, 250 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 2e10357..7117220 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -30,6 +30,7 @@
 #include <plat/common.h>
 #include <plat/board.h>
 #include <plat/omap_device.h>
+#include <plat/omap-pm.h>
 
 #include "prm2xxx_3xxx.h"
 #include "pm.h"
@@ -246,6 +247,51 @@ static void omap_uart_idle_init(struct omap_uart_port_info *uart,
 	}
 }
 
+static bool omap_uart_chk_wakeup(struct platform_device *pdev)
+{
+	struct omap_uart_port_info *up = pdev->dev.platform_data;
+	struct omap_device *od;
+	u32 wkst = 0;
+	bool ret = false;
+
+	if (up->wk_st && up->wk_en && up->wk_mask) {
+		/* Check for normal UART wakeup (and clear it) */
+		wkst = __raw_readl(up->wk_st) & up->wk_mask;
+		if (wkst) {
+			__raw_writel(up->wk_mask, up->wk_st);
+			ret = true;
+		}
+	}
+
+	od = to_omap_device(pdev);
+	if (omap_hwmod_pad_get_wakeup_status(od->hwmods[0]) == true)
+		ret = true;
+
+	return ret;
+}
+
+static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
+{
+	struct omap_device *od;
+	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;
+		else
+			v &= ~up->wk_mask;
+		__raw_writel(v, up->wk_en);
+	}
+
+	od = to_omap_device(pdev);
+	if (enable)
+		omap_hwmod_enable_wakeup(od->hwmods[0]);
+	else
+		omap_hwmod_disable_wakeup(od->hwmods[0]);
+}
+
 struct omap_hwmod *omap_uart_hwmod_lookup(int num)
 {
 	struct omap_hwmod *oh;
@@ -317,6 +363,9 @@ 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;
+	pdata->chk_wakeup = omap_uart_chk_wakeup;
+	pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count;
 
 	pdev = omap_device_build(name, bdata->id, oh, pdata,
 				sizeof(*pdata),	omap_uart_latency,
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 06e3aa2..7fc63b8 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -67,6 +67,9 @@ struct omap_uart_port_info {
 	void __iomem *wk_st;
 	void __iomem *wk_en;
 	u32 wk_mask;
+	void (*enable_wakeup)(struct platform_device *, bool);
+	bool (*chk_wakeup)(struct platform_device *);
+	u32 (*get_context_loss_count)(struct device *);
 };
 
 struct uart_omap_dma {
@@ -118,7 +121,14 @@ struct uart_omap_port {
 	unsigned char		msr_saved_flags;
 	char			name[20];
 	unsigned long		port_activity;
+	u32			context_loss_cnt;
+	u8			wake_status;
+
 	unsigned int		errata;
+	unsigned int		clocked;
+	u8			console_locked;
+
+	bool (*chk_wakeup)(struct platform_device *);
 };
 
 #endif /* __OMAP_SERIAL_H__ */
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6ac0ade..30bdd05 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -37,10 +37,14 @@
 #include <linux/clk.h>
 #include <linux/serial_core.h>
 #include <linux/irq.h>
+#include <linux/pm_runtime.h>
 
 #include <plat/dma.h>
 #include <plat/dmtimer.h>
 #include <plat/omap-serial.h>
+#include <plat/omap_device.h>
+
+#define OMAP_UART_AUTOSUSPEND_DELAY 3000	/* Value is msecs */
 
 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 
@@ -102,6 +106,8 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)
 		omap_free_dma(up->uart_dma.rx_dma_channel);
 		up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 		up->uart_dma.rx_dma_used = false;
+		pm_runtime_mark_last_busy(&up->pdev->dev);
+		pm_runtime_put_autosuspend(&up->pdev->dev);
 	}
 }
 
@@ -110,8 +116,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);
+
+	pm_runtime_get_sync(&up->pdev->dev);
 	up->ier |= UART_IER_MSI;
 	serial_out(up, UART_IER, up->ier);
+	pm_runtime_put(&up->pdev->dev);
 }
 
 static void serial_omap_stop_tx(struct uart_port *port)
@@ -129,23 +138,32 @@ static void serial_omap_stop_tx(struct uart_port *port)
 		omap_stop_dma(up->uart_dma.tx_dma_channel);
 		omap_free_dma(up->uart_dma.tx_dma_channel);
 		up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
+		pm_runtime_mark_last_busy(&up->pdev->dev);
+		pm_runtime_put_autosuspend(&up->pdev->dev);
 	}
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	if (up->ier & UART_IER_THRI) {
 		up->ier &= ~UART_IER_THRI;
 		serial_out(up, UART_IER, up->ier);
 	}
+
+	pm_runtime_mark_last_busy(&up->pdev->dev);
+	pm_runtime_put_autosuspend(&up->pdev->dev);
 }
 
 static void serial_omap_stop_rx(struct uart_port *port)
 {
 	struct uart_omap_port *up = (struct uart_omap_port *)port;
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	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);
+	pm_runtime_mark_last_busy(&up->pdev->dev);
+	pm_runtime_put_autosuspend(&up->pdev->dev);
 }
 
 static inline void receive_chars(struct uart_omap_port *up, int *status)
@@ -262,7 +280,10 @@ static void serial_omap_start_tx(struct uart_port *port)
 	int ret = 0;
 
 	if (!up->use_dma) {
+		pm_runtime_get_sync(&up->pdev->dev);
 		serial_omap_enable_ier_thri(up);
+		pm_runtime_mark_last_busy(&up->pdev->dev);
+		pm_runtime_put_autosuspend(&up->pdev->dev);
 		return;
 	}
 
@@ -272,6 +293,7 @@ static void serial_omap_start_tx(struct uart_port *port)
 	xmit = &up->port.state->xmit;
 
 	if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
+		pm_runtime_get_sync(&up->pdev->dev);
 		ret = omap_request_dma(up->uart_dma.uart_dma_tx,
 				"UART Tx DMA",
 				(void *)uart_tx_dma_callback, up,
@@ -354,9 +376,12 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 	unsigned int iir, lsr;
 	unsigned long flags;
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	iir = serial_in(up, UART_IIR);
-	if (iir & UART_IIR_NO_INT)
+	if (iir & UART_IIR_NO_INT) {
+		pm_runtime_put(&up->pdev->dev);
 		return IRQ_NONE;
+	}
 
 	spin_lock_irqsave(&up->port.lock, flags);
 	lsr = serial_in(up, UART_LSR);
@@ -378,6 +403,9 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
 		transmit_chars(up);
 
 	spin_unlock_irqrestore(&up->port.lock, flags);
+	pm_runtime_mark_last_busy(&up->pdev->dev);
+	pm_runtime_put_autosuspend(&up->pdev->dev);
+
 	up->port_activity = jiffies;
 	return IRQ_HANDLED;
 }
@@ -388,11 +416,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
 	unsigned long flags = 0;
 	unsigned int ret = 0;
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	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);
-
+	pm_runtime_put(&up->pdev->dev);
 	return ret;
 }
 
@@ -402,7 +431,10 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
 	unsigned char status;
 	unsigned int ret = 0;
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	status = check_modem_status(up);
+	pm_runtime_put(&up->pdev->dev);
+
 	dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
 
 	if (status & UART_MSR_DCD)
@@ -433,9 +465,11 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	if (mctrl & TIOCM_LOOP)
 		mcr |= UART_MCR_LOOP;
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	up->mcr = serial_in(up, UART_MCR);
 	up->mcr |= mcr;
 	serial_out(up, UART_MCR, up->mcr);
+	pm_runtime_put(&up->pdev->dev);
 }
 
 static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -444,6 +478,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);
+	pm_runtime_get_sync(&up->pdev->dev);
 	spin_lock_irqsave(&up->port.lock, flags);
 	if (break_state == -1)
 		up->lcr |= UART_LCR_SBC;
@@ -451,6 +486,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);
+	pm_runtime_put(&up->pdev->dev);
 }
 
 static int serial_omap_startup(struct uart_port *port)
@@ -469,6 +505,7 @@ static int serial_omap_startup(struct uart_port *port)
 
 	dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
 
+	pm_runtime_get_sync(&up->pdev->dev);
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())
@@ -524,6 +561,8 @@ 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);
 
+	pm_runtime_mark_last_busy(&up->pdev->dev);
+	pm_runtime_put_autosuspend(&up->pdev->dev);
 	up->port_activity = jiffies;
 	return 0;
 }
@@ -534,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);
+
+	pm_runtime_get_sync(&up->pdev->dev);
 	/*
 	 * Disable interrupts from this port
 	 */
@@ -567,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;
 	}
+	pm_runtime_put(&up->pdev->dev);
 	free_irq(up->port.irq, up);
 }
 
@@ -682,6 +724,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.
 	 */
+	pm_runtime_get_sync(&up->pdev->dev);
 	spin_lock_irqsave(&up->port.lock, flags);
 
 	/*
@@ -814,6 +857,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);
+	pm_runtime_put(&up->pdev->dev);
 	dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
 }
 
@@ -825,6 +869,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);
+
+	pm_runtime_get_sync(&up->pdev->dev);
 	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);
@@ -834,6 +880,7 @@ 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);
+	pm_runtime_put(&up->pdev->dev);
 }
 
 static void serial_omap_release_port(struct uart_port *port)
@@ -911,25 +958,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;
+
+	pm_runtime_get_sync(&up->pdev->dev);
 	wait_for_xmitr(up);
 	serial_out(up, UART_TX, ch);
+	pm_runtime_put(&up->pdev->dev);
 }
 
 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;
 
+	pm_runtime_get_sync(&up->pdev->dev);
+	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);
+	pm_runtime_put(&up->pdev->dev);
+	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;
@@ -951,6 +1004,8 @@ serial_omap_console_write(struct console *co, const char *s,
 	unsigned int ier;
 	int locked = 1;
 
+	pm_runtime_get_sync(&up->pdev->dev);
+
 	local_irq_save(flags);
 	if (up->port.sysrq)
 		locked = 0;
@@ -983,9 +1038,12 @@ serial_omap_console_write(struct console *co, const char *s,
 	if (up->msr_saved_flags)
 		check_modem_status(up);
 
+	pm_runtime_mark_last_busy(&up->pdev->dev);
+	pm_runtime_put_autosuspend(&up->pdev->dev);
 	if (locked)
 		spin_unlock(&up->port.lock);
 	local_irq_restore(flags);
+	up->port_activity = jiffies;
 }
 
 static int __init
@@ -1065,22 +1123,33 @@ 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);
 
-	if (up)
+	if (up) {
 		uart_suspend_port(&serial_omap_reg, &up->port);
+		if (up->port.line == up->port.cons->index &&
+				!is_console_locked())
+			up->console_locked = console_trylock();
+	}
+
 	return 0;
 }
 
-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);
+		if (up->port.line == up->port.cons->index &&
+					up->console_locked) {
+			console_unlock();
+			up->console_locked = 0;
+		}
+	}
+
 	return 0;
 }
 
@@ -1140,6 +1209,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
 	int ret = 0;
 
 	if (up->uart_dma.rx_dma_channel == -1) {
+		pm_runtime_get_sync(&up->pdev->dev);
 		ret = omap_request_dma(up->uart_dma.uart_dma_rx,
 				"UART Rx DMA",
 				(void *)uart_rx_dma_callback, up,
@@ -1228,9 +1298,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);
@@ -1280,12 +1351,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->chk_wakeup = omap_up_info->chk_wakeup;
 
 	if (omap_up_info->dma_enabled) {
 		up->uart_dma.uart_dma_tx = dma_tx->start;
@@ -1299,18 +1378,34 @@ 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_irq_safe(&pdev->dev);
+	if (device_may_wakeup(&pdev->dev)) {
+		pm_runtime_enable(&pdev->dev);
+		od = to_omap_device(up->pdev);
+		omap_hwmod_idle(od->hwmods[0]);
+		pm_runtime_get_sync(&up->pdev->dev);
+		up->clocked = 1;
+	}
+
 	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;
 
 	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, resource_size(mem));
 	return ret;
@@ -1322,20 +1417,96 @@ 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)
+{
+	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
+	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, UART_LCR_CONF_MODE_B); /* 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, UART_LCR_CONF_MODE_A);
+	serial_out(up, UART_MCR, up->mcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
+	serial_out(up, UART_EFR, up->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);
+	struct omap_uart_port_info *pdata = dev->platform_data;
+
+	if (!up)
+		return -EINVAL;
+
+	if (pdata->get_context_loss_count)
+		up->context_loss_cnt = pdata->get_context_loss_count(dev);
+
+	if (device_may_wakeup(dev)) {
+		if (!up->wake_status) {
+			pdata->enable_wakeup(up->pdev, true);
+			up->wake_status = true;
+		}
+	} else {
+		if (up->wake_status) {
+			pdata->enable_wakeup(up->pdev, false);
+			up->wake_status = false;
+		}
+	}
+
+	return 0;
+}
+
+static int omap_serial_runtime_resume(struct device *dev)
+{
+	struct uart_omap_port *up = dev_get_drvdata(dev);
+	struct omap_uart_port_info *pdata = dev->platform_data;
+
+	if (up) {
+		if (pdata->get_context_loss_count) {
+			u32 loss_cnt = pdata->get_context_loss_count(dev);
+
+			if (up->context_loss_cnt != loss_cnt)
+				omap_uart_restore_context(up);
+		}
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_RUNTIME
+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,
+};
+#define SERIAL_PM_OPS (&omap_serial_dev_pm_ops)
+#else
+#define SERIAL_PM_OPS NULL
+#endif
+
 static struct platform_driver serial_omap_driver = {
 	.probe          = serial_omap_probe,
 	.remove         = serial_omap_remove,
-
-	.suspend	= serial_omap_suspend,
-	.resume		= serial_omap_resume,
 	.driver		= {
 		.name	= DRIVER_NAME,
+		.pm = SERIAL_PM_OPS,
 	},
 };
 
-- 
1.7.4.1

[PATCH v4 08/11] Serial: OMAP2+: Move erratum handling from serial.c

[Govindraj.R]

Move the erratum handling mechanism from serial.c to driver file
and utilise the same func in driver file.

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 drivers/tty/serial/omap-serial.c |   58 ++++++++++++++++++++++++++++++++++---
 1 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 30bdd05..96fc860 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -52,6 +52,7 @@ static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 static void uart_tx_dma_callback(int lch, u16 ch_status, void *data);
 static void serial_omap_rx_timeout(unsigned long uart_no);
 static int serial_omap_start_rxdma(struct uart_omap_port *up);
+static void omap_uart_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
 
 static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
 {
@@ -805,7 +806,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* Protocol, Baud Rate, and Interrupt Settings */
 
-	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+	if (up->errata & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(up, up->mdr1);
+	else
+		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);
@@ -830,7 +835,10 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	else
 		up->mdr1 = UART_OMAP_MDR1_16X_MODE;
 
-	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+	if (up->errata & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(up, up->mdr1);
+	else
+		serial_out(up, UART_OMAP_MDR1, up->mdr1);
 
 	/* Hardware Flow Control Configuration */
 
@@ -1424,9 +1432,47 @@ static int serial_omap_remove(struct platform_device *dev)
 	return 0;
 }
 
+/*
+ * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)
+ * The access to uart register after MDR1 Access
+ * causes UART to corrupt data.
+ *
+ * Need a delay =
+ * 5 L4 clock cycles + 5 UART functional clock cycle (@48MHz = ~0.2uS)
+ * give 10 times as much
+ */
+static void omap_uart_mdr1_errataset(struct uart_omap_port *up, u8 mdr1)
+{
+	u8 timeout = 255;
+
+	serial_out(up, UART_OMAP_MDR1, mdr1);
+	udelay(2);
+	serial_out(up, UART_FCR, up->fcr | UART_FCR_CLEAR_XMIT |
+			UART_FCR_CLEAR_RCVR);
+	/*
+	 * Wait for FIFO to empty: when empty, RX_FIFO_E bit is 0 and
+	 * TX_FIFO_E bit is 1.
+	 */
+	while (UART_LSR_THRE != (serial_in(up, UART_LSR) &
+				(UART_LSR_THRE | UART_LSR_DR))) {
+		timeout--;
+		if (!timeout) {
+			/* Should *never* happen. we warn and carry on */
+			dev_crit(&up->pdev->dev, "Errata i202: timedout %x\n",
+						serial_in(up, UART_LSR));
+			break;
+		}
+		udelay(1);
+	}
+}
+
 static void omap_uart_restore_context(struct uart_omap_port *up)
 {
-	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+	if (up->errata & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(up, UART_OMAP_MDR1_DISABLE);
+	else
+		serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
+
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
 	serial_out(up, UART_EFR, UART_EFR_ECB);
 	serial_out(up, UART_LCR, 0x0); /* Operational mode */
@@ -1442,8 +1488,10 @@ static void omap_uart_restore_context(struct uart_omap_port *up)
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
 	serial_out(up, UART_EFR, up->efr);
 	serial_out(up, UART_LCR, up->lcr);
-	/* UART 16x mode */
-	serial_out(up, UART_OMAP_MDR1, up->mdr1);
+	if (up->errata & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(up, up->mdr1);
+	else
+		serial_out(up, UART_OMAP_MDR1, up->mdr1);
 }
 
 static int omap_serial_runtime_suspend(struct device *dev)
-- 
1.7.4.1

[PATCH v4 09/11] OMAP2+: Serial: Use prepare and resume calls to gate uart clocks

[Govindraj.R]

Currently we are using uart prepare and resume calls to gate uart clocks
retaining the same method.

More details on reason to retain this design is provided here:
http://www.spinics.net/lists/linux-serial/msg04128.html

Since prepare and resume hooks are moved to driver itself we can just use
a single prepare and resume call. As in driver we traverse on number of uart
instance and handle it accordingly.

In 34xx uart can wakeup using module level PM_WKEN or IO PAD wakeup use
resume_call from prcm irq handler to wakeup uart, based on chk_wakeup_status
from io_pad or PM_WKST.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/pm24xx.c             |    8 +----
 arch/arm/mach-omap2/pm34xx.c             |   17 +++++-------
 arch/arm/plat-omap/include/plat/serial.h |    4 +-
 drivers/tty/serial/omap-serial.c         |   40 ++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
index 39f26af..91eacef 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -138,18 +138,14 @@ static void omap2_enter_full_retention(void)
 		if (!console_trylock())
 			goto no_sleep;
 
-	omap_uart_prepare_idle(0);
-	omap_uart_prepare_idle(1);
-	omap_uart_prepare_idle(2);
+	omap_uart_prepare_idle();
 
 	/* Jump to SRAM suspend code */
 	omap2_sram_suspend(sdrc_read_reg(SDRC_DLLA_CTRL),
 			   OMAP_SDRC_REGADDR(SDRC_DLLA_CTRL),
 			   OMAP_SDRC_REGADDR(SDRC_POWER));
 
-	omap_uart_resume_idle(2);
-	omap_uart_resume_idle(1);
-	omap_uart_resume_idle(0);
+	omap_uart_resume_idle();
 
 	if (!is_suspending())
 		console_unlock();
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 9f3bf2c..26ddd56 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -210,6 +210,7 @@ static int prcm_clear_mod_irqs(s16 module, u8 regs)
 
 	wkst = omap2_prm_read_mod_reg(module, wkst_off);
 	wkst &= omap2_prm_read_mod_reg(module, grpsel_off);
+	c += omap_uart_resume_idle();
 	if (wkst) {
 		iclk = omap2_cm_read_mod_reg(module, iclk_off);
 		fclk = omap2_cm_read_mod_reg(module, fclk_off);
@@ -380,17 +381,19 @@ void omap_sram_idle(void)
 	}
 
 	/* Block console output in case it is on one of the OMAP UARTs */
-	if (!is_suspending())
+	if (!is_suspending()) {
 		if (per_next_state < PWRDM_POWER_ON ||
-		    core_next_state < PWRDM_POWER_ON)
+		    core_next_state < PWRDM_POWER_ON) {
 			if (!console_trylock())
 				goto console_still_active;
+			else
+				omap_uart_prepare_idle();
+		}
+	}
 
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
-		omap_uart_prepare_idle(2);
-		omap_uart_prepare_idle(3);
 		omap2_gpio_prepare_for_idle(per_going_off);
 		if (per_next_state == PWRDM_POWER_OFF)
 				omap3_per_save_context();
@@ -398,8 +401,6 @@ void omap_sram_idle(void)
 
 	/* CORE */
 	if (core_next_state < PWRDM_POWER_ON) {
-		omap_uart_prepare_idle(0);
-		omap_uart_prepare_idle(1);
 		if (core_next_state == PWRDM_POWER_OFF) {
 			omap3_core_save_context();
 			omap3_cm_save_context();
@@ -446,8 +447,6 @@ void omap_sram_idle(void)
 			omap3_sram_restore_context();
 			omap2_sms_restore_context();
 		}
-		omap_uart_resume_idle(0);
-		omap_uart_resume_idle(1);
 		if (core_next_state == PWRDM_POWER_OFF)
 			omap2_prm_clear_mod_reg_bits(OMAP3430_AUTO_OFF_MASK,
 					       OMAP3430_GR_MOD,
@@ -461,8 +460,6 @@ void omap_sram_idle(void)
 		omap2_gpio_resume_after_idle();
 		if (per_prev_state == PWRDM_POWER_OFF)
 			omap3_per_restore_context();
-		omap_uart_resume_idle(2);
-		omap_uart_resume_idle(3);
 	}
 
 	if (!is_suspending())
diff --git a/arch/arm/plat-omap/include/plat/serial.h b/arch/arm/plat-omap/include/plat/serial.h
index bfd73b7..5b8c8f2 100644
--- a/arch/arm/plat-omap/include/plat/serial.h
+++ b/arch/arm/plat-omap/include/plat/serial.h
@@ -109,8 +109,8 @@ struct omap_board_data;
 
 extern void omap_serial_init(void);
 extern void omap_serial_init_port(struct omap_board_data *bdata);
-extern void omap_uart_prepare_idle(int num);
-extern void omap_uart_resume_idle(int num);
+extern void omap_uart_prepare_idle(void);
+extern int omap_uart_resume_idle(void);
 #endif
 
 #endif
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 96fc860..05c2f52 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -54,6 +54,46 @@ static void serial_omap_rx_timeout(unsigned long uart_no);
 static int serial_omap_start_rxdma(struct uart_omap_port *up);
 static void omap_uart_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
 
+int omap_uart_resume_idle()
+{
+	struct uart_omap_port *up;
+	int i, ret = 0;
+
+	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
+		up = ui[i];
+		if (!up)
+			continue;
+
+		if (!up->clocked && up->chk_wakeup(up->pdev)) {
+			pm_runtime_get_sync(&up->pdev->dev);
+			up->clocked = 1;
+			ret++;
+		}
+	}
+
+	return ret;
+}
+
+void omap_uart_prepare_idle()
+{
+	struct uart_omap_port *up;
+	int i;
+
+	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
+		up = ui[i];
+		if (!up)
+			continue;
+
+		if (up->clocked &&
+				jiffies_to_msecs(jiffies - up->port_activity)
+					> 3000) {
+			pm_runtime_mark_last_busy(&up->pdev->dev);
+			pm_runtime_put_autosuspend(&up->pdev->dev);
+			up->clocked = 0;
+		}
+	}
+}
+
 static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
 {
 	offset <<= up->port.regshift;
-- 
1.7.4.1

[PATCH v4 10/11] OMAP: Serial: Allow UART parameters to be configured from board file.

[Govindraj.R]

The following UART parameters are defined within the UART driver:

1). Whether the UART uses DMA (dma_enabled), by default set to 0
2). The size of dma buffer (set to 4096 bytes)
3). The time after which the dma should stop if no more data is received.
4). The auto suspend delay that will be passed for pm_runtime_autosuspend
    where uart will be disabled after timeout

Different UARTs may be used for different purpose such as the console,
for interfacing bluetooth chip, for interfacing to a modem chip, etc.
Therefore, it is necessary to be able to customize the above settings
for a given board on a per UART basis.

This change allows these parameters to be configured from the board file
and allows the parameters to be configured for each UART independently.

If a board does not define its own custom parameters for the UARTs, then
use the default parameters in the structure "omap_serial_default_info".
The default parameters are defined to be the same as the current settings
in the UART driver to avoid breaking the UART for any board. By default,
make all boards use the default UART parameters.

Signed-off-by: Deepak K <deepak.k@ti.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/board-4430sdp.c           |    8 ++--
 arch/arm/mach-omap2/board-n8x0.c              |    6 ++--
 arch/arm/mach-omap2/board-omap4panda.c        |    8 ++--
 arch/arm/mach-omap2/serial.c                  |   46 ++++++++++++++++++++++--
 arch/arm/plat-omap/include/plat/omap-serial.h |    7 +++-
 arch/arm/plat-omap/include/plat/serial.h      |    5 ++-
 drivers/tty/serial/omap-serial.c              |    8 ++---
 7 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index c7cef44..41b43cb 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -743,11 +743,11 @@ static inline void board_serial_init(void)
 	bdata.pads_cnt	= 0;
 	bdata.id	= 0;
 	/* pass dummy data for UART1 */
-	omap_serial_init_port(&bdata);
+	omap_serial_init_port(&bdata, NULL);
 
-	omap_serial_init_port(&serial2_data);
-	omap_serial_init_port(&serial3_data);
-	omap_serial_init_port(&serial4_data);
+	omap_serial_init_port(&serial2_data, NULL);
+	omap_serial_init_port(&serial3_data, NULL);
+	omap_serial_init_port(&serial4_data, NULL);
 }
 #else
 #define board_mux	NULL
diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index e11f0c5..3408726 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -656,15 +656,15 @@ static inline void board_serial_init(void)
 	bdata.pads_cnt = 0;
 
 	bdata.id = 0;
-	omap_serial_init_port(&bdata);
+	omap_serial_init_port(&bdata, NULL);
 
 	bdata.id = 1;
-	omap_serial_init_port(&bdata);
+	omap_serial_init_port(&bdata, NULL);
 
 	bdata.id = 2;
 	bdata.pads = serial2_pads;
 	bdata.pads_cnt = ARRAY_SIZE(serial2_pads);
-	omap_serial_init_port(&bdata);
+	omap_serial_init_port(&bdata, NULL);
 }
 
 #else
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index 9aaa960..fdfac0c 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -425,11 +425,11 @@ static inline void board_serial_init(void)
 	bdata.pads_cnt  = 0;
 	bdata.id        = 0;
 	/* pass dummy data for UART1 */
-	omap_serial_init_port(&bdata);
+	omap_serial_init_port(&bdata, NULL);
 
-	omap_serial_init_port(&serial2_data);
-	omap_serial_init_port(&serial3_data);
-	omap_serial_init_port(&serial4_data);
+	omap_serial_init_port(&serial2_data, NULL);
+	omap_serial_init_port(&serial3_data, NULL);
+	omap_serial_init_port(&serial4_data, NULL);
 }
 #else
 #define board_mux	NULL
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 7117220..dd4d872 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -41,6 +41,19 @@
 
 #define MAX_UART_HWMOD_NAME_LEN		16
 
+#define DEFAULT_RXDMA_TIMEOUT		1	/* RX DMA polling rate (us) */
+#define DEFAULT_RXDMA_BUFSIZE		4096	/* RX DMA buffer size */
+#define DEFAULT_AUTOSUSPEND_DELAY	3000	/* Runtime autosuspend (msecs)*/
+
+static struct omap_uart_port_info omap_serial_default_info[] = {
+	{
+		.dma_enabled	= 0,
+		.dma_rx_buf_size = DEFAULT_RXDMA_BUFSIZE,
+		.dma_rx_timeout = DEFAULT_RXDMA_TIMEOUT,
+		.auto_sus_timeout = DEFAULT_AUTOSUSPEND_DELAY,
+	},
+};
+
 static int uart_idle_hwmod(struct omap_device *od)
 {
 	omap_hwmod_idle(od->hwmods[0]);
@@ -323,6 +336,7 @@ core_initcall(omap_serial_early_init);
 /**
  * omap_serial_init_port() - initialize single serial port
  * @bdata: port specific board data pointer
+ * @info: platform specific data pointer
  *
  * This function initialies serial driver for given port only.
  * Platforms can call this function instead of omap_serial_init()
@@ -331,7 +345,8 @@ core_initcall(omap_serial_early_init);
  * Don't mix calls to omap_serial_init_port() and omap_serial_init(),
  * use only one of the two.
  */
-void __init omap_serial_init_port(struct omap_board_data *bdata)
+void __init omap_serial_init_port(struct omap_board_data *bdata,
+			struct omap_uart_port_info *info)
 {
 	struct omap_hwmod *oh;
 	struct platform_device *pdev;
@@ -349,6 +364,9 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 	if (!oh)
 		return;
 
+	if (info == NULL)
+		info = omap_serial_default_info;
+
 	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
 	if (!pdata) {
 		pr_err("Memory allocation for UART pdata failed\n");
@@ -366,6 +384,10 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 	pdata->enable_wakeup = omap_uart_wakeup_enable;
 	pdata->chk_wakeup = omap_uart_chk_wakeup;
 	pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count;
+	pdata->dma_enabled = info->dma_enabled;
+	pdata->dma_rx_buf_size = info->dma_rx_buf_size;
+	pdata->dma_rx_timeout = info->dma_rx_timeout;
+	pdata->auto_sus_timeout = info->auto_sus_timeout;
 
 	pdev = omap_device_build(name, bdata->id, oh, pdata,
 				sizeof(*pdata),	omap_uart_latency,
@@ -382,13 +404,14 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 }
 
 /**
- * omap_serial_init() - initialize all supported serial ports
+ * omap_serial_board_init() - initialize all supported serial ports
+ * @info: platform specific data pointer
  *
  * Initializes all available UARTs as serial ports. Platforms
  * can call this function when they want to have default behaviour
  * for serial ports (e.g initialize them all as serial ports).
  */
-void __init omap_serial_init(void)
+void __init omap_serial_board_init(struct omap_uart_port_info *info)
 {
 	struct omap_board_data bdata;
 	u8 i;
@@ -402,6 +425,21 @@ void __init omap_serial_init(void)
 		if (cpu_is_omap44xx() || cpu_is_omap34xx())
 			omap_serial_fill_default_pads(&bdata);
 
-		omap_serial_init_port(&bdata);
+		if (info == NULL)
+			omap_serial_init_port(&bdata, NULL);
+		else
+			omap_serial_init_port(&bdata, &info[i]);
 	}
 }
+
+/**
+ * omap_serial_init() - initialize all supported serial ports
+ *
+ * Initializes all available UARTs.
+ * Platforms can call this function when they want to have default behaviour
+ * for serial ports (e.g initialize them all as serial ports).
+ */
+void __init omap_serial_init(void)
+{
+	omap_serial_board_init(NULL);
+}
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 7fc63b8..211cb0a 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -63,6 +63,9 @@ struct omap_uart_port_info {
 	unsigned int		uartclk;	/* UART clock rate */
 	upf_t			flags;		/* UPF_* flags */
 	unsigned int		errata;
+	unsigned int		dma_rx_buf_size;/* DMA Rx Buffer Size */
+	unsigned int		dma_rx_timeout;	/* DMA RX timeout */
+	unsigned int		auto_sus_timeout; /* Auto_suspend timeout */
 
 	void __iomem *wk_st;
 	void __iomem *wk_en;
@@ -93,8 +96,8 @@ struct uart_omap_dma {
 	spinlock_t		rx_lock;
 	/* timer to poll activity on rx dma */
 	struct timer_list	rx_timer;
-	int			rx_buf_size;
-	int			rx_timeout;
+	unsigned int		rx_buf_size;
+	unsigned int		rx_timeout;
 };
 
 struct uart_omap_port {
diff --git a/arch/arm/plat-omap/include/plat/serial.h b/arch/arm/plat-omap/include/plat/serial.h
index 5b8c8f2..9c3e9e3 100644
--- a/arch/arm/plat-omap/include/plat/serial.h
+++ b/arch/arm/plat-omap/include/plat/serial.h
@@ -106,11 +106,14 @@
 #ifndef __ASSEMBLER__
 
 struct omap_board_data;
+struct omap_uart_port_info;
 
 extern void omap_serial_init(void);
-extern void omap_serial_init_port(struct omap_board_data *bdata);
 extern void omap_uart_prepare_idle(void);
 extern int omap_uart_resume_idle(void);
+extern void omap_serial_board_init(struct omap_uart_port_info *platform_data);
+extern void omap_serial_init_port(struct omap_board_data *bdata,
+		struct omap_uart_port_info *platform_data);
 #endif
 
 #endif
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 05c2f52..7e13036 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -44,8 +44,6 @@
 #include <plat/omap-serial.h>
 #include <plat/omap_device.h>
 
-#define OMAP_UART_AUTOSUSPEND_DELAY 3000	/* Value is msecs */
-
 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 
 /* Forward declaration of functions */
@@ -1418,8 +1416,8 @@ static int serial_omap_probe(struct platform_device *pdev)
 		up->uart_dma.uart_dma_tx = dma_tx->start;
 		up->uart_dma.uart_dma_rx = dma_rx->start;
 		up->use_dma = 1;
-		up->uart_dma.rx_buf_size = 4096;
-		up->uart_dma.rx_timeout = 2;
+		up->uart_dma.rx_buf_size = omap_up_info->dma_rx_buf_size;
+		up->uart_dma.rx_timeout = omap_up_info->dma_rx_timeout;
 		spin_lock_init(&(up->uart_dma.tx_lock));
 		spin_lock_init(&(up->uart_dma.rx_lock));
 		up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
@@ -1428,7 +1426,7 @@ static int serial_omap_probe(struct platform_device *pdev)
 
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev,
-			OMAP_UART_AUTOSUSPEND_DELAY);
+			omap_up_info->auto_sus_timeout);
 
 	pm_runtime_irq_safe(&pdev->dev);
 	if (device_may_wakeup(&pdev->dev)) {
-- 
1.7.4.1

[PATCH v4 11/11] Serial: OMAP2+: Make the RX_TIMEOUT for DMA configurable for each UART

[Govindraj.R]

From: Jon Hunter <jon-hunter@ti.com>

When using DMA there are two timeouts defined. The first timeout,
rx_timeout, is really a polling rate in which software polls the
DMA status to see if the DMA has finished. This is necessary for
the RX side because we do not know how much data we will receive.
The second timeout, RX_TIMEOUT, is a timeout after which the
DMA will be stopped if no more data is received. To make this
clearer, rename rx_timeout as rx_poll_rate and rename the
function serial_omap_rx_timeout() to serial_omap_rxdma_poll().

The OMAP-Serial driver defines an RX_TIMEOUT of 3 seconds that is
used to indicate when the DMA for UART can be stopped if no more
data is received. The value is a global definition that is applied
to all instances of the UART.

Each UART may be used for a different purpose and so the timeout
required may differ. Make this value configurable for each UART so
that this value can be optimised for power savings.

Acked-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/serial.c                  |    5 ++++-
 arch/arm/plat-omap/include/plat/omap-serial.h |    3 ++-
 drivers/tty/serial/omap-serial.c              |   15 ++++++++-------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index dd4d872..942438c 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -41,14 +41,16 @@
 
 #define MAX_UART_HWMOD_NAME_LEN		16
 
-#define DEFAULT_RXDMA_TIMEOUT		1	/* RX DMA polling rate (us) */
+#define DEFAULT_RXDMA_POLLRATE		1	/* RX DMA polling rate (us) */
 #define DEFAULT_RXDMA_BUFSIZE		4096	/* RX DMA buffer size */
 #define DEFAULT_AUTOSUSPEND_DELAY	3000	/* Runtime autosuspend (msecs)*/
+#define DEFAULT_RXDMA_TIMEOUT		(3 * HZ)/* RX DMA timeout (jiffies) */
 
 static struct omap_uart_port_info omap_serial_default_info[] = {
 	{
 		.dma_enabled	= 0,
 		.dma_rx_buf_size = DEFAULT_RXDMA_BUFSIZE,
+		.dma_rx_poll_rate = DEFAULT_RXDMA_POLLRATE,
 		.dma_rx_timeout = DEFAULT_RXDMA_TIMEOUT,
 		.auto_sus_timeout = DEFAULT_AUTOSUSPEND_DELAY,
 	},
@@ -386,6 +388,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count;
 	pdata->dma_enabled = info->dma_enabled;
 	pdata->dma_rx_buf_size = info->dma_rx_buf_size;
+	pdata->dma_rx_poll_rate = info->dma_rx_poll_rate;
 	pdata->dma_rx_timeout = info->dma_rx_timeout;
 	pdata->auto_sus_timeout = info->auto_sus_timeout;
 
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 211cb0a..0d4423d 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -51,7 +51,6 @@
 
 #define OMAP_UART_DMA_CH_FREE	-1
 
-#define RX_TIMEOUT		(3 * HZ)
 #define OMAP_MAX_HSUART_PORTS	4
 
 #define MSR_SAVE_FLAGS		UART_MSR_ANY_DELTA
@@ -65,6 +64,7 @@ struct omap_uart_port_info {
 	unsigned int		errata;
 	unsigned int		dma_rx_buf_size;/* DMA Rx Buffer Size */
 	unsigned int		dma_rx_timeout;	/* DMA RX timeout */
+	unsigned int		dma_rx_poll_rate; /* DMA RX timeout */
 	unsigned int		auto_sus_timeout; /* Auto_suspend timeout */
 
 	void __iomem *wk_st;
@@ -97,6 +97,7 @@ struct uart_omap_dma {
 	/* timer to poll activity on rx dma */
 	struct timer_list	rx_timer;
 	unsigned int		rx_buf_size;
+	unsigned int		rx_poll_rate;
 	unsigned int		rx_timeout;
 };
 
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 7e13036..cd79733 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -48,7 +48,7 @@ static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 
 /* Forward declaration of functions */
 static void uart_tx_dma_callback(int lch, u16 ch_status, void *data);
-static void serial_omap_rx_timeout(unsigned long uart_no);
+static void serial_omap_rxdma_poll(unsigned long uart_no);
 static int serial_omap_start_rxdma(struct uart_omap_port *up);
 static void omap_uart_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
 
@@ -582,7 +582,7 @@ static int serial_omap_startup(struct uart_port *port)
 			(dma_addr_t *)&(up->uart_dma.tx_buf_dma_phys),
 			0);
 		init_timer(&(up->uart_dma.rx_timer));
-		up->uart_dma.rx_timer.function = serial_omap_rx_timeout;
+		up->uart_dma.rx_timer.function = serial_omap_rxdma_poll;
 		up->uart_dma.rx_timer.data = up->pdev->id;
 		/* Currently the buffer size is 4KB. Can increase it */
 		up->uart_dma.rx_buf = dma_alloc_coherent(NULL,
@@ -1199,7 +1199,7 @@ static int serial_omap_resume(struct device *dev)
 	return 0;
 }
 
-static void serial_omap_rx_timeout(unsigned long uart_no)
+static void serial_omap_rxdma_poll(unsigned long uart_no)
 {
 	struct uart_omap_port *up = ui[uart_no];
 	unsigned int curr_dma_pos, curr_transmitted_size;
@@ -1209,9 +1209,9 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 	if ((curr_dma_pos == up->uart_dma.prev_rx_dma_pos) ||
 			     (curr_dma_pos == 0)) {
 		if (jiffies_to_msecs(jiffies - up->port_activity) <
-							RX_TIMEOUT) {
+						up->uart_dma.rx_timeout) {
 			mod_timer(&up->uart_dma.rx_timer, jiffies +
-				usecs_to_jiffies(up->uart_dma.rx_timeout));
+				usecs_to_jiffies(up->uart_dma.rx_poll_rate));
 		} else {
 			serial_omap_stop_rxdma(up);
 			up->ier |= (UART_IER_RDI | UART_IER_RLSI);
@@ -1240,7 +1240,7 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
 		}
 	} else  {
 		mod_timer(&up->uart_dma.rx_timer, jiffies +
-			usecs_to_jiffies(up->uart_dma.rx_timeout));
+			usecs_to_jiffies(up->uart_dma.rx_poll_rate));
 	}
 	up->port_activity = jiffies;
 }
@@ -1279,7 +1279,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
 	/* FIXME: Cache maintenance needed here? */
 	omap_start_dma(up->uart_dma.rx_dma_channel);
 	mod_timer(&up->uart_dma.rx_timer, jiffies +
-				usecs_to_jiffies(up->uart_dma.rx_timeout));
+				usecs_to_jiffies(up->uart_dma.rx_poll_rate));
 	up->uart_dma.rx_dma_used = true;
 	return ret;
 }
@@ -1418,6 +1418,7 @@ static int serial_omap_probe(struct platform_device *pdev)
 		up->use_dma = 1;
 		up->uart_dma.rx_buf_size = omap_up_info->dma_rx_buf_size;
 		up->uart_dma.rx_timeout = omap_up_info->dma_rx_timeout;
+		up->uart_dma.rx_poll_rate = omap_up_info->dma_rx_poll_rate;
 		spin_lock_init(&(up->uart_dma.tx_lock));
 		spin_lock_init(&(up->uart_dma.rx_lock));
 		up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
-- 
1.7.4.1

[PATCH v4 04/11] OMAP2+: UART: Remove uart clock handling code from serial.c

[Kevin Hilman]

"Govindraj.R" <govindraj.raja@ti.com> writes:

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

I don't see that as part of this patch.

> 4.) Erratum handling informed as flag to driver and func to handle erratum
>     can be moved to omap-serial driver itself.

OK, but I see two erratum flags removed, but only flag added back.
Also, the erratum handling is completely removed here and not added to
the driver.

In general, this way of moving things makes it very difficult to review.
You remove a bunch of things in this patch (and the previous one) and
imply that some of it is added back to the driver.  However, that's
really difficult to verify with patch review.

If code is being moved, it is customary to remove it and add it to the
new place in the same patch.

> Acked-by: Alan Cox <alan@linux.intel.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  arch/arm/mach-omap2/serial.c                  |  742 ++-----------------------
>  arch/arm/plat-omap/include/plat/omap-serial.h |   11 +-
>  2 files changed, 53 insertions(+), 700 deletions(-)

[...]

> -static DEVICE_ATTR(sleep_timeout, 0644, sleep_timeout_show,
> -		sleep_timeout_store);
> -#define DEV_CREATE_FILE(dev, attr) WARN_ON(device_create_file(dev, attr))
> -#else
> -static inline void omap_uart_idle_init(struct omap_uart_state *uart) {}
> -static void omap_uart_block_sleep(struct omap_uart_state *uart)
> +struct omap_hwmod *omap_uart_hwmod_lookup(int num)

function should be static

[...]

> +	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
> +		oh = omap_uart_hwmod_lookup(i);
>  		if (!oh)
> -			break;
> -
> -		uart = kzalloc(sizeof(struct omap_uart_state), GFP_KERNEL);
> -		if (WARN_ON(!uart))
> -			return -ENODEV;
> -
> -		uart->oh = oh;
> -		uart->num = i++;
> -		list_add_tail(&uart->node, &uart_list);
> -		num_uarts++;
> -
> -		/*
> -		 * NOTE: omap_hwmod_setup*() has not yet been called,
> -		 *       so no hwmod functions will work yet.
> -		 */
> -
> -		/*
> -		 * During UART early init, device need to be probed
> -		 * to determine SoC specific init before omap_device
> -		 * is ready.  Therefore, don't allow idle here
> -		 */
> -		uart->oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
> -	} while (1);
> +			continue;
>  
> +		oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;

With proper runtime PM in the driver, I did not expect these to be
needed any longer by the end of the series, but I see they are still
there.  Are they really needed?

[...]

> @@ -864,15 +213,14 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
>   */
>  void __init omap_serial_init(void)
>  {
> -	struct omap_uart_state *uart;
>  	struct omap_board_data bdata;
> +	u8 i;
>  
> -	list_for_each_entry(uart, &uart_list, node) {
> -		bdata.id = uart->num;
> +	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {

This should probably use the hwmod class iterator.

Kevin

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Kevin Hilman]

"Govindraj.R" <govindraj.raja@ti.com> writes:

> Adapts omap-serial driver to use pm_runtime API's.
>
> 1.) Moving context_restore func to driver from serial.c
> 2.) Use runtime irq safe to use get_sync from irq context.
> 3.) autosuspend for port specific activities and put for reg access.

Re: autosuspend, it looks like the driver now has 2 mechanisms for
tracking activity.  The runtime PM 'mark last busy' and the existing
'up->port_activity = jiffies' stuff.  Do you think those could be
unified?

At first glance, it looks like most places with a _mark_last_busy() also
have a up->port_activity update.  I'm not familiar enough with the
driver to make the call, but they look awfully similar.

> 4.) for earlyprintk usage the debug macro will write to console uart
>     so keep uart clocks active from boot, idle using hwmod API's re-enable back
>     using runtime API's.

/me doesn't understand

> 5.) Moving context restore to runtime suspend and using reg values from port
>     structure to restore the uart port context based on context_loss_count.
>     Maintain internal state machine for avoiding repeated enable/disable of
>     uart port wakeup mechanism.
> 6.) Add API to enable wakeup and check wakeup status.
>
> Acked-by: Alan Cox <alan@linux.intel.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  arch/arm/mach-omap2/serial.c                  |   49 ++++++
>  arch/arm/plat-omap/include/plat/omap-serial.h |   10 ++
>  drivers/tty/serial/omap-serial.c              |  211 ++++++++++++++++++++++---
>  3 files changed, 250 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 2e10357..7117220 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -30,6 +30,7 @@
>  #include <plat/common.h>
>  #include <plat/board.h>
>  #include <plat/omap_device.h>
> +#include <plat/omap-pm.h>
>  
>  #include "prm2xxx_3xxx.h"
>  #include "pm.h"
> @@ -246,6 +247,51 @@ static void omap_uart_idle_init(struct omap_uart_port_info *uart,
>  	}
>  }
>  
> +static bool omap_uart_chk_wakeup(struct platform_device *pdev)
> +{
> +	struct omap_uart_port_info *up = pdev->dev.platform_data;
> +	struct omap_device *od;
> +	u32 wkst = 0;
> +	bool ret = false;
> +
> +	if (up->wk_st && up->wk_en && up->wk_mask) {
> +		/* Check for normal UART wakeup (and clear it) */
> +		wkst = __raw_readl(up->wk_st) & up->wk_mask;
> +		if (wkst) {
> +			__raw_writel(up->wk_mask, up->wk_st);
> +			ret = true;
> +		}
> +	}
> +
> +	od = to_omap_device(pdev);
> +	if (omap_hwmod_pad_get_wakeup_status(od->hwmods[0]) == true)
> +		ret = true;
> +
> +	return ret;
> +}
> +
> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
> +{
> +	struct omap_device *od;
> +	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;
> +		else
> +			v &= ~up->wk_mask;
> +		__raw_writel(v, up->wk_en);
> +	}
> +
> +	od = to_omap_device(pdev);
> +	if (enable)
> +		omap_hwmod_enable_wakeup(od->hwmods[0]);
> +	else
> +		omap_hwmod_disable_wakeup(od->hwmods[0]);
> +}
> +

Here again, this is difficult to review because you removed the code in
[4/11] and add it back here, but with rather different functionality
with no description as to why.

The previous version enabled wakeups at the PRCM and at the IO ring.
This version enables wakeups at the PRCM as well but instead of enabling
them at the IO ring, enables them at the module.

>  struct omap_hwmod *omap_uart_hwmod_lookup(int num)
>  {
>  	struct omap_hwmod *oh;
> @@ -317,6 +363,9 @@ 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;
> +	pdata->chk_wakeup = omap_uart_chk_wakeup;
> +	pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count;
>  
>  	pdev = omap_device_build(name, bdata->id, oh, pdata,
>  				sizeof(*pdata),	omap_uart_latency,
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 06e3aa2..7fc63b8 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -67,6 +67,9 @@ struct omap_uart_port_info {
>  	void __iomem *wk_st;
>  	void __iomem *wk_en;
>  	u32 wk_mask;
> +	void (*enable_wakeup)(struct platform_device *, bool);
> +	bool (*chk_wakeup)(struct platform_device *);
> +	u32 (*get_context_loss_count)(struct device *);
>  };
>  
>  struct uart_omap_dma {
> @@ -118,7 +121,14 @@ struct uart_omap_port {
>  	unsigned char		msr_saved_flags;
>  	char			name[20];
>  	unsigned long		port_activity;
> +	u32			context_loss_cnt;
> +	u8			wake_status;
> +
>  	unsigned int		errata;
> +	unsigned int		clocked;

This is a legacy from serial.c and should not be needed.  Checking
pm_runtime_suspended() will provide the same info.

> +	u8			console_locked;
> +
> +	bool (*chk_wakeup)(struct platform_device *);

s/chk/check/ please.  It's not that much longer.

>  };
>  
>  #endif /* __OMAP_SERIAL_H__ */
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 6ac0ade..30bdd05 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -37,10 +37,14 @@
>  #include <linux/clk.h>
>  #include <linux/serial_core.h>
>  #include <linux/irq.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <plat/dma.h>
>  #include <plat/dmtimer.h>
>  #include <plat/omap-serial.h>
> +#include <plat/omap_device.h>
> +
> +#define OMAP_UART_AUTOSUSPEND_DELAY 3000	/* Value is msecs */

Because of the character lost problem when UARTs are idled, Tony prefers
that the default on this be no auto timeout, like the current mainline
code does.

>  static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
>  
> @@ -102,6 +106,8 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)
>  		omap_free_dma(up->uart_dma.rx_dma_channel);
>  		up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
>  		up->uart_dma.rx_dma_used = false;
> +		pm_runtime_mark_last_busy(&up->pdev->dev);
> +		pm_runtime_put_autosuspend(&up->pdev->dev);
>  	}
>  }
>  
> @@ -110,8 +116,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);
> +
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	up->ier |= UART_IER_MSI;
>  	serial_out(up, UART_IER, up->ier);
> +	pm_runtime_put(&up->pdev->dev);
>  }
>  
>  static void serial_omap_stop_tx(struct uart_port *port)
> @@ -129,23 +138,32 @@ static void serial_omap_stop_tx(struct uart_port *port)
>  		omap_stop_dma(up->uart_dma.tx_dma_channel);
>  		omap_free_dma(up->uart_dma.tx_dma_channel);
>  		up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
> +		pm_runtime_mark_last_busy(&up->pdev->dev);
> +		pm_runtime_put_autosuspend(&up->pdev->dev);
>  	}
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	if (up->ier & UART_IER_THRI) {
>  		up->ier &= ~UART_IER_THRI;
>  		serial_out(up, UART_IER, up->ier);
>  	}
> +
> +	pm_runtime_mark_last_busy(&up->pdev->dev);
> +	pm_runtime_put_autosuspend(&up->pdev->dev);
>  }
>  
>  static void serial_omap_stop_rx(struct uart_port *port)
>  {
>  	struct uart_omap_port *up = (struct uart_omap_port *)port;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	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);
> +	pm_runtime_mark_last_busy(&up->pdev->dev);
> +	pm_runtime_put_autosuspend(&up->pdev->dev);
>  }
>  
>  static inline void receive_chars(struct uart_omap_port *up, int *status)
> @@ -262,7 +280,10 @@ static void serial_omap_start_tx(struct uart_port *port)
>  	int ret = 0;
>  
>  	if (!up->use_dma) {
> +		pm_runtime_get_sync(&up->pdev->dev);
>  		serial_omap_enable_ier_thri(up);
> +		pm_runtime_mark_last_busy(&up->pdev->dev);
> +		pm_runtime_put_autosuspend(&up->pdev->dev);
>  		return;
>  	}
>  
> @@ -272,6 +293,7 @@ static void serial_omap_start_tx(struct uart_port *port)
>  	xmit = &up->port.state->xmit;
>  
>  	if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
> +		pm_runtime_get_sync(&up->pdev->dev);
>  		ret = omap_request_dma(up->uart_dma.uart_dma_tx,
>  				"UART Tx DMA",
>  				(void *)uart_tx_dma_callback, up,
> @@ -354,9 +376,12 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
>  	unsigned int iir, lsr;
>  	unsigned long flags;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	iir = serial_in(up, UART_IIR);
> -	if (iir & UART_IIR_NO_INT)
> +	if (iir & UART_IIR_NO_INT) {
> +		pm_runtime_put(&up->pdev->dev);
>  		return IRQ_NONE;
> +	}
>  
>  	spin_lock_irqsave(&up->port.lock, flags);
>  	lsr = serial_in(up, UART_LSR);
> @@ -378,6 +403,9 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
>  		transmit_chars(up);
>  
>  	spin_unlock_irqrestore(&up->port.lock, flags);
> +	pm_runtime_mark_last_busy(&up->pdev->dev);
> +	pm_runtime_put_autosuspend(&up->pdev->dev);
> +
>  	up->port_activity = jiffies;
>  	return IRQ_HANDLED;
>  }
> @@ -388,11 +416,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
>  	unsigned long flags = 0;
>  	unsigned int ret = 0;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	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);
> -
> +	pm_runtime_put(&up->pdev->dev);
>  	return ret;
>  }
>  
> @@ -402,7 +431,10 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
>  	unsigned char status;
>  	unsigned int ret = 0;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	status = check_modem_status(up);
> +	pm_runtime_put(&up->pdev->dev);
> +
>  	dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
>  
>  	if (status & UART_MSR_DCD)
> @@ -433,9 +465,11 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  	if (mctrl & TIOCM_LOOP)
>  		mcr |= UART_MCR_LOOP;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	up->mcr = serial_in(up, UART_MCR);
>  	up->mcr |= mcr;
>  	serial_out(up, UART_MCR, up->mcr);
> +	pm_runtime_put(&up->pdev->dev);
>  }
>  
>  static void serial_omap_break_ctl(struct uart_port *port, int break_state)
> @@ -444,6 +478,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);
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	spin_lock_irqsave(&up->port.lock, flags);
>  	if (break_state == -1)
>  		up->lcr |= UART_LCR_SBC;
> @@ -451,6 +486,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);
> +	pm_runtime_put(&up->pdev->dev);
>  }
>  
>  static int serial_omap_startup(struct uart_port *port)
> @@ -469,6 +505,7 @@ static int serial_omap_startup(struct uart_port *port)
>  
>  	dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	/*
>  	 * Clear the FIFO buffers and disable them.
>  	 * (they will be reenabled in set_termios())
> @@ -524,6 +561,8 @@ 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);
>  
> +	pm_runtime_mark_last_busy(&up->pdev->dev);
> +	pm_runtime_put_autosuspend(&up->pdev->dev);
>  	up->port_activity = jiffies;
>  	return 0;
>  }
> @@ -534,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);
> +
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	/*
>  	 * Disable interrupts from this port
>  	 */
> @@ -567,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;
>  	}
> +	pm_runtime_put(&up->pdev->dev);
>  	free_irq(up->port.irq, up);
>  }
>  
> @@ -682,6 +724,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.
>  	 */
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	spin_lock_irqsave(&up->port.lock, flags);
>  
>  	/*
> @@ -814,6 +857,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);
> +	pm_runtime_put(&up->pdev->dev);
>  	dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
>  }
>  
> @@ -825,6 +869,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);
> +
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	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);
> @@ -834,6 +880,7 @@ 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);
> +	pm_runtime_put(&up->pdev->dev);
>  }
>  
>  static void serial_omap_release_port(struct uart_port *port)
> @@ -911,25 +958,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;
> +
> +	pm_runtime_get_sync(&up->pdev->dev);
>  	wait_for_xmitr(up);
>  	serial_out(up, UART_TX, ch);
> +	pm_runtime_put(&up->pdev->dev);
>  }
>  
>  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;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
> +	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);
> +	pm_runtime_put(&up->pdev->dev);
> +	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;
> @@ -951,6 +1004,8 @@ serial_omap_console_write(struct console *co, const char *s,
>  	unsigned int ier;
>  	int locked = 1;
>  
> +	pm_runtime_get_sync(&up->pdev->dev);
> +
>  	local_irq_save(flags);
>  	if (up->port.sysrq)
>  		locked = 0;
> @@ -983,9 +1038,12 @@ serial_omap_console_write(struct console *co, const char *s,
>  	if (up->msr_saved_flags)
>  		check_modem_status(up);
>  
> +	pm_runtime_mark_last_busy(&up->pdev->dev);
> +	pm_runtime_put_autosuspend(&up->pdev->dev);
>  	if (locked)
>  		spin_unlock(&up->port.lock);
>  	local_irq_restore(flags);
> +	up->port_activity = jiffies;

hmm, why?  this change looks suspicious.

>  }
>  
>  static int __init
> @@ -1065,22 +1123,33 @@ 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);
>  
> -	if (up)
> +	if (up) {
>  		uart_suspend_port(&serial_omap_reg, &up->port);
> +		if (up->port.line == up->port.cons->index &&
> +				!is_console_locked())
> +			up->console_locked = console_trylock();
> +	}
> +

Motiviation for console locking in this version of the series is not
clear, and not described in the changelog.

It's also present here in static suspend/resume but not in runtime
suspend/resume, which also is suspicious.

>  	return 0;
>  }
>  
> -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);
> +		if (up->port.line == up->port.cons->index &&
> +					up->console_locked) {
> +			console_unlock();
> +			up->console_locked = 0;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1140,6 +1209,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
>  	int ret = 0;
>  
>  	if (up->uart_dma.rx_dma_channel == -1) {
> +		pm_runtime_get_sync(&up->pdev->dev);
>  		ret = omap_request_dma(up->uart_dma.uart_dma_rx,
>  				"UART Rx DMA",
>  				(void *)uart_rx_dma_callback, up,
> @@ -1228,9 +1298,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);
> @@ -1280,12 +1351,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);

The size is (end - start + 1), but please use the resource_size() helper
for this to avoid this common problem.

> +	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->chk_wakeup = omap_up_info->chk_wakeup;
>  
>  	if (omap_up_info->dma_enabled) {
>  		up->uart_dma.uart_dma_tx = dma_tx->start;
> @@ -1299,18 +1378,34 @@ 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_irq_safe(&pdev->dev);
> +	if (device_may_wakeup(&pdev->dev)) {
> +		pm_runtime_enable(&pdev->dev);
> +		od = to_omap_device(up->pdev);
> +		omap_hwmod_idle(od->hwmods[0]);

No hwmod calls in drivers please.

In this case, this is a legacy of using HWMOD_INIT_NO_IDLE and
_NO_RESET.  That should be removed so this could be removed.

> +		pm_runtime_get_sync(&up->pdev->dev);
> +		up->clocked = 1;
> +	}
> +
>  	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;
>  
>  	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, resource_size(mem));
>  	return ret;
> @@ -1322,20 +1417,96 @@ 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)
> +{
> +	serial_out(up, UART_OMAP_MDR1, up->mdr1);
> +	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
> +	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, UART_LCR_CONF_MODE_B); /* 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, UART_LCR_CONF_MODE_A);
> +	serial_out(up, UART_MCR, up->mcr);
> +	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
> +	serial_out(up, UART_EFR, up->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);
> +	struct omap_uart_port_info *pdata = dev->platform_data;
> +
> +	if (!up)
> +		return -EINVAL;
> +
> +	if (pdata->get_context_loss_count)
> +		up->context_loss_cnt = pdata->get_context_loss_count(dev);

You need

	if (!pdata->enable_wakeup)
		return 0;

here in case there is no ->enable_wakeup provided.  Otherwise...

> +	if (device_may_wakeup(dev)) {
> +		if (!up->wake_status) {
> +			pdata->enable_wakeup(up->pdev, true);

...boom.

> +			up->wake_status = true;
> +		}
> +	} else {
> +		if (up->wake_status) {
> +			pdata->enable_wakeup(up->pdev, false);
> +			up->wake_status = false;
> +		}
> +	}

up->wake_status would be better named ->wakeups_enabled;

> +	return 0;
> +}
> +
> +static int omap_serial_runtime_resume(struct device *dev)
> +{
> +	struct uart_omap_port *up = dev_get_drvdata(dev);
> +	struct omap_uart_port_info *pdata = dev->platform_data;
> +
> +	if (up) {
> +		if (pdata->get_context_loss_count) {
> +			u32 loss_cnt = pdata->get_context_loss_count(dev);
> +
> +			if (up->context_loss_cnt != loss_cnt)
> +				omap_uart_restore_context(up);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_RUNTIME
> +static const struct dev_pm_ops omap_serial_dev_pm_ops = {
> +	.suspend = serial_omap_suspend,
> +	.resume	= serial_omap_resume,

Static suspend operations should exists even when runtime PM is
disabled.

> +	.runtime_suspend = omap_serial_runtime_suspend,
> +	.runtime_resume = omap_serial_runtime_resume,

Note that some functions have serial_omap_ prefix and others have
omap_serial_.  It looks like serial_omap_ is used througout the driver.
Please unify.

> +};
> +#define SERIAL_PM_OPS (&omap_serial_dev_pm_ops)
> +#else
> +#define SERIAL_PM_OPS NULL
> +#endif

Instead of this ifdef construct, please use SET_SYSTEM_SLEEP_PM_OPS()
and SET_RUNTIME_PM_OPS() which take care of the right ifdefs for you,
and also ensure that callbacks are available for suspend-to-disk (hibernate):

static const struct dev_pm_ops omap_serial_dev_pm_ops = {
       SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume),
       SET_RUNTIME_PM_OPS(runtime_suspend, runtime_resume, NULL),
};

>  static struct platform_driver serial_omap_driver = {
>  	.probe          = serial_omap_probe,
>  	.remove         = serial_omap_remove,
> -
> -	.suspend	= serial_omap_suspend,
> -	.resume		= serial_omap_resume,
>  	.driver		= {
>  		.name	= DRIVER_NAME,
> +		.pm = SERIAL_PM_OPS,
>  	},
>  };

Kevin

[PATCH v4 04/11] OMAP2+: UART: Remove uart clock handling code from serial.c

[Govindraj]

Hi Kevin,

Thanks for the review.

On Fri, Sep 9, 2011 at 5:09 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> 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.
>
> I don't see that as part of this patch.

Yes I have to remove that line.

>
>> 4.) Erratum handling informed as flag to driver and func to handle erratum
>> ? ? can be moved to omap-serial driver itself.
>
> OK, but I see two erratum flags removed, but only flag added back.
> Also, the erratum handling is completely removed here and not added to
> the driver.
>

errata 2.15, yes correct force idle mode if in dma mode
need to incorporated into runtime suspend call back.

> In general, this way of moving things makes it very difficult to review.
> You remove a bunch of things in this patch (and the previous one) and
> imply that some of it is added back to the driver. ?However, that's
> really difficult to verify with patch review.
>

the things I moved from here to driver :

1.) prepare and resume calls, Yes I can incorporate 9/11 into this patch.

2.) mdr errata movement - yes can be part of this change.

3.) context restore is part of runtime resume call back so I think
that should be part of runtime patches itself. Also force idle mode errata 2.15
missed out also should be part of runtime suspend hook.
so cant be part of this change here.


> If code is being moved, it is customary to remove it and add it to the
> new place in the same patch.
>

Ok.

>> Acked-by: Alan Cox <alan@linux.intel.com>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>> ---
>> ?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ?742 ++-----------------------
>> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? 11 +-
>> ?2 files changed, 53 insertions(+), 700 deletions(-)
>
> [...]
>
>> -static DEVICE_ATTR(sleep_timeout, 0644, sleep_timeout_show,
>> - ? ? ? ? ? ? sleep_timeout_store);
>> -#define DEV_CREATE_FILE(dev, attr) WARN_ON(device_create_file(dev, attr))
>> -#else
>> -static inline void omap_uart_idle_init(struct omap_uart_state *uart) {}
>> -static void omap_uart_block_sleep(struct omap_uart_state *uart)
>> +struct omap_hwmod *omap_uart_hwmod_lookup(int num)
>
> function should be static
>

OK.

> [...]
>
>> + ? ? for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
>> + ? ? ? ? ? ? oh = omap_uart_hwmod_lookup(i);
>> ? ? ? ? ? ? ? if (!oh)
>> - ? ? ? ? ? ? ? ? ? ? break;
>> -
>> - ? ? ? ? ? ? uart = kzalloc(sizeof(struct omap_uart_state), GFP_KERNEL);
>> - ? ? ? ? ? ? if (WARN_ON(!uart))
>> - ? ? ? ? ? ? ? ? ? ? return -ENODEV;
>> -
>> - ? ? ? ? ? ? uart->oh = oh;
>> - ? ? ? ? ? ? uart->num = i++;
>> - ? ? ? ? ? ? list_add_tail(&uart->node, &uart_list);
>> - ? ? ? ? ? ? num_uarts++;
>> -
>> - ? ? ? ? ? ? /*
>> - ? ? ? ? ? ? ?* NOTE: omap_hwmod_setup*() has not yet been called,
>> - ? ? ? ? ? ? ?* ? ? ? so no hwmod functions will work yet.
>> - ? ? ? ? ? ? ?*/
>> -
>> - ? ? ? ? ? ? /*
>> - ? ? ? ? ? ? ?* During UART early init, device need to be probed
>> - ? ? ? ? ? ? ?* to determine SoC specific init before omap_device
>> - ? ? ? ? ? ? ?* is ready. ?Therefore, don't allow idle here
>> - ? ? ? ? ? ? ?*/
>> - ? ? ? ? ? ? uart->oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
>> - ? ? } while (1);
>> + ? ? ? ? ? ? ? ? ? ? continue;
>>
>> + ? ? ? ? ? ? oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
>
> With proper runtime PM in the driver, I did not expect these to be
> needed any longer by the end of the series, but I see they are still
> there. ?Are they really needed?
>

Actually when we use early_printk option from CONFIG_DEBUG_LL
we use the debug-macro.s file to do put-char of early kernel prints until
a console driver is available, So while debug-macros does putchar from uart
if we try to reset console uart during hwmod_init this will cause uart
to go to bad state
and still no console driver available and can halt uart during bootup.

reset from hwmod -> putchar from earlyprintk driver doing tx on uart ??

Since softreset from hwmod can loose uart configuration from bootloader
and earlyprintk driver is unaware of this and does tx on uart.

Hence we still need this code to avoid uart reset & idle during bootup.

> [...]
>
>> @@ -864,15 +213,14 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
>> ? */
>> ?void __init omap_serial_init(void)
>> ?{
>> - ? ? struct omap_uart_state *uart;
>> ? ? ? struct omap_board_data bdata;
>> + ? ? u8 i;
>>
>> - ? ? list_for_each_entry(uart, &uart_list, node) {
>> - ? ? ? ? ? ? bdata.id = uart->num;
>> + ? ? for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
>
> This should probably use the hwmod class iterator.

yes, will check that option.

--
Thanks,
Govindraj.R


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

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Govindraj]

On Fri, Sep 9, 2011 at 5:54 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> Adapts omap-serial driver to use pm_runtime API's.
>>
>> 1.) Moving context_restore func to driver from serial.c
>> 2.) Use runtime irq safe to use get_sync from irq context.
>> 3.) autosuspend for port specific activities and put for reg access.
>
> Re: autosuspend, it looks like the driver now has 2 mechanisms for
> tracking activity. ?The runtime PM 'mark last busy' and the existing
> 'up->port_activity = jiffies' stuff. ?Do you think those could be
> unified?
>

Is there a way where we can retrieve the last busy jiffie value
using runtime API? I don't find that available.


> At first glance, it looks like most places with a _mark_last_busy() also
> have a up->port_activity update. ?I'm not familiar enough with the
> driver to make the call, but they look awfully similar.
>

Ok, I will check whether I can get rid if it.

>> 4.) for earlyprintk usage the debug macro will write to console uart
>> ? ? so keep uart clocks active from boot, idle using hwmod API's re-enable back
>> ? ? using runtime API's.
>
> /me doesn't understand

I have added this reason in early mail reply to 04/11 patch review
[needed for earlyprintk option from low level debug ]


>
>> 5.) Moving context restore to runtime suspend and using reg values from port
>> ? ? structure to restore the uart port context based on context_loss_count.
>> ? ? Maintain internal state machine for avoiding repeated enable/disable of
>> ? ? uart port wakeup mechanism.
>> 6.) Add API to enable wakeup and check wakeup status.
>>
>> Acked-by: Alan Cox <alan@linux.intel.com>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>> ---
>> ?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 49 ++++++
>> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? 10 ++
>> ?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?211 ++++++++++++++++++++++---
>> ?3 files changed, 250 insertions(+), 20 deletions(-)
>>

[..]

>> +
>> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
>> +{
>> + ? ? struct omap_device *od;
>> + ? ? 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;
>> + ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
>> + ? ? ? ? ? ? __raw_writel(v, up->wk_en);
>> + ? ? }
>> +
>> + ? ? od = to_omap_device(pdev);
>> + ? ? if (enable)
>> + ? ? ? ? ? ? omap_hwmod_enable_wakeup(od->hwmods[0]);
>> + ? ? else
>> + ? ? ? ? ? ? omap_hwmod_disable_wakeup(od->hwmods[0]);
>> +}
>> +
>
> Here again, this is difficult to review because you removed the code in
> [4/11] and add it back here, but with rather different functionality
> with no description as to why.
>

will move this change as part of 4/11 itself.

> The previous version enabled wakeups at the PRCM and at the IO ring.
> This version enables wakeups at the PRCM as well but instead of enabling
> them at the IO ring, enables them at the module.
>

Since we are gating using prepare idle call I think
we can use this enable wake-up call as part of prepare idle call itself,
as done earlier.

>> ?struct omap_hwmod *omap_uart_hwmod_lookup(int num)
>> ?{
>> ? ? ? struct omap_hwmod *oh;
>> @@ -317,6 +363,9 @@ 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;
>> + ? ? pdata->chk_wakeup = omap_uart_chk_wakeup;
>> + ? ? pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count;
>>
>> ? ? ? pdev = omap_device_build(name, bdata->id, oh, pdata,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(*pdata), omap_uart_latency,
>> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
>> index 06e3aa2..7fc63b8 100644
>> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
>> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
>> @@ -67,6 +67,9 @@ struct omap_uart_port_info {
>> ? ? ? void __iomem *wk_st;
>> ? ? ? void __iomem *wk_en;
>> ? ? ? u32 wk_mask;
>> + ? ? void (*enable_wakeup)(struct platform_device *, bool);
>> + ? ? bool (*chk_wakeup)(struct platform_device *);
>> + ? ? u32 (*get_context_loss_count)(struct device *);
>> ?};
>>
>> ?struct uart_omap_dma {
>> @@ -118,7 +121,14 @@ struct uart_omap_port {
>> ? ? ? unsigned char ? ? ? ? ? msr_saved_flags;
>> ? ? ? char ? ? ? ? ? ? ? ? ? ?name[20];
>> ? ? ? unsigned long ? ? ? ? ? port_activity;
>> + ? ? u32 ? ? ? ? ? ? ? ? ? ? context_loss_cnt;
>> + ? ? u8 ? ? ? ? ? ? ? ? ? ? ?wake_status;
>> +
>> ? ? ? unsigned int ? ? ? ? ? ?errata;
>> + ? ? unsigned int ? ? ? ? ? ?clocked;
>
> This is a legacy from serial.c and should not be needed. ?Checking
> pm_runtime_suspended() will provide the same info.
>

Yes will try to use that.

>> + ? ? u8 ? ? ? ? ? ? ? ? ? ? ?console_locked;
>> +
>> + ? ? bool (*chk_wakeup)(struct platform_device *);
>
> s/chk/check/ please. ?It's not that much longer.
>

will change.

>> ?};
>>
>> ?#endif /* __OMAP_SERIAL_H__ */
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 6ac0ade..30bdd05 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -37,10 +37,14 @@
>> ?#include <linux/clk.h>
>> ?#include <linux/serial_core.h>
>> ?#include <linux/irq.h>
>> +#include <linux/pm_runtime.h>
>>
>> ?#include <plat/dma.h>
>> ?#include <plat/dmtimer.h>
>> ?#include <plat/omap-serial.h>
>> +#include <plat/omap_device.h>
>> +
>> +#define OMAP_UART_AUTOSUSPEND_DELAY 3000 ? ? /* Value is msecs */
>
> Because of the character lost problem when UARTs are idled, Tony prefers
> that the default on this be no auto timeout, like the current mainline
> code does.
>

Yes fine, IIUC this value should be -1 by default and can be later set using
sysfs entry.


>> ?static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
>>
>> @@ -102,6 +106,8 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)
>> ? ? ? ? ? ? ? omap_free_dma(up->uart_dma.rx_dma_channel);
>> ? ? ? ? ? ? ? up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
>> ? ? ? ? ? ? ? up->uart_dma.rx_dma_used = false;
>> + ? ? ? ? ? ? pm_runtime_mark_last_busy(&up->pdev->dev);
>> + ? ? ? ? ? ? pm_runtime_put_autosuspend(&up->pdev->dev);
>> ? ? ? }
>> ?}
>>
>> @@ -110,8 +116,11 @@ static void serial_omap_enable_ms(struct uart_port *port)

[..]

>>
>> ?#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;
>> @@ -951,6 +1004,8 @@ serial_omap_console_write(struct console *co, const char *s,
>> ? ? ? unsigned int ier;
>> ? ? ? int locked = 1;
>>
>> + ? ? pm_runtime_get_sync(&up->pdev->dev);
>> +
>> ? ? ? local_irq_save(flags);
>> ? ? ? if (up->port.sysrq)
>> ? ? ? ? ? ? ? locked = 0;
>> @@ -983,9 +1038,12 @@ serial_omap_console_write(struct console *co, const char *s,
>> ? ? ? if (up->msr_saved_flags)
>> ? ? ? ? ? ? ? check_modem_status(up);
>>
>> + ? ? pm_runtime_mark_last_busy(&up->pdev->dev);
>> + ? ? pm_runtime_put_autosuspend(&up->pdev->dev);
>> ? ? ? if (locked)
>> ? ? ? ? ? ? ? spin_unlock(&up->port.lock);
>> ? ? ? local_irq_restore(flags);
>> + ? ? up->port_activity = jiffies;
>
> hmm, why? ?this change looks suspicious.
>

Yes will try to unify with mark last busy.
But as said earlier can we have option to retrieve
last busy jiffies value.


>> ?}
>>
>> ?static int __init
>> @@ -1065,22 +1123,33 @@ 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);
>>
>> - ? ? if (up)
>> + ? ? if (up) {
>> ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
>> + ? ? ? ? ? ? if (up->port.line == up->port.cons->index &&
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? !is_console_locked())
>> + ? ? ? ? ? ? ? ? ? ? up->console_locked = console_trylock();
>> + ? ? }
>> +
>
> Motiviation for console locking in this version of the series is not
> clear, and not described in the changelog.
>
> It's also present here in static suspend/resume but not in runtime
> suspend/resume, which also is suspicious.
>

Yes will add to change log,

We needed this for no console suspend use case
no console lock is taken in no_console_suspend is specified,

Since our pwr_dmn hooks disable clocks left enabled during suspend,
so any prints after pwr_dmn hooks can cause problems since clocks
are already cut from pwr_dmn hooks.

So buffer prints while entering suspend by taking console lock
if not taken already and print back after uart state machine restores
console uart.


>> ? ? ? return 0;
>> ?}
>>
>> -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) {

[..]

>> + ? ? up->port.mapbase = mem->start;
>> + ? ? up->port.membase = ioremap(mem->start, mem->end - mem->start);
>
> The size is (end - start + 1), but please use the resource_size() helper
> for this to avoid this common problem.
>

will change that.

>> + ? ? 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->chk_wakeup = omap_up_info->chk_wakeup;
>>
>> ? ? ? if (omap_up_info->dma_enabled) {
>> ? ? ? ? ? ? ? up->uart_dma.uart_dma_tx = dma_tx->start;
>> @@ -1299,18 +1378,34 @@ 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_irq_safe(&pdev->dev);
>> + ? ? if (device_may_wakeup(&pdev->dev)) {
>> + ? ? ? ? ? ? pm_runtime_enable(&pdev->dev);
>> + ? ? ? ? ? ? od = to_omap_device(up->pdev);
>> + ? ? ? ? ? ? omap_hwmod_idle(od->hwmods[0]);
>
> No hwmod calls in drivers please.
>
> In this case, this is a legacy of using HWMOD_INIT_NO_IDLE and
> _NO_RESET. ?That should be removed so this could be removed.
>

low level debug early printk use case as said earlier.


>> + ? ? ? ? ? ? pm_runtime_get_sync(&up->pdev->dev);
>> + ? ? ? ? ? ? up->clocked = 1;
>> + ? ? }
>> +
>> ? ? ? ui[pdev->id] = up;
>> ? ? ? serial_omap_add_console_port(up);
>>

[..]

>> + ? ? if (pdata->get_context_loss_count)
>> + ? ? ? ? ? ? up->context_loss_cnt = pdata->get_context_loss_count(dev);
>
> You need
>
> ? ? ? ?if (!pdata->enable_wakeup)
> ? ? ? ? ? ? ? ?return 0;
>
> here in case there is no ->enable_wakeup provided. ?Otherwise...
>
>> + ? ? if (device_may_wakeup(dev)) {
>> + ? ? ? ? ? ? if (!up->wake_status) {
>> + ? ? ? ? ? ? ? ? ? ? pdata->enable_wakeup(up->pdev, true);
>
> ...boom.
>

will correct it.

>> + ? ? ? ? ? ? ? ? ? ? up->wake_status = true;
>> + ? ? ? ? ? ? }
>> + ? ? } else {
>> + ? ? ? ? ? ? if (up->wake_status) {
>> + ? ? ? ? ? ? ? ? ? ? pdata->enable_wakeup(up->pdev, false);
>> + ? ? ? ? ? ? ? ? ? ? up->wake_status = false;
>> + ? ? ? ? ? ? }
>> + ? ? }
>
> up->wake_status would be better named ->wakeups_enabled;

sure.

>
>> + ? ? return 0;
>> +}
>> +
>> +static int omap_serial_runtime_resume(struct device *dev)
>> +{
>> + ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
>> + ? ? struct omap_uart_port_info *pdata = dev->platform_data;
>> +
>> + ? ? if (up) {
>> + ? ? ? ? ? ? if (pdata->get_context_loss_count) {
>> + ? ? ? ? ? ? ? ? ? ? u32 loss_cnt = pdata->get_context_loss_count(dev);
>> +
>> + ? ? ? ? ? ? ? ? ? ? if (up->context_loss_cnt != loss_cnt)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? omap_uart_restore_context(up);
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_RUNTIME
>> +static const struct dev_pm_ops omap_serial_dev_pm_ops = {
>> + ? ? .suspend = serial_omap_suspend,
>> + ? ? .resume = serial_omap_resume,
>
> Static suspend operations should exists even when runtime PM is
> disabled.
>
>> + ? ? .runtime_suspend = omap_serial_runtime_suspend,
>> + ? ? .runtime_resume = omap_serial_runtime_resume,
>
> Note that some functions have serial_omap_ prefix and others have
> omap_serial_. ?It looks like serial_omap_ is used througout the driver.
> Please unify.
>

yes sure. will change.

>> +};
>> +#define SERIAL_PM_OPS (&omap_serial_dev_pm_ops)
>> +#else
>> +#define SERIAL_PM_OPS NULL
>> +#endif
>
> Instead of this ifdef construct, please use SET_SYSTEM_SLEEP_PM_OPS()
> and SET_RUNTIME_PM_OPS() which take care of the right ifdefs for you,
> and also ensure that callbacks are available for suspend-to-disk (hibernate):
>
> static const struct dev_pm_ops omap_serial_dev_pm_ops = {
> ? ? ? SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume),
> ? ? ? SET_RUNTIME_PM_OPS(runtime_suspend, runtime_resume, NULL),
> };
>

yes will incorporate this.

Will give a quick respin and try to repost asap.

--
Thanks,
Govindraj.R


>> ?static struct platform_driver serial_omap_driver = {
>> ? ? ? .probe ? ? ? ? ?= serial_omap_probe,
>> ? ? ? .remove ? ? ? ? = serial_omap_remove,
>> -
>> - ? ? .suspend ? ? ? ?= serial_omap_suspend,
>> - ? ? .resume ? ? ? ? = serial_omap_resume,
>> ? ? ? .driver ? ? ? ? = {
>> ? ? ? ? ? ? ? .name ? = DRIVER_NAME,
>> + ? ? ? ? ? ? .pm = SERIAL_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
>

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Kevin Hilman]

Govindraj <govindraj.ti@gmail.com> writes:

> On Fri, Sep 9, 2011 at 5:54 AM, Kevin Hilman <khilman@ti.com> wrote:
>> "Govindraj.R" <govindraj.raja@ti.com> writes:
>>
>>> Adapts omap-serial driver to use pm_runtime API's.
>>>
>>> 1.) Moving context_restore func to driver from serial.c
>>> 2.) Use runtime irq safe to use get_sync from irq context.
>>> 3.) autosuspend for port specific activities and put for reg access.
>>
>> Re: autosuspend, it looks like the driver now has 2 mechanisms for
>> tracking activity. ?The runtime PM 'mark last busy' and the existing
>> 'up->port_activity = jiffies' stuff. ?Do you think those could be
>> unified?
>>
>
> Is there a way where we can retrieve the last busy jiffie value
> using runtime API? I don't find that available.
>

Not currently.  The question is more along the lines of: what is the
port_activity jiffies used for, and can it be replaced by using
_mark_last_busy().

If it cannot, that's fine, but it should be described.

>> At first glance, it looks like most places with a _mark_last_busy() also
>> have a up->port_activity update. ?I'm not familiar enough with the
>> driver to make the call, but they look awfully similar.
>>
>
> Ok, I will check whether I can get rid if it.
>
>>> 4.) for earlyprintk usage the debug macro will write to console uart
>>> ? ? so keep uart clocks active from boot, idle using hwmod API's re-enable back
>>> ? ? using runtime API's.
>>
>> /me doesn't understand
>
> I have added this reason in early mail reply to 04/11 patch review
> [needed for earlyprintk option from low level debug ]

OK, but AFAIK, DEBUG_LL + earlyprintk do not use this driver.  Instead
they use the debug macros, so any hacks to deal with that do not belong
in the driver.

If there are hacks required, they should be in a separate patch, with a
separate descriptive changelog.

>>
>>> 5.) Moving context restore to runtime suspend and using reg values from port
>>> ? ? structure to restore the uart port context based on context_loss_count.
>>> ? ? Maintain internal state machine for avoiding repeated enable/disable of
>>> ? ? uart port wakeup mechanism.
>>> 6.) Add API to enable wakeup and check wakeup status.
>>>
>>> Acked-by: Alan Cox <alan@linux.intel.com>
>>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>>> ---
>>> ?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 49 ++++++
>>> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? 10 ++
>>> ?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?211 ++++++++++++++++++++++---
>>> ?3 files changed, 250 insertions(+), 20 deletions(-)
>>>
>
> [..]
>
>>> +
>>> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
>>> +{
>>> + ? ? struct omap_device *od;
>>> + ? ? 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;
>>> + ? ? ? ? ? ? else
>>> + ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
>>> + ? ? ? ? ? ? __raw_writel(v, up->wk_en);
>>> + ? ? }
>>> +
>>> + ? ? od = to_omap_device(pdev);
>>> + ? ? if (enable)
>>> + ? ? ? ? ? ? omap_hwmod_enable_wakeup(od->hwmods[0]);
>>> + ? ? else
>>> + ? ? ? ? ? ? omap_hwmod_disable_wakeup(od->hwmods[0]);
>>> +}
>>> +
>>
>> Here again, this is difficult to review because you removed the code in
>> [4/11] and add it back here, but with rather different functionality
>> with no description as to why.
>>
>
> will move this change as part of 4/11 itself.
>

The point was not just that the move was confusing, but that you changed
the functionality along with the move without any description.

>> The previous version enabled wakeups at the PRCM and at the IO ring.
>> This version enables wakeups at the PRCM as well but instead of enabling
>> them at the IO ring, enables them at the module.
>>
>
> Since we are gating using prepare idle call I think
> we can use this enable wake-up call as part of prepare idle call itself,
> as done earlier.

Not sure what that has to with my comment.

In moving this code, you removed the enable/disable of IO ring wakeups.
It probably continues to work because they're enabled by the bootloader,
but that is not correct and the driver should be managing the IO ring
wakeups as before.

[...]

>>> ?}
>>>
>>> ?static int __init
>>> @@ -1065,22 +1123,33 @@ 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);
>>>
>>> - ? ? if (up)
>>> + ? ? if (up) {
>>> ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
>>> + ? ? ? ? ? ? if (up->port.line == up->port.cons->index &&
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? !is_console_locked())
>>> + ? ? ? ? ? ? ? ? ? ? up->console_locked = console_trylock();
>>> + ? ? }
>>> +
>>
>> Motiviation for console locking in this version of the series is not
>> clear, and not described in the changelog.
>>
>> It's also present here in static suspend/resume but not in runtime
>> suspend/resume, which also is suspicious.
>>
>
> Yes will add to change log,
>
> We needed this for no console suspend use case
> no console lock is taken in no_console_suspend is specified,
>
> Since our pwr_dmn hooks disable clocks left enabled during suspend,
> so any prints after pwr_dmn hooks can cause problems since clocks
> are already cut from pwr_dmn hooks.
>
> So buffer prints while entering suspend by taking console lock
> if not taken already and print back after uart state machine restores
> console uart.

Any special consideration for features like no_console_suspend should be
done in a separate patch with a separate changelog.

However, as I've stated before during previous reviews, if someone has
put no_console_suspend on the command line, that means they've
specifically stated they *want* to see console writes during suspend.
That means, we should not cut clocks at all.

Of course, that means the system will not hit the low power state becase
the UART clocks are not gated, but that is what the user requested.


Kevin

[PATCH v4 09/11] OMAP2+: Serial: Use prepare and resume calls to gate uart clocks

[Kevin Hilman]

"Govindraj.R" <govindraj.raja@ti.com> writes:

> Currently we are using uart prepare and resume calls to gate uart clocks
> retaining the same method.
>
> More details on reason to retain this design is provided here:
> http://www.spinics.net/lists/linux-serial/msg04128.html

This type of thing can go after the '---' since it doesn't belong in the
permanent git history.

> Since prepare and resume hooks are moved to driver itself we can just use
> a single prepare and resume call. As in driver we traverse on number of uart
> instance and handle it accordingly.

Some important functionality has been removed (and not documented.)
Namely, the current code checks the next power when deciding whether or
not to call prepare_idle, and vice versa for resume_idle.

It's quite possible that it is no longer needed with the runtime PM 

> In 34xx uart can wakeup using module level PM_WKEN or IO PAD wakeup use
> resume_call from prcm irq handler to wakeup uart, based on chk_wakeup_status
> from io_pad or PM_WKST.

IMO, this patch should just replace the mulitple calls with a single
call, but still in the idle path.

Using the PRCM IRQ handler should be a separate patch with it's own
changelog.

> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>

Also, moving these calls to the driver means that the driver cannot be
built as a module:

arch/arm/mach-omap2/built-in.o: In function `omap2_enter_full_retention':
/work/kernel/omap/pm/arch/arm/mach-omap2/pm24xx.c:141: undefined reference to `omap_uart_prepare_idle'
/work/kernel/omap/pm/arch/arm/mach-omap2/pm24xx.c:148: undefined reference to `omap_uart_resume_idle'
arch/arm/mach-omap2/built-in.o: In function `prcm_clear_mod_irqs':
/work/kernel/omap/pm/arch/arm/mach-omap2/pm34xx.c:213: undefined reference to `omap_uart_resume_idle'
arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle':
/work/kernel/omap/pm/arch/arm/mach-omap2/pm34xx.c:390: undefined reference to `omap_uart_prepare_idle'

> ---
>  arch/arm/mach-omap2/pm24xx.c             |    8 +----
>  arch/arm/mach-omap2/pm34xx.c             |   17 +++++-------
>  arch/arm/plat-omap/include/plat/serial.h |    4 +-
>  drivers/tty/serial/omap-serial.c         |   40 ++++++++++++++++++++++++++++++
>  4 files changed, 51 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
> index 39f26af..91eacef 100644
> --- a/arch/arm/mach-omap2/pm24xx.c
> +++ b/arch/arm/mach-omap2/pm24xx.c
> @@ -138,18 +138,14 @@ static void omap2_enter_full_retention(void)
>  		if (!console_trylock())
>  			goto no_sleep;
>  
> -	omap_uart_prepare_idle(0);
> -	omap_uart_prepare_idle(1);
> -	omap_uart_prepare_idle(2);
> +	omap_uart_prepare_idle();
>  
>  	/* Jump to SRAM suspend code */
>  	omap2_sram_suspend(sdrc_read_reg(SDRC_DLLA_CTRL),
>  			   OMAP_SDRC_REGADDR(SDRC_DLLA_CTRL),
>  			   OMAP_SDRC_REGADDR(SDRC_POWER));
>  
> -	omap_uart_resume_idle(2);
> -	omap_uart_resume_idle(1);
> -	omap_uart_resume_idle(0);
> +	omap_uart_resume_idle();
>  
>  	if (!is_suspending())
>  		console_unlock();
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 9f3bf2c..26ddd56 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -210,6 +210,7 @@ static int prcm_clear_mod_irqs(s16 module, u8 regs)
>  
>  	wkst = omap2_prm_read_mod_reg(module, wkst_off);
>  	wkst &= omap2_prm_read_mod_reg(module, grpsel_off);
> +	c += omap_uart_resume_idle();
>  	if (wkst) {
>  		iclk = omap2_cm_read_mod_reg(module, iclk_off);
>  		fclk = omap2_cm_read_mod_reg(module, fclk_off);
> @@ -380,17 +381,19 @@ void omap_sram_idle(void)
>  	}
>  
>  	/* Block console output in case it is on one of the OMAP UARTs */
> -	if (!is_suspending())
> +	if (!is_suspending()) {
>  		if (per_next_state < PWRDM_POWER_ON ||
> -		    core_next_state < PWRDM_POWER_ON)
> +		    core_next_state < PWRDM_POWER_ON) {
>  			if (!console_trylock())
>  				goto console_still_active;
> +			else
> +				omap_uart_prepare_idle();
> +		}
> +	}
>  
>  	/* PER */
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
> -		omap_uart_prepare_idle(2);
> -		omap_uart_prepare_idle(3);
>  		omap2_gpio_prepare_for_idle(per_going_off);
>  		if (per_next_state == PWRDM_POWER_OFF)
>  				omap3_per_save_context();
> @@ -398,8 +401,6 @@ void omap_sram_idle(void)
>  
>  	/* CORE */
>  	if (core_next_state < PWRDM_POWER_ON) {
> -		omap_uart_prepare_idle(0);
> -		omap_uart_prepare_idle(1);
>  		if (core_next_state == PWRDM_POWER_OFF) {
>  			omap3_core_save_context();
>  			omap3_cm_save_context();
> @@ -446,8 +447,6 @@ void omap_sram_idle(void)
>  			omap3_sram_restore_context();
>  			omap2_sms_restore_context();
>  		}
> -		omap_uart_resume_idle(0);
> -		omap_uart_resume_idle(1);
>  		if (core_next_state == PWRDM_POWER_OFF)
>  			omap2_prm_clear_mod_reg_bits(OMAP3430_AUTO_OFF_MASK,
>  					       OMAP3430_GR_MOD,
> @@ -461,8 +460,6 @@ void omap_sram_idle(void)
>  		omap2_gpio_resume_after_idle();
>  		if (per_prev_state == PWRDM_POWER_OFF)
>  			omap3_per_restore_context();
> -		omap_uart_resume_idle(2);
> -		omap_uart_resume_idle(3);
>  	}
>  
>  	if (!is_suspending())
> diff --git a/arch/arm/plat-omap/include/plat/serial.h b/arch/arm/plat-omap/include/plat/serial.h
> index bfd73b7..5b8c8f2 100644
> --- a/arch/arm/plat-omap/include/plat/serial.h
> +++ b/arch/arm/plat-omap/include/plat/serial.h
> @@ -109,8 +109,8 @@ struct omap_board_data;
>  
>  extern void omap_serial_init(void);
>  extern void omap_serial_init_port(struct omap_board_data *bdata);
> -extern void omap_uart_prepare_idle(int num);
> -extern void omap_uart_resume_idle(int num);
> +extern void omap_uart_prepare_idle(void);
> +extern int omap_uart_resume_idle(void);
>  #endif
>  
>  #endif
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 96fc860..05c2f52 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -54,6 +54,46 @@ static void serial_omap_rx_timeout(unsigned long uart_no);
>  static int serial_omap_start_rxdma(struct uart_omap_port *up);
>  static void omap_uart_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
>  
> +int omap_uart_resume_idle()
> +{
> +	struct uart_omap_port *up;
> +	int i, ret = 0;
> +
> +	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
> +		up = ui[i];
> +		if (!up)
> +			continue;
> +
> +		if (!up->clocked && up->chk_wakeup(up->pdev)) {
> +			pm_runtime_get_sync(&up->pdev->dev);
> +			up->clocked = 1;
> +			ret++;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +void omap_uart_prepare_idle()
> +{
> +	struct uart_omap_port *up;
> +	int i;
> +
> +	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
> +		up = ui[i];
> +		if (!up)
> +			continue;
> +
> +		if (up->clocked &&
> +				jiffies_to_msecs(jiffies - up->port_activity)
> +					> 3000) {

What is this check for?

> +			pm_runtime_mark_last_busy(&up->pdev->dev);
> +			pm_runtime_put_autosuspend(&up->pdev->dev);
> +			up->clocked = 0;
> +		}
> +	}
> +}
> +
>  static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
>  {
>  	offset <<= up->port.regshift;

Kevin

[PATCH v4 10/11] OMAP: Serial: Allow UART parameters to be configured from board file.

[Kevin Hilman]

"Govindraj.R" <govindraj.raja@ti.com> writes:

> The following UART parameters are defined within the UART driver:
>
> 1). Whether the UART uses DMA (dma_enabled), by default set to 0
> 2). The size of dma buffer (set to 4096 bytes)
> 3). The time after which the dma should stop if no more data is received.
> 4). The auto suspend delay that will be passed for pm_runtime_autosuspend
>     where uart will be disabled after timeout
>
> Different UARTs may be used for different purpose such as the console,
> for interfacing bluetooth chip, for interfacing to a modem chip, etc.
> Therefore, it is necessary to be able to customize the above settings
> for a given board on a per UART basis.
>
> This change allows these parameters to be configured from the board file
> and allows the parameters to be configured for each UART independently.
>
> If a board does not define its own custom parameters for the UARTs, then
> use the default parameters in the structure "omap_serial_default_info".
> The default parameters are defined to be the same as the current settings
> in the UART driver to avoid breaking the UART for any board. By default,
> make all boards use the default UART parameters.
>
> Signed-off-by: Deepak K <deepak.k@ti.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>

Who is the author of this patch?  The first version says Jon[1], v2 says
Deepak[2].  Either way, please ensure proper authorship is attributed
using From: at the beginning of the changelog.  (git-format-patch will
do this for you when the authorship is correct in the git history.)

Even better would be to summarize the changes since the first version
after the '---' so it would be clearer why the authorship was changed.

Only 1/3 of my comments from v3 were addressed in this version.  Please
re-read my comments there[3]. 

Kevin


[1] http://marc.info/?l=linux-omap&m=129890257812478&w=2
[2] http://marc.info/?l=linux-omap&m=130408096416887&w=2
[3] http://marc.info/?l=linux-omap&m=130896078622810&w=2

[PATCH v4 10/11] OMAP: Serial: Allow UART parameters to be configured from board file.

[Govindraj]

On Sat, Sep 10, 2011 at 12:41 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> The following UART parameters are defined within the UART driver:
>>
>> 1). Whether the UART uses DMA (dma_enabled), by default set to 0
>> 2). The size of dma buffer (set to 4096 bytes)
>> 3). The time after which the dma should stop if no more data is received.
>> 4). The auto suspend delay that will be passed for pm_runtime_autosuspend
>> ? ? where uart will be disabled after timeout
>>
>> Different UARTs may be used for different purpose such as the console,
>> for interfacing bluetooth chip, for interfacing to a modem chip, etc.
>> Therefore, it is necessary to be able to customize the above settings
>> for a given board on a per UART basis.
>>
>> This change allows these parameters to be configured from the board file
>> and allows the parameters to be configured for each UART independently.
>>
>> If a board does not define its own custom parameters for the UARTs, then
>> use the default parameters in the structure "omap_serial_default_info".
>> The default parameters are defined to be the same as the current settings
>> in the UART driver to avoid breaking the UART for any board. By default,
>> make all boards use the default UART parameters.
>>
>> Signed-off-by: Deepak K <deepak.k@ti.com>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>
> Who is the author of this patch? ?The first version says Jon[1], v2 says
> Deepak[2]. ?Either way, please ensure proper authorship is attributed
> using From: at the beginning of the changelog. ?(git-format-patch will
> do this for you when the authorship is correct in the git history.)
>
> Even better would be to summarize the changes since the first version
> after the '---' so it would be clearer why the authorship was changed.
>
> Only 1/3 of my comments from v3 were addressed in this version. ?Please
> re-read my comments there[3].
>

Will fix this and repost.

--
Thanks,
Govindraj.R


> Kevin
>
>
> [1] http://marc.info/?l=linux-omap&m=129890257812478&w=2
> [2] http://marc.info/?l=linux-omap&m=130408096416887&w=2
> [3] http://marc.info/?l=linux-omap&m=130896078622810&w=2
> --
> 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
>

[PATCH v4 09/11] OMAP2+: Serial: Use prepare and resume calls to gate uart clocks

[Govindraj]

On Sat, Sep 10, 2011 at 12:29 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> Currently we are using uart prepare and resume calls to gate uart clocks
>> retaining the same method.
>>
>> More details on reason to retain this design is provided here:
>> http://www.spinics.net/lists/linux-serial/msg04128.html
>
> This type of thing can go after the '---' since it doesn't belong in the
> permanent git history.
>

Okay.

>> Since prepare and resume hooks are moved to driver itself we can just use
>> a single prepare and resume call. As in driver we traverse on number of uart
>> instance and handle it accordingly.
>
> Some important functionality has been removed (and not documented.)
> Namely, the current code checks the next power when deciding whether or
> not to call prepare_idle, and vice versa for resume_idle.
>
> It's quite possible that it is no longer needed with the runtime PM
>
>> In 34xx uart can wakeup using module level PM_WKEN or IO PAD wakeup use
>> resume_call from prcm irq handler to wakeup uart, based on chk_wakeup_status
>> from io_pad or PM_WKST.
>
> IMO, this patch should just replace the mulitple calls with a single
> call, but still in the idle path.
>

Yes fine, will make resume from prcm_irq a separate patch.

> Using the PRCM IRQ handler should be a separate patch with it's own
> changelog.
>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>
> Also, moving these calls to the driver means that the driver cannot be
> built as a module:
>
> arch/arm/mach-omap2/built-in.o: In function `omap2_enter_full_retention':
> /work/kernel/omap/pm/arch/arm/mach-omap2/pm24xx.c:141: undefined reference to `omap_uart_prepare_idle'
> /work/kernel/omap/pm/arch/arm/mach-omap2/pm24xx.c:148: undefined reference to `omap_uart_resume_idle'
> arch/arm/mach-omap2/built-in.o: In function `prcm_clear_mod_irqs':
> /work/kernel/omap/pm/arch/arm/mach-omap2/pm34xx.c:213: undefined reference to `omap_uart_resume_idle'
> arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle':
> /work/kernel/omap/pm/arch/arm/mach-omap2/pm34xx.c:390: undefined reference to `omap_uart_prepare_idle'
>

Will check and fix this in next version.

--
Thanks,
Govindraj.R

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Govindraj]

On Fri, Sep 9, 2011 at 11:44 PM, Kevin Hilman <khilman@ti.com> wrote:
> Govindraj <govindraj.ti@gmail.com> writes:
>
>> On Fri, Sep 9, 2011 at 5:54 AM, Kevin Hilman <khilman@ti.com> wrote:
>>> "Govindraj.R" <govindraj.raja@ti.com> writes:
>>>
>>>> Adapts omap-serial driver to use pm_runtime API's.
>>>>
>>>> 1.) Moving context_restore func to driver from serial.c
>>>> 2.) Use runtime irq safe to use get_sync from irq context.
>>>> 3.) autosuspend for port specific activities and put for reg access.
>>>
>>> Re: autosuspend, it looks like the driver now has 2 mechanisms for
>>> tracking activity. ?The runtime PM 'mark last busy' and the existing
>>> 'up->port_activity = jiffies' stuff. ?Do you think those could be
>>> unified?
>>>
>>
>> Is there a way where we can retrieve the last busy jiffie value
>> using runtime API? I don't find that available.
>>
>
> Not currently. ?The question is more along the lines of: what is the
> port_activity jiffies used for, and can it be replaced by using
> _mark_last_busy().
>
> If it cannot, that's fine, but it should be described.

Okay.

>
>>> At first glance, it looks like most places with a _mark_last_busy() also
>>> have a up->port_activity update. ?I'm not familiar enough with the
>>> driver to make the call, but they look awfully similar.
>>>
>>
>> Ok, I will check whether I can get rid if it.
>>
>>>> 4.) for earlyprintk usage the debug macro will write to console uart
>>>> ? ? so keep uart clocks active from boot, idle using hwmod API's re-enable back
>>>> ? ? using runtime API's.
>>>
>>> /me doesn't understand
>>
>> I have added this reason in early mail reply to 04/11 patch review
>> [needed for earlyprintk option from low level debug ]
>
> OK, but AFAIK, DEBUG_LL + earlyprintk do not use this driver. ?Instead
> they use the debug macros, so any hacks to deal with that do not belong
> in the driver.
>
> If there are hacks required, they should be in a separate patch, with a
> separate descriptive changelog.
>

Yes these are required, because early printk relies on clocks from bootloader
and if clocks are gated even before console_driver is available it can
lead to boot failures.

I can keep that part as a separate patch.


>>>
>>>> 5.) Moving context restore to runtime suspend and using reg values from port
>>>> ? ? structure to restore the uart port context based on context_loss_count.
>>>> ? ? Maintain internal state machine for avoiding repeated enable/disable of
>>>> ? ? uart port wakeup mechanism.
>>>> 6.) Add API to enable wakeup and check wakeup status.
>>>>
>>>> Acked-by: Alan Cox <alan@linux.intel.com>
>>>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>>>> ---
>>>> ?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 49 ++++++
>>>> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? 10 ++
>>>> ?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?211 ++++++++++++++++++++++---
>>>> ?3 files changed, 250 insertions(+), 20 deletions(-)
>>>>
>>
>> [..]
>>
>>>> +
>>>> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
>>>> +{
>>>> + ? ? struct omap_device *od;
>>>> + ? ? 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;
>>>> + ? ? ? ? ? ? else
>>>> + ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
>>>> + ? ? ? ? ? ? __raw_writel(v, up->wk_en);
>>>> + ? ? }
>>>> +
>>>> + ? ? od = to_omap_device(pdev);
>>>> + ? ? if (enable)
>>>> + ? ? ? ? ? ? omap_hwmod_enable_wakeup(od->hwmods[0]);
>>>> + ? ? else
>>>> + ? ? ? ? ? ? omap_hwmod_disable_wakeup(od->hwmods[0]);
>>>> +}
>>>> +
>>>
>>> Here again, this is difficult to review because you removed the code in
>>> [4/11] and add it back here, but with rather different functionality
>>> with no description as to why.
>>>
>>
>> will move this change as part of 4/11 itself.
>>
>
> The point was not just that the move was confusing, but that you changed
> the functionality along with the move without any description.
>

If are referring to uart rx pad wakeup its still retained but done
using mux framework.

>>> The previous version enabled wakeups at the PRCM and at the IO ring.
>>> This version enables wakeups at the PRCM as well but instead of enabling
>>> them at the IO ring, enables them at the module.
>>>
>>
>> Since we are gating using prepare idle call I think
>> we can use this enable wake-up call as part of prepare idle call itself,
>> as done earlier.
>
> Not sure what that has to with my comment.
>
> In moving this code, you removed the enable/disable of IO ring wakeups.
> It probably continues to work because they're enabled by the bootloader,
> but that is not correct and the driver should be managing the IO ring
> wakeups as before.
>

Prior to this we used io-pad offsets address and modified to enable wakeup
mechanism, But since we received some comments from Tony to use mux framework
for any mux changes, I removed using mux address and use mux framework from
hwmod, I have not removed rx-pad wakeup still enable io-pad wakeup.
otherwise wakeup from off-mode will fail on 3430SDP.

omap_uart_wakeup_enable
    --> omap_hwmod_enable_wakeup
           --> omap_hwmod_enable_ioring_wakeup [PATCH v4 01/11]
OMAP2+: hwmod: Add API to enable IO ring wakeup.]

boot-loaders doesn't enable io pad wakeup for uart.


> [...]
>
>>>> ?}
>>>>
>>>> ?static int __init
>>>> @@ -1065,22 +1123,33 @@ 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);
>>>>
>>>> - ? ? if (up)
>>>> + ? ? if (up) {
>>>> ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
>>>> + ? ? ? ? ? ? if (up->port.line == up->port.cons->index &&
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? !is_console_locked())
>>>> + ? ? ? ? ? ? ? ? ? ? up->console_locked = console_trylock();
>>>> + ? ? }
>>>> +
>>>
>>> Motiviation for console locking in this version of the series is not
>>> clear, and not described in the changelog.
>>>
>>> It's also present here in static suspend/resume but not in runtime
>>> suspend/resume, which also is suspicious.
>>>
>>
>> Yes will add to change log,
>>
>> We needed this for no console suspend use case
>> no console lock is taken in no_console_suspend is specified,
>>
>> Since our pwr_dmn hooks disable clocks left enabled during suspend,
>> so any prints after pwr_dmn hooks can cause problems since clocks
>> are already cut from pwr_dmn hooks.
>>
>> So buffer prints while entering suspend by taking console lock
>> if not taken already and print back after uart state machine restores
>> console uart.
>
> Any special consideration for features like no_console_suspend should be
> done in a separate patch with a separate changelog.
>

will add as separate patch.

> However, as I've stated before during previous reviews, if someone has
> put no_console_suspend on the command line, that means they've
> specifically stated they *want* to see console writes during suspend.
> That means, we should not cut clocks at all.
>
> Of course, that means the system will not hit the low power state becase
> the UART clocks are not gated, but that is what the user requested.

Yes correct, But pwr domain callback will now cut all enabled clocks
now, so only available method was to use console_lock and print later.

pwr_domain callback done in omap-device.c
will force to low powers state by gating all clocks that where left
enabled by driver.

--
Thanks,
Govindraj.R


>
>
> Kevin
>

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Govindraj]

On Mon, Sep 12, 2011 at 12:25 PM, Govindraj <govindraj.ti@gmail.com> wrote:
> On Fri, Sep 9, 2011 at 11:44 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Govindraj <govindraj.ti@gmail.com> writes:
>>
>>> On Fri, Sep 9, 2011 at 5:54 AM, Kevin Hilman <khilman@ti.com> wrote:
>>>> "Govindraj.R" <govindraj.raja@ti.com> writes:
>>>>
>>>>> Adapts omap-serial driver to use pm_runtime API's.
>>>>>
>>>>> 1.) Moving context_restore func to driver from serial.c
>>>>> 2.) Use runtime irq safe to use get_sync from irq context.
>>>>> 3.) autosuspend for port specific activities and put for reg access.
>>>>
>>>> Re: autosuspend, it looks like the driver now has 2 mechanisms for
>>>> tracking activity. ?The runtime PM 'mark last busy' and the existing
>>>> 'up->port_activity = jiffies' stuff. ?Do you think those could be
>>>> unified?
>>>>
>>>
>>> Is there a way where we can retrieve the last busy jiffie value
>>> using runtime API? I don't find that available.
>>>
>>
>> Not currently. ?The question is more along the lines of: what is the
>> port_activity jiffies used for, and can it be replaced by using
>> _mark_last_busy().
>>
>> If it cannot, that's fine, but it should be described.
>
> Okay.
>
>>
>>>> At first glance, it looks like most places with a _mark_last_busy() also
>>>> have a up->port_activity update. ?I'm not familiar enough with the
>>>> driver to make the call, but they look awfully similar.
>>>>
>>>
>>> Ok, I will check whether I can get rid if it.
>>>
>>>>> 4.) for earlyprintk usage the debug macro will write to console uart
>>>>> ? ? so keep uart clocks active from boot, idle using hwmod API's re-enable back
>>>>> ? ? using runtime API's.
>>>>
>>>> /me doesn't understand
>>>
>>> I have added this reason in early mail reply to 04/11 patch review
>>> [needed for earlyprintk option from low level debug ]
>>
>> OK, but AFAIK, DEBUG_LL + earlyprintk do not use this driver. ?Instead
>> they use the debug macros, so any hacks to deal with that do not belong
>> in the driver.
>>
>> If there are hacks required, they should be in a separate patch, with a
>> separate descriptive changelog.
>>
>
> Yes these are required, because early printk relies on clocks from bootloader
> and if clocks are gated even before console_driver is available it can
> lead to boot failures.
>
> I can keep that part as a separate patch.
>
>
>>>>
>>>>> 5.) Moving context restore to runtime suspend and using reg values from port
>>>>> ? ? structure to restore the uart port context based on context_loss_count.
>>>>> ? ? Maintain internal state machine for avoiding repeated enable/disable of
>>>>> ? ? uart port wakeup mechanism.
>>>>> 6.) Add API to enable wakeup and check wakeup status.
>>>>>
>>>>> Acked-by: Alan Cox <alan@linux.intel.com>
>>>>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>>>>> ---
>>>>> ?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 49 ++++++
>>>>> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? 10 ++
>>>>> ?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?211 ++++++++++++++++++++++---
>>>>> ?3 files changed, 250 insertions(+), 20 deletions(-)
>>>>>
>>>
>>> [..]
>>>
>>>>> +
>>>>> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
>>>>> +{
>>>>> + ? ? struct omap_device *od;
>>>>> + ? ? 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;
>>>>> + ? ? ? ? ? ? else
>>>>> + ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
>>>>> + ? ? ? ? ? ? __raw_writel(v, up->wk_en);
>>>>> + ? ? }
>>>>> +
>>>>> + ? ? od = to_omap_device(pdev);
>>>>> + ? ? if (enable)
>>>>> + ? ? ? ? ? ? omap_hwmod_enable_wakeup(od->hwmods[0]);
>>>>> + ? ? else
>>>>> + ? ? ? ? ? ? omap_hwmod_disable_wakeup(od->hwmods[0]);
>>>>> +}
>>>>> +
>>>>
>>>> Here again, this is difficult to review because you removed the code in
>>>> [4/11] and add it back here, but with rather different functionality
>>>> with no description as to why.
>>>>
>>>
>>> will move this change as part of 4/11 itself.
>>>
>>
>> The point was not just that the move was confusing, but that you changed
>> the functionality along with the move without any description.
>>
>
> If are referring to uart rx pad wakeup its still retained but done
> using mux framework.
>
>>>> The previous version enabled wakeups at the PRCM and at the IO ring.
>>>> This version enables wakeups at the PRCM as well but instead of enabling
>>>> them at the IO ring, enables them at the module.
>>>>
>>>
>>> Since we are gating using prepare idle call I think
>>> we can use this enable wake-up call as part of prepare idle call itself,
>>> as done earlier.
>>
>> Not sure what that has to with my comment.
>>
>> In moving this code, you removed the enable/disable of IO ring wakeups.
>> It probably continues to work because they're enabled by the bootloader,
>> but that is not correct and the driver should be managing the IO ring
>> wakeups as before.
>>
>
> Prior to this we used io-pad offsets address and modified to enable wakeup
> mechanism, But since we received some comments from Tony to use mux framework
> for any mux changes, I removed using mux address and use mux framework from
> hwmod, I have not removed rx-pad wakeup still enable io-pad wakeup.
> otherwise wakeup from off-mode will fail on 3430SDP.
>
> omap_uart_wakeup_enable
> ? ?--> omap_hwmod_enable_wakeup
> ? ? ? ? ? --> omap_hwmod_enable_ioring_wakeup [PATCH v4 01/11]
> OMAP2+: hwmod: Add API to enable IO ring wakeup.]
>
> boot-loaders doesn't enable io pad wakeup for uart.
>
>
>> [...]
>>
>>>>> ?}
>>>>>
>>>>> ?static int __init
>>>>> @@ -1065,22 +1123,33 @@ 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);
>>>>>
>>>>> - ? ? if (up)
>>>>> + ? ? if (up) {
>>>>> ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
>>>>> + ? ? ? ? ? ? if (up->port.line == up->port.cons->index &&
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? !is_console_locked())
>>>>> + ? ? ? ? ? ? ? ? ? ? up->console_locked = console_trylock();
>>>>> + ? ? }
>>>>> +
>>>>
>>>> Motiviation for console locking in this version of the series is not
>>>> clear, and not described in the changelog.
>>>>
>>>> It's also present here in static suspend/resume but not in runtime
>>>> suspend/resume, which also is suspicious.
>>>>
>>>
>>> Yes will add to change log,
>>>
>>> We needed this for no console suspend use case
>>> no console lock is taken in no_console_suspend is specified,
>>>
>>> Since our pwr_dmn hooks disable clocks left enabled during suspend,
>>> so any prints after pwr_dmn hooks can cause problems since clocks
>>> are already cut from pwr_dmn hooks.
>>>
>>> So buffer prints while entering suspend by taking console lock
>>> if not taken already and print back after uart state machine restores
>>> console uart.
>>
>> Any special consideration for features like no_console_suspend should be
>> done in a separate patch with a separate changelog.
>>
>
> will add as separate patch.
>
>> However, as I've stated before during previous reviews, if someone has
>> put no_console_suspend on the command line, that means they've
>> specifically stated they *want* to see console writes during suspend.
>> That means, we should not cut clocks at all.
>>
>> Of course, that means the system will not hit the low power state becase
>> the UART clocks are not gated, but that is what the user requested.
>
> Yes correct, But pwr domain callback will now cut all enabled clocks
> now, so only available method was to use console_lock and print later.
>
> pwr_domain callback done in omap-device.c
> will force to low powers state by gating all clocks that where left
> enabled by driver.
>

Forgot to specify that the above statement is in context to system wide
suspend scenario and not cpu_idle path use cases.



> --
> Thanks,
> Govindraj.R
>
>
>>
>>
>> Kevin
>>
>

[PATCH v4 07/11] Serial: OMAP: Add runtime pm support for omap-serial driver

[Kevin Hilman]

Govindraj <govindraj.ti@gmail.com> writes:

> On Fri, Sep 9, 2011 at 11:44 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Govindraj <govindraj.ti@gmail.com> writes:
>>
>>> On Fri, Sep 9, 2011 at 5:54 AM, Kevin Hilman <khilman@ti.com> wrote:
>>>> "Govindraj.R" <govindraj.raja@ti.com> writes:
>>>>
>>>>> Adapts omap-serial driver to use pm_runtime API's.
>>>>>
>>>>> 1.) Moving context_restore func to driver from serial.c
>>>>> 2.) Use runtime irq safe to use get_sync from irq context.
>>>>> 3.) autosuspend for port specific activities and put for reg access.
>>>>
>>>> Re: autosuspend, it looks like the driver now has 2 mechanisms for
>>>> tracking activity. ?The runtime PM 'mark last busy' and the existing
>>>> 'up->port_activity = jiffies' stuff. ?Do you think those could be
>>>> unified?
>>>>
>>>
>>> Is there a way where we can retrieve the last busy jiffie value
>>> using runtime API? I don't find that available.
>>>
>>
>> Not currently. ?The question is more along the lines of: what is the
>> port_activity jiffies used for, and can it be replaced by using
>> _mark_last_busy().
>>
>> If it cannot, that's fine, but it should be described.
>
> Okay.
>
>>
>>>> At first glance, it looks like most places with a _mark_last_busy() also
>>>> have a up->port_activity update. ?I'm not familiar enough with the
>>>> driver to make the call, but they look awfully similar.
>>>>
>>>
>>> Ok, I will check whether I can get rid if it.
>>>
>>>>> 4.) for earlyprintk usage the debug macro will write to console uart
>>>>> ? ? so keep uart clocks active from boot, idle using hwmod API's re-enable back
>>>>> ? ? using runtime API's.
>>>>
>>>> /me doesn't understand
>>>
>>> I have added this reason in early mail reply to 04/11 patch review
>>> [needed for earlyprintk option from low level debug ]
>>
>> OK, but AFAIK, DEBUG_LL + earlyprintk do not use this driver. ?Instead
>> they use the debug macros, so any hacks to deal with that do not belong
>> in the driver.
>>
>> If there are hacks required, they should be in a separate patch, with a
>> separate descriptive changelog.
>>
>
> Yes these are required, because early printk relies on clocks from bootloader
> and if clocks are gated even before console_driver is available it can
> lead to boot failures.

My point is that hacks/workaround for DEBUG_LL + earlyprintk don't
belong in this driver, since this driver is not involved.

> I can keep that part as a separate patch.

Yes please.

>>>>
>>>>> 5.) Moving context restore to runtime suspend and using reg values from port
>>>>> ? ? structure to restore the uart port context based on context_loss_count.
>>>>> ? ? Maintain internal state machine for avoiding repeated enable/disable of
>>>>> ? ? uart port wakeup mechanism.
>>>>> 6.) Add API to enable wakeup and check wakeup status.
>>>>>
>>>>> Acked-by: Alan Cox <alan@linux.intel.com>
>>>>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>>>>> ---
>>>>> ?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ? 49 ++++++
>>>>> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? 10 ++
>>>>> ?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ?211 ++++++++++++++++++++++---
>>>>> ?3 files changed, 250 insertions(+), 20 deletions(-)
>>>>>
>>>
>>> [..]
>>>
>>>>> +
>>>>> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
>>>>> +{
>>>>> + ? ? struct omap_device *od;
>>>>> + ? ? 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;
>>>>> + ? ? ? ? ? ? else
>>>>> + ? ? ? ? ? ? ? ? ? ? v &= ~up->wk_mask;
>>>>> + ? ? ? ? ? ? __raw_writel(v, up->wk_en);
>>>>> + ? ? }
>>>>> +
>>>>> + ? ? od = to_omap_device(pdev);
>>>>> + ? ? if (enable)
>>>>> + ? ? ? ? ? ? omap_hwmod_enable_wakeup(od->hwmods[0]);
>>>>> + ? ? else
>>>>> + ? ? ? ? ? ? omap_hwmod_disable_wakeup(od->hwmods[0]);
>>>>> +}
>>>>> +
>>>>
>>>> Here again, this is difficult to review because you removed the code in
>>>> [4/11] and add it back here, but with rather different functionality
>>>> with no description as to why.
>>>>
>>>
>>> will move this change as part of 4/11 itself.
>>>
>>
>> The point was not just that the move was confusing, but that you changed
>> the functionality along with the move without any description.
>>
>
> If are referring to uart rx pad wakeup its still retained but done
> using mux framework.

OK, let me try this a slightly different way.  Here's what you did:

- remove some code in patch 4
- add it back in patch 7, but in different form (with no description of diffs)
- the missing parts of original code are not documented
- moved code has some new (but undocumented) features (module-level wakeup)
- turns out the missing parts (IO pad wakeups) are done using a
  different framework in yet another patch (which would be fine, if it
  the reviewers were given a hint.)

I hope this is enough to demonstrate that trying to decipher what what
you meant to do, what you did, and what you didn't do is extremely
time consuming for a reviewer.

Well-constructed patches, broken up into *logical* chunks with
descriptive changelogs are invaluable to an efficient review process.

>>>> The previous version enabled wakeups at the PRCM and at the IO ring.
>>>> This version enables wakeups at the PRCM as well but instead of enabling
>>>> them at the IO ring, enables them at the module.
>>>>
>>>
>>> Since we are gating using prepare idle call I think
>>> we can use this enable wake-up call as part of prepare idle call itself,
>>> as done earlier.
>>
>> Not sure what that has to with my comment.
>>
>> In moving this code, you removed the enable/disable of IO ring wakeups.
>> It probably continues to work because they're enabled by the bootloader,
>> but that is not correct and the driver should be managing the IO ring
>> wakeups as before.
>>
>
> Prior to this we used io-pad offsets address and modified to enable wakeup
> mechanism, But since we received some comments from Tony to use mux framework
> for any mux changes, I removed using mux address and use mux framework from
> hwmod, I have not removed rx-pad wakeup still enable io-pad wakeup.
> otherwise wakeup from off-mode will fail on 3430SDP.
>
> omap_uart_wakeup_enable
>     --> omap_hwmod_enable_wakeup
>            --> omap_hwmod_enable_ioring_wakeup [PATCH v4 01/11]
> OMAP2+: hwmod: Add API to enable IO ring wakeup.]
>
> boot-loaders doesn't enable io pad wakeup for uart.
>
>
>> [...]
>>
>>>>> ?}
>>>>>
>>>>> ?static int __init
>>>>> @@ -1065,22 +1123,33 @@ 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);
>>>>>
>>>>> - ? ? if (up)
>>>>> + ? ? if (up) {
>>>>> ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
>>>>> + ? ? ? ? ? ? if (up->port.line == up->port.cons->index &&
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? !is_console_locked())
>>>>> + ? ? ? ? ? ? ? ? ? ? up->console_locked = console_trylock();
>>>>> + ? ? }
>>>>> +
>>>>
>>>> Motiviation for console locking in this version of the series is not
>>>> clear, and not described in the changelog.
>>>>
>>>> It's also present here in static suspend/resume but not in runtime
>>>> suspend/resume, which also is suspicious.
>>>>
>>>
>>> Yes will add to change log,
>>>
>>> We needed this for no console suspend use case
>>> no console lock is taken in no_console_suspend is specified,
>>>
>>> Since our pwr_dmn hooks disable clocks left enabled during suspend,
>>> so any prints after pwr_dmn hooks can cause problems since clocks
>>> are already cut from pwr_dmn hooks.
>>>
>>> So buffer prints while entering suspend by taking console lock
>>> if not taken already and print back after uart state machine restores
>>> console uart.
>>
>> Any special consideration for features like no_console_suspend should be
>> done in a separate patch with a separate changelog.
>>
>
> will add as separate patch.

yes please.

>> However, as I've stated before during previous reviews, if someone has
>> put no_console_suspend on the command line, that means they've
>> specifically stated they *want* to see console writes during suspend.
>> That means, we should not cut clocks at all.
>>
>> Of course, that means the system will not hit the low power state becase
>> the UART clocks are not gated, but that is what the user requested.
>
> Yes correct, But pwr domain callback will now cut all enabled clocks
> now, so only available method was to use console_lock and print later.
>
> pwr_domain callback done in omap-device.c
> will force to low powers state by gating all clocks that where left
> enabled by driver.

These are the kinds of things that belong in a descriptive changelog.

Kevin