Govindraj [off-list ref] writes:

On Fri, Oct 14, 2011 at 2:52 AM, Kevin Hilman [off-list ref] wrote:

quoted

Govindraj [off-list ref] writes:

quoted

On Thu, Oct 13, 2011 at 5:36 AM, Kevin Hilman [off-list ref] wrote:

quoted

"Govindraj.R" [off-list ref] writes:

[...]

quoted

Use device_may_wakeup to check whether uart has wakeup capabilities
and then enable uart runtime usage for the uart.

Curious about what happens when device_may_wakeup() is not set during
device init.

[...]

quoted

@@ -1305,6 +1363,16 @@ static int serial_omap_probe(struct platform_device *pdev)

? ? ? ? ? ? ? up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
? ? ? }

+ ? ? pm_runtime_use_autosuspend(&pdev->dev);
+ ? ? pm_runtime_set_autosuspend_delay(&pdev->dev,
+ ? ? ? ? ? ? ? ? ? ? OMAP_UART_AUTOSUSPEND_DELAY);
+
+ ? ? pm_runtime_irq_safe(&pdev->dev);
+ ? ? if (device_may_wakeup(&pdev->dev)) {
+ ? ? ? ? ? ? pm_runtime_enable(&pdev->dev);

So if device_may_wakeup() is false, runtime PM is not enabled, then...

quoted

+ ? ? ? ? ? ? pm_runtime_get_sync(&pdev->dev);

...this get doesn't happen, and the first register access causes a crash.

Actually no crash, clocks will left enabled from boot up (hwmod_no_reset/idle)
that are idled and enabled back here.

Since hwmod_idle is binded here later ([PATCH v6 15/16]),

IMO, That's not a very maintainable solution.

What happens when when someone fixes serial.c to only set
HWMOD_INIT_NO_IDLE on the console UART? ?or if we fix things so we don't
need INIT_NO_IDLE anymore? ?Then this will crash.

Driver code should not make assumptions like this about what device init
code is or isn't doing.

Okay, How about doing as below ensuring get_sync always
and forbid runtime to avoid clock_gating if wakeup is not set.

This is better, I'm not crazy about that either.

Consider a platform that doesn't have wakeups, but the UARTs are not
used (at least initially.)   That means on power up, all the UARTs are
left enabled to consume power even when they're not used.

Kevin