linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 15/21] OMAP2+: UART: Make the RX_TIMEOUT for DMA configurable for each UART
@ 2011-10-18 15:35 Govindraj.R
  2011-10-18 15:35 ` [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs Govindraj.R
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Govindraj.R @ 2011-10-18 15:35 UTC (permalink / raw)
  To: linux-arm-kernel

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 secound 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 e70e273..7c65410 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -64,13 +64,15 @@ struct omap_uart_state {
 static LIST_HEAD(uart_list);
 static u8 num_uarts;
 
-#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_RXDMA_TIMEOUT		(3 * HZ)/* RX DMA timeout (jiffies) */
 
 static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
 	{
 		.dma_enabled	= false,
 		.dma_rx_buf_size = DEFAULT_RXDMA_BUFSIZE,
+		.dma_rx_poll_rate = DEFAULT_RXDMA_POLLRATE,
 		.dma_rx_timeout = DEFAULT_RXDMA_TIMEOUT,
 		.autosuspend_timeout = DEFAULT_AUTOSUSPEND_DELAY,
 	},
@@ -373,6 +375,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	omap_up.enable_wakeup = omap_uart_enable_wakeup;
 	omap_up.dma_rx_buf_size = info->dma_rx_buf_size;
 	omap_up.dma_rx_timeout = info->dma_rx_timeout;
+	omap_up.dma_rx_poll_rate = info->dma_rx_poll_rate;
 	omap_up.autosuspend_timeout = info->autosuspend_timeout;
 
 	/* Enable the MDR1 errata for OMAP3 */
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 26f8cbd..9a6879c 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -53,7 +53,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
@@ -68,6 +67,7 @@ struct omap_uart_port_info {
 	unsigned int		dma_rx_buf_size;
 	unsigned int		dma_rx_timeout;
 	unsigned int		autosuspend_timeout;
+	unsigned int		dma_rx_poll_rate;
 
 	u32 (*get_context_loss_count)(struct device *);
 	void (*set_forceidle)(struct platform_device *);
@@ -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 10f8a89..1714bd2 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -47,7 +47,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 serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
 
@@ -542,7 +542,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,
@@ -1159,7 +1159,7 @@ static int serial_omap_resume(struct device *dev)
 }
 #endif
 
-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;
@@ -1169,9 +1169,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);
@@ -1200,7 +1200,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;
 }
@@ -1239,7 +1239,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;
 }
@@ -1375,6 +1375,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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs.
  2011-10-18 15:35 [PATCH v7 15/21] OMAP2+: UART: Make the RX_TIMEOUT for DMA configurable for each UART Govindraj.R
@ 2011-10-18 15:35 ` Govindraj.R
  2011-11-04 22:00   ` Kevin Hilman
  2011-10-18 15:35 ` [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos Govindraj.R
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Govindraj.R @ 2011-10-18 15:35 UTC (permalink / raw)
  To: linux-arm-kernel

The custom hwmod activate and deactivate funcs does hwmod_enable
and idle same can be done with omap_device generic API's.

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

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 7c65410..7658a03 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -78,24 +78,10 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
 	},
 };
 
-static int uart_idle_hwmod(struct omap_device *od)
-{
-	omap_hwmod_idle(od->hwmods[0]);
-
-	return 0;
-}
-
-static int uart_enable_hwmod(struct omap_device *od)
-{
-	omap_hwmod_enable(od->hwmods[0]);
-
-	return 0;
-}
-
 static struct omap_device_pm_latency omap_uart_latency[] = {
 	{
-		.deactivate_func = uart_idle_hwmod,
-		.activate_func	 = uart_enable_hwmod,
+		.activate_func	 = omap_device_enable_hwmods,
+		.deactivate_func = omap_device_idle_hwmods,
 		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
 	},
 };
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-10-18 15:35 [PATCH v7 15/21] OMAP2+: UART: Make the RX_TIMEOUT for DMA configurable for each UART Govindraj.R
  2011-10-18 15:35 ` [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs Govindraj.R
@ 2011-10-18 15:35 ` Govindraj.R
  2011-11-04 22:42   ` Kevin Hilman
  2011-10-18 15:35 ` [PATCH v7 18/21] OMAP2+: UART: remove temporary variable used to count uart instance Govindraj.R
  2011-10-18 15:35 ` [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart Govindraj.R
  3 siblings, 1 reply; 15+ messages in thread
From: Govindraj.R @ 2011-10-18 15:35 UTC (permalink / raw)
  To: linux-arm-kernel

Omap_uart_can_sleep function blocks system wide low power state until
uart is active remove this func and add qos requests to prevent
MPU from transitioning while uart is active and remove qos request
if uart is auto-idled.

qos requests are blocking notifier calls so put these requests to
work queue to avoid warn on slow path warning while using qos
API's from runtime callbacks. Flush_sync any pending qos jobs
in work queue while suspending.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c             |    5 ---
 arch/arm/mach-omap2/pm24xx.c                  |    2 -
 arch/arm/mach-omap2/pm34xx.c                  |   10 ------
 arch/arm/mach-omap2/serial.c                  |   24 +-------------
 arch/arm/plat-omap/include/plat/omap-serial.h |    8 +++++
 drivers/tty/serial/omap-serial.c              |   41 ++++++++++++++++++++++++-
 6 files changed, 50 insertions(+), 40 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 4bf6e6e..98b7d3f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -226,11 +226,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	struct omap3_idle_statedata *cx;
 	int ret;
 
-	if (!omap3_can_sleep()) {
-		new_state = dev->safe_state;
-		goto select_state;
-	}
-
 	/*
 	 * Prevent idle completely if CAM is active.
 	 * CAM does not have wakeup capability in OMAP3.
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
index a75f764..192f0a4 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -248,8 +248,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())
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 6e7f276..a635fa1 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -425,21 +425,11 @@ void omap_sram_idle(void)
 	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
-int omap3_can_sleep(void)
-{
-	if (!omap_uart_can_sleep())
-		return 0;
-	return 1;
-}
-
 static void omap3_pm_idle(void)
 {
 	local_irq_disable();
 	local_fiq_disable();
 
-	if (!omap3_can_sleep())
-		goto out;
-
 	if (omap_irq_pending() || need_resched())
 		goto out;
 
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 7658a03..55ce950 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -75,6 +75,7 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
 		.dma_rx_poll_rate = DEFAULT_RXDMA_POLLRATE,
 		.dma_rx_timeout = DEFAULT_RXDMA_TIMEOUT,
 		.autosuspend_timeout = DEFAULT_AUTOSUSPEND_DELAY,
+		.use_pm_qos = true,
 	},
 };
 
@@ -87,28 +88,6 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
 };
 
 #ifdef CONFIG_PM
-
-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;
-}
-
 static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
 {
 	struct omap_device *od = to_omap_device(pdev);
@@ -363,6 +342,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	omap_up.dma_rx_timeout = info->dma_rx_timeout;
 	omap_up.dma_rx_poll_rate = info->dma_rx_poll_rate;
 	omap_up.autosuspend_timeout = info->autosuspend_timeout;
+	omap_up.use_pm_qos = info->use_pm_qos;
 
 	/* Enable the MDR1 errata for OMAP3 */
 	if (cpu_is_omap34xx() && !cpu_is_ti816x())
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 9a6879c..41eda3c 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -19,6 +19,7 @@
 
 #include <linux/serial_core.h>
 #include <linux/platform_device.h>
+#include <linux/pm_qos_params.h>
 
 #include <plat/mux.h>
 
@@ -68,6 +69,7 @@ struct omap_uart_port_info {
 	unsigned int		dma_rx_timeout;
 	unsigned int		autosuspend_timeout;
 	unsigned int		dma_rx_poll_rate;
+	u8			use_pm_qos;
 
 	u32 (*get_context_loss_count)(struct device *);
 	void (*set_forceidle)(struct platform_device *);
@@ -129,6 +131,12 @@ struct uart_omap_port {
 	u32			context_loss_cnt;
 	u32			errata;
 	u8			wakeups_enabled;
+
+	struct pm_qos_request_list pm_qos_request;
+	u32			latency;
+	struct work_struct	work;
+	u8			request_qos;
+	u8			use_pm_qos;
 };
 
 #endif /* __OMAP_SERIAL_H__ */
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 1714bd2..956736c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -51,6 +51,8 @@ static void serial_omap_rxdma_poll(unsigned long uart_no);
 static int serial_omap_start_rxdma(struct uart_omap_port *up);
 static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
 
+static struct workqueue_struct *serial_omap_uart_wq;
+
 static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
 {
 	offset <<= up->port.regshift;
@@ -674,6 +676,21 @@ serial_omap_configure_xonxoff
 	serial_out(up, UART_LCR, up->lcr);
 }
 
+static void serial_omap_uart_qos_work(struct work_struct *work)
+{
+	struct uart_omap_port *up = container_of(work, struct uart_omap_port,
+						work);
+
+	if (!up->request_qos) {
+		pm_qos_add_request(&up->pm_qos_request,
+			PM_QOS_CPU_DMA_LATENCY, up->latency);
+		up->request_qos = true;
+	} else {
+		pm_qos_remove_request(&up->pm_qos_request);
+		up->request_qos = false;
+	}
+}
+
 static void
 serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 			struct ktermios *old)
@@ -869,6 +886,13 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial_omap_configure_xonxoff(up, termios);
 
 	spin_unlock_irqrestore(&up->port.lock, flags);
+
+	if (up->use_pm_qos) {
+		/* calculate wakeup latency constraint */
+		up->latency = (1000000 * up->port.fifosize) / (1000 * baud / 8);
+		schedule_work(&up->work);
+	}
+
 	pm_runtime_put(&up->pdev->dev);
 	dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
 }
@@ -1144,8 +1168,11 @@ static int serial_omap_suspend(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
 
-	if (up)
+	if (up) {
 		uart_suspend_port(&serial_omap_reg, &up->port);
+		flush_work_sync(&up->work);
+	}
+
 	return 0;
 }
 
@@ -1368,6 +1395,7 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->port.uartclk = omap_up_info->uartclk;
 	up->uart_dma.uart_base = mem->start;
 	up->errata = omap_up_info->errata;
+	up->use_pm_qos = omap_up_info->use_pm_qos;
 
 	if (omap_up_info->dma_enabled) {
 		up->uart_dma.uart_dma_tx = dma_tx->start;
@@ -1382,6 +1410,11 @@ static int serial_omap_probe(struct platform_device *pdev)
 		up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
 	}
 
+	if (up->use_pm_qos) {
+		serial_omap_uart_wq = create_workqueue(up->name);
+		INIT_WORK(&up->work, serial_omap_uart_qos_work);
+	}
+
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev,
 			omap_up_info->autosuspend_timeout);
@@ -1516,6 +1549,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
 	if (up->use_dma && pdata->set_forceidle)
 		pdata->set_forceidle(up->pdev);
 
+	if (up->use_pm_qos && up->request_qos)
+		schedule_work(&up->work);
+
 	return 0;
 }
 
@@ -1535,6 +1571,9 @@ static int serial_omap_runtime_resume(struct device *dev)
 		/* Errata i291 */
 		if (up->use_dma && pdata->set_noidle)
 			pdata->set_noidle(up->pdev);
+
+		if (up->use_pm_qos && !up->request_qos)
+			schedule_work(&up->work);
 	}
 
 	return 0;
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 18/21] OMAP2+: UART: remove temporary variable used to count uart instance
  2011-10-18 15:35 [PATCH v7 15/21] OMAP2+: UART: Make the RX_TIMEOUT for DMA configurable for each UART Govindraj.R
  2011-10-18 15:35 ` [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs Govindraj.R
  2011-10-18 15:35 ` [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos Govindraj.R
@ 2011-10-18 15:35 ` Govindraj.R
  2011-10-18 15:35 ` [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart Govindraj.R
  3 siblings, 0 replies; 15+ messages in thread
From: Govindraj.R @ 2011-10-18 15:35 UTC (permalink / raw)
  To: linux-arm-kernel

Reuse the num_uarts variable itself to count number of uarts.

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

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 55ce950..e1eba7f 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -253,15 +253,13 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata) {}
 
 static int __init omap_serial_early_init(void)
 {
-	int i = 0;
-
 	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);
+			 "uart%d", num_uarts + 1);
 		oh = omap_hwmod_lookup(oh_name);
 		if (!oh)
 			break;
@@ -271,9 +269,8 @@ static int __init omap_serial_early_init(void)
 			return -ENODEV;
 
 		uart->oh = oh;
-		uart->num = i++;
+		uart->num = num_uarts++;
 		list_add_tail(&uart->node, &uart_list);
-		num_uarts++;
 
 		/*
 		 * NOTE: omap_hwmod_setup*() has not yet been called,
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart.
  2011-10-18 15:35 [PATCH v7 15/21] OMAP2+: UART: Make the RX_TIMEOUT for DMA configurable for each UART Govindraj.R
                   ` (2 preceding siblings ...)
  2011-10-18 15:35 ` [PATCH v7 18/21] OMAP2+: UART: remove temporary variable used to count uart instance Govindraj.R
@ 2011-10-18 15:35 ` Govindraj.R
  2011-11-04 23:00   ` Kevin Hilman
  3 siblings, 1 reply; 15+ messages in thread
From: Govindraj.R @ 2011-10-18 15:35 UTC (permalink / raw)
  To: linux-arm-kernel

Omap-uart can be used as console uart to print early boot
messages using earlyprintk so for console uart prevent
hwmod reset or idling during bootup.

Identify the console_uart set the id and use the custom
pm_latency ops for console uart for the first time
to idle console uart left enabled from bootup and then enable
them back and reset pm_latency ops to default ops.

Thanks to Kevin Hilman <khilman@ti.com> for suggesting
this approach.

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

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index e1eba7f..55903f0 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -63,6 +63,7 @@ struct omap_uart_state {
 
 static LIST_HEAD(uart_list);
 static u8 num_uarts;
+static u8 console_uart_id = -1;
 
 #define DEFAULT_RXDMA_POLLRATE		1	/* RX DMA polling rate (us) */
 #define DEFAULT_RXDMA_BUFSIZE		4096	/* RX DMA buffer size */
@@ -87,6 +88,29 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
 	},
 };
 
+static int console_uart_enable_hwmod(struct omap_device *od)
+{
+	/* For early console we prevented hwmod reset and idle
+	 * So before we enable the uart clocks idle and then
+	 */
+	omap_hwmod_idle(od->hwmods[0]);
+	omap_hwmod_enable(od->hwmods[0]);
+
+	/* restore the default activate/deactivate funcs,
+	 * since now we have set the hwmod state machine right
+	 * with the idle/enable for console uart
+	 */
+	od->pm_lats = omap_uart_latency;
+
+	return 0;
+}
+
+static struct omap_device_pm_latency console_uart_latency[] = {
+	{
+		.activate_func	 = console_uart_enable_hwmod,
+	},
+};
+
 #ifdef CONFIG_PM
 static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
 {
@@ -251,12 +275,20 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
 static void omap_serial_fill_default_pads(struct omap_board_data *bdata) {}
 #endif
 
+char *cmdline_find_option(char *str)
+{
+	extern char *saved_command_line;
+
+	return strstr(saved_command_line, str);
+}
+
 static int __init omap_serial_early_init(void)
 {
 	do {
 		char oh_name[MAX_UART_HWMOD_NAME_LEN];
 		struct omap_hwmod *oh;
 		struct omap_uart_state *uart;
+		char uart_name[MAX_UART_HWMOD_NAME_LEN];
 
 		snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN,
 			 "uart%d", num_uarts + 1);
@@ -271,18 +303,23 @@ static int __init omap_serial_early_init(void)
 		uart->oh = oh;
 		uart->num = num_uarts++;
 		list_add_tail(&uart->node, &uart_list);
+		snprintf(uart_name, MAX_UART_HWMOD_NAME_LEN,
+				"%s%d", OMAP_SERIAL_NAME, uart->num);
+
+		if (cmdline_find_option(uart_name)) {
+			console_uart_id = uart->num;
+			/*
+			 * omap-uart can be used for earlyprintk logs
+			 * So if omap-uart is used as console then prevent
+			 * uart reset and idle to get logs from omap-uart
+			 * until uart console driver is available to take
+			 * care for console messages.
+			 * Idling or resetting omap-uart while printing logs
+			 * early boot logs can stall the boot-up.
+			 */
+			oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
+		}
 
-		/*
-		 * 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);
 
 	return 0;
@@ -311,6 +348,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	u32 pdata_size = 0;
 	char *name;
 	struct omap_uart_port_info omap_up;
+	struct omap_device *od;
 
 	if (WARN_ON(!bdata))
 		return;
@@ -357,6 +395,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	WARN(IS_ERR(pdev), "Could not build omap_device for %s: %s.\n",
 	     name, oh->name);
 
+	if (console_uart_id == bdata->id) {
+		od = to_omap_device(pdev);
+		od->pm_lats = console_uart_latency;
+	}
+
 	omap_device_disable_idle_on_suspend(pdev);
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
@@ -364,20 +407,6 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 
 	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_device_idle(uart->pdev);
-
-	console_unlock();
-
 	if ((cpu_is_omap34xx() || cpu_is_omap44xx()) && bdata->pads)
 		device_init_wakeup(&pdev->dev, true);
 }
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs.
  2011-10-18 15:35 ` [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs Govindraj.R
@ 2011-11-04 22:00   ` Kevin Hilman
  2011-11-07  8:39     ` Govindraj
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-11-04 22:00 UTC (permalink / raw)
  To: linux-arm-kernel

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

> The custom hwmod activate and deactivate funcs does hwmod_enable
> and idle same can be done with omap_device generic API's.
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>

This one needs a minor update for current mainline...

> ---
>  arch/arm/mach-omap2/serial.c |   18 ++----------------
>  1 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 7c65410..7658a03 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -78,24 +78,10 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
>  	},
>  };
>  
> -static int uart_idle_hwmod(struct omap_device *od)
> -{
> -	omap_hwmod_idle(od->hwmods[0]);
> -
> -	return 0;
> -}
> -
> -static int uart_enable_hwmod(struct omap_device *od)
> -{
> -	omap_hwmod_enable(od->hwmods[0]);
> -
> -	return 0;
> -}
> -
>  static struct omap_device_pm_latency omap_uart_latency[] = {
>  	{
> -		.deactivate_func = uart_idle_hwmod,
> -		.activate_func	 = uart_enable_hwmod,
> +		.activate_func	 = omap_device_enable_hwmods,
> +		.deactivate_func = omap_device_idle_hwmods,
>  		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>  	},
>  };

If we're just using default pm_latency struct, it can be passed to
omap_device_build as NULL, and a default one will be configured.

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-10-18 15:35 ` [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos Govindraj.R
@ 2011-11-04 22:42   ` Kevin Hilman
  2011-11-08  9:16     ` Rajendra Nayak
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-11-04 22:42 UTC (permalink / raw)
  To: linux-arm-kernel

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

> Omap_uart_can_sleep function blocks system wide low power state until
> uart is active remove this func and add qos requests to prevent
                ^
missing some punctuation.

> MPU from transitioning while uart is active and remove qos request
> if uart is auto-idled.

It would be helpful to summarize my previous comments[1] about why this
happens on some UARTs (PER) and not others (CORE.)  

> qos requests are blocking notifier calls so put these requests to
> work queue to avoid warn on slow path warning while using qos
> API's from runtime callbacks. 

OK.  

Rather than a vague reference to 'warn on slow path', please describe
that this is needed because the driver uses the irq_safe mode of runtime
PM, so callbacks may be run with interrupts disabled.

> Flush_sync any pending qos jobs in work queue while suspending.

Nice.

> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  arch/arm/mach-omap2/cpuidle34xx.c             |    5 ---
>  arch/arm/mach-omap2/pm24xx.c                  |    2 -
>  arch/arm/mach-omap2/pm34xx.c                  |   10 ------
>  arch/arm/mach-omap2/serial.c                  |   24 +-------------
>  arch/arm/plat-omap/include/plat/omap-serial.h |    8 +++++
>  drivers/tty/serial/omap-serial.c              |   41 ++++++++++++++++++++++++-
>  6 files changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 4bf6e6e..98b7d3f 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -226,11 +226,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  	struct omap3_idle_statedata *cx;
>  	int ret;
>  
> -	if (!omap3_can_sleep()) {
> -		new_state = dev->safe_state;
> -		goto select_state;
> -	}
> -
>  	/*
>  	 * Prevent idle completely if CAM is active.
>  	 * CAM does not have wakeup capability in OMAP3.
> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
> index a75f764..192f0a4 100644
> --- a/arch/arm/mach-omap2/pm24xx.c
> +++ b/arch/arm/mach-omap2/pm24xx.c
> @@ -248,8 +248,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())
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 6e7f276..a635fa1 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -425,21 +425,11 @@ void omap_sram_idle(void)
>  	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>  }
>  
> -int omap3_can_sleep(void)
> -{
> -	if (!omap_uart_can_sleep())
> -		return 0;
> -	return 1;
> -}
> -
>  static void omap3_pm_idle(void)
>  {
>  	local_irq_disable();
>  	local_fiq_disable();
>  
> -	if (!omap3_can_sleep())
> -		goto out;
> -
>  	if (omap_irq_pending() || need_resched())
>  		goto out;
>  
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 7658a03..55ce950 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -75,6 +75,7 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
>  		.dma_rx_poll_rate = DEFAULT_RXDMA_POLLRATE,
>  		.dma_rx_timeout = DEFAULT_RXDMA_TIMEOUT,
>  		.autosuspend_timeout = DEFAULT_AUTOSUSPEND_DELAY,
> +		.use_pm_qos = true,

You're enabling the MPU constraint for all UARTs.

However, as mentioned previously[1], due to a HW sleepdep between MPU
and CORE, this constraint isn't actually needed for CORE UARTs, so it's
a bit wasteful to go through all the constraint setting for no reason.

The changelog should summarize this, and the init code should only
enable the constraints for PER UARTs, at least on OMAP3.  I'm not sure
about OMAP4.

>  	},
>  };
>  
> @@ -87,28 +88,6 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
>  };
>  
>  #ifdef CONFIG_PM
> -
> -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;
> -}
> -
>  static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
>  {
>  	struct omap_device *od = to_omap_device(pdev);
> @@ -363,6 +342,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
>  	omap_up.dma_rx_timeout = info->dma_rx_timeout;
>  	omap_up.dma_rx_poll_rate = info->dma_rx_poll_rate;
>  	omap_up.autosuspend_timeout = info->autosuspend_timeout;
> +	omap_up.use_pm_qos = info->use_pm_qos;
>  
>  	/* Enable the MDR1 errata for OMAP3 */
>  	if (cpu_is_omap34xx() && !cpu_is_ti816x())
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 9a6879c..41eda3c 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -19,6 +19,7 @@
>  
>  #include <linux/serial_core.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_qos_params.h>
>  
>  #include <plat/mux.h>
>  
> @@ -68,6 +69,7 @@ struct omap_uart_port_info {
>  	unsigned int		dma_rx_timeout;
>  	unsigned int		autosuspend_timeout;
>  	unsigned int		dma_rx_poll_rate;
> +	u8			use_pm_qos;
>  
>  	u32 (*get_context_loss_count)(struct device *);
>  	void (*set_forceidle)(struct platform_device *);
> @@ -129,6 +131,12 @@ struct uart_omap_port {
>  	u32			context_loss_cnt;
>  	u32			errata;
>  	u8			wakeups_enabled;
> +
> +	struct pm_qos_request_list pm_qos_request;
> +	u32			latency;
> +	struct work_struct	work;

minor: call this qos_work.

> +	u8			request_qos;
> +	u8			use_pm_qos;
>  };
>  
>  #endif /* __OMAP_SERIAL_H__ */
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 1714bd2..956736c 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -51,6 +51,8 @@ static void serial_omap_rxdma_poll(unsigned long uart_no);
>  static int serial_omap_start_rxdma(struct uart_omap_port *up);
>  static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
>  
> +static struct workqueue_struct *serial_omap_uart_wq;
> +
>  static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
>  {
>  	offset <<= up->port.regshift;
> @@ -674,6 +676,21 @@ serial_omap_configure_xonxoff
>  	serial_out(up, UART_LCR, up->lcr);
>  }
>  
> +static void serial_omap_uart_qos_work(struct work_struct *work)
> +{
> +	struct uart_omap_port *up = container_of(work, struct uart_omap_port,
> +						work);
> +
> +	if (!up->request_qos) {
> +		pm_qos_add_request(&up->pm_qos_request,
> +			PM_QOS_CPU_DMA_LATENCY, up->latency);
> +		up->request_qos = true;

->request_qos is a confusing name.  ->qos_requested would help
readability, IMO.

> +	} else {
> +		pm_qos_remove_request(&up->pm_qos_request);
> +		up->request_qos = false;
> +	}
> +}
> +
>  static void
>  serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>  			struct ktermios *old)
> @@ -869,6 +886,13 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>  	serial_omap_configure_xonxoff(up, termios);
>  
>  	spin_unlock_irqrestore(&up->port.lock, flags);
> +
> +	if (up->use_pm_qos) {
> +		/* calculate wakeup latency constraint */
> +		up->latency = (1000000 * up->port.fifosize) / (1000 * baud / 8);
> +		schedule_work(&up->work);

Is this one really needed?  There's a pm_runtime_put() right after this
that will immediately remove the constraint.

Actually, it seems like it would be better to calculate the latency
earlier in this function (right after the baud rate) so that the
subsequent pm_runtime_get() sets the constraint...

> +	}
> +
>  	pm_runtime_put(&up->pdev->dev);

...and this will remove the constraint.

>  	dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
>  }
> @@ -1144,8 +1168,11 @@ static int serial_omap_suspend(struct device *dev)
>  {
>  	struct uart_omap_port *up = dev_get_drvdata(dev);
>  
> -	if (up)
> +	if (up) {
>  		uart_suspend_port(&serial_omap_reg, &up->port);
> +		flush_work_sync(&up->work);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1368,6 +1395,7 @@ static int serial_omap_probe(struct platform_device *pdev)
>  	up->port.uartclk = omap_up_info->uartclk;
>  	up->uart_dma.uart_base = mem->start;
>  	up->errata = omap_up_info->errata;
> +	up->use_pm_qos = omap_up_info->use_pm_qos;
>  
>  	if (omap_up_info->dma_enabled) {
>  		up->uart_dma.uart_dma_tx = dma_tx->start;
> @@ -1382,6 +1410,11 @@ static int serial_omap_probe(struct platform_device *pdev)
>  		up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
>  	}
>  
> +	if (up->use_pm_qos) {
> +		serial_omap_uart_wq = create_workqueue(up->name);
> +		INIT_WORK(&up->work, serial_omap_uart_qos_work);
> +	}
> +
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev,
>  			omap_up_info->autosuspend_timeout);
> @@ -1516,6 +1549,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
>  	if (up->use_dma && pdata->set_forceidle)
>  		pdata->set_forceidle(up->pdev);
>  
> +	if (up->use_pm_qos && up->request_qos)
> +		schedule_work(&up->work);
> +
>  	return 0;
>  }
>  
> @@ -1535,6 +1571,9 @@ static int serial_omap_runtime_resume(struct device *dev)
>  		/* Errata i291 */
>  		if (up->use_dma && pdata->set_noidle)
>  			pdata->set_noidle(up->pdev);
> +
> +		if (up->use_pm_qos && !up->request_qos)
> +			schedule_work(&up->work);
>  	}
>  
>  	return 0;

Kevin

[1] http://www.mail-archive.com/linux-omap at vger.kernel.org/msg57577.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart.
  2011-10-18 15:35 ` [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart Govindraj.R
@ 2011-11-04 23:00   ` Kevin Hilman
  2011-11-07  8:42     ` Govindraj
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-11-04 23:00 UTC (permalink / raw)
  To: linux-arm-kernel

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

> Omap-uart can be used as console uart to print early boot
> messages using earlyprintk so for console uart prevent
> hwmod reset or idling during bootup.
>
> Identify the console_uart set the id and use the custom
> pm_latency ops for console uart for the first time
> to idle console uart left enabled from bootup and then enable
> them back and reset pm_latency ops to default ops.
>
> Thanks to Kevin Hilman <khilman@ti.com> for suggesting
> this approach.
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  arch/arm/mach-omap2/serial.c |   79 ++++++++++++++++++++++++++++-------------
>  1 files changed, 54 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index e1eba7f..55903f0 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -63,6 +63,7 @@ struct omap_uart_state {
>  
>  static LIST_HEAD(uart_list);
>  static u8 num_uarts;
> +static u8 console_uart_id = -1;
>  
>  #define DEFAULT_RXDMA_POLLRATE		1	/* RX DMA polling rate (us) */
>  #define DEFAULT_RXDMA_BUFSIZE		4096	/* RX DMA buffer size */
> @@ -87,6 +88,29 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
>  	},
>  };
>  
> +static int console_uart_enable_hwmod(struct omap_device *od)
> +{
> +	/* For early console we prevented hwmod reset and idle
> +	 * So before we enable the uart clocks idle and then
> +	 */

minor: fix multi-line comment style (search for multi-line in
Documentation/CodingStyle).  Another one below.

Notice that you're moving existing code here, but also changing the
functionality without a description as to why.    Based on the existing code:

You need a console_lock() here...

> +	omap_hwmod_idle(od->hwmods[0]);
> +	omap_hwmod_enable(od->hwmods[0]);

...and this should be omap_device_enable().

And  you need a console_unlock() here.

Yes, you're subsequent patches avoid this problem, but we still need a real
fix other than checking for 'debug/loglevel', and when that happens,
we'll need a console lock here.

> +	/* restore the default activate/deactivate funcs,
> +	 * since now we have set the hwmod state machine right
> +	 * with the idle/enable for console uart
> +	 */
> +	od->pm_lats = omap_uart_latency;
> +
> +	return 0;
> +}
> +
> +static struct omap_device_pm_latency console_uart_latency[] = {
> +	{
> +		.activate_func	 = console_uart_enable_hwmod,
> +	},
> +};
> +
>  #ifdef CONFIG_PM
>  static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
>  {
> @@ -251,12 +275,20 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
>  static void omap_serial_fill_default_pads(struct omap_board_data *bdata) {}
>  #endif
>  
> +char *cmdline_find_option(char *str)

static inline

> +{
> +	extern char *saved_command_line;
> +
> +	return strstr(saved_command_line, str);
> +}
> +
>  static int __init omap_serial_early_init(void)
>  {
>  	do {
>  		char oh_name[MAX_UART_HWMOD_NAME_LEN];
>  		struct omap_hwmod *oh;
>  		struct omap_uart_state *uart;
> +		char uart_name[MAX_UART_HWMOD_NAME_LEN];
>  
>  		snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN,
>  			 "uart%d", num_uarts + 1);
> @@ -271,18 +303,23 @@ static int __init omap_serial_early_init(void)
>  		uart->oh = oh;
>  		uart->num = num_uarts++;
>  		list_add_tail(&uart->node, &uart_list);
> +		snprintf(uart_name, MAX_UART_HWMOD_NAME_LEN,
> +				"%s%d", OMAP_SERIAL_NAME, uart->num);
> +
> +		if (cmdline_find_option(uart_name)) {
> +			console_uart_id = uart->num;
> +			/*
> +			 * omap-uart can be used for earlyprintk logs
> +			 * So if omap-uart is used as console then prevent
> +			 * uart reset and idle to get logs from omap-uart
> +			 * until uart console driver is available to take
> +			 * care for console messages.
> +			 * Idling or resetting omap-uart while printing logs
> +			 * early boot logs can stall the boot-up.
> +			 */
> +			oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
> +		}
>  
> -		/*
> -		 * 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);
>  
>  	return 0;
> @@ -311,6 +348,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
>  	u32 pdata_size = 0;
>  	char *name;
>  	struct omap_uart_port_info omap_up;
> +	struct omap_device *od;
>  
>  	if (WARN_ON(!bdata))
>  		return;
> @@ -357,6 +395,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
>  	WARN(IS_ERR(pdev), "Could not build omap_device for %s: %s.\n",
>  	     name, oh->name);
>  
> +	if (console_uart_id == bdata->id) {
> +		od = to_omap_device(pdev);
> +		od->pm_lats = console_uart_latency;
> +	}
> +
>  	omap_device_disable_idle_on_suspend(pdev);
>  	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
>  
> @@ -364,20 +407,6 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
>  
>  	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_device_idle(uart->pdev);
> -
> -	console_unlock();
> -
>  	if ((cpu_is_omap34xx() || cpu_is_omap44xx()) && bdata->pads)
>  		device_init_wakeup(&pdev->dev, true);
>  }

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs.
  2011-11-04 22:00   ` Kevin Hilman
@ 2011-11-07  8:39     ` Govindraj
  0 siblings, 0 replies; 15+ messages in thread
From: Govindraj @ 2011-11-07  8:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Nov 5, 2011 at 3:30 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> The custom hwmod activate and deactivate funcs does hwmod_enable
>> and idle same can be done with omap_device generic API's.
>>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>
> This one needs a minor update for current mainline...
>
>> ---
>> ?arch/arm/mach-omap2/serial.c | ? 18 ++----------------
>> ?1 files changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index 7c65410..7658a03 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -78,24 +78,10 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
>> ? ? ? },
>> ?};
>>
>> -static int uart_idle_hwmod(struct omap_device *od)
>> -{
>> - ? ? omap_hwmod_idle(od->hwmods[0]);
>> -
>> - ? ? return 0;
>> -}
>> -
>> -static int uart_enable_hwmod(struct omap_device *od)
>> -{
>> - ? ? omap_hwmod_enable(od->hwmods[0]);
>> -
>> - ? ? return 0;
>> -}
>> -
>> ?static struct omap_device_pm_latency omap_uart_latency[] = {
>> ? ? ? {
>> - ? ? ? ? ? ? .deactivate_func = uart_idle_hwmod,
>> - ? ? ? ? ? ? .activate_func ? = uart_enable_hwmod,
>> + ? ? ? ? ? ? .activate_func ? = omap_device_enable_hwmods,
>> + ? ? ? ? ? ? .deactivate_func = omap_device_idle_hwmods,
>> ? ? ? ? ? ? ? .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>> ? ? ? },
>> ?};
>
> If we're just using default pm_latency struct, it can be passed to
> omap_device_build as NULL, and a default one will be configured.
>

yes, will update this.

--
Thanks,
Govindraj.R

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart.
  2011-11-04 23:00   ` Kevin Hilman
@ 2011-11-07  8:42     ` Govindraj
  0 siblings, 0 replies; 15+ messages in thread
From: Govindraj @ 2011-11-07  8:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Nov 5, 2011 at 4:30 AM, Kevin Hilman <khilman@ti.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> Omap-uart can be used as console uart to print early boot
>> messages using earlyprintk so for console uart prevent
>> hwmod reset or idling during bootup.
>>
>> Identify the console_uart set the id and use the custom
>> pm_latency ops for console uart for the first time
>> to idle console uart left enabled from bootup and then enable
>> them back and reset pm_latency ops to default ops.
>>
>> Thanks to Kevin Hilman <khilman@ti.com> for suggesting
>> this approach.
>>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>> ---
>> ?arch/arm/mach-omap2/serial.c | ? 79 ++++++++++++++++++++++++++++-------------
>> ?1 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index e1eba7f..55903f0 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -63,6 +63,7 @@ struct omap_uart_state {
>>
>> ?static LIST_HEAD(uart_list);
>> ?static u8 num_uarts;
>> +static u8 console_uart_id = -1;
>>
>> ?#define DEFAULT_RXDMA_POLLRATE ? ? ? ? ? ? ? 1 ? ? ? /* RX DMA polling rate (us) */
>> ?#define DEFAULT_RXDMA_BUFSIZE ? ? ? ? ? ? ? ?4096 ? ?/* RX DMA buffer size */
>> @@ -87,6 +88,29 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
>> ? ? ? },
>> ?};
>>
>> +static int console_uart_enable_hwmod(struct omap_device *od)
>> +{
>> + ? ? /* For early console we prevented hwmod reset and idle
>> + ? ? ?* So before we enable the uart clocks idle and then
>> + ? ? ?*/
>
> minor: fix multi-line comment style (search for multi-line in
> Documentation/CodingStyle). ?Another one below.
>

will correct this.

> Notice that you're moving existing code here, but also changing the
> functionality without a description as to why. ? ?Based on the existing code:
>
> You need a console_lock() here...
>
>> + ? ? omap_hwmod_idle(od->hwmods[0]);
>> + ? ? omap_hwmod_enable(od->hwmods[0]);
>
> ...and this should be omap_device_enable().
>
> And ?you need a console_unlock() here.
>
> Yes, you're subsequent patches avoid this problem, but we still need a real
> fix other than checking for 'debug/loglevel', and when that happens,
> we'll need a console lock here.
>

yes correct, we need console_lock/unlock binding here will update this.

--
Thanks,
Govindraj.R

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-11-04 22:42   ` Kevin Hilman
@ 2011-11-08  9:16     ` Rajendra Nayak
  2011-11-08 19:20       ` Kevin Hilman
  0 siblings, 1 reply; 15+ messages in thread
From: Rajendra Nayak @ 2011-11-08  9:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin,

On Saturday 05 November 2011 04:12 AM, Kevin Hilman wrote:
> However, as mentioned previously[1], due to a HW sleepdep between MPU
> and CORE, this constraint isn't actually needed for CORE UARTs, so it's
> a bit wasteful to go through all the constraint setting for no reason.

I had a short chat with Govind on this and was trying to understand
this better.
Are you referring to the 'autodeps' for omap3 here, because they would
prevent any clock domain from idling as long as MPU or IVA are active,
but not the other way round. Which means MPU can still idle, while CORE
does not.

My guess was, its probably the CORE domain idling itself thats causing
the UART sluggishness, (and not MPU idling), due to higher latency,
which is prevented with an active UART module in CORE, but not in PER.
So Govind did a small experiment to prevent just CORE idling and let MPU
idle alone and that did not show any sluggishness.

Now, putting a pm-qos constraint for a UART in CORE still looks
redundant because the latency requirement that UART has is in
some way *indirectly* met (because the active UART in CORE prevents
CORE transitions in idle).
But don't you think the UART driver should express its
latency constraints regardless, without thinking of any indirect ways
in which the same requirements would have already been met?

regards,
Rajendra

>
> The changelog should summarize this, and the init code should only
> enable the constraints for PER UARTs, at least on OMAP3.  I'm not sure
> about OMAP4.
>
>> >    	},
>> >    };
>> >
>> >  @@ -87,28 +88,6 @@ static struct omap_device_pm_latency omap_uart_latency[] = {
>> >    };
>> >
>> >    #ifdef CONFIG_PM
>> >  -
>> >  -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;
>> >  -}
>> >  -
>> >    static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
>> >    {
>> >    	struct omap_device *od = to_omap_device(pdev);
>> >  @@ -363,6 +342,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
>> >    	omap_up.dma_rx_timeout = info->dma_rx_timeout;
>> >    	omap_up.dma_rx_poll_rate = info->dma_rx_poll_rate;
>> >    	omap_up.autosuspend_timeout = info->autosuspend_timeout;
>> >  +	omap_up.use_pm_qos = info->use_pm_qos;
>> >
>> >    	/* Enable the MDR1 errata for OMAP3 */
>> >    	if (cpu_is_omap34xx()&&  !cpu_is_ti816x())
>> >  diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
>> >  index 9a6879c..41eda3c 100644
>> >  --- a/arch/arm/plat-omap/include/plat/omap-serial.h
>> >  +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
>> >  @@ -19,6 +19,7 @@
>> >
>> >    #include<linux/serial_core.h>
>> >    #include<linux/platform_device.h>
>> >  +#include<linux/pm_qos_params.h>
>> >
>> >    #include<plat/mux.h>
>> >
>> >  @@ -68,6 +69,7 @@ struct omap_uart_port_info {
>> >    	unsigned int		dma_rx_timeout;
>> >    	unsigned int		autosuspend_timeout;
>> >    	unsigned int		dma_rx_poll_rate;
>> >  +	u8			use_pm_qos;
>> >
>> >    	u32 (*get_context_loss_count)(struct device *);
>> >    	void (*set_forceidle)(struct platform_device *);
>> >  @@ -129,6 +131,12 @@ struct uart_omap_port {
>> >    	u32			context_loss_cnt;
>> >    	u32			errata;
>> >    	u8			wakeups_enabled;
>> >  +
>> >  +	struct pm_qos_request_list pm_qos_request;
>> >  +	u32			latency;
>> >  +	struct work_struct	work;
> minor: call this qos_work.
>
>> >  +	u8			request_qos;
>> >  +	u8			use_pm_qos;
>> >    };
>> >
>> >    #endif /* __OMAP_SERIAL_H__ */
>> >  diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> >  index 1714bd2..956736c 100644
>> >  --- a/drivers/tty/serial/omap-serial.c
>> >  +++ b/drivers/tty/serial/omap-serial.c
>> >  @@ -51,6 +51,8 @@ static void serial_omap_rxdma_poll(unsigned long uart_no);
>> >    static int serial_omap_start_rxdma(struct uart_omap_port *up);
>> >    static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
>> >
>> >  +static struct workqueue_struct *serial_omap_uart_wq;
>> >  +
>> >    static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
>> >    {
>> >    	offset<<= up->port.regshift;
>> >  @@ -674,6 +676,21 @@ serial_omap_configure_xonxoff
>> >    	serial_out(up, UART_LCR, up->lcr);
>> >    }
>> >
>> >  +static void serial_omap_uart_qos_work(struct work_struct *work)
>> >  +{
>> >  +	struct uart_omap_port *up = container_of(work, struct uart_omap_port,
>> >  +						work);
>> >  +
>> >  +	if (!up->request_qos) {
>> >  +		pm_qos_add_request(&up->pm_qos_request,
>> >  +			PM_QOS_CPU_DMA_LATENCY, up->latency);
>> >  +		up->request_qos = true;
> ->request_qos is a confusing name.  ->qos_requested would help
> readability, IMO.
>
>> >  +	} else {
>> >  +		pm_qos_remove_request(&up->pm_qos_request);
>> >  +		up->request_qos = false;
>> >  +	}
>> >  +}
>> >  +
>> >    static void
>> >    serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>> >    			struct ktermios *old)
>> >  @@ -869,6 +886,13 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>> >    	serial_omap_configure_xonxoff(up, termios);
>> >
>> >    	spin_unlock_irqrestore(&up->port.lock, flags);
>> >  +
>> >  +	if (up->use_pm_qos) {
>> >  +		/* calculate wakeup latency constraint */
>> >  +		up->latency = (1000000 * up->port.fifosize) / (1000 * baud / 8);
>> >  +		schedule_work(&up->work);
> Is this one really needed?  There's a pm_runtime_put() right after this
> that will immediately remove the constraint.
>
> Actually, it seems like it would be better to calculate the latency
> earlier in this function (right after the baud rate) so that the
> subsequent pm_runtime_get() sets the constraint...
>
>> >  +	}
>> >  +
>> >    	pm_runtime_put(&up->pdev->dev);
> ...and this will remove the constraint.
>
>> >    	dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
>> >    }
>> >  @@ -1144,8 +1168,11 @@ static int serial_omap_suspend(struct device *dev)
>> >    {
>> >    	struct uart_omap_port *up = dev_get_drvdata(dev);
>> >
>> >  -	if (up)
>> >  +	if (up) {
>> >    		uart_suspend_port(&serial_omap_reg,&up->port);
>> >  +		flush_work_sync(&up->work);
>> >  +	}
>> >  +
>> >    	return 0;
>> >    }
>> >
>> >  @@ -1368,6 +1395,7 @@ static int serial_omap_probe(struct platform_device *pdev)
>> >    	up->port.uartclk = omap_up_info->uartclk;
>> >    	up->uart_dma.uart_base = mem->start;
>> >    	up->errata = omap_up_info->errata;
>> >  +	up->use_pm_qos = omap_up_info->use_pm_qos;
>> >
>> >    	if (omap_up_info->dma_enabled) {
>> >    		up->uart_dma.uart_dma_tx = dma_tx->start;
>> >  @@ -1382,6 +1410,11 @@ static int serial_omap_probe(struct platform_device *pdev)
>> >    		up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
>> >    	}
>> >
>> >  +	if (up->use_pm_qos) {
>> >  +		serial_omap_uart_wq = create_workqueue(up->name);
>> >  +		INIT_WORK(&up->work, serial_omap_uart_qos_work);
>> >  +	}
>> >  +
>> >    	pm_runtime_use_autosuspend(&pdev->dev);
>> >    	pm_runtime_set_autosuspend_delay(&pdev->dev,
>> >    			omap_up_info->autosuspend_timeout);
>> >  @@ -1516,6 +1549,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
>> >    	if (up->use_dma&&  pdata->set_forceidle)
>> >    		pdata->set_forceidle(up->pdev);
>> >
>> >  +	if (up->use_pm_qos&&  up->request_qos)
>> >  +		schedule_work(&up->work);
>> >  +
>> >    	return 0;
>> >    }
>> >
>> >  @@ -1535,6 +1571,9 @@ static int serial_omap_runtime_resume(struct device *dev)
>> >    		/* Errata i291 */
>> >    		if (up->use_dma&&  pdata->set_noidle)
>> >    			pdata->set_noidle(up->pdev);
>> >  +
>> >  +		if (up->use_pm_qos&&  !up->request_qos)
>> >  +			schedule_work(&up->work);
>> >    	}
>> >
>> >    	return 0;
> Kevin
>
> [1]http://www.mail-archive.com/linux-omap at vger.kernel.org/msg57577.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-11-08  9:16     ` Rajendra Nayak
@ 2011-11-08 19:20       ` Kevin Hilman
  2011-11-10 12:00         ` Govindraj
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-11-08 19:20 UTC (permalink / raw)
  To: linux-arm-kernel

Rajendra Nayak <rnayak@ti.com> writes:

> Hi Kevin,
>
> On Saturday 05 November 2011 04:12 AM, Kevin Hilman wrote:
>> However, as mentioned previously[1], due to a HW sleepdep between MPU
>> and CORE, this constraint isn't actually needed for CORE UARTs, so it's
>> a bit wasteful to go through all the constraint setting for no reason.
>
> I had a short chat with Govind on this and was trying to understand
> this better.
> Are you referring to the 'autodeps' for omap3 here, because they would
> prevent any clock domain from idling as long as MPU or IVA are active,

No, I was thinking of HW sleepdeps.  However, I looked back at the
OMAP3430 TRM and see that MPU does not have a HW sleepdep on CORE like I
thought.

> but not the other way round. Which means MPU can still idle, while CORE
> does not.
>
> My guess was, its probably the CORE domain idling itself thats causing
> the UART sluggishness, (and not MPU idling), due to higher latency,
> which is prevented with an active UART module in CORE, but not in PER.

OK, that indeed makes sense.  Thanks for correcting me.

> So Govind did a small experiment to prevent just CORE idling and let MPU
> idle alone and that did not show any sluggishness.

OK, good.

> Now, putting a pm-qos constraint for a UART in CORE still looks
> redundant because the latency requirement that UART has is in
> some way *indirectly* met (because the active UART in CORE prevents
> CORE transitions in idle).
> But don't you think the UART driver should express its
> latency constraints regardless, without thinking of any indirect ways
> in which the same requirements would have already been met?

Yes, you're right.  The driver should not need to know which powerdomain
a given UART is in.  It's probably best (and most portable) to have UART
always express its requirements all the time.

Thanks for digging into this,

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-11-08 19:20       ` Kevin Hilman
@ 2011-11-10 12:00         ` Govindraj
  2011-11-10 19:02           ` Kevin Hilman
  0 siblings, 1 reply; 15+ messages in thread
From: Govindraj @ 2011-11-10 12:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin,

On Wed, Nov 9, 2011 at 12:50 AM, Kevin Hilman <khilman@ti.com> wrote:
> Rajendra Nayak <rnayak@ti.com> writes:
>
>> Hi Kevin,
>>
>> On Saturday 05 November 2011 04:12 AM, Kevin Hilman wrote:
>>> However, as mentioned previously[1], due to a HW sleepdep between MPU
>>> and CORE, this constraint isn't actually needed for CORE UARTs, so it's
>>> a bit wasteful to go through all the constraint setting for no reason.
>>
>> I had a short chat with Govind on this and was trying to understand
>> this better.
>> Are you referring to the 'autodeps' for omap3 here, because they would
>> prevent any clock domain from idling as long as MPU or IVA are active,
>
> No, I was thinking of HW sleepdeps. ?However, I looked back at the
> OMAP3430 TRM and see that MPU does not have a HW sleepdep on CORE like I
> thought.
>
>> but not the other way round. Which means MPU can still idle, while CORE
>> does not.
>>
>> My guess was, its probably the CORE domain idling itself thats causing
>> the UART sluggishness, (and not MPU idling), due to higher latency,
>> which is prevented with an active UART module in CORE, but not in PER.
>
> OK, that indeed makes sense. ?Thanks for correcting me.
>
>> So Govind did a small experiment to prevent just CORE idling and let MPU
>> idle alone and that did not show any sluggishness.
>
> OK, good.
>
>> Now, putting a pm-qos constraint for a UART in CORE still looks
>> redundant because the latency requirement that UART has is in
>> some way *indirectly* met (because the active UART in CORE prevents
>> CORE transitions in idle).
>> But don't you think the UART driver should express its
>> latency constraints regardless, without thinking of any indirect ways
>> in which the same requirements would have already been met?
>
> Yes, you're right. ?The driver should not need to know which powerdomain
> a given UART is in. ?It's probably best (and most portable) to have UART
> always express its requirements all the time.
>
> Thanks for digging into this,
>

I have fixed this and other uart_v7 comments and have re-based the
patch series on top
of 3.2-rc1 along with Tero's v9 irq chaining patches and tested the same.
Available here [1].

Can this patches series be added to a test branch for upstreaming or do you
think there are still some outstanding comments that needs to be discussed?

--
Thanks,
Govindraj.R

[1]:

git://gitorious.org/runtime_3-0/runtime_3-0.git
3.2-rc1_uart_runtime

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-11-10 12:00         ` Govindraj
@ 2011-11-10 19:02           ` Kevin Hilman
  2011-11-11 10:17             ` Govindraj
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-11-10 19:02 UTC (permalink / raw)
  To: linux-arm-kernel

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

> Hi Kevin,
>
> On Wed, Nov 9, 2011 at 12:50 AM, Kevin Hilman <khilman@ti.com> wrote:
>> Rajendra Nayak <rnayak@ti.com> writes:
>>
>>> Hi Kevin,
>>>
>>> On Saturday 05 November 2011 04:12 AM, Kevin Hilman wrote:
>>>> However, as mentioned previously[1], due to a HW sleepdep between MPU
>>>> and CORE, this constraint isn't actually needed for CORE UARTs, so it's
>>>> a bit wasteful to go through all the constraint setting for no reason.
>>>
>>> I had a short chat with Govind on this and was trying to understand
>>> this better.
>>> Are you referring to the 'autodeps' for omap3 here, because they would
>>> prevent any clock domain from idling as long as MPU or IVA are active,
>>
>> No, I was thinking of HW sleepdeps. ?However, I looked back at the
>> OMAP3430 TRM and see that MPU does not have a HW sleepdep on CORE like I
>> thought.
>>
>>> but not the other way round. Which means MPU can still idle, while CORE
>>> does not.
>>>
>>> My guess was, its probably the CORE domain idling itself thats causing
>>> the UART sluggishness, (and not MPU idling), due to higher latency,
>>> which is prevented with an active UART module in CORE, but not in PER.
>>
>> OK, that indeed makes sense. ?Thanks for correcting me.
>>
>>> So Govind did a small experiment to prevent just CORE idling and let MPU
>>> idle alone and that did not show any sluggishness.
>>
>> OK, good.
>>
>>> Now, putting a pm-qos constraint for a UART in CORE still looks
>>> redundant because the latency requirement that UART has is in
>>> some way *indirectly* met (because the active UART in CORE prevents
>>> CORE transitions in idle).
>>> But don't you think the UART driver should express its
>>> latency constraints regardless, without thinking of any indirect ways
>>> in which the same requirements would have already been met?
>>
>> Yes, you're right. ?The driver should not need to know which powerdomain
>> a given UART is in. ?It's probably best (and most portable) to have UART
>> always express its requirements all the time.
>>
>> Thanks for digging into this,
>>
>
> I have fixed this and other uart_v7 comments and have re-based the
> patch series on top
> of 3.2-rc1 along with Tero's v9 irq chaining patches and tested the same.
> Available here [1].

Please repost your updated series.

> Can this patches series be added to a test branch for upstreaming or do you
> think there are still some outstanding comments that needs to be discussed?

The PRCM IRQ chaining series is not yet finalized.

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
  2011-11-10 19:02           ` Kevin Hilman
@ 2011-11-11 10:17             ` Govindraj
  0 siblings, 0 replies; 15+ messages in thread
From: Govindraj @ 2011-11-11 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 11, 2011 at 12:32 AM, Kevin Hilman <khilman@ti.com> wrote:
> Govindraj <govindraj.ti@gmail.com> writes:
>

[..]

>>
>> I have fixed this and other uart_v7 comments and have re-based the
>> patch series on top
>> of 3.2-rc1 along with Tero's v9 irq chaining patches and tested the same.
>> Available here [1].
>
> Please repost your updated series.

Yes done. v8 posted [1]

>
>> Can this patches series be added to a test branch for upstreaming or do you
>> think there are still some outstanding comments that needs to be discussed?
>
> The PRCM IRQ chaining series is not yet finalized.

okay fine.

--
Thanks,
Govindraj.R

[1]:
http://www.spinics.net/lists/linux-omap/msg60115.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-11-11 10:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-18 15:35 [PATCH v7 15/21] OMAP2+: UART: Make the RX_TIMEOUT for DMA configurable for each UART Govindraj.R
2011-10-18 15:35 ` [PATCH v7 16/21] OMAP2+: UART: Remove custom activate funcs and use generic funcs Govindraj.R
2011-11-04 22:00   ` Kevin Hilman
2011-11-07  8:39     ` Govindraj
2011-10-18 15:35 ` [PATCH v7 17/21] OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos Govindraj.R
2011-11-04 22:42   ` Kevin Hilman
2011-11-08  9:16     ` Rajendra Nayak
2011-11-08 19:20       ` Kevin Hilman
2011-11-10 12:00         ` Govindraj
2011-11-10 19:02           ` Kevin Hilman
2011-11-11 10:17             ` Govindraj
2011-10-18 15:35 ` [PATCH v7 18/21] OMAP2+: UART: remove temporary variable used to count uart instance Govindraj.R
2011-10-18 15:35 ` [PATCH v7 19/21] OMAP2+: UART: Use custom activate func for console uart Govindraj.R
2011-11-04 23:00   ` Kevin Hilman
2011-11-07  8:42     ` Govindraj

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).