ARM: OMAP2: UART: fix console UART mismatched runtime PM status

PATCH New Kevin Hilman 4 participants 7 messages Oct 15, 2012 – Oct 17, 2012

Subsystems (3)

Arm (32-bit) Arm sub-architectures OMAP2+ support

lore

Share & tools

lore

Kevin Hilman

author

2012-10-15 23:49 UTC #0 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

From: Kevin Hilman <[email protected]>

The runtime PM framework assumes that the hardware state of devices when initialized is disabled. For all omap_devices, we idle/disable device by default. However, the console uart uses a "no idle" option during omap_device init in order to allow earlyprintk usage to work seamlessly during boot.

Because the hardware is left partially enabled after init (whatever the bootloader settings were), the omap_device should later be fully initialized (including mux) and the runtime PM framework should be told that the device is active, and not disabled so that the hardware state is in sync with runtime PM state.

To fix, after the device has been created/registered, call omap_device_enable() to finialize init and use pm_runtime_set_active() to tell the runtime PM core the device is enabled.

Tested on 2420/n810, 3530/Overo, 3530/Beagle, 3730/OveroSTORM, 3730/Beagle-xM, 4460/PandaES.

Reported-by Paul Walmsley <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Sourav Poddar <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

---
Applies against v3.7-rc1.
Fix targetted for v3.7.

I'm still not entirely sure why this has worked up to now on OMAP3/4 but not on OMAP2. Even so, this fix is needed for all platforms to ensure matching hardware state and runtime PM state.

 arch/arm/mach-omap2/serial.c |    5 +++++
 1 file changed, 5 insertions(+)

arch/arm/mach-omap2/serial.c +5 -0

index 0405c81..37b5dbe 100644
@@ -327,6 +327,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	if ((console_uart_id == bdata->id) && no_console_suspend)
 		omap_device_disable_idle_on_suspend(pdev);
 
+	if (console_uart_id == bdata->id) {
+		omap_device_enable(pdev);
+		pm_runtime_set_active(&pdev->dev);
+	}
+
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
 	oh->dev_attr = uart;

Signature · 1 line

1.7.9.2

Tony Lindgren

maintainer

2012-10-16 00:01 UTC #1 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

Quoted 18 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>
The runtime PM framework assumes that the hardware state of devices when initialized is disabled. For all omap_devices, we idle/disable device by default. However, the console uart uses a "no idle" option during omap_device init in order to allow earlyprintk usage to work seamlessly during boot.
Because the hardware is left partially enabled after init (whatever the bootloader settings were), the omap_device should later be fully initialized (including mux) and the runtime PM framework should be told that the device is active, and not disabled so that the hardware state is in sync with runtime PM state.
To fix, after the device has been created/registered, call omap_device_enable() to finialize init and use pm_runtime_set_active() to tell the runtime PM core the device is enabled.

Quoted 8 lines

Tested on 2420/n810, 3530/Overo, 3530/Beagle, 3730/OveroSTORM, 3730/Beagle-xM, 4460/PandaES.
Reported-by Paul Walmsley <[email protected]>
Suggested-by Russell King <[email protected]>
Cc Felipe Balbi <[email protected]>
Cc Sourav Poddar <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>

Seems to work for my n800:

Tested-by Tony Lindgren <[email protected]>

Felipe Balbi

2012-10-16 10:25 UTC #2 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

In reply to Kevin Hilman

Hi,

Quoted 43 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>
The runtime PM framework assumes that the hardware state of devices when initialized is disabled. For all omap_devices, we idle/disable device by default. However, the console uart uses a "no idle" option during omap_device init in order to allow earlyprintk usage to work seamlessly during boot.
Because the hardware is left partially enabled after init (whatever the bootloader settings were), the omap_device should later be fully initialized (including mux) and the runtime PM framework should be told that the device is active, and not disabled so that the hardware state is in sync with runtime PM state.
To fix, after the device has been created/registered, call omap_device_enable() to finialize init and use pm_runtime_set_active() to tell the runtime PM core the device is enabled.
Tested on 2420/n810, 3530/Overo, 3530/Beagle, 3730/OveroSTORM, 3730/Beagle-xM, 4460/PandaES.
Reported-by Paul Walmsley <[email protected]>
Suggested-by Russell King <[email protected]>
Cc Felipe Balbi <[email protected]>
Cc Sourav Poddar <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
Applies against v3.7-rc1.
Fix targetted for v3.7.
I'm still not entirely sure why this has worked up to now on OMAP3/4 but not on OMAP2. Even so, this fix is needed for all platforms to ensure matching hardware state and runtime PM state.
 arch/arm/mach-omap2/serial.c |    5 +++++
 1 file changed, 5 insertions(+)
arch/arm/mach-omap2/serial.c +0 -0
index 0405c81..37b5dbe 100644
@@ -327,6 +327,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	if ((console_uart_id == bdata->id) && no_console_suspend)

arch/arm/mach-omap2/serial.c void __init omap_serial_init_port(struct omap_board_data *bdata, · line 337

Quoted 10 lines

arch/arm/mach-omap2/serial.c +5 -0

 		omap_device_disable_idle_on_suspend(pdev);
 
+	if (console_uart_id == bdata->id) {
+		omap_device_enable(pdev);
+		pm_runtime_set_active(&pdev->dev);
+	}
+
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
 	oh->dev_attr = uart;

looks good to me. Even took care of doing that only for the console UART.

Reviewed-by Felipe Balbi <[email protected]>

sorry for the issues caused.

Signature · 8 lines

balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121016/7f249c3b/attachment.sig>

Tony Lindgren

maintainer

2012-10-16 21:18 UTC #3 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

Quoted 55 lines · Felipe Balbi

Hi,
Quoted 52 lines · Kevin Hilman
From: Kevin Hilman <[email protected]>
The runtime PM framework assumes that the hardware state of devices when initialized is disabled. For all omap_devices, we idle/disable device by default. However, the console uart uses a "no idle" option during omap_device init in order to allow earlyprintk usage to work seamlessly during boot.
Because the hardware is left partially enabled after init (whatever the bootloader settings were), the omap_device should later be fully initialized (including mux) and the runtime PM framework should be told that the device is active, and not disabled so that the hardware state is in sync with runtime PM state.
To fix, after the device has been created/registered, call omap_device_enable() to finialize init and use pm_runtime_set_active() to tell the runtime PM core the device is enabled.
Tested on 2420/n810, 3530/Overo, 3530/Beagle, 3730/OveroSTORM, 3730/Beagle-xM, 4460/PandaES.
Reported-by Paul Walmsley <[email protected]>
Suggested-by Russell King <[email protected]>
Cc Felipe Balbi <[email protected]>
Cc Sourav Poddar <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
Applies against v3.7-rc1.
Fix targetted for v3.7.
I'm still not entirely sure why this has worked up to now on OMAP3/4 but not on OMAP2. Even so, this fix is needed for all platforms to ensure matching hardware state and runtime PM state.
 arch/arm/mach-omap2/serial.c |    5 +++++
 1 file changed, 5 insertions(+)
arch/arm/mach-omap2/serial.c +5 -0
index 0405c81..37b5dbe 100644
@@ -327,6 +327,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	if ((console_uart_id == bdata->id) && no_console_suspend)
 		omap_device_disable_idle_on_suspend(pdev);
 
+	if (console_uart_id == bdata->id) {
+		omap_device_enable(pdev);
+		pm_runtime_set_active(&pdev->dev);
+	}
+
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 

Quoted 4 lines

arch/arm/mach-omap2/serial.c void __init omap_serial_init_port(struct omap_board_data *bdata, · line 337
Quoted 1 line
arch/arm/mach-omap2/serial.c +0 -0
 	oh->dev_attr = uart;
looks good to me. Even took care of doing that only for the console UART.

Applying into omap-for-v3.7-rc1/fixes-take3.

Regards,

Tony

Kevin Hilman

author

2012-10-17 01:03 UTC #4 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

Quoted 56 lines · Tony Lindgren

Quoted 55 lines · Felipe Balbi
Hi,
Quoted 52 lines · Kevin Hilman
From: Kevin Hilman <[email protected]>
The runtime PM framework assumes that the hardware state of devices when initialized is disabled. For all omap_devices, we idle/disable device by default. However, the console uart uses a "no idle" option during omap_device init in order to allow earlyprintk usage to work seamlessly during boot.
Because the hardware is left partially enabled after init (whatever the bootloader settings were), the omap_device should later be fully initialized (including mux) and the runtime PM framework should be told that the device is active, and not disabled so that the hardware state is in sync with runtime PM state.
To fix, after the device has been created/registered, call omap_device_enable() to finialize init and use pm_runtime_set_active() to tell the runtime PM core the device is enabled.
Tested on 2420/n810, 3530/Overo, 3530/Beagle, 3730/OveroSTORM, 3730/Beagle-xM, 4460/PandaES.
Reported-by Paul Walmsley <[email protected]>
Suggested-by Russell King <[email protected]>
Cc Felipe Balbi <[email protected]>
Cc Sourav Poddar <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
Applies against v3.7-rc1.
Fix targetted for v3.7.
I'm still not entirely sure why this has worked up to now on OMAP3/4 but not on OMAP2. Even so, this fix is needed for all platforms to ensure matching hardware state and runtime PM state.
 arch/arm/mach-omap2/serial.c |    5 +++++
 1 file changed, 5 insertions(+)
arch/arm/mach-omap2/serial.c +5 -0
index 0405c81..37b5dbe 100644
@@ -327,6 +327,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	if ((console_uart_id == bdata->id) && no_console_suspend)
 		omap_device_disable_idle_on_suspend(pdev);
 
+	if (console_uart_id == bdata->id) {
+		omap_device_enable(pdev);
+		pm_runtime_set_active(&pdev->dev);
+	}
+
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 

Quoted 7 lines

Quoted 4 lines
arch/arm/mach-omap2/serial.c void __init omap_serial_init_port(struct omap_board_data *bdata, · line 337
Quoted 1 line
arch/arm/mach-omap2/serial.c +0 -0
 	oh->dev_attr = uart;
looks good to me. Even took care of doing that only for the console UART.
Applying into omap-for-v3.7-rc1/fixes-take3.

Doh, this isn't quite right yet. Some more PM testing shows that UART wakeups are broken, so this isn't ready for v3.7 yet.

To be continued...

Kevin

Tony Lindgren

maintainer

2012-10-17 02:14 UTC #5 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

Quoted 3 lines · Kevin Hilman

Doh, this isn't quite right yet. Some more PM testing shows that UART wakeups are broken, so this isn't ready for v3.7 yet.

OK I'll drop it then.

Tony

Russell King - ARM Linux

2012-10-17 08:28 UTC #6 lore raw

PATCH 1/1 ARM: OMAP2: UART: fix console UART mismatched runtime PM status

In reply to Kevin Hilman

Quoted 14 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>
The runtime PM framework assumes that the hardware state of devices when initialized is disabled. For all omap_devices, we idle/disable device by default. However, the console uart uses a "no idle" option during omap_device init in order to allow earlyprintk usage to work seamlessly during boot.
Because the hardware is left partially enabled after init (whatever the bootloader settings were), the omap_device should later be fully initialized (including mux) and the runtime PM framework should be told that the device is active, and not disabled so that the hardware state is in sync with runtime PM state.

Quoted 3 lines

To fix, after the device has been created/registered, call omap_device_enable() to finialize init and use pm_runtime_set_active() to tell the runtime PM core the device is enabled.

I still believe you're better off with the approach I gave. Why? Because every driver you have contains virtually the same sequence of runtime PM "initialization" which is to runtime PM enable the device and then do a get on it.

Why not do that in bus code if all your drivers are doing the same thing, and kill off a bunch of code in the drivers?

It's fairly easy to do with the BUS_NOTIFY_BIND_DRIVER / BUS_NOTIFY_UNBIND_DRIVER bus level notifies, and you've already hooked the notifier list for these callbacks in omap_device.c