[PATCH] mackerel: Do not enable TMU timer in default config

[PATCH] mackerel: Do not enable TMU timer in default config

[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 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

Signed-off-by: Simon Horman <horms@verge.net.au>

---

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(-)

diff --git a/arch/arm/configs/mackerel_defconfig b/arch/arm/configs/mackerel_defconfig
index 306a2e2..41d014e 100644
--- a/arch/arm/configs/mackerel_defconfig
+++ b/arch/arm/configs/mackerel_defconfig
@@ -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\x15
-- 
1.7.2.3

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Paul Mundt]

On Mon, Feb 13, 2012 at 02:36:17PM +0900, Simon Horman wrote:
> 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
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> 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?

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Simon Horman]

On Tue, Feb 14, 2012 at 08:33:04AM +0900, Paul Mundt wrote:
> On Mon, Feb 13, 2012 at 02:36:17PM +0900, Simon Horman wrote:
> > 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
> > 
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > 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?

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] mackerel: Do not enable TMU timer in default config

[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 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

Signed-off-by: Simon Horman <horms@verge.net.au>

---

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(-)

diff --git a/arch/arm/configs/mackerel_defconfig b/arch/arm/configs/mackerel_defconfig
index 306a2e2..41d014e 100644
--- a/arch/arm/configs/mackerel_defconfig
+++ b/arch/arm/configs/mackerel_defconfig
@@ -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\x15
-- 
1.7.2.3

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Paul Mundt]

On Thu, Feb 23, 2012 at 07:06:41PM +0900, Simon Horman wrote:
> 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.

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Rafael J. Wysocki]

On Thursday, February 23, 2012, Paul Mundt wrote:
> On Thu, Feb 23, 2012 at 07:06:41PM +0900, Simon Horman wrote:
> > 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.

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

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Rafael J. Wysocki]

On Thursday, February 23, 2012, Simon Horman wrote:
> 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
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> 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.

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.
 

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Simon Horman]

On Sun, Feb 26, 2012 at 11:57:01PM +0100, Rafael J. Wysocki wrote:
> On Thursday, February 23, 2012, Simon Horman wrote:
> > 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
> > 
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > 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.
> 
> >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.

Hi Rafael,

thanks for taking time to look into this.
Your proposal seems good to me.

Tested-by: Simon Horman <horms@verge.net.au>

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Paul Mundt]

On Sun, Feb 26, 2012 at 11:57:01PM +0100, Rafael J. Wysocki wrote:
> On Thursday, February 23, 2012, Simon Horman wrote:
> > 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.
> 
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.

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Rafael J. Wysocki]

On Monday, February 27, 2012, Paul Mundt wrote:
> On Sun, Feb 26, 2012 at 11:57:01PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, February 23, 2012, Simon Horman wrote:
> > > 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.
> > 
> 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.

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.

> 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.

> 	- Ensure the TMU bits are represented in the appropriate power
> 	  domain.

The two above together.

> 	- 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.

> 	  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

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Rafael J. Wysocki]

On Monday, February 27, 2012, Simon Horman wrote:
> On Sun, Feb 26, 2012 at 11:57:01PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, February 23, 2012, Simon Horman wrote:
> > > 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
> > > 
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > 
> > > ---
> > > 
> > > 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.
> > 
> > >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.
> 
> Hi Rafael,
> 
> thanks for taking time to look into this.
> Your proposal seems good to me.
> 
> Tested-by: Simon Horman <horms@verge.net.au>

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

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Simon Horman]

On Tue, Feb 28, 2012 at 02:23:45AM +0100, Rafael J. Wysocki wrote:
> On Monday, February 27, 2012, Simon Horman wrote:
> > On Sun, Feb 26, 2012 at 11:57:01PM +0100, Rafael J. Wysocki wrote:
> > > On Thursday, February 23, 2012, Simon Horman wrote:
> > > > 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
> > > > 
> > > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > > 
> > > > ---
> > > > 
> > > > 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.
> > > 
> > > >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.
> > 
> > Hi Rafael,
> > 
> > thanks for taking time to look into this.
> > Your proposal seems good to me.
> > 
> > Tested-by: Simon Horman <horms@verge.net.au>
> 
> 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.

Hi Rafael,

thanks. Please let me know if you would like anything tested
with the caveat that I will be on vacation next week.

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Paul Mundt]

On Tue, Feb 28, 2012 at 02:23:45AM +0100, Rafael J. Wysocki wrote:
> 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.
> 
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.

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Simon Horman]

On Tue, Feb 28, 2012 at 03:09:09PM +0900, Paul Mundt wrote:
> On Tue, Feb 28, 2012 at 02:23:45AM +0100, Rafael J. Wysocki wrote:
> > 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.
> > 
> 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]

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Rafael J. Wysocki]

On Tuesday, February 28, 2012, Simon Horman wrote:
> On Tue, Feb 28, 2012 at 03:09:09PM +0900, Paul Mundt wrote:
> > On Tue, Feb 28, 2012 at 02:23:45AM +0100, Rafael J. Wysocki wrote:
> > > 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.
> > > 
> > 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]

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

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Simon Horman]

On Wed, Feb 29, 2012 at 12:59:36AM +0100, Rafael J. Wysocki wrote:
> On Tuesday, February 28, 2012, Simon Horman wrote:
> > On Tue, Feb 28, 2012 at 03:09:09PM +0900, Paul Mundt wrote:
> > > On Tue, Feb 28, 2012 at 02:23:45AM +0100, Rafael J. Wysocki wrote:
> > > > 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.
> > > > 
> > > 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]
> 
> 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,

please don't hesitate to let me know if you need more testing done.

Re: [PATCH] mackerel: Do not enable TMU timer in default config

[Rafael J. Wysocki]

On Wednesday, February 29, 2012, Simon Horman wrote:
> On Wed, Feb 29, 2012 at 12:59:36AM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, February 28, 2012, Simon Horman wrote:
> > > On Tue, Feb 28, 2012 at 03:09:09PM +0900, Paul Mundt wrote:
> > > > On Tue, Feb 28, 2012 at 02:23:45AM +0100, Rafael J. Wysocki wrote:
> > > > > 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.
> > > > > 
> > > > 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]
> > 
> > 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,
> 
> 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 <rjw@sisk.pl>
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().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 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);
 	}