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

"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]),

So either this get isn't needed, because there is no device access in
this path, or this path is broken and needs validation.

quoted

+ ? ? }
+
? ? ? ui[pdev->id] = up;
? ? ? serial_omap_add_console_port(up);

@@ -1312,6 +1380,7 @@ static int serial_omap_probe(struct platform_device *pdev)

? ? ? if (ret != 0)
? ? ? ? ? ? ? goto do_release_region;

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

...also, this put is not balanced with a corresponding get.

this needs to happen only when runtime is enabled.

will correct this.

--
Thanks,
Govindraj.,R