[ This series is targeted for merging during v3.3-rc ]
On v3.3-rc1, the OMAP serial console doesn't behave properly when
power management is enabled (the default with omap2plus_defconfig).
This seems to be due to a combination of a silicon bug in the UART IP
block and broken FIFO settings in the OMAP serial driver.
This patch series attempts to work around these problems. The workarounds
are not perfect; see the patch descriptions for more details.
Tested on OMAP3530 BeagleBoard. This series will need to be tested on
OMAP2 and OMAP4 before it is ready to merge.
- Paul
---
Paul Walmsley (2):
tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA mode
tty: serial: OMAP: transmit FIFO threshold interrupts don't wake the chip
arch/arm/plat-omap/include/plat/omap-serial.h | 1
drivers/tty/serial/omap-serial.c | 86 ++++++++++++++++++++++++-
2 files changed, 82 insertions(+), 5 deletions(-)
Ensure FIFO levels are set correctly in non-DMA mode (the default).
This patch will cause a receive FIFO threshold interrupt to be raised when
there is at least one byte in the RX FIFO. It will also cause a transmit
FIFO threshold interrupt when there is only one byte remaining in the TX
FIFO.
These changes fix the receive interrupt problem and part of the
transmit interrupt problem. A separate set of issues must be worked
around for the transmit path to have a basic level of functionality; a
subsequent patch will address these.
DMA operation is unaffected by this patch.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <redacted>
Cc: Govindraj Raja <redacted>
Cc: Kevin Hilman <redacted>
---
drivers/tty/serial/omap-serial.c | 35 +++++++++++++++++++++++++++++++----
1 files changed, 31 insertions(+), 4 deletions(-)
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.
DMA operation is unaffected by this patch.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <redacted>
Cc: Govindraj Raja <redacted>
Cc: Kevin Hilman <redacted>
---
arch/arm/plat-omap/include/plat/omap-serial.h | 1
drivers/tty/serial/omap-serial.c | 51 +++++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletions(-)
On Sat, Jan 21, 2012 at 12:27:37AM -0700, Paul Walmsley wrote:
[ This series is targeted for merging during v3.3-rc ]
On v3.3-rc1, the OMAP serial console doesn't behave properly when
power management is enabled (the default with omap2plus_defconfig).
This seems to be due to a combination of a silicon bug in the UART IP
block and broken FIFO settings in the OMAP serial driver.
This patch series attempts to work around these problems. The workarounds
are not perfect; see the patch descriptions for more details.
Tested on OMAP3530 BeagleBoard. This series will need to be tested on
OMAP2 and OMAP4 before it is ready to merge.
Who is going to do that testing? When? Why isn't it done already?
I guess this means I can ignore them for now, right?
greg k-h
On Sat, Jan 21, 2012 at 12:27:37AM -0700, Paul Walmsley wrote:
quoted
[ This series is targeted for merging during v3.3-rc ]
On v3.3-rc1, the OMAP serial console doesn't behave properly when
power management is enabled (the default with omap2plus_defconfig).
This seems to be due to a combination of a silicon bug in the UART IP
block and broken FIFO settings in the OMAP serial driver.
This patch series attempts to work around these problems. The workarounds
are not perfect; see the patch descriptions for more details.
Tested on OMAP3530 BeagleBoard. This series will need to be tested on
OMAP2 and OMAP4 before it is ready to merge.
Who is going to do that testing?
I will do it, although I do cling to the hope that others will help.
When?
I'll do it within the next two or three days, unless others beat me to it.
Why isn't it done already?
That's always the question about testing, isn't it?
In fact, now that you mention it, why haven't these patches been tested
yet on every possible combination of OMAP SoC and console UART?
Unbelievable, how negligent people are.
I guess this means I can ignore them for now, right?
Yes. I'll follow up with a pull request (or revisions) after I've tested
them on some OMAP2 and OMAP4 boards.
regards
- Paul
I will do it, although I do cling to the hope that others will help.
I always work on the basis that if its had some coverage testing then the
other folks who might have been affected should have reviewed the patch.
They opted not to, within reason - their lookout.
They'll only test it later when it his the tree if they don't do it
earlier 8)
Alan
[ This series is targeted for merging during v3.3-rc ]
On v3.3-rc1, the OMAP serial console doesn't behave properly when
power management is enabled (the default with omap2plus_defconfig).
This seems to be due to a combination of a silicon bug in the UART IP
block and broken FIFO settings in the OMAP serial driver.
This patch series attempts to work around these problems. The workarounds
are not perfect; see the patch descriptions for more details.
Tested on OMAP3530 BeagleBoard. This series will need to be tested on
OMAP2 and OMAP4 before it is ready to merge.
Just did a quick test on an OMAP4430 ES2 Pandaboard and the
(OMAP2420-based) N800. This series has no obvious effect on either N800
or OMAP4430 Pandaboard, aside from the expected increase in interrupts in
the non-CPUIdle case. Probably this is due to the relative immaturity of
the PM code on those two chip families.
- Paul
From: Tomi Valkeinen <hidden> Date: 2012-01-23 08:15:22
On Sat, 2012-01-21 at 00:27 -0700, Paul Walmsley wrote:
[ This series is targeted for merging during v3.3-rc ]
On v3.3-rc1, the OMAP serial console doesn't behave properly when
power management is enabled (the default with omap2plus_defconfig).
This seems to be due to a combination of a silicon bug in the UART IP
block and broken FIFO settings in the OMAP serial driver.
This patch series attempts to work around these problems. The workarounds
are not perfect; see the patch descriptions for more details.
Tested on OMAP3530 BeagleBoard. This series will need to be tested on
OMAP2 and OMAP4 before it is ready to merge.
On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley [off-list ref] 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.
--
Thanks,
Govindraj.R
quoted hunk
DMA operation is unaffected by this patch.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <redacted>
Cc: Govindraj Raja <redacted>
Cc: Kevin Hilman <redacted>
---
?arch/arm/plat-omap/include/plat/omap-serial.h | ? ?1
?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ? 51 +++++++++++++++++++++++++
?2 files changed, 51 insertions(+), 1 deletions(-)
? ? ? ?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
@@ -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
On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley [off-list ref] wrote:
quoted
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.
If you re-read the patch description that you quoted above, you'll see
that this behavior was clearly mentioned:
quoted
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.
/ # cat /proc/interrupts | grep "UART2"
74: 3902 INTC OMAP UART2
/ #
[...]
without these two patches + cpu_idle enabled
You should compare apples to apples. The omap-serial.c without these two
patches is unusable as a console when CONFIG_CPU_IDLE=n; there are long
lags between transmit FIFO drains. CONFIG_CPU_IDLE is the setting in the
standard omap2plus_defconfig.
I guess it was never tested with that standard config? Or were the
v3.3 omap-serial patch series submitted with this bug known?
The only reason why the driver 'works' when CONFIG_CPU_IDLE=y is because
the omap-serial.c RX path wakeup latency constraint calculation is broken.
It adds a 1 microsecond latency constraint to the CPU, when it should be
adding a ~ 1100 microsecond latency constraint to the CPU. (Well, ~ 5500
microsecond, but that's another issue.)
And this keeps the CPU from entering a low-power state not just during
transmits, but during the entire time the driver is active for the console
UART.
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.
Yes, that's known behavior, as described in the quoted patch description.
It's not clear why the number of interrupts is ~5x the number of
transmitted bytes, rather than say 2x the number of transmitted bytes.
But the IRQ handler in omap-serial.c is also a mess, so that may have
something to do with it.
Do you have a better workaround for the CONFIG_CPU_IDLE=n case that
is acceptable for the -rc series? If so, perhaps you can post it?
- Paul
On Mon, Jan 23, 2012 at 2:20 PM, Govindraj [off-list ref] wrote:
On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley [off-list ref] wrote:
quoted
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
quoted
DMA operation is unaffected by this patch.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <redacted>
Cc: Govindraj Raja <redacted>
Cc: Kevin Hilman <redacted>
---
?arch/arm/plat-omap/include/plat/omap-serial.h | ? ?1
?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ? 51 +++++++++++++++++++++++++
?2 files changed, 51 insertions(+), 1 deletions(-)
? ? ? ?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
@@ -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
On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley [off-list ref] wrote:
quoted hunk
Ensure FIFO levels are set correctly in non-DMA mode (the default).
This patch will cause a receive FIFO threshold interrupt to be raised when
there is at least one byte in the RX FIFO. ?It will also cause a transmit
FIFO threshold interrupt when there is only one byte remaining in the TX
FIFO.
These changes fix the receive interrupt problem and part of the
transmit interrupt problem. ?A separate set of issues must be worked
around for the transmit path to have a basic level of functionality; a
subsequent patch will address these.
DMA operation is unaffected by this patch.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <redacted>
Cc: Govindraj Raja <redacted>
Cc: Kevin Hilman <redacted>
---
?drivers/tty/serial/omap-serial.c | ? 35 +++++++++++++++++++++++++++++++----
?1 files changed, 31 insertions(+), 4 deletions(-)
On Mon, Jan 23, 2012 at 3:51 PM, Govindraj [off-list ref] wrote:
On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley [off-list ref] wrote:
quoted
Ensure FIFO levels are set correctly in non-DMA mode (the default).
This patch will cause a receive FIFO threshold interrupt to be raised when
there is at least one byte in the RX FIFO. ?It will also cause a transmit
FIFO threshold interrupt when there is only one byte remaining in the TX
FIFO.
These changes fix the receive interrupt problem and part of the
transmit interrupt problem. ?A separate set of issues must be worked
around for the transmit path to have a basic level of functionality; a
subsequent patch will address these.
DMA operation is unaffected by this patch.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <redacted>
Cc: Govindraj Raja <redacted>
Cc: Kevin Hilman <redacted>
---
?drivers/tty/serial/omap-serial.c | ? 35 +++++++++++++++++++++++++++++++----
?1 files changed, 31 insertions(+), 4 deletions(-)