From mboxrd@z Thu Jan 1 00:00:00 1970 From: govindraj.ti@gmail.com (Govindraj) Date: Mon, 23 Jan 2012 15:20:16 +0530 Subject: [PATCH 2/2] tty: serial: OMAP: transmit FIFO threshold interrupts don't wake the chip In-Reply-To: References: <20120121071934.18707.64258.stgit@dusk> <20120121072740.18707.87632.stgit@dusk> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Jan 23, 2012 at 2:20 PM, Govindraj wrote: > On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley wrote: >> It seems that when the transmit FIFO threshold is reached on OMAP >> UARTs, it does not result in a PRCM wakeup. ?This appears to be a >> silicon bug. ?This means that if the MPU powerdomain is in a low-power >> state, the MPU will not be awakened to refill the FIFO until the next >> interrupt from another device. >> >> The best solution, at least for the short term, would be for the OMAP >> serial driver to call a OMAP subarchitecture function to prevent the >> MPU powerdomain from entering a low power state while the FIFO has >> data to transmit. ?However, we no longer have a clean way to do this, >> since patches that add platform_data function pointers have been >> deprecated by the OMAP maintainer. ?So we attempt to work around this >> as well. ?The workarounds depend on the setting of CONFIG_CPU_IDLE. >> >> When CONFIG_CPU_IDLE=n, the driver will now only transmit one byte at >> a time. ?This causes the transmit FIFO threshold interrupt to stay >> active until there is no more data to be sent. ?Thus, the MPU >> powerdomain stays on during transmits. ?Aside from that energy >> consumption penalty, each transmitted byte results in a huge number of >> UART interrupts -- about five per byte. ?This wastes CPU time and is >> quite inefficient, but is probably the most expedient workaround in >> this case. >> >> When CONFIG_CPU_IDLE=y, there is a slightly more direct workaround: >> the PM QoS constraint can be abused to keep the MPU powerdomain on. >> This results in a normal number of interrupts, but, similar to the >> above workaround, wastes power by preventing the MPU from entering >> WFI. >> >> Future patches are planned for the 3.4 merge window to implement more >> efficient, but also more disruptive, workarounds to these problems. >> > > With these two patches number of interrupts seems to increase by 24x > after boot up. > > [...] > > / # cat /proc/interrupts | grep "UART2" > ?74: ? ? ? 3902 ? ? ?INTC ?OMAP UART2 > / # > [...] > > without these two patches + cpu_idle enabled > > [...] > > / # > / # cat /proc/interrupts | grep "UART2" > ?74: ? ? ? ?158 ? ? ?INTC ?OMAP UART2 > / # > > [...] > > > I am using beagle xm and 3.3rc1 > > Looks like there are far two many uart irqs which > keeps the mpu busy and thus preventing uart sluggishness. > after looking in closely, looks like every byte is getting transferred in case of non cpu_idle cases causing 1 irq for each tx. > -- > Thanks, > Govindraj.R > > >> DMA operation is unaffected by this patch. >> >> Signed-off-by: Paul Walmsley >> Cc: Tomi Valkeinen >> Cc: Govindraj Raja >> Cc: Kevin Hilman >> --- >> ?arch/arm/plat-omap/include/plat/omap-serial.h | ? ?1 >> ?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ? 51 +++++++++++++++++++++++++ >> ?2 files changed, 51 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h >> index 9ff4444..12a64eb 100644 >> --- a/arch/arm/plat-omap/include/plat/omap-serial.h >> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h >> @@ -131,6 +131,7 @@ struct uart_omap_port { >> ? ? ? ?u32 ? ? ? ? ? ? ? ? ? ? context_loss_cnt; >> ? ? ? ?u32 ? ? ? ? ? ? ? ? ? ? errata; >> ? ? ? ?u8 ? ? ? ? ? ? ? ? ? ? ?wakeups_enabled; >> + ? ? ? u8 ? ? ? ? ? ? ? ? ? ? ?max_tx_count; >> >> ? ? ? ?struct pm_qos_request ? pm_qos_request; >> ? ? ? ?u32 ? ? ? ? ? ? ? ? ? ? latency; >> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c >> index 9de7d71..621fde5 100644 >> --- a/drivers/tty/serial/omap-serial.c >> +++ b/drivers/tty/serial/omap-serial.c >> @@ -88,6 +88,49 @@ static inline void serial_omap_clear_fifos(struct uart_omap_port *up) >> ? ? ? ?serial_out(up, UART_FCR, 0); >> ?} >> >> +/** >> + * serial_omap_block_cpu_low_power_state - prevent MPU pwrdm from leaving ON >> + * @up: struct uart_omap_port * >> + * >> + * Prevent the MPU powerdomain from entering a power state lower than >> + * ON. ?(It should be sufficient to prevent it from entering INACTIVE, >> + * but there is presently no easy way to do this.) ?This works around >> + * a suspected silicon bug in the OMAP UART IP blocks. ?The UARTs should >> + * wake the PRCM when the transmit FIFO threshold interrupt is raised, but >> + * they do not. ? See also serial_omap_allow_cpu_low_power_state(). ?No >> + * return value. >> + */ >> +static void serial_omap_block_cpu_low_power_state(struct uart_omap_port *up) >> +{ >> +#ifdef CONFIG_CPU_IDLE >> + ? ? ? up->latency = 1; >> + ? ? ? schedule_work(&up->qos_work); >> +#else >> + ? ? ? up->max_tx_count = 1; >> +#endif >> +} >> + >> +/** >> + * serial_omap_allow_cpu_low_power_state - remove power state restriction on MPU >> + * @up: struct uart_omap_port * >> + * >> + * Cancel the effects of serial_omap_block_cpu_low_power_state(). >> + * This should allow the MPU powerdomain to enter a power state lower >> + * than ON, assuming the rest of the kernel is not restricting it. >> + * This works around a suspected silicon bug in the OMAP UART IP >> + * blocks. ?The UARTs should wake the PRCM when the transmit FIFO >> + * threshold interrupt is raised, but they do not. ?No return value. >> + */ >> +static void serial_omap_allow_cpu_low_power_state(struct uart_omap_port *up) >> +{ >> +#ifdef CONFIG_CPU_IDLE >> + ? ? ? up->latency = up->calc_latency; >> + ? ? ? schedule_work(&up->qos_work); >> +#else >> + ? ? ? up->max_tx_count = up->port.fifosize / 4; >> +#endif >> +} >> + >> ?/* >> ?* serial_omap_get_divisor - calculate divisor value >> ?* @port: uart port info >> @@ -163,6 +206,9 @@ static void serial_omap_stop_tx(struct uart_port *port) >> ? ? ? ? ? ? ? ?serial_out(up, UART_IER, up->ier); >> ? ? ? ?} >> >> + ? ? ? if (!up->use_dma) >> + ? ? ? ? ? ? ? serial_omap_allow_cpu_low_power_state(up); >> + >> ? ? ? ?pm_runtime_mark_last_busy(&up->pdev->dev); >> ? ? ? ?pm_runtime_put_autosuspend(&up->pdev->dev); >> ?} >> @@ -264,7 +310,7 @@ static void transmit_chars(struct uart_omap_port *up) >> ? ? ? ? ? ? ? ?serial_omap_stop_tx(&up->port); >> ? ? ? ? ? ? ? ?return; >> ? ? ? ?} >> - ? ? ? count = up->port.fifosize / 4; >> + ? ? ? count = up->max_tx_count; >> ? ? ? ?do { >> ? ? ? ? ? ? ? ?serial_out(up, UART_TX, xmit->buf[xmit->tail]); >> ? ? ? ? ? ? ? ?xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1); >> @@ -297,6 +343,7 @@ static void serial_omap_start_tx(struct uart_port *port) >> >> ? ? ? ?if (!up->use_dma) { >> ? ? ? ? ? ? ? ?pm_runtime_get_sync(&up->pdev->dev); >> + ? ? ? ? ? ? ? serial_omap_block_cpu_low_power_state(up); >> ? ? ? ? ? ? ? ?serial_omap_enable_ier_thri(up); >> ? ? ? ? ? ? ? ?pm_runtime_mark_last_busy(&up->pdev->dev); >> ? ? ? ? ? ? ? ?pm_runtime_put_autosuspend(&up->pdev->dev); >> @@ -1421,6 +1468,8 @@ static int serial_omap_probe(struct platform_device *pdev) >> ? ? ? ?up->port.fifosize = 64; >> ? ? ? ?up->port.ops = &serial_omap_pops; >> >> + ? ? ? up->max_tx_count = up->port.fifosize / 4; >> + >> ? ? ? ?if (pdev->dev.of_node) >> ? ? ? ? ? ? ? ?up->port.line = of_alias_get_id(pdev->dev.of_node, "serial"); >> ? ? ? ?else >> >> >> -- >> 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