mackerel: Do not enable TMU timer in default config
Subsystems (2)
PATCH 1/1 mackerel: Do not enable TMU timer in default config
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled
---
This resolves a regression since 3.2.
I am unsure if there is a cleaner way to resolve this problem nor if other boards are affected.
--- arch/arm/configs/mackerel_defconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/configs/mackerel_defconfig +1 -0
@@ -18,6 +18,7 @@ CONFIG_ARCH_SHMOBILE=y CONFIG_ARCH_SH7372=y CONFIG_MACH_MACKEREL=y CONFIG_MEMORY_SIZE=0x10000000 +# CONFIG_SH_TIMER_TMU is not set CONFIG_AEABI=y # CONFIG_OABI_COMPAT is not set CONFIG_FORCE_MAX_ZONEORDER
Signature · 1 line
1.7.2.3
Quoted 11 lines · Simon Horman
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled
Quoted 7 lines
Signed-off-by Simon Horman <[email protected]>---
This resolves a regression since 3.2.
I am unsure if there is a cleaner way to resolve this problem nor if other boards are affected.
That's different. Do you see the start latency exceeded thing with TMU disabled? Perhaps there is a pinmux conflict between some of the channels and SCIF functions, it'd be interesting to figure out what the issue is rather than simply hiding it. You mention this as a regression, so presumably it worked ok up until some point? Perhaps something in the recent batch of sh-sci updates broke existing behaviour?
Quoted 20 lines · Paul Mundt
Quoted 11 lines · Simon Horman
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabledQuoted 7 lines
Signed-off-by Simon Horman <[email protected]>---
This resolves a regression since 3.2.
I am unsure if there is a cleaner way to resolve this problem nor if other boards are affected.
Quoted 6 lines
That's different. Do you see the start latency exceeded thing with TMU disabled? Perhaps there is a pinmux conflict between some of the channels and SCIF functions, it'd be interesting to figure out what the issue is rather than simply hiding it. You mention this as a regression, so presumably it worked ok up until some point? Perhaps something in the recent batch of sh-sci updates broke existing behaviour?
Hi Paul,
At this point I don't have any data other than that without TMU the boot is successful and with TMU the boot stops at the point indicated above.
Is there something else that I should look for?
Do you think it would be worth doing a bisection on the problem?
PATCH 1/1 mackerel: Do not enable TMU timer in default config
In reply to Simon HormanThis allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled
---
Hi Rafael,
I would appreciate it if you could consider this change for 3.3.
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
--- arch/arm/configs/mackerel_defconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/configs/mackerel_defconfig +1 -0
@@ -18,6 +18,7 @@ CONFIG_ARCH_SHMOBILE=y CONFIG_ARCH_SH7372=y CONFIG_MACH_MACKEREL=y CONFIG_MEMORY_SIZE=0x10000000 +# CONFIG_SH_TIMER_TMU is not set CONFIG_AEABI=y # CONFIG_OABI_COMPAT is not set CONFIG_FORCE_MAX_ZONEORDER
Signature · 1 line
1.7.2.3
Quoted 11 lines · Simon Horman
I would appreciate it if you could consider this change for 3.3.
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
If there is a problem with the TMU driver then it needs to be fixed, especially if this is a regression. We aren't going to paper over real bugs by just disabling the offending driver in the defconfig, all it does is encourage people to not fix the problem later, and the same workaround can be done locally if there's no desire to actually hunt down and fix the regression.
Quoted 12 lines · Paul Mundt
Quoted 11 lines · Simon Horman
I would appreciate it if you could consider this change for 3.3.
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
Quoted 6 lines
If there is a problem with the TMU driver then it needs to be fixed, especially if this is a regression. We aren't going to paper over real bugs by just disabling the offending driver in the defconfig, all it does is encourage people to not fix the problem later, and the same workaround can be done locally if there's no desire to actually hunt down and fix the regression.
There still is some time to final 3.3, so let's try to fix this properly and see how intrusive that's going to be after all.
I'll look into this tomorrow.
Thanks,
Rafael
Quoted 19 lines · Simon Horman
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabledSigned-off-by Simon Horman <[email protected]>---
Hi Rafael,
I would appreciate it if you could consider this change for 3.3.
Quoted 8 lines
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
From my inspection of the sh_tmu driver it looks like that driver doesn't support power management and the device it handles isn't taken into account in our PM domains configuration (it belongs to the A4R domain, so that domain can't be powered off while the driver is present).
For this reason, I propose to apply the appended change (which reflects the current status of the driver AFAICS) for now and revisit the driver at one point in future.
Thanks,
Rafael
--- arch/arm/mach-shmobile/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/arch/arm/mach-shmobile/Kconfig =================================--- linux.orig/arch/arm/mach-shmobile/Kconfig +++ linux/arch/arm/mach-shmobile/Kconfig
@@ -151,7 +151,7 @@ config SH_TIMER_CMT
config SH_TIMER_TMU
bool "TMU timer driver"
- default y
+ depends on !PM
help
This enables build of the TMU timer driver.
Quoted 29 lines · Rafael J. Wysocki
Quoted 27 lines · Simon Horman
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabledSigned-off-by Simon Horman <[email protected]>---
Hi Rafael,
I would appreciate it if you could consider this change for 3.3.
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
Quoted 8 lines
Quoted 1 line
From my inspection of the
sh_tmudriver it looks like that driver doesn'tsupport power management and the device it handles isn't taken into account in our PM domains configuration (it belongs to the A4R domain, so that domain can't be powered off while the driver is present).
For this reason, I propose to apply the appended change (which reflects the current status of the driver AFAICS) for now and revisit the driver at one point in future.
Hi Rafael,
thanks for taking time to look into this.
Your proposal seems good to me.
Quoted 6 lines · Rafael J. Wysocki
Quoted 4 lines · Simon Horman
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
Quoted 9 lines
From my inspection of the
sh_tmudriver it looks like that driver doesn't support power management and the device it handles isn't taken into account in our PM domains configuration (it belongs to the A4R domain, so that domain can't be powered off while the driver is present).For this reason, I propose to apply the appended change (which reflects the current status of the driver AFAICS) for now and revisit the driver at one point in future.
This is just more band-aiding, and is functionally no different from disabling the TMU driver in the defconfig.
No, the TMU driver doesn't support power management, as support was never added. Presently none of the sh clocksource/clockevent drivers do, and we're certainly not going to be doing a !PM dependency for those, either.
At present we do have the clock framework interaction, but there are also fundamental ordering issues there with regards to use as an early timer (something I don't expect the power domains code has any concept of, either). If the device needs to be included in the A4R domain then patches to setup-sh7372.c doing that are fine, but I'm not interested in symptom patches that only attempt to sweep the problem under the rug.
Note that none of the MTU2/CMT/TMU drivers have been touched in ages, so the idea that regressions were introduced by any of them or that they're doing something wrong now is utterly absurd. If the power domain code wants to change the rules, that's fine, but disabling stuff randomly that you or Magnus failed to take in to account at the time is not.
The options as I see it are any of:
- Fix the driver(s) (possibly not much time left now, due to all of the time wasted on trying to fix the symptoms instead of the problem).
- Ensure the TMU bits are represented in the appropriate power domain.
- Revert whatever change prevented PM && TMU from booting when it was something that always worked before. It can be re-applied when the change is done in less of a half-assed fashion.
Quoted 30 lines · Paul Mundt
Quoted 6 lines · Rafael J. Wysocki
Quoted 4 lines · Simon Horman
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
Quoted 9 lines
From my inspection of the
sh_tmudriver it looks like that driver doesn't support power management and the device it handles isn't taken into account in our PM domains configuration (it belongs to the A4R domain, so that domain can't be powered off while the driver is present).For this reason, I propose to apply the appended change (which reflects the current status of the driver AFAICS) for now and revisit the driver at one point in future.
This is just more band-aiding, and is functionally no different from disabling the TMU driver in the defconfig.
No, the TMU driver doesn't support power management, as support was never added. Presently none of the sh clocksource/clockevent drivers do, and we're certainly not going to be doing a !PM dependency for those, either.
At present we do have the clock framework interaction, but there are also fundamental ordering issues there with regards to use as an early timer (something I don't expect the power domains code has any concept of, either). If the device needs to be included in the A4R domain then patches to
setup-sh7372.cdoing that are fine, but I'm not interested in symptom patches that only attempt to sweep the problem under the rug.
Quoted 5 lines
Note that none of the MTU2/CMT/TMU drivers have been touched in ages, so the idea that regressions were introduced by any of them or that they're doing something wrong now is utterly absurd. If the power domain code wants to change the rules, that's fine, but disabling stuff randomly that you or Magnus failed to take in to account at the time is not.
I'm not talking about the power domains code, but of the PM code in general.
Actually, I can make it work with runtime PM (to some extent at least), but that won't be enough, because it doesn't handle system suspend/resume correctly.
Quoted 5 lines
The options as I see it are any of:
- Fix the driver(s) (possibly not much time left now, due to all of the time wasted on trying to fix the symptoms instead of the problem).
That is clearly the way to go, but not before 3.3 is out. And it is not sufficient, because the TMU bits have to be represented in the appropriate PM domain as you correctly noted below.
Quoted 2 lines
- Ensure the TMU bits are represented in the appropriate power domain.
The two above together.
Quoted 2 lines
- Revert whatever change prevented PM && TMU from booting when it was something that always worked before.
That's changes that were merged a couple of kernel releases before, so I don't think we can do that realistically. If you can give me some advice we can actually use, I'll appreciate that very much.
And it boots along with PM if the early_platform_init part is removed from it just fine, but then it doesn't work with PM, so it's not very useful when PM is enabled anyway.
Quoted 2 lines
It can be re-applied when the change is done in less of a half-assed fashion.
Well, I think the technical discussion ends here.
Thank you for your kind response.
Rafael
Quoted 39 lines · Simon Horman
Quoted 29 lines · Rafael J. Wysocki
Quoted 27 lines · Simon Horman
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabledSigned-off-by Simon Horman <[email protected]>---
Hi Rafael,
I would appreciate it if you could consider this change for 3.3.
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
Quoted 8 lines
Quoted 1 line
From my inspection of the
sh_tmudriver it looks like that driver doesn'tsupport power management and the device it handles isn't taken into account in our PM domains configuration (it belongs to the A4R domain, so that domain can't be powered off while the driver is present).
For this reason, I propose to apply the appended change (which reflects the current status of the driver AFAICS) for now and revisit the driver at one point in future.
Quoted 6 lines
Hi Rafael,
thanks for taking time to look into this.
Your proposal seems good to me.Tested-by Simon Horman <[email protected]>
However, having looked at both sh_tmu and sh_cmt I don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang with sh_tmu enabled go away. So perhaps sh_tmu is just a messenger here and the real problem is with sh-sci.
I think that more investigation is needed.
Thanks,
Rafael
Quoted 47 lines · Rafael J. Wysocki
Quoted 39 lines · Simon Horman
Quoted 29 lines · Rafael J. Wysocki
Quoted 27 lines · Simon Horman
This allows the mackerel to boot using a kernel compiled using the mackerel default config. Without this change the kernel boot hangs during or immediately after SCIF initialisation.
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5416 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabledSigned-off-by Simon Horman <[email protected]>---
Hi Rafael,
I would appreciate it if you could consider this change for 3.3.
It is a band-aid solution, however, it resolves a regression - the boot or lack thereof of a mackerel board using the default config - since 3.2.
I have discussed this a little with Magnus Damm and he is of the opinion that this problem likely relates to an interaction between TMU and power domains and that a full fix would be difficult within the 3.3 time-frame.
Quoted 8 lines
Quoted 1 line
From my inspection of the
sh_tmudriver it looks like that driver doesn'tsupport power management and the device it handles isn't taken into account in our PM domains configuration (it belongs to the A4R domain, so that domain can't be powered off while the driver is present).
For this reason, I propose to apply the appended change (which reflects the current status of the driver AFAICS) for now and revisit the driver at one point in future.
Quoted 6 lines
Hi Rafael,
thanks for taking time to look into this.
Your proposal seems good to me.Tested-by Simon Horman <[email protected]>
Quoted 8 lines
However, having looked at both
sh_tmuandsh_cmtI don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang withsh_tmuenabled go away. So perhapssh_tmuis just a messenger here and the real problem is with sh-sci.I think that more investigation is needed.
Hi Rafael,
thanks. Please let me know if you would like anything tested with the caveat that I will be on vacation next week.
Quoted 8 lines · Rafael J. Wysocki
However, having looked at both
sh_tmuandsh_cmtI don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang withsh_tmuenabled go away. So perhapssh_tmuis just a messenger here and the real problem is with sh-sci.I think that more investigation is needed.
Do you see any different behaviour with early printk enabled vs disabled? We did have the ordering issues before with the pm calls hanging or oopsing in the early path, but all of those should have been fixed already. There have been quite a few sh-sci changes though, so if it worked previously it should be bisectable at least.
Quoted 7 lines · Paul Mundt
Quoted 6 lines · Rafael J. Wysocki
However, having looked at both
sh_tmuandsh_cmtI don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang withsh_tmuenabled go away. So perhapssh_tmuis just a messenger here and the real problem is with sh-sci.
Quoted 7 lines
Quoted 2 lines
I think that more investigation is needed.
Do you see any different behaviour with early printk enabled vs disabled? We did have the ordering issues before with the pm calls hanging or oopsing in the early path, but all of those should have been fixed already. There have been quite a few sh-sci changes though, so if it worked previously it should be bisectable at least.
Hi Paul, Hi Rafael,
I asked Hiep-san (CCed) to see if there was any difference booting a 3.3-rc3 on a Mackerel without earlyprintk in the kernel commandline. I tested 3.3-rc5 as well. There does not seem to be any change in behaviour, other than that specificly relating the bootconsole.
Without earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 6916 ns console [ttySC0] enabled
[no more output]
With earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5917 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled [no more output]
Quoted 31 lines · Simon Horman
Quoted 7 lines · Paul Mundt
Quoted 6 lines · Rafael J. Wysocki
However, having looked at both
sh_tmuandsh_cmtI don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang withsh_tmuenabled go away. So perhapssh_tmuis just a messenger here and the real problem is with sh-sci.Quoted 7 lines
Quoted 2 lines
I think that more investigation is needed.
Do you see any different behaviour with early printk enabled vs disabled? We did have the ordering issues before with the pm calls hanging or oopsing in the early path, but all of those should have been fixed already. There have been quite a few sh-sci changes though, so if it worked previously it should be bisectable at least.
Hi Paul, Hi Rafael,
I asked Hiep-san (CCed) to see if there was any difference booting a 3.3-rc3 on a Mackerel without earlyprintk in the kernel commandline. I tested 3.3-rc5 as well. There does not seem to be any change in behaviour, other than that specificly relating the bootconsole.
Without earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 6916 ns console [ttySC0] enabled
[no more output]
Quoted 8 lines
With earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5917 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled [no more output]
OK, I know what the problem is.
The enabling of sh_tmu apparently triggers the dev_warn() in GENPD_DEV_TIMED_CALLBACK() for sh-sci.0, which leads to a deadlock in the runtime PM core, because synchronous runtime resume for the device is then run from its own runtime suspend callback.
I have an idea how to fix this, but it's going to take a couple of days due to my time constraints.
Thanks,
Rafael
Quoted 43 lines · Rafael J. Wysocki
Quoted 31 lines · Simon Horman
Quoted 7 lines · Paul Mundt
Quoted 6 lines · Rafael J. Wysocki
However, having looked at both
sh_tmuandsh_cmtI don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang withsh_tmuenabled go away. So perhapssh_tmuis just a messenger here and the real problem is with sh-sci.Quoted 7 lines
Quoted 2 lines
I think that more investigation is needed.
Do you see any different behaviour with early printk enabled vs disabled? We did have the ordering issues before with the pm calls hanging or oopsing in the early path, but all of those should have been fixed already. There have been quite a few sh-sci changes though, so if it worked previously it should be bisectable at least.
Hi Paul, Hi Rafael,
I asked Hiep-san (CCed) to see if there was any difference booting a 3.3-rc3 on a Mackerel without earlyprintk in the kernel commandline. I tested 3.3-rc5 as well. There does not seem to be any change in behaviour, other than that specificly relating the bootconsole.
Without earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 6916 ns console [ttySC0] enabled
[no more output]Quoted 8 lines
With earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5917 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled [no more output]OK, I know what the problem is.
Quoted 7 lines
The enabling of
sh_tmuapparently triggers thedev_warn()inGENPD_DEV_TIMED_CALLBACK()for sh-sci.0, which leads to a deadlock in the runtime PM core, because synchronous runtime resume for the device is then run from its own runtime suspend callback.I have an idea how to fix this, but it's going to take a couple of days due to my time constraints.
Thanks Rafael,
please don't hesitate to let me know if you need more testing done.
Quoted 49 lines · Simon Horman
Quoted 48 lines · Rafael J. Wysocki
Quoted 31 lines · Simon Horman
Quoted 7 lines · Paul Mundt
Quoted 6 lines · Rafael J. Wysocki
However, having looked at both
sh_tmuandsh_cmtI don't see why the former should break things (during boot), while the latter doesn't, so I've done some more testing and, surprisingly enough, it turns out that disabling runtime PM in sh-sci makes the boot hang withsh_tmuenabled go away. So perhapssh_tmuis just a messenger here and the real problem is with sh-sci.Quoted 7 lines
Quoted 2 lines
I think that more investigation is needed.
Do you see any different behaviour with early printk enabled vs disabled? We did have the ordering issues before with the pm calls hanging or oopsing in the early path, but all of those should have been fixed already. There have been quite a few sh-sci changes though, so if it worked previously it should be bisectable at least.
Hi Paul, Hi Rafael,
I asked Hiep-san (CCed) to see if there was any difference booting a 3.3-rc3 on a Mackerel without earlyprintk in the kernel commandline. I tested 3.3-rc5 as well. There does not seem to be any change in behaviour, other than that specificly relating the bootconsole.
Without earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 6916 ns console [ttySC0] enabled
[no more output]Quoted 8 lines
With earlyprintk
...
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO0xe6c40000(irq = 80) is a scifa sh-sci sh-sci.0: start latency exceeded, new value 5917 ns console [ttySC0] enabled, bootconsole disabled console [ttySC0] enabled, bootconsole disabled [no more output]OK, I know what the problem is.
The enabling of
sh_tmuapparently triggers thedev_warn()inGENPD_DEV_TIMED_CALLBACK()for sh-sci.0, which leads to a deadlock in the runtime PM core, because synchronous runtime resume for the device is then run from its own runtime suspend callback.
Quoted 6 lines
Quoted 2 lines
I have an idea how to fix this, but it's going to take a couple of days due to my time constraints.
Thanks Rafael,
please don't hesitate to let me know if you need more testing done.
Below is a patch that fixes the boot problem for me. Please test if it works for you too.
Unfortunately, we can't detect that deadlock at the core level (at least I'm not aware of any generally reliable method of doing that) so it has to be taken care of by the driver. The alternative would be to remove all diagnostic messages from runtime PM callbacks that may be used in a console runtime resume code path, but I think that would be overkill (those messages are actually useful for other types of devices).
However, this patch only addresses the boot issue, but then, if sh_tmu is enabled, runtime PM involving the A4R domain and system-level PM lead to nasty crashes. Since Paul doesn't like the idea of enabling sh_tmu only if PM is not enabled, I propose to make it actively fail all PM operations that would lead to the removal of power from TMU devices. This is done in a second patch that will be sent in a reply to this message.
Thanks,
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: sh-sci / PM: Avoid deadlocking runtime PM
The runtime PM of sh-sci devices is enabled when sci_probe() returns, so the pm_runtime_put_sync() executed by driver_probe_device() attempts to suspend the device. Then, in some situations, a diagnostic message is printed to the console by one of the runtime suspend routines handling the sh-sci device, which causes synchronous runtime resume to be started from the device's own runtime suspend callback. This causes rpm_resume() to be run eventually, which sees the RPM_SUSPENDING status set by rpm_suspend() and waits for it to change. However, the device's runtime PM status cannot change at that point, because the routine that has set it waits for the rpm_suspend() to return. A deadlock occurs as a result.
To avoid that make sci_init_single() increment the device's runtime PM usage counter, so that it cannot be suspended by driver_probe_device(). That counter has to be decremented eventually, so make sci_startup() do that before starting to actually use the device and make sci_shutdown() increment it again before returning to balance the incrementation carried out by sci_startup().
--- drivers/tty/serial/sh-sci.c | 5 +++++ 1 file changed, 5 insertions(+)
Index: linux/drivers/tty/serial/sh-sci.c =================================--- linux.orig/drivers/tty/serial/sh-sci.c +++ linux/drivers/tty/serial/sh-sci.c
@@ -1710,6 +1710,8 @@ static int sci_startup(struct uart_port
dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
+ pm_runtime_put_noidle(port->dev);
+
sci_port_enable(s);
ret = sci_request_irq(s);
@@ -1737,6 +1739,8 @@ static void sci_shutdown(struct uart_por
sci_free_irq(s);
sci_port_disable(s);
+
+ pm_runtime_get_noresume(port->dev);
}
static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
@@ -2075,6 +2079,7 @@ static int __devinit sci_init_single(str
sci_init_gpios(sci_port);
pm_runtime_irq_safe(&dev->dev);
+ pm_runtime_get_noresume(&dev->dev);
pm_runtime_enable(&dev->dev);
}