[3/6] 2.6.21-rc4: known regressions

[3/6] 2.6.21-rc4: known regressions

[Adrian Bunk]

This email lists some known regressions in Linus' tree compared to 2.6.20.

If you find your name in the Cc header, you are either submitter of one
of the bugs, maintainer of an affectected subsystem or driver, a patch
of you caused a breakage or I'm considering you in any other way
possibly involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.


Subject    : ThinkPad X60: resume no longer works  (PCI related?)
References : http://lkml.org/lkml/2007/3/13/3
Submitter  : Dave Jones <davej@redhat.com>
             Jeremy Fitzhardinge <jeremy@goop.org>
Caused-By  : PCI merge
             commit 78149df6d565c36675463352d0bfe0000b02b7a7
Handled-By : Eric W. Biederman <ebiederm@xmission.com>
             Rafael J. Wysocki <rjw@sisk.pl>
Status     : problem is being debugged


Subject    : ThinkPad doesn't resume from suspend to RAM
References : http://lkml.org/lkml/2007/2/27/80
             http://lkml.org/lkml/2007/2/28/348
Submitter  : Jens Axboe <jens.axboe@oracle.com>
             Jeff Chua <jeff.chua.linux@gmail.com>
Status     : unknown


Subject    : suspend to disk hangs
References : http://lkml.org/lkml/2007/3/6/142
Submitter  : Jeff Chua <jeff.chua.linux@gmail.com>
Status     : unknown


Subject    : suspend to disk hangs
References : http://lkml.org/lkml/2007/3/16/126
Submitter  : Maxim Levitsky <maximlevitsky@gmail.com>
Status     : unknown


Subject    : suspend to disk hangs
References : http://bugzilla.kernel.org/show_bug.cgi?id=8224
Submitter  : Mike Harris <atarimike@wavecable.com>
Status     : unknown


Subject    : ThinkPad R60: suspend broken
References : http://lkml.org/lkml/2007/3/16/57
Submitter  : Marcus Better <marcus@better.se>
Status     : unknown


Subject    : laptop immediately resumes after suspend
References : http://lkml.org/lkml/2007/3/8/469
Submitter  : Ray Lee <ray-lk@madrabbit.org>
Caused-By  : Alexey Starikovskiy <alexey.y.starikovskiy@linux.intel.com>
             commit ed41dab90eb40ac4911e60406bc653661f0e4ce1
Handled-By : Len Brown <lenb@kernel.org>
Patch      : http://lkml.org/lkml/2007/3/12/228
Status     : patch available


Subject    : second suspend to disk in a row results in an oops  (libata?)
References : http://lkml.org/lkml/2007/3/17/43
Submitter  : Thomas Meyer <thomas@m3y3r.de>
Status     : unknown


Subject    : SATA breakage on resume
References : http://lkml.org/lkml/2007/3/7/233
Submitter  : Thomas Gleixner <tglx@linutronix.de>
             Soeren Sonnenburg <kernel@nn7.de>
Status     : unknown

Re: [3/6] 2.6.21-rc4: known regressions

[Marcus Better]

[-- Attachment #1.1: Type: text/plain, Size: 353 bytes --]

> Subject    : ThinkPad R60: suspend broken
> References : http://lkml.org/lkml/2007/3/16/57
> Submitter  : Marcus Better <marcus@better.se>
> Status     : unknown

FWIW, it's still broken as of commit cd05a1f818073a623455a58e756c5b419fc98db9.

The in-kernel suspend to disk or RAM both hang after "Suspending consoles", 
with the fan stopped.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [3/6] 2.6.21-rc4: known regressions

[Jeff Chua]

On 3/19/07, Adrian Bunk <bunk@stusta.de> wrote:

> Subject    : ThinkPad doesn't resume from suspend to RAM
> References : http://lkml.org/lkml/2007/2/27/80
>              http://lkml.org/lkml/2007/2/28/348
> Submitter  : Jens Axboe <jens.axboe@oracle.com>
>              Jeff Chua <jeff.chua.linux@gmail.com>
> Status     : unknown
>
>
> Subject    : suspend to disk hangs
> References : http://lkml.org/lkml/2007/3/6/142
> Submitter  : Jeff Chua <jeff.chua.linux@gmail.com>
> Status     : unknown

The good news is on 2.6.21-rc5, suspend to disk, and resume from disk works.
But, it only works with CONFIG_NO_HZ unset.

Setting CONFIG_NO_HZ will cause suspend to disk to hang just before
saving memory to disk.

Resume from RAM (s2ram) still broke (tried with or without
CONFIG_NO_HZ). Suspend to RAM seems ok, but upon resume, the screen
will only display "inu" and only after pressing the power button will
the system return to console. But "date" still doesn't advance.

Thanks,
Jeff.

Re: [3/6] 2.6.21-rc4: known regressions

[Adrian Bunk]

On Mon, Mar 26, 2007 at 09:25:51AM +0800, Jeff Chua wrote:
> On 3/19/07, Adrian Bunk <bunk@stusta.de> wrote:
> 
> >Subject    : ThinkPad doesn't resume from suspend to RAM
> >References : http://lkml.org/lkml/2007/2/27/80
> >             http://lkml.org/lkml/2007/2/28/348
> >Submitter  : Jens Axboe <jens.axboe@oracle.com>
> >             Jeff Chua <jeff.chua.linux@gmail.com>
> >Status     : unknown
> >
> >
> >Subject    : suspend to disk hangs
> >References : http://lkml.org/lkml/2007/3/6/142
> >Submitter  : Jeff Chua <jeff.chua.linux@gmail.com>
> >Status     : unknown
> 
> The good news is on 2.6.21-rc5, suspend to disk, and resume from disk works.
> But, it only works with CONFIG_NO_HZ unset.

Thanks for this information.

> Setting CONFIG_NO_HZ will cause suspend to disk to hang just before
> saving memory to disk.

Thanks, noted.

> Resume from RAM (s2ram) still broke (tried with or without
> CONFIG_NO_HZ). Suspend to RAM seems ok, but upon resume, the screen
> will only display "inu" and only after pressing the power button will
> the system return to console. But "date" still doesn't advance.

This might be related to the following regression:

Subject    : first disk access after resume takes several minutes
             ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
References : http://lkml.org/lkml/2007/3/8/117
             http://lkml.org/lkml/2007/3/25/20
Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
Handled-By : Thomas Gleixner <tglx@linutronix.de>
             Ingo Molnar <mingo@elte.hu>
Status     : problem is being debugged

> Thanks,
> Jeff.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [3/6] 2.6.21-rc4: known regressions

[Jeff Chua]

On 3/26/07, Adrian Bunk <bunk@stusta.de> wrote:

> > Resume from RAM (s2ram) still broke (tried with or without
> > CONFIG_NO_HZ). Suspend to RAM seems ok, but upon resume, the screen
> > will only display "inu" and only after pressing the power button will
> > the system return to console. But "date" still doesn't advance.
>
> This might be related to the following regression:
>
> Subject    : first disk access after resume takes several minutes
>              ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
> References : http://lkml.org/lkml/2007/3/8/117
>              http://lkml.org/lkml/2007/3/25/20
> Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
> Handled-By : Thomas Gleixner <tglx@linutronix.de>
>              Ingo Molnar <mingo@elte.hu>
> Status     : problem is being debugged

Adrian,

It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
suspend and resume from RAM (s2ram). Even better, it works
with/without CONFIG_NO_HZ.

But, suspend to disk still broke with CONFIG_NO_HZ set.

Thanks,
Jeff.

Re: [3/6] 2.6.21-rc4: known regressions

[Thomas Gleixner]

On Mon, 2007-03-26 at 13:37 +0800, Jeff Chua wrote:
> On 3/26/07, Adrian Bunk <bunk@stusta.de> wrote:
> 
> > > Resume from RAM (s2ram) still broke (tried with or without
> > > CONFIG_NO_HZ). Suspend to RAM seems ok, but upon resume, the screen
> > > will only display "inu" and only after pressing the power button will
> > > the system return to console. But "date" still doesn't advance.
> >
> > This might be related to the following regression:
> >
> > Subject    : first disk access after resume takes several minutes
> >              ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
> > References : http://lkml.org/lkml/2007/3/8/117
> >              http://lkml.org/lkml/2007/3/25/20
> > Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
> > Handled-By : Thomas Gleixner <tglx@linutronix.de>
> >              Ingo Molnar <mingo@elte.hu>
> > Status     : problem is being debugged
> 
> Adrian,
> 
> It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
> suspend and resume from RAM (s2ram). Even better, it works
> with/without CONFIG_NO_HZ.

Does the patch below fix the HPET_TIMER=y case ?

	tglx

diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index f3ab61e..76afea6 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -197,7 +197,7 @@ static int hpet_next_event(unsigned long delta,
 	cnt += delta;
 	hpet_writel(cnt, HPET_T0_CMP);
 
-	return ((long)(hpet_readl(HPET_COUNTER) - cnt ) > 0);
+	return ((long)(hpet_readl(HPET_COUNTER) - cnt ) > 0) ? -ETIME : 0;
 }
 
 /*

Re: [3/6] 2.6.21-rc4: known regressions

[Jeff Chua]

On 3/27/07, Thomas Gleixner <tglx@linutronix.de> wrote:

> > It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
> > suspend and resume from RAM (s2ram). Even better, it works
> > with/without CONFIG_NO_HZ.
>
> Does the patch below fix the HPET_TIMER=y case ?

Thomas, I tried, but it didn't help. Upon resume from ram, "date"
still didn't advance.

Thanks,
Jeff.

Re: [3/6] 2.6.21-rc4: known regressions

[Thomas Gleixner]

On Tue, 2007-03-27 at 01:46 +0800, Jeff Chua wrote:
> On 3/27/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > > It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
> > > suspend and resume from RAM (s2ram). Even better, it works
> > > with/without CONFIG_NO_HZ.
> >
> > Does the patch below fix the HPET_TIMER=y case ?
> 
> Thomas, I tried, but it didn't help. Upon resume from ram, "date"
> still didn't advance.

Can you please issue a SysRq-Q in this situation and provide the dmesg
output ?

Thanks,

	tglx

Re: [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Wednesday 28 March 2007 09:04:28 Thomas Gleixner wrote:
> On Tue, 2007-03-27 at 01:46 +0800, Jeff Chua wrote:
> > On 3/27/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > > It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
> > > > suspend and resume from RAM (s2ram). Even better, it works
> > > > with/without CONFIG_NO_HZ.
> > >
> > > Does the patch below fix the HPET_TIMER=y case ?
> > 
> > Thomas, I tried, but it didn't help. Upon resume from ram, "date"
> > still didn't advance.
> 
> Can you please issue a SysRq-Q in this situation and provide the dmesg
> output ?
> 
> Thanks,
> 
> 	tglx
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Hi,
	I almost sure Iknow why this happens,
		The problem is that both hpet clock source and hpet clockevents doesn't have a suspend/resume function
		On resume we should enable the main counter _and_ enable legacy replacement mode,
		On my system main counter in enabled, by I think by bios, but legacy replacement mode is not, so if
		a system doesn't use lapic as a tick source, but use hpet+broadcast, it will hang for sure on resume, and i tested it

		The patch below is a temporally fix, until clock-events and clocksources will get proper suspend/resume hooks:

		Regards,
			Maxim Levitsky

---

Add suspend/resume for HPET
Signed-off-by: Maxim Levitsky <maximlevisky@gmail.com>

---

diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index 0fd9fba..a1ec79e 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode,
 	unsigned long cfg, cmp, now;
 	uint64_t delta;
 
+
+	if ( mode != CLOCK_EVT_MODE_UNUSED && mode != CLOCK_EVT_MODE_SHUTDOWN)
+	{
+		unsigned long cfg = hpet_readl(HPET_CFG);
+		cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
+		hpet_writel(cfg, HPET_CFG);
+		
+	}
+		
+
 	switch(mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult;

Re: [3/6] 2.6.21-rc4: known regressions

[Ingo Molnar]

* Maxim <maximlevitsky@gmail.com> wrote:

> I almost sure I know why this happens,

cool! Your patch is a definite improvement on my t60 (where 
suspend/resume never worked with hpet enabled). But it does not fix 
everything - for example the timings are way off after resume. Thomas?

	Ingo

Re: [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Wednesday 28 March 2007 16:41:48 Ingo Molnar wrote:
> 
> * Maxim <maximlevitsky@gmail.com> wrote:
> 
> > I almost sure I know why this happens,
> 
> cool! Your patch is a definite improvement on my t60 (where 
> suspend/resume never worked with hpet enabled). But it does not fix 
> everything - for example the timings are way off after resume. Thomas?
> 
> 	Ingo
> 

The problem is that HPET  consists of two parts:
	one is a main counter and second is a a timers.

	To enable main counter one must set HPET_CFG_ENABLE.
	It is set only on boot, and not on resume now.

	But on my system I think bios re-enables it.

	Secondary to enable HPET to act as a timer on IRQ0 and to make it replace RTC IRQ8 one must set
	HPET_CFG_LEGACY

	This bit is definitely not set on resume, so on my system I get a hang if I use HPET as clockevents device,
	and on other system where bios doesn't reenable HPET, then even if HPET is used as timing device 
	'date won't advance'

	But I set those bits only in initialization of HPET clockevents device, and I set always, when it is turned on,
	It is not that good,but works.

	Now I don't have a clue how to set those bits if only HPET is used as clock source because now clocksources
	don't have _any_ resume hook.

	The timing problem you mention I think isd connected to the fact that HPET is not enabled instantly after resume, but after some time when clockevents
	core calls HPET to enable event timer.

	Best regards,
		Maxim Levitsky

Re: [3/6] 2.6.21-rc4: known regressions

[Linus Torvalds]

On Wed, 28 Mar 2007, Maxim wrote:
> 
> 	Now I don't have a clue how to set those bits if only HPET is used as clock source because now clocksources
> 	don't have _any_ resume hook.

One thing that drives me wild about that "clocksource resume" thing is 
that it seems to think that clocksources are somehow different from any 
other system devices..

Why isn't the HPET considered a "device", and has it's own *device* 
"suspend" and "resume"? Why do we seem to think that only "set_mode()" 
etc should wake up clock sources?

It's a *device*, dammit. It should save and resume like one (probably as a 
system device). The "set_mode()" etc stuff is at a completely different 
(higher) conceptual level.

Thomas? It does seem like Maxim has hit the nail on the head (at least 
partly) on the HPET timer resume problems..

		Linus

Re: [3/6] 2.6.21-rc4: known regressions

[David Brownell]

On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote:

> It's a *device*, dammit. It should save and resume like one (probably as a 
> system device). The "set_mode()" etc stuff is at a completely different 
> (higher) conceptual level.

Agreed, except about "probably as a system device".

Last I checked, there was no good reason to use sysdev suspend()/resume()
rather than platform_device suspend_late()/early_resume().  Which more
or less means no good reason to use sysdev in new code...


Also, making HPET use the legacy mode seems like a step backwards.

- Dave

Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Wednesday 28 March 2007 21:38:55 David Brownell wrote:
> On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote:
> 
> > It's a *device*, dammit. It should save and resume like one (probably as a 
> > system device). The "set_mode()" etc stuff is at a completely different 
> > (higher) conceptual level.
> 
> Agreed, except about "probably as a system device".
> 
> Last I checked, there was no good reason to use sysdev suspend()/resume()
> rather than platform_device suspend_late()/early_resume().  Which more
> or less means no good reason to use sysdev in new code...
> 
> 
> Also, making HPET use the legacy mode seems like a step backwards.
> 
> - Dave
> 


Hi,
	It is not 'legacy' mode,
It is a legacy replacement mode.
It this mode HPET takes over IRQ0 and IRQ 8 and provides this way replacement for PIT and RTC periodic function

	Best regards,
		Maxim Levitsky

Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions

[David Brownell]

On Wednesday 28 March 2007 1:19 pm, Maxim wrote:
> On Wednesday 28 March 2007 21:38:55 David Brownell wrote:

> > 
> > Also, making HPET use the legacy mode seems like a step backwards.

> 	It is not 'legacy' mode,
> It is a legacy replacement mode.

Typo, sorry.


> It this mode HPET takes over IRQ0 and IRQ 8 and provides this way
> replacement for PIT and RTC periodic function 

It's that RTC periodic thing that bothers me, I don't mind about
the PIT.  Remember that IRQ8 is also used for other RTC functions.

Now, if there were a way to tell rtc-cmos that HPET is active,
and arrange some kind of handshake ... that would be different.

- Dave

Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Wednesday 28 March 2007 22:59:26 David Brownell wrote:
> On Wednesday 28 March 2007 1:19 pm, Maxim wrote:
> > On Wednesday 28 March 2007 21:38:55 David Brownell wrote:
> 
> > > 
> > > Also, making HPET use the legacy mode seems like a step backwards.
> 
> > 	It is not 'legacy' mode,
> > It is a legacy replacement mode.
> 
> Typo, sorry.
> 
> 
> > It this mode HPET takes over IRQ0 and IRQ 8 and provides this way
> > replacement for PIT and RTC periodic function 
> 
> It's that RTC periodic thing that bothers me, I don't mind about
> the PIT.  Remember that IRQ8 is also used for other RTC functions.
> 
> Now, if there were a way to tell rtc-cmos that HPET is active,
> and arrange some kind of handshake ... that would be different.
> 
> - Dave
> 

Yes,
	When HPET is active it eats RTC IRQ,
	So the only way out is to emulate RTC using HPET,
	It is done this way in old rtc driver, rtc-cmos should do the same.

	Of course suspend resume is not supported at all by old rtc driver

	I already wrote complete support for suspend/resume for old rtc driver (I wrote it long time ago)

	Now I fixed it to support HPET , and this way I discovered that HPET doesn't have suspend resume functions

	I will do last checks now and send this patch very soon

	I am also planning to add support of HPET and suspend/resume for rtc-cmos, but I didn't start this yet.

	Best regards,
		Maxim Levitsky




	

Re: [3/6] 2.6.21-rc4: known regressions

[David Brownell]

On Wednesday 28 March 2007 2:27 pm, Maxim wrote:
> On Wednesday 28 March 2007 22:59:26 David Brownell wrote:

> 	When HPET is active it eats RTC IRQ,

Only when HPET timers 0 and 1 are set up for "Legacy Replacement Mode".
In the more sensible "Standard Mode", they have their own IRQs.


> 	So the only way out is to emulate RTC using HPET,
> 	It is done this way in old rtc driver, rtc-cmos should do the same.

No.  Patches like

  http://marc.info/?l=linux-kernel&m=117219531503973&w=2

should be merged (I hope they're in the 2.6.22 queue!), making
HPET run in "Standard Mode" so that HPET can stop sticking its
fingers in code where they don't belong.


> 	I am also planning to add support of HPET and suspend/resume
> 	for rtc-cmos, but I didn't start this yet. 

It's already got suspend/resume support, and in the 2.6.22 queue
are RTC framework updates which will let the RTC framework replace
a lot more platform-specific RTC support.  (Platform changes can come
later, where they're needed.  ARM for example doesn't need any.)

Once HPET stops using "Legacy Replacement Mode" you won't need to
touch anything in the RTC stack (except maybe the legacy char/rtc.c
driver, removing HPET stuff).

The open issue with suspend/resume support in rtc-cmos relates to
how ACPI wakeup alarms should trigger.  I've not made time to test
those patches.

- Dave

Re: [3/6] 2.6.21-rc4: known regressions

[Maxim Levitsky]

On Friday 30 March 2007 00:33:35 David Brownell wrote:
> On Wednesday 28 March 2007 2:27 pm, Maxim wrote:
> > On Wednesday 28 March 2007 22:59:26 David Brownell wrote:
> 
> > 	When HPET is active it eats RTC IRQ,
> 
> Only when HPET timers 0 and 1 are set up for "Legacy Replacement Mode".
> In the more sensible "Standard Mode", they have their own IRQs.
> 
> 
> > 	So the only way out is to emulate RTC using HPET,
> > 	It is done this way in old rtc driver, rtc-cmos should do the same.
> 
> No.  Patches like
> 
>   http://marc.info/?l=linux-kernel&m=117219531503973&w=2
> 
> should be merged (I hope they're in the 2.6.22 queue!), making
> HPET run in "Standard Mode" so that HPET can stop sticking its
> fingers in code where they don't belong.
> 
> 
> > 	I am also planning to add support of HPET and suspend/resume
> > 	for rtc-cmos, but I didn't start this yet. 
> 
> It's already got suspend/resume support, and in the 2.6.22 queue
> are RTC framework updates which will let the RTC framework replace
> a lot more platform-specific RTC support.  (Platform changes can come
> later, where they're needed.  ARM for example doesn't need any.)
> 
> Once HPET stops using "Legacy Replacement Mode" you won't need to
> touch anything in the RTC stack (except maybe the legacy char/rtc.c
> driver, removing HPET stuff).
> 
> The open issue with suspend/resume support in rtc-cmos relates to
> how ACPI wakeup alarms should trigger.  I've not made time to test
> those patches.
> 
> - Dave
> 

Hi,
	It is not that simple,

	Only in legacy replacement mode HPET can be put on IRQ0 (and sadly  IRQ8)
	At least this is true on some systems, on mine for example
	
	On my system first 2 hpet timers can only be assigned to IRQ21-23
	and third to ether IRQ11, IRQ21-IRQ23

	Or in legacy replacement mode first is assigned IRQ0 and second IRQ8

	this will make it difficult to use it as a clockevents source

	Not to mention the fact that current code assumes that BIOS assigned IRQs to all timers which is not true on my system.

	I have brand new intel DG965 motherboard.

	What is wrong with relying on HPET to provide RTC IRQ ?

	Best regards,
		Maxim Levitsky

Re: [3/6] 2.6.21-rc4: known regressions

[David Brownell]

On Thursday 29 March 2007 4:29 pm, Maxim Levitsky wrote:
> On Friday 30 March 2007 00:33:35 David Brownell wrote:
> > On Wednesday 28 March 2007 2:27 pm, Maxim wrote:

> > > 	So the only way out is to emulate RTC using HPET,
> > > 	It is done this way in old rtc driver, rtc-cmos should do the same.
> > 
> > No.  Patches like
> > 
> >   http://marc.info/?l=linux-kernel&m=117219531503973&w=2

also:   http://marc.info/?l=linux-kernel&m=117219531526317&w=2

> > should be merged (I hope they're in the 2.6.22 queue!), making
> > HPET run in "Standard Mode" so that HPET can stop sticking its
> > fingers in code where they don't belong.
> >
> > ...
>
> 	It is not that simple,
> 
> 	Only in legacy replacement mode HPET can be put on IRQ0 (and sadly  IRQ8)
> 	At least this is true on some systems, on mine for example

Right, that's the entire point of legacy replacement mode.

But so what?  In standard mode, HPET just uses other IRQs.
Nothing would care about irq0, and irq8 would be used by
the RTC as necessary.


> 	this will make it difficult to use it as a clockevents source

Why?  It's not like the clockevent logic cares what IRQ a given
programmable timer uses.  So long as the HPET driver can receive
its IRQ, it'll make a fine clockevent.  There's no reason to have
HPET prevent other drivers from working, by insisting it use that
nasty "prevent other hardware from issuing IRQs" mode.

The patches from Venkatesh sure seem to have behaved for him.
And while he might have complained about difficulty, I think
that'd be more likely due to the SMP issues he also addressed
than because of some (historical?) attachment to irq0.


> 	Not to mention the fact that current code assumes that BIOS
> 	assigned IRQs to all timers which is not true on my system. 

Getting IRQ routing sorted out is a problem that's been solved
numerous times before.  And again, the patches referenced above
seem to have resolved any such issues.


> 	What is wrong with relying on HPET to provide RTC IRQ ?

For starters, it's not an RTC.  Why in the world would you want to
make the OS think it's an RTC ... unless you're Microsoft, and are
desperate to get another periodic timer (and don't much care about
the other RTC functionality?


... that's all off-topic for 2.6.21 regressions though; it's too
late to merge x86_64 clockevent support, or fix HPET issues like
not using "standard mode".

- Dave

Re: [3/6] 2.6.21-rc4: known regressions

[Maxim Levitsky]

On Friday 30 March 2007 03:09:14 David Brownell wrote:
> On Thursday 29 March 2007 4:29 pm, Maxim Levitsky wrote:
> > On Friday 30 March 2007 00:33:35 David Brownell wrote:
> > > On Wednesday 28 March 2007 2:27 pm, Maxim wrote:
> 
> > > > 	So the only way out is to emulate RTC using HPET,
> > > > 	It is done this way in old rtc driver, rtc-cmos should do the same.
> > > 
> > > No.  Patches like
> > > 
> > >   http://marc.info/?l=linux-kernel&m=117219531503973&w=2
> 
> also:   http://marc.info/?l=linux-kernel&m=117219531526317&w=2
> 
> > > should be merged (I hope they're in the 2.6.22 queue!), making
> > > HPET run in "Standard Mode" so that HPET can stop sticking its
> > > fingers in code where they don't belong.
> > >
> > > ...
> >
> > 	It is not that simple,
> > 
> > 	Only in legacy replacement mode HPET can be put on IRQ0 (and sadly  IRQ8)
> > 	At least this is true on some systems, on mine for example
> 
> Right, that's the entire point of legacy replacement mode.
> 
> But so what?  In standard mode, HPET just uses other IRQs.
> Nothing would care about irq0, and irq8 would be used by
> the RTC as necessary.
> 
> 
> > 	this will make it difficult to use it as a clockevents source
> 
> Why?  It's not like the clockevent logic cares what IRQ a given
> programmable timer uses.  So long as the HPET driver can receive
> its IRQ, it'll make a fine clockevent.  There's no reason to have
> HPET prevent other drivers from working, by insisting it use that
> nasty "prevent other hardware from issuing IRQs" mode.
> 
> The patches from Venkatesh sure seem to have behaved for him.
> And while he might have complained about difficulty, I think
> that'd be more likely due to the SMP issues he also addressed
> than because of some (historical?) attachment to irq0.
> 
> 
> > 	Not to mention the fact that current code assumes that BIOS
> > 	assigned IRQs to all timers which is not true on my system. 
> 
> Getting IRQ routing sorted out is a problem that's been solved
> numerous times before.  And again, the patches referenced above
> seem to have resolved any such issues.
No they don't, 

First patch does that:

                        hd.hd_irq[i] = (timer->hpet_config &
                                        Tn_INT_ROUTE_CNF_MASK) >>                                
					Tn_INT_ROUTE_CNF_SHIFT;

This just reads values assigned by BIOS.


> 
> 
> > 	What is wrong with relying on HPET to provide RTC IRQ ?
> 
> For starters, it's not an RTC.  Why in the world would you want to
> make the OS think it's an RTC ... unless you're Microsoft, and are
> desperate to get another periodic timer (and don't much care about
> the other RTC functionality?
> 
> 
> ... that's all off-topic for 2.6.21 regressions though; it's too
> late to merge x86_64 clockevent support, or fix HPET issues like
> not using "standard mode".
> 
> - Dave
> 
> 

Hi,
	I feel that you are right,

	You meant that one of HPET timers will be used as clock source and will be assigned some IRQ (not IRQ0)
	
	Seems fine,

	Only one thing: the kernel must assign IRQs to HPETS , relying on bios is not good,
	Also the IRQ for clocksource should be not shared, but maybe I am wrong here 
	(I am afraid that latencies might be a problem here)

	By the way I never thought about the fact that legacy replacement mode  is a 'virtual legacy'

	I mean that it is intended to simulate RTCs and PITs on systems that don't have them, am I right here ?
	HPET spec says that RTC is still requred to provide all its usial functions except periodic freq.

	Best regards,
		Maxim Levitsky

Re: [3/6] 2.6.21-rc4: known regressions

[Linus Torvalds]

On Wed, 28 Mar 2007, David Brownell wrote:
>
> On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote:
> 
> > It's a *device*, dammit. It should save and resume like one (probably as a 
> > system device). The "set_mode()" etc stuff is at a completely different 
> > (higher) conceptual level.
> 
> Agreed, except about "probably as a system device".
> 
> Last I checked, there was no good reason to use sysdev suspend()/resume()
> rather than platform_device suspend_late()/early_resume().  Which more
> or less means no good reason to use sysdev in new code...

I won't disagree - it might well be much nicer to just show it in the 
"real" device tree. I'm not 100% sure where in the tree it would go, 
though. It should probably be "inside" the root entry, before any of the 
PCI buses. It's generally what we've used those "system device" things 
for, but I agree that it would be better to just make system devices show 
up early on the regular device list than it is to have them be special 
cases.

Bit I think that's a separate (and fairly small) issue compared to the 
"don't use the clocksource infrastructure as a make-believe suspend/resume 
mechanism" problem that Maxim's patch had.

(Maxim, don't take that the wrong way - I think your analysis and patch 
were great, I just think another organization would be better)

> Also, making HPET use the legacy mode seems like a step backwards.

I don't think that's actually "legacy" in any sense but the interrupt 
delivery, where the "legacy mode" bit is not so much that the HPET itself 
is "legacy" but that it *replaces* legacy devices.

But I may have misunderstood the thing. I'm an old fart, so I know the old 
timers much better than I know the new ones ;). Somebody feel free to hit 
me with the clue-2x4.

			Linus

Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions

[David Brownell]

On Wednesday 28 March 2007 1:42 pm, Linus Torvalds wrote:
> 
> I won't disagree - it might well be much nicer to just show it in the 
> "real" device tree. I'm not 100% sure where in the tree it would go, 
> though. It should probably be "inside" the root entry, before any of the 
> PCI buses. 

Mixing "inside" and "before" is a small linguistic clue about
one of the issues with driver model PM.  Off topic here; and
in terms of suspend/resume callback sequencing that answer
shouldn't matter much for HPET (as I understand things).


> It's generally what we've used those "system device" things  
> for, but I agree that it would be better to just make system devices show 
> up early on the regular device list than it is to have them be special 
> cases.

Yes -- where "platform_device" is a regular Joe-Sixpack kind of
device, but "sysdev" is a special case.


> Bit I think that's a separate (and fairly small) issue compared to the 
> "don't use the clocksource infrastructure as a make-believe suspend/resume 
> mechanism" problem that Maxim's patch had.

Agreed -- although isn't it the "clockevent" change which is at issue?

A "clockevent" thingie wraps various kinds of timer IRQs; the clocksource
is conceptually just a free run counter.  Clocksources have been around
for a while, with no particular problems.

It's clockevent sources have been the problem with dynamic tick solutions
all along, since they mask such chaos inside x86 hardware and interact
with so many different parts of the kernel.  ;)

- Dave


> (Maxim, don't take that the wrong way - I think your analysis and patch 
> were great, I just think another organization would be better)

Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Wednesday 28 March 2007 22:42:00 Linus Torvalds wrote:
> 
> On Wed, 28 Mar 2007, David Brownell wrote:
> >
> > On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote:
> > 
> > > It's a *device*, dammit. It should save and resume like one (probably as a 
> > > system device). The "set_mode()" etc stuff is at a completely different 
> > > (higher) conceptual level.
> > 
> > Agreed, except about "probably as a system device".
> > 
> > Last I checked, there was no good reason to use sysdev suspend()/resume()
> > rather than platform_device suspend_late()/early_resume().  Which more
> > or less means no good reason to use sysdev in new code...
> 
> I won't disagree - it might well be much nicer to just show it in the 
> "real" device tree. I'm not 100% sure where in the tree it would go, 
> though. It should probably be "inside" the root entry, before any of the 
> PCI buses. It's generally what we've used those "system device" things 
> for, but I agree that it would be better to just make system devices show 
> up early on the regular device list than it is to have them be special 
> cases.
> 
> Bit I think that's a separate (and fairly small) issue compared to the 
> "don't use the clocksource infrastructure as a make-believe suspend/resume 
> mechanism" problem that Maxim's patch had.
> 
> (Maxim, don't take that the wrong way - I think your analysis and patch 
> were great, I just think another organization would be better)

Exactly, I agree completely
I said that my patch was a  temporary fix, and I agree that the best way is to create a new system device
and use its suspend/resume hooks to bring HPET back to life on resume.

> 
> > Also, making HPET use the legacy mode seems like a step backwards.
> 
> I don't think that's actually "legacy" in any sense but the interrupt 
> delivery, where the "legacy mode" bit is not so much that the HPET itself 
> is "legacy" but that it *replaces* legacy devices.
> 
> But I may have misunderstood the thing. I'm an old fart, so I know the old 
> timers much better than I know the new ones ;). Somebody feel free to hit 
> me with the clue-2x4.
> 
> 			Linus
> 

Best regards,
	Maxim Levitsky

[ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Wednesday 28 March 2007 18:38:48 Linus Torvalds wrote:
> 
> On Wed, 28 Mar 2007, Maxim wrote:
> > 
> > 	Now I don't have a clue how to set those bits if only HPET is used as clock source because now clocksources
> > 	don't have _any_ resume hook.
> 
> One thing that drives me wild about that "clocksource resume" thing is 
> that it seems to think that clocksources are somehow different from any 
> other system devices..
> 
> Why isn't the HPET considered a "device", and has it's own *device* 
> "suspend" and "resume"? Why do we seem to think that only "set_mode()" 
> etc should wake up clock sources?
> 
> It's a *device*, dammit. It should save and resume like one (probably as a 
> system device). The "set_mode()" etc stuff is at a completely different 
> (higher) conceptual level.
> 
> Thomas? It does seem like Maxim has hit the nail on the head (at least 
> partly) on the HPET timer resume problems..
> 
> 		Linus
> 


Hi,
	I am sending here a patch that as was discussed here adds hpet to list of system devices
	and adds suspend/resume hooks this way.
	I tested it and it works fine.

---
Add suspend/resume support for HPET
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

---
 arch/i386/kernel/hpet.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index 0fd9fba..ac41476 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -3,6 +3,8 @@
 #include <linux/errno.h>
 #include <linux/hpet.h>
 #include <linux/init.h>
+#include <linux/sysdev.h>
+#include <linux/pm.h>
 
 #include <asm/hpet.h>
 #include <asm/io.h>
@@ -524,3 +526,65 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif
-- 
1.4.4.2

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Linus Torvalds]

On Thu, 29 Mar 2007, Maxim wrote:
>
> 	I am sending here a patch that as was discussed here adds hpet to list of system devices
> 	and adds suspend/resume hooks this way.
> 	I tested it and it works fine.

Ok, it certainly looks better, but it *also* looks like it just assumes 
the HPET is there. Which would work in testing _with_ a HPET, but would 
likely break on hardware without one, no?

Shouldn't there be at least something like a

	if (!is_hpet_capable())
		return 0;

at the top of that init routine? I'd also expect that you'd need to check 
that "hpet_virt_address" is valid or something?

(Or, better yet, shouldn't we set "boot_hpet_disable" when we decide not 
to use the HPET, and set hpet_virt_address to NULL?)

		Linus

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Thursday 29 March 2007 07:08:58 Linus Torvalds wrote:
> 
> On Thu, 29 Mar 2007, Maxim wrote:
> >
> > 	I am sending here a patch that as was discussed here adds hpet to list of system devices
> > 	and adds suspend/resume hooks this way.
> > 	I tested it and it works fine.
> 
> Ok, it certainly looks better, but it *also* looks like it just assumes 
> the HPET is there. Which would work in testing _with_ a HPET, but would 
> likely break on hardware without one, no?
> 
> Shouldn't there be at least something like a
> 
> 	if (!is_hpet_capable())
> 		return 0;
> 
> at the top of that init routine? I'd also expect that you'd need to check 
> that "hpet_virt_address" is valid or something?
> 
> (Or, better yet, shouldn't we set "boot_hpet_disable" when we decide not 
> to use the HPET, and set hpet_virt_address to NULL?)

This is done here

out_nohpet:
	iounmap(hpet_virt_address);
	hpet_virt_address = NULL;
> 
> 		Linus
> 

Hi, 
	Of course, I forgot.

	I was planning to put sysdev code in hpet_enable()
	but it is not possible because this function is called too early.

	Thus I put sysdev initialization  in separate function but forgot to test for HPET

	Thanks a lot.

	Best regards
		Maxim Levitsky

---
This adds support of suspend/resume on i386 for HPET
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

---
 arch/i386/kernel/hpet.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index 0fd9fba..7c67780 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -3,6 +3,8 @@
 #include <linux/errno.h>
 #include <linux/hpet.h>
 #include <linux/init.h>
+#include <linux/sysdev.h>
+#include <linux/pm.h>
 
 #include <asm/hpet.h>
 #include <asm/io.h>
@@ -310,6 +312,7 @@ int __init hpet_enable(void)
 out_nohpet:
 	iounmap(hpet_virt_address);
 	hpet_virt_address = NULL;
+	boot_hpet_disable = 1;
 	return 0;
 }
 
@@ -524,3 +527,68 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	if (!is_hpet_capable())
+		return 0;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif
-- 
1.4.4.2

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Sergei Shtylyov]

Hello.

Maxim wrote:

> ---
> This adds support of suspend/resume on i386 for HPET

   The part after usually "---" gets cut off, the patch description and 
signoff should actially *precede* it.

> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

> diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
> index 0fd9fba..7c67780 100644
> --- a/arch/i386/kernel/hpet.c
> +++ b/arch/i386/kernel/hpet.c
[...]
> +static __init int hpet_register_sysfs(void)
> +{
> +	int err;
> +
> +	if (!is_hpet_capable())
> +		return 0;
> +
> +	err = sysdev_class_register(&hpet_class);
> +
> +	if (!err) {
> +		sysdev_register(&hpet_device);
> +		if (err)
> +			sysdev_class_unregister(&hpet_class);

    This doesn't make sense, err will always be 0.  Perhaps you actually 
intended to check the result of sysdev_register()?

> +	}
> +
> +	return err;
> +}
> +
> +device_initcall(hpet_register_sysfs);
> +
> +#endif

WBR, Sergei

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Maxim]

On Thursday 29 March 2007 15:20:27 Sergei Shtylyov wrote:
> Hello.
> 
> Maxim wrote:
> 
> > ---
> > This adds support of suspend/resume on i386 for HPET
> 
>    The part after usually "---" gets cut off, the patch description and 
> signoff should actially *precede* it.
> 
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> 
> > diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
> > index 0fd9fba..7c67780 100644
> > --- a/arch/i386/kernel/hpet.c
> > +++ b/arch/i386/kernel/hpet.c
> [...]
> > +static __init int hpet_register_sysfs(void)
> > +{
> > +	int err;
> > +
> > +	if (!is_hpet_capable())
> > +		return 0;
> > +
> > +	err = sysdev_class_register(&hpet_class);
> > +
> > +	if (!err) {
> > +		sysdev_register(&hpet_device);
> > +		if (err)
> > +			sysdev_class_unregister(&hpet_class);
> 
>     This doesn't make sense, err will always be 0.  Perhaps you actually 
> intended to check the result of sysdev_register()?
> 
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +device_initcall(hpet_register_sysfs);
> > +
> > +#endif
> 
> WBR, Sergei
> 

Hi,
	Big thanks for pointing this out,
		I will resend that updated patch.

	Best regards,
		Maxim Levitsky

[PATCH v2] Add suspend/resume for HPET

[Maxim Levitsky]

Subject: Add suspend/resume for HPET
This adds support of suspend/resume on i386 for HPET
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

---
 arch/i386/kernel/hpet.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index 0fd9fba..7c67780 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -3,6 +3,8 @@
 #include <linux/errno.h>
 #include <linux/hpet.h>
 #include <linux/init.h>
+#include <linux/sysdev.h>
+#include <linux/pm.h>
 
 #include <asm/hpet.h>
 #include <asm/io.h>
@@ -310,6 +312,7 @@ int __init hpet_enable(void)
 out_nohpet:
 	iounmap(hpet_virt_address);
 	hpet_virt_address = NULL;
+	boot_hpet_disable = 1;
 	return 0;
 }
 
@@ -524,3 +527,68 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	if (!is_hpet_capable())
+		return 0;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		err = sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif
-- 
1.4.4.2

Re: [PATCH v2] Add suspend/resume for HPET

[Linus Torvalds]

On Thu, 29 Mar 2007, Maxim Levitsky wrote:
>
> Subject: Add suspend/resume for HPET
>
> This adds support of suspend/resume on i386 for HPET
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> 
> ---
>  arch/i386/kernel/hpet.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++

Btw, what about arch/x86_64/kernel/hpet.c?

That thing seems totally broken. Lookie here:

  arch/x86_64/kernel/hpet.c:irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id, struct pt_regs *regs)
  drivers/char/rtc.c:extern irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id);

anybody see a problem? The x86-64 version doesn't seem to be very well 
maintained. Is there some fundamental reason why this file isn't shared 
across architectures?

			Linus

Re: [PATCH v2] Add suspend/resume for HPET

[Maxim Levitsky]

On Thursday 29 March 2007 18:53:37 Linus Torvalds wrote:
> 
> On Thu, 29 Mar 2007, Maxim Levitsky wrote:
> >
> > Subject: Add suspend/resume for HPET
> >
> > This adds support of suspend/resume on i386 for HPET
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > 
> > ---
> >  arch/i386/kernel/hpet.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++
> 
> Btw, what about arch/x86_64/kernel/hpet.c?
> 
> That thing seems totally broken. Lookie here:
> 
>   arch/x86_64/kernel/hpet.c:irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id, struct pt_regs *regs)
>   drivers/char/rtc.c:extern irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id);
> 
> anybody see a problem? The x86-64 version doesn't seem to be very well 
> maintained. Is there some fundamental reason why this file isn't shared 
> across architectures?
> 
> 			Linus
> 

Hi,
	I agree with that, there seems to be lot of code duplication between i386 and x86_64.
	By the way, x86_64 does take care of suspend/resume for hpet, it is done by 

	linux-2.6/arch/x86_64/kernel/time.c:timer_resume(struct sys_device *dev):
		hpet_reenable()


	on i386 PIT driver goes out of way when HPET is detected
	So it seems that there is lot of work to do to remove redundant code.


	Best regards,
		Maxim Levitsky

Re: [PATCH v2] Add suspend/resume for HPET

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Btw, what about arch/x86_64/kernel/hpet.c?

at least wrt. suspend/resume it should be fine, because in 
arch/x86_64/kernel/time.c it does this upon resume:

 static int timer_resume(struct sys_device *dev)
 {
         if (hpet_address)
                 hpet_reenable();
         else
                 i8254_timer_resume();

[ barring the issue that mixing two pieces of hardware like this in a 
  single resume function is wrong - all timer hardware should be 
  separated like we did it for i386. I've got 64-bit clockevents code in 
  -rt which does this separation. ]

> That thing seems totally broken. Lookie here:
> 
>   arch/x86_64/kernel/hpet.c:irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id, struct pt_regs *regs)
>   drivers/char/rtc.c:extern irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id);
> 
> anybody see a problem? The x86-64 version doesn't seem to be very well 
> maintained. Is there some fundamental reason why this file isn't 
> shared across architectures?

there's no fundamental reason. x86_64 COW-ed hpet_timer.c and 
time_hpet.c years ago and drifted off into different areas.
They should be unified: more power to arch/x86/ ;-)

	Ingo

Re: [PATCH v2] Add suspend/resume for HPET

[Andi Kleen]

Ingo Molnar <mingo@elte.hu> writes:
> 
> there's no fundamental reason. x86_64 COW-ed hpet_timer.c and 
> time_hpet.c years ago and drifted off into different areas.

Not quite -- x86-64 did HPET long before i386; the only stuff cowed
was the character driver support code. But the core HPET code
was always totally different code streams. We never did the complicated 
pluggable clock code i386 did though -- i never quite saw the point of that
because there aren't that many timers there.
Of course it is already obsolete with clocksources now.

-Andi

Re: [PATCH v2] Add suspend/resume for HPET

[Jeff Chua]

On 3/29/07, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> Subject: Add suspend/resume for HPET
> This adds support of suspend/resume on i386 for HPET
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

Confirmed that suspend/resume disk/ram works on X60s with
CONFIG_HPET_TIMER=y and CONFIG_NO_HZ unset.

But suspend to disk still hang with CONFIG_NO_HZ unset.

Thanks,
Jeff.

Re: [PATCH v2] Add suspend/resume for HPET

[Thomas Gleixner]

On Thu, 2007-03-29 at 15:46 +0200, Maxim Levitsky wrote:
> Subject: Add suspend/resume for HPET
> This adds support of suspend/resume on i386 for HPET
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
>
> +static struct sysdev_class hpet_class = {
> +	set_kset_name("hpet"),
> +	.suspend	= hpet_suspend,
> +	.resume		= hpet_resume,
> +};
> +
> +static struct sys_device hpet_device = {
> +	.id		= 0,
> +	.cls		= &hpet_class,
> +};

Sorry for being inresponsive. I was travelling and unexpectedly cut off
from the internet for some days.

While I agree in principle with the patch, I'm a bit uncomfortable. The
sys device suspend / resume ordering is not guaranteed and relies on the
registering order.

Jeff still seems to have problems with CONFIG_NO_HZ=n and it might be
caused by time keeping / tick management resume happening before the
HPET resume.

The required resume order is:

clocksources
timekeeping
clockevents
tick management

I'm not sure how to do this properly with the sys device facilities, but
I look into it.

/me goes off to understand the sys device magic.

	tglx

Re: [PATCH v2] Add suspend/resume for HPET

[Jeff Chua]

On 3/31/07, Thomas Gleixner <tglx@linutronix.de> wrote:

> Jeff still seems to have problems with CONFIG_NO_HZ=n and it might be
> caused by time keeping / tick management resume happening before the
> HPET resume.


me>Confirmed that suspend/resume disk/ram works on X60s with
me>CONFIG_HPET_TIMER=y and CONFIG_NO_HZ unset.
me> But suspend to disk still hang with CONFIG_NO_HZ unset.


Oops, sorry. Typo (as a result copy/paste using mouse)
    ... I actually meant CONFIG_NO_HZ "set".

Just to be clear, suspend to disk still hang with CONFIG_NO_HZ=y. It
hang just before you see the percent saving %.


Jeff.

Re: [PATCH v2] Add suspend/resume for HPET

[Thomas Gleixner]

On Sun, 2007-04-01 at 00:01 +0800, Jeff Chua wrote:
> On 3/31/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > Jeff still seems to have problems with CONFIG_NO_HZ=n and it might be
> > caused by time keeping / tick management resume happening before the
> > HPET resume.
> 
> 
> me>Confirmed that suspend/resume disk/ram works on X60s with
> me>CONFIG_HPET_TIMER=y and CONFIG_NO_HZ unset.
> me> But suspend to disk still hang with CONFIG_NO_HZ unset.
> 
> 
> Oops, sorry. Typo (as a result copy/paste using mouse)
>     ... I actually meant CONFIG_NO_HZ "set".
> 
> Just to be clear, suspend to disk still hang with CONFIG_NO_HZ=y. It
> hang just before you see the percent saving %.

Ah, that's a different one then. In that path the timers should be
alive, but who knows.

	tglx

Re: [PATCH v2] Add suspend/resume for HPET

[Linus Torvalds]

On Sat, 31 Mar 2007, Thomas Gleixner wrote:
> 
> While I agree in principle with the patch, I'm a bit uncomfortable. The
> sys device suspend / resume ordering is not guaranteed and relies on the
> registering order.

Well, this is why we probably should try to get away from the "system 
device" issue, exactly because system devices are totally outside the 
normal ordering and only have a random linear order.

If the clocksources were actually in the device tree, you'd get all the 
normal guarantees about hierarchical ordering..

		Linus

Re: [PATCH v2] Add suspend/resume for HPET

[Thomas Gleixner]

On Sat, 2007-03-31 at 09:09 -0700, Linus Torvalds wrote:
> 
> On Sat, 31 Mar 2007, Thomas Gleixner wrote:
> > 
> > While I agree in principle with the patch, I'm a bit uncomfortable. The
> > sys device suspend / resume ordering is not guaranteed and relies on the
> > registering order.
> 
> Well, this is why we probably should try to get away from the "system 
> device" issue, exactly because system devices are totally outside the 
> normal ordering and only have a random linear order.
> 
> If the clocksources were actually in the device tree, you'd get all the 
> normal guarantees about hierarchical ordering..

Right, but clock - sources/events need to be extremly late suspended and
early resumed. How can we ensure this ?

	tglx

Re: [PATCH v2] Add suspend/resume for HPET

[Greg KH]

On Sat, Mar 31, 2007 at 06:33:20PM +0200, Thomas Gleixner wrote:
> On Sat, 2007-03-31 at 09:09 -0700, Linus Torvalds wrote:
> > 
> > On Sat, 31 Mar 2007, Thomas Gleixner wrote:
> > > 
> > > While I agree in principle with the patch, I'm a bit uncomfortable. The
> > > sys device suspend / resume ordering is not guaranteed and relies on the
> > > registering order.
> > 
> > Well, this is why we probably should try to get away from the "system 
> > device" issue, exactly because system devices are totally outside the 
> > normal ordering and only have a random linear order.
> > 
> > If the clocksources were actually in the device tree, you'd get all the 
> > normal guarantees about hierarchical ordering..
> 
> Right, but clock - sources/events need to be extremly late suspended and
> early resumed. How can we ensure this ?

Put it at the top of the device tree.

thanks,

greg k-h

Re: [PATCH v2] Add suspend/resume for HPET

[Linus Torvalds]

On Sat, 31 Mar 2007, Thomas Gleixner wrote:
> 
> Right, but clock - sources/events need to be extremly late suspended and
> early resumed. How can we ensure this ?

Make them be at the top of the device tree by adding them early. That's 
the whole point of the device tree after all - we have an ordering that is 
enforced by its topology, and that we sort by when things were added.

Right now the way things work is (iirc - somebody like Greg should 
double-check me) is that we add all devices to the power list at 
device_add() time by traversing the devices fromt he root all the way out, 
and doing a device_add() which does a device_pm_add(), which in turn adds 
it to the power-management list - so that the list is always topologically 
sorted.

So the only thing that needs to be done is to make sure that we add the 
timer devices early during bootup - something we have to do *anyway*. If a 
device is added early in bootup, that automatically means that it will be 
suspended late, and resumed early - because we maintain that order all the 
way through..

		Linus

Re: [PATCH v2] Add suspend/resume for HPET

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sat, 31 Mar 2007, Thomas Gleixner wrote:
> > 
> > Right, but clock - sources/events need to be extremly late suspended and
> > early resumed. How can we ensure this ?

[...]
> So the only thing that needs to be done is to make sure that we add 
> the timer devices early during bootup - something we have to do 
> *anyway*. If a device is added early in bootup, that automatically 
> means that it will be suspended late, and resumed early - because we 
> maintain that order all the way through..

IIRC hpet is particularly hard to initialize early on in the bootup 
sequence. So the way the clockevents code works is that it will always 
try to make the best out of all available devices, and dynamically 
adapts things as devices 'arrive' or 'depart' - no matter how late that 
happens. (That way there's no dependency on how late a device gets 
registered - it will only delay the switch to high-res mode for 
example.) A given time device might 'depart' because for example the 
watchdog mechanism finds that its quality is not good enough, or because 
someone initiated cpufreq which breaks the TSC clocksource.

i dont think there's any particular problem here because suspend/resume 
wont be done during bootup - but we might need a way to move a device to 
earlier spots in the device tree, even if they got registered later on - 
instead of forcing the time devices to be registered very early?

	Ingo

Re: [PATCH v2] Add suspend/resume for HPET

[David Brownell]

( please remove obsolute linux-pm@lists.osdl.org  from further messages!! )

On Saturday 31 March 2007 10:02 am, Ingo Molnar wrote:
> 
> i dont think there's any particular problem here because suspend/resume 
> wont be done during bootup - but we might need a way to move a device to 
> earlier spots in the device tree, even if they got registered later on - 
> instead of forcing the time devices to be registered very early?

I'm about ready to test the appended patch... a "move one device" call
might be safest at this point in the release cycle though.

- Dave

========================	SNIP!
Change how the PM list is constructed, so that devices are added right
after their parents (when they have one) rather than at the end of the
list.  This preserves sequencing guarantees, but enables sequencing of
suspend/resume operations by more important characteristics than "when
device happened to enumerate" ... e.g. clocksources and clockevents
at a clearly defined point during suspend and resume.

This patch has a potential downside for devices that have multiple
power dependencies and which "just happened to work" before.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

--- g26.orig/drivers/base/power/main.c	2006-07-02 12:30:30.000000000 -0700
+++ g26/drivers/base/power/main.c	2007-03-31 11:02:28.000000000 -0700
@@ -52,12 +52,17 @@ EXPORT_SYMBOL_GPL(device_pm_set_parent);
 int device_pm_add(struct device * dev)
 {
 	int error;
+	struct device *parent = dev->parent;
 
-	pr_debug("PM: Adding info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name);
+	pr_debug("PM: Adding info for %s:%s, after %s\n",
+		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name,
+		 parent ? parent->bus_id : "(no parent)");
 	down(&dpm_list_sem);
-	list_add_tail(&dev->power.entry, &dpm_active);
-	device_pm_set_parent(dev, dev->parent);
+	if (parent)
+		list_add(&dev->power.entry, &parent->power.entry);
+	else
+		list_add_tail(&dev->power.entry, &dpm_active);
+	device_pm_set_parent(dev, parent);
 	if ((error = dpm_sysfs_add(dev)))
 		list_del(&dev->power.entry);
 	up(&dpm_list_sem);

Re: [PATCH v2] Add suspend/resume for HPET

[David Brownell]

On Saturday 31 March 2007 11:18 am, David Brownell wrote:
> ( please remove obsolute linux-pm@lists.osdl.org  from further messages!! )
> 
> On Saturday 31 March 2007 10:02 am, Ingo Molnar wrote:
> > 
> > i dont think there's any particular problem here because suspend/resume 
> > wont be done during bootup - but we might need a way to move a device to 
> > earlier spots in the device tree, even if they got registered later on - 
> > instead of forcing the time devices to be registered very early?
> 
> I'm about ready to test the appended patch... a "move one device" call
> might be safest at this point in the release cycle though.

As expected, this behaved OK on an x86 laptop.  I'll see if it breaks
some of the ARM boards I have handy.

To be clear, what this means is that if "clockevent" and "clocksource"
devices get registered "very early", and the various clockevent and
clock source devices get registered, then the suspend/resume methods
for those will all get grouped together ... suspended "very late" and
resumed "very early", regardless of when they get registered.  Pretty
much the driver model parts of what Linus was suggesting; clockevent
bits would still be needed.

- Dave



> - Dave
> 
> ========================	SNIP!
> Change how the PM list is constructed, so that devices are added right
> after their parents (when they have one) rather than at the end of the
> list.  This preserves sequencing guarantees, but enables sequencing of
> suspend/resume operations by more important characteristics than "when
> device happened to enumerate" ... e.g. clocksources and clockevents
> at a clearly defined point during suspend and resume.
> 
> This patch has a potential downside for devices that have multiple
> power dependencies and which "just happened to work" before.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> 
> --- g26.orig/drivers/base/power/main.c	2006-07-02 12:30:30.000000000 -0700
> +++ g26/drivers/base/power/main.c	2007-03-31 11:02:28.000000000 -0700
> @@ -52,12 +52,17 @@ EXPORT_SYMBOL_GPL(device_pm_set_parent);
>  int device_pm_add(struct device * dev)
>  {
>  	int error;
> +	struct device *parent = dev->parent;
>  
> -	pr_debug("PM: Adding info for %s:%s\n",
> -		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name);
> +	pr_debug("PM: Adding info for %s:%s, after %s\n",
> +		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name,
> +		 parent ? parent->bus_id : "(no parent)");
>  	down(&dpm_list_sem);
> -	list_add_tail(&dev->power.entry, &dpm_active);
> -	device_pm_set_parent(dev, dev->parent);
> +	if (parent)
> +		list_add(&dev->power.entry, &parent->power.entry);
> +	else
> +		list_add_tail(&dev->power.entry, &dpm_active);
> +	device_pm_set_parent(dev, parent);
>  	if ((error = dpm_sysfs_add(dev)))
>  		list_del(&dev->power.entry);
>  	up(&dpm_list_sem);
> 

Re: [PATCH v2] Add suspend/resume for HPET

[Jeff Chua]

On 4/1/07, David Brownell <david-b@pacbell.net> wrote:
> for those will all get grouped together ... suspended "very late" and
> resumed "very early", regardless of when they get registered.  Pretty
> much the driver model parts of what Linus was suggesting; clockevent
> bits would still be needed.

I tested your patch with CONFIG_NO_HZ=y, but it still hangs while
suspending to disk (before the percent saving).

But one discovery. I get tired of all these hangs, so I decided to
press some keys and the power button. Accidentally, the suspend
proceeded and successfully suspended!

I tried many more times, and discovered that to proceed with the
suspend, hit any 4 keys slowly. (e.g. "F1 F2 F3 F4", or "1 2 3 4").

My .config has CONFIG_NO_HZ=y and CONFIG_HIGH_RES_TIMERS=y, but I
suppose CONFIG_HIGH_RES_TIMERS=y gots nothing to do with the hang.

I went back with my vanilla 2.6.21-rc5 with Maxim's patch, and with
hitting the keys, I could suspend to disk with CONFIG_NO_HZ=y and
CONFIG_HIGH_RES_TIMERS=y.

Jeff.

Re: [PATCH v2] Add suspend/resume for HPET

[David Brownell]

On Saturday 31 March 2007 8:13 pm, Jeff Chua wrote:
> On 4/1/07, David Brownell <david-b@pacbell.net> wrote:
> > for those will all get grouped together ... suspended "very late" and
> > resumed "very early", regardless of when they get registered.  Pretty
> > much the driver model parts of what Linus was suggesting; clockevent
> > bits would still be needed.
> 
> I tested your patch with CONFIG_NO_HZ=y, but it still hangs while
> suspending to disk (before the percent saving).

Of course.  No clockevent updates...

Re: [PATCH v2] Add suspend/resume for HPET

[Greg KH]

On Sat, Mar 31, 2007 at 09:53:43AM -0700, Linus Torvalds wrote:
> Make them be at the top of the device tree by adding them early. That's 
> the whole point of the device tree after all - we have an ordering that is 
> enforced by its topology, and that we sort by when things were added.
> 
> Right now the way things work is (iirc - somebody like Greg should 
> double-check me) is that we add all devices to the power list at 
> device_add() time by traversing the devices fromt he root all the way out, 
> and doing a device_add() which does a device_pm_add(), which in turn adds 
> it to the power-management list - so that the list is always topologically 
> sorted.

Yes, this is how it works (or if not, then there's a bug that needs to
be fixed, as that is how it _should_ work...)

thanks,

greg k-h

Re: [linux-pm] [PATCH v2] Add suspend/resume for HPET

[David Brownell]

On Saturday 31 March 2007 9:53 am, Linus Torvalds wrote:
> 
> On Sat, 31 Mar 2007, Thomas Gleixner wrote:
> > 
> > Right, but clock - sources/events need to be extremly late suspended and
> > early resumed. How can we ensure this ?
> 
> Make them be at the top of the device tree by adding them early. That's 
> the whole point of the device tree after all - we have an ordering that is 
> enforced by its topology, and that we sort by when things were added.

Right, but "when things get added" doesn't correlate well to
"when they should get suspended/resumed".  It's also in basic
conflict with runtime PM models, where devices may be suspended
at essentially any time.  And sysdevs are even stranger.


One way out:  rather than constructing that list as devices get
enumerated, it could be constructed by a (linear-time, non-recursive)
walk of the device tree(s) before they get suspended.

(Or equivalently:  construct lists at enumeration time, but just
adding them *right after their parent* rather than at the end of
the list.)

Would that solve the problem here?  Potentially ... if the tree is
structured to meet Thomas' rules:

> > The required resume order is:
> > 
> > clocksources
> > timekeeping
> > clockevents
> > tick management

> So the only thing that needs to be done is to make sure that we add the 
> timer devices early during bootup - something we have to do *anyway*. If a 
> device is added early in bootup, that automatically means that it will be 
> suspended late, and resumed early - because we maintain that order all the 
> way through..

>             -- "clocksource" -- +-- HPET
>                                 |
>                                 +-- TSC
>                                 |
>                                 +-- i8259
>                                 |
>                                 +-- lapic timer
>                                 |
>                                 .. whatever else

If each of those were a device node, with "clocksource" suspend/resume
methods handling Thomas' "timekeeping" item, and simlarly for "clockevent"
devices ... I could see that all working neatly.

- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Add suspend/resume for HPET

[Maxim Levitsky]

On Saturday 31 March 2007 18:51:11 Thomas Gleixner wrote:
> On Thu, 2007-03-29 at 15:46 +0200, Maxim Levitsky wrote:
> > Subject: Add suspend/resume for HPET
> > This adds support of suspend/resume on i386 for HPET
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> >
> > +static struct sysdev_class hpet_class = {
> > +	set_kset_name("hpet"),
> > +	.suspend	= hpet_suspend,
> > +	.resume		= hpet_resume,
> > +};
> > +
> > +static struct sys_device hpet_device = {
> > +	.id		= 0,
> > +	.cls		= &hpet_class,
> > +};
> 
> Sorry for being inresponsive. I was travelling and unexpectedly cut off
> from the internet for some days.
> 
> While I agree in principle with the patch, I'm a bit uncomfortable. The
> sys device suspend / resume ordering is not guaranteed and relies on the
> registering order.
> 
> Jeff still seems to have problems with CONFIG_NO_HZ=n and it might be
> caused by time keeping / tick management resume happening before the
> HPET resume.
> 
> The required resume order is:
> 
> clocksources
> timekeeping
> clockevents
> tick management
> 
> I'm not sure how to do this properly with the sys device facilities, but
> I look into it.
> 
> /me goes off to understand the sys device magic.
> 
> 	tglx
> 
> 
> 

Hi,

So maybe I was right afrer all,
Maybe it is better to add a suspend/resume hook to each clock source and call 
it from timekeeping_resume() ?

Or maybe even unite clocksources with clockevents, don't know 

By the way I want to report maybe a bug / maybe a feature :-)  : 
(sorry for long explanation)

Basically I have two clockevent sources : PIT(HPET) and APIC
(Actually I it seems that in next version of kernel HPET will be switched out 
of 'legacy replacement mode' , so PIT and HPET and RTC could coexist of same system,
But HPET won't be able to generate IRQ0, and it will be assigned some IRQ, possibly shared with other devices)

APIC timer  is chosen by default and works fine, 
since I don't have C2/C2 states on my system (ICH8 doesn't support them :-( )

But if I force it off (nolapic_timer) HPET or PIC is chosen and strangely they are
 put in _periodic_ mode although they are capable of one-shot mode
Is this a bug ?

Secondary I am getting a very strange behavior if I use CONFIG_NOHZ + !CONFIG_HIGH_RES_TIMERS
and try to suspend to ram:

System resumes, but gets crazy:
'top' shows that  ksoftirqd consumes 9999 % of cpu time (this is not a typo)
And other 'normal' programs that are running show same 9999 too.
System slows to crawl.

Also I found that one of APICS is in periodic mode,  and second is in one shoot mode.
And I tested this with or without my patch (thank goodness it is not my fault)

CONFIG_NOHZ + CONFIG_HIGH_RES_TIMERS work just fine.

Best regards,
	Maxim Levitsky

Re: [PATCH v2] Add suspend/resume for HPET

[Linus Torvalds]

On Sat, 31 Mar 2007, Maxim Levitsky wrote:
> 
> So maybe I was right afrer all,
> Maybe it is better to add a suspend/resume hook to each clock source and call 
> it from timekeeping_resume() ?

Umm.. WHy not make the device tree look like this:

	    -- "clocksource" -- +-- HPET
				|
				+-- TSC
				|
				+-- i8259
				|
				+-- lapic timer
				|
				.. whatever else

and use the "struct device" that we *have* for this? The whole "struct 
device" is literally designed to do this, and to be embedded into whatever 
bigger structures you have that describes higher-level behaviour. Ie you'd 
put a "struct device" inside the "struct clocksource".

That thingalready *has* the suspend/resume hooks, and it will mean that 
people will see the clocks in the device tree rather than have a new 
notion.


			Linus

Re: [PATCH v2] Add suspend/resume for HPET

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Umm.. WHy not make the device tree look like this:
> 
> 	    -- "clocksource" -- +-- HPET
> 				|
> 				+-- TSC
> 				|
> 				+-- i8259
> 				|
> 				+-- lapic timer
> 				|
> 				.. whatever else
> 
> and use the "struct device" that we *have* for this? The whole "struct 
> device" is literally designed to do this, and to be embedded into 
> whatever bigger structures you have that describes higher-level 
> behaviour. Ie you'd put a "struct device" inside the "struct 
> clocksource".

yeah. There's some practical problems that need to be sorted out: much 
of the current GTOD code is irq-driven (and all GTOD locks are 
irq-safe), while the sysfs code needs to run in process-context level. 

Clocksources 'arrive' and 'depart' in hardirq context (which is the 
primary place where we notice their breakage, determine that they are 
now verified to be usable, etc.). This came partly from legacy: the 
gradual conversion of the monolithic time code, and the need to preserve 
GTOD and non-GTOD architectures without too much duplication. It also 
came partly because there's also a fundamental need to have accurate 
time, which is better served from irq context.

i very much agree that this should and must be cleaned up, but it needs 
quite a bit more logistics than it might appear at first sight. 
Clockevents basically just followed (and had to follow) the direction of 
clocksources in this regard.

	Ingo

Re: [PATCH v2] Add suspend/resume for HPET

[Daniel Walker]

On Sat, 2007-03-31 at 19:17 +0200, Ingo Molnar wrote:

> yeah. There's some practical problems that need to be sorted out: much 
> of the current GTOD code is irq-driven (and all GTOD locks are 
> irq-safe), while the sysfs code needs to run in process-context level. 
> 
> Clocksources 'arrive' and 'depart' in hardirq context (which is the 
> primary place where we notice their breakage, determine that they are 
> now verified to be usable, etc.). This came partly from legacy: the 
> gradual conversion of the monolithic time code, and the need to preserve 
> GTOD and non-GTOD architectures without too much duplication. It also 
> came partly because there's also a fundamental need to have accurate 
> time, which is better served from irq context.
> 

Is this in reference to the irq-context clocksource polling stuff? I
don't see a dire reason to keep that code, and I agree removing that is
a certainly a worth while cleanup .. I added this cleanup to one of my
trees when you first suggested it , and there is some infrastructure
that really should be added to facilitate it.

Daniel

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Linus Torvalds]

On Thu, 29 Mar 2007, Maxim wrote:

> On Thursday 29 March 2007 07:08:58 Linus Torvalds wrote:
> > 
> > (Or, better yet, shouldn't we set "boot_hpet_disable" when we decide not 
> > to use the HPET, and set hpet_virt_address to NULL?)
> 
> This is done here
> 
> out_nohpet:
> 	iounmap(hpet_virt_address);
> 	hpet_virt_address = NULL;

No, that only clears hpet_virt_address, and thus makes all subsequent 
"hpet_readl()" and "hpet_writel()" calls oops.

But it doesn't actually *tell* anybody that the HPET is disabled, so if 
you later on do

	if (is_hpet_capable()) {
		time = hpet_readl(..);
		..

you will just Oops!

So as far as I can see, even with your latest patch, if hpet_enable() 
fails (and triggers the "goto out_nohpet" cases), you'll just oops 
immediately when you try to suspend/resume the HPET.

THAT was what I meant - when we clear hpet_virt_address, we should also 
tell all potential subsequent users that the HPET is not there!

		Linus

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Maxim Levitsky]

On Thursday 29 March 2007 18:35:21 Linus Torvalds wrote:
> 
> On Thu, 29 Mar 2007, Maxim wrote:
> 
> > On Thursday 29 March 2007 07:08:58 Linus Torvalds wrote:
> > > 
> > > (Or, better yet, shouldn't we set "boot_hpet_disable" when we decide not 
> > > to use the HPET, and set hpet_virt_address to NULL?)
> > 
> > This is done here
> > 
> > out_nohpet:
> > 	iounmap(hpet_virt_address);
> > 	hpet_virt_address = NULL;
> 
> No, that only clears hpet_virt_address, and thus makes all subsequent 
> "hpet_readl()" and "hpet_writel()" calls oops.
> 
> But it doesn't actually *tell* anybody that the HPET is disabled, so if 
> you later on do
> 
> 	if (is_hpet_capable()) {
> 		time = hpet_readl(..);
> 		..
> 
> you will just Oops!
> 
> So as far as I can see, even with your latest patch, if hpet_enable() 
> fails (and triggers the "goto out_nohpet" cases), you'll just oops 
> immediately when you try to suspend/resume the HPET.
> 
> THAT was what I meant - when we clear hpet_virt_address, we should also 
> tell all potential subsequent users that the HPET is not there!
> 
> 		Linus
> 

Hi,
	I agree, and as you said I did exactly that:

out_nohpet:
	iounmap(hpet_virt_address);
	hpet_virt_address = NULL;
	boot_hpet_disable = 1; <<<--- this ensures that is_hpet_capable() will never return positive value


	I also sent an updated version on my patch with subject line "[PATCH v2] Add suspend/resume for HPET"

	I forgot (a typo) to check error code in  hpet_register_sysfs
	Thanks to Sergei Shtylyov for pointing me on that.

	This patch should be ok.

	Best regards,
		Maxim Levitsky

Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions

[Linus Torvalds]

On Thu, 29 Mar 2007, Maxim Levitsky wrote:
>
> 	I agree, and as you said I did exactly that:

Gaah. I'm blind. Sorry. Your patch did indeed do exactly that, I somehow 
overlooked it.

		Linus

Re: [patch, v2] add suspend/resume for HPET

[Ingo Molnar]

update: i've tested Maxim's v2 patch on both a hpet-capable and a 
hpet-less system, and it works fine here.

on a dual-core hpet-capable system, running a NO_HZ+!HIGH_RES_TIMERS 
kernel:

  europe:~> grep Clock /proc/timer_list
  Clock Event Device: hpet
  Clock Event Device: lapic
  Clock Event Device: lapic

s2ram works fine now - it hung on resume before.

on a dual-core non-hpet system, with a NO_HZ+!HIGH_RES_TIMERS kernel:

  neptune:~> grep Clock /proc/timer_list
  Clock Event Device: pit
  Clock Event Device: lapic
  Clock Event Device: lapic

s2ram worked fine before - and it still works now.

(The combination of NO_HZ+!HIGH_RES_TIMERS was the most fragile wrt. 
suspend because in the !HIGH_RES_TIMERS there's just a single instance 
after resume that we touch the timer hardware, and we very much rely on 
the periodic interrupt being set to the precise value.)

So this is a go on my systems - good work Maxim! (I've reproduced 
Maxim's patch below with minor patch-metadata updates.)

	Ingo

---------------------------->
Subject: [patch] add suspend/resume for HPET
From: Maxim Levitsky <maximlevitsky@gmail.com>

This adds support for suspend/resume on i386 for HPET.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

---
 arch/i386/kernel/hpet.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

Index: linux/arch/i386/kernel/hpet.c
===================================================================
--- linux.orig/arch/i386/kernel/hpet.c
+++ linux/arch/i386/kernel/hpet.c
@@ -3,6 +3,8 @@
 #include <linux/errno.h>
 #include <linux/hpet.h>
 #include <linux/init.h>
+#include <linux/sysdev.h>
+#include <linux/pm.h>
 
 #include <asm/hpet.h>
 #include <asm/io.h>
@@ -307,6 +309,7 @@ int __init hpet_enable(void)
 out_nohpet:
 	iounmap(hpet_virt_address);
 	hpet_virt_address = NULL;
+	boot_hpet_disable = 1;
 	return 0;
 }
 
@@ -523,3 +526,68 @@ irqreturn_t hpet_rtc_interrupt(int irq, 
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	if (!is_hpet_capable())
+		return 0;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		err = sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif

Re: [3/6] 2.6.21-rc4: known regressions

[Michael S. Tsirkin]

> Subject    : first disk access after resume takes several minutes
>              ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
> References : http://lkml.org/lkml/2007/3/8/117
>              http://lkml.org/lkml/2007/3/25/20
> Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>

...

> Subject    : after resume: X hangs after drawing a couple of windows
> References : http://lkml.org/lkml/2007/3/8/117
> Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
> Status     : unknown

...

> > > > From: Jeff Chua <jeff.chua.linux@gmail.com>
> > > > > It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
> > > > > suspend and resume from RAM (s2ram). Even better, it works
> > > > > with/without CONFIG_NO_HZ.
> 
> Quoting Maxim <maximlevitsky@gmail.com>:
> 
> Hi,
> 	I almost sure Iknow why this happens,
> 			The problem is that both hpet clock source
> 	and hpet clockevents doesn't have a suspend/resume function
> 	On resume we should enable the main counter _and_ enable
> 	legacy replacement mode, On my system main counter in
> 	enabled, by I think by bios, but legacy replacement mode is
> 	not, so if a system doesn't use lapic as a tick source, but
> 	use hpet+broadcast, it will hang for sure on resume, and i
> 	tested it
> 	
> 			The patch below is a temporally fix, until
> 	clock-events and clocksources will get proper suspend/resume
> 	hooks:
> 
> 		Regards,
> 			Maxim Levitsky

Bingo!

The patch below fixes the two problems (listed above) with
resume from RAM that I have observed on my T60 with
2.6.21-rc5: with this patch applied, and with CONFIG_NO_HZ
unset, date advances correctly, X functions properly and
there is no delay on first disk access.

Thanks very much.

---
> Add suspend/resume for HPET
> Signed-off-by: Maxim Levitsky <maximlevisky@gmail.com>

Maxim, do you plan to send this upstream?

Acked-by: Michael S. Tsirkin <mst@dev.mellanox.co.il>

---

diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index 0fd9fba..a1ec79e 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode,
 	unsigned long cfg, cmp, now;
 	uint64_t delta;
 
+
+	if ( mode != CLOCK_EVT_MODE_UNUSED && mode != CLOCK_EVT_MODE_SHUTDOWN)
+	{
+		unsigned long cfg = hpet_readl(HPET_CFG);
+		cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
+		hpet_writel(cfg, HPET_CFG);
+		
+	}
+		
+
 	switch(mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult;

-- 
MST

Re: [3/6] 2.6.21-rc4: known regressions

[Ingo Molnar]

* Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:

> Bingo!
> 
> The patch below fixes the two problems (listed above) with resume from 
> RAM that I have observed on my T60 with 2.6.21-rc5: with this patch 
> applied, and with CONFIG_NO_HZ unset, date advances correctly, X 
> functions properly and there is no delay on first disk access.
> 
> Thanks very much.
[...]
> 
> Maxim, do you plan to send this upstream?

we'll fix this the right way tomorrow, by adding a proper suspend/resume 
sysdev handler - the lapic clockevents driver already has that. Maxim 
definitely deserves alot of kudos for having found this bug!

	Ingo

Re: [3/6] 2.6.21-rc4: known regressions

[Randy Dunlap]

On Wed, 28 Mar 2007 20:04:57 +0200 Michael S. Tsirkin wrote:

> > Subject    : first disk access after resume takes several minutes
> >              ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
> > References : http://lkml.org/lkml/2007/3/8/117
> >              http://lkml.org/lkml/2007/3/25/20
> > Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
> 
> ...
> 
> > Subject    : after resume: X hangs after drawing a couple of windows
> > References : http://lkml.org/lkml/2007/3/8/117
> > Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
> > Status     : unknown
> 
> ...
> 
> > > > > From: Jeff Chua <jeff.chua.linux@gmail.com>
> > > > > > It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can
> > > > > > suspend and resume from RAM (s2ram). Even better, it works
> > > > > > with/without CONFIG_NO_HZ.
> > 
> > Quoting Maxim <maximlevitsky@gmail.com>:
> > 
> > Hi,
> > 	I almost sure Iknow why this happens,
> > 			The problem is that both hpet clock source
> > 	and hpet clockevents doesn't have a suspend/resume function
> > 	On resume we should enable the main counter _and_ enable
> > 	legacy replacement mode, On my system main counter in
> > 	enabled, by I think by bios, but legacy replacement mode is
> > 	not, so if a system doesn't use lapic as a tick source, but
> > 	use hpet+broadcast, it will hang for sure on resume, and i
> > 	tested it
> > 	
> > 			The patch below is a temporally fix, until
> > 	clock-events and clocksources will get proper suspend/resume
> > 	hooks:
> > 
> > 		Regards,
> > 			Maxim Levitsky
> 
> Bingo!
> 
> The patch below fixes the two problems (listed above) with
> resume from RAM that I have observed on my T60 with
> 2.6.21-rc5: with this patch applied, and with CONFIG_NO_HZ
> unset, date advances correctly, X functions properly and
> there is no delay on first disk access.
> 
> Thanks very much.
> 
> ---
> > Add suspend/resume for HPET
> > Signed-off-by: Maxim Levitsky <maximlevisky@gmail.com>
> 
> Maxim, do you plan to send this upstream?

with whitespace fixes, please...


> Acked-by: Michael S. Tsirkin <mst@dev.mellanox.co.il>
> 
> ---
> 
> diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
> index 0fd9fba..a1ec79e 100644
> --- a/arch/i386/kernel/hpet.c
> +++ b/arch/i386/kernel/hpet.c
> @@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode,
>  	unsigned long cfg, cmp, now;
>  	uint64_t delta;
>  
> +
> +	if ( mode != CLOCK_EVT_MODE_UNUSED && mode != CLOCK_EVT_MODE_SHUTDOWN)
> +	{

	if (mode != CLOCK_EVT_MODE_UNUSED && mode != CLOCK_EVT_MODE_SHUTDOWN) {

> +		unsigned long cfg = hpet_readl(HPET_CFG);
> +		cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
> +		hpet_writel(cfg, HPET_CFG);
> +		

delete above line.

> +	}
> +		
> +
>  	switch(mode) {
>  	case CLOCK_EVT_MODE_PERIODIC:
>  		delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult;


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [3/6] 2.6.21-rc4: known regressions

[Jeff Chua]

On 3/29/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> > Quoting Maxim <maximlevitsky@gmail.com>:
> >                       The patch below is a temporally fix, until
> >       clock-events and clocksources will get proper suspend/resume
> >       hooks:
> Bingo!

I confirmed that it suspend/resume disk/ram all works on my X60s with
CONFIG_HPET_TIMER=y and CONFIG_NO_HZ unset.

But suspend to disk still hang with CONFIG_NO_HZ=y no matter what
setting CONFIG_HPET_TIMER is set to.

Thanks,
Jeff.

[4/6] 2.6.21-rc4: known regressions

[Adrian Bunk]

This email lists some known regressions in Linus' tree compared to 2.6.20.

If you find your name in the Cc header, you are either submitter of one
of the bugs, maintainer of an affectected subsystem or driver, a patch
of you caused a breakage or I'm considering you in any other way
possibly involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.


Subject    : suspend/resume hangs (until keypress)  (CONFIG_NO_HZ)
References : http://bugzilla.kernel.org/show_bug.cgi?id=8181
             http://lkml.org/lkml/2007/3/11/112
Submitter  : Tomas Janousek <tomi@nomi.cz>
             Thomas Meyer <thomas@m3y3r.de>
             Milan Broz <mbroz@redhat.com>
Handled-By : Thomas Gleixner <tglx@linutronix.de>
Patch      : http://lkml.org/lkml/2007/3/16/406
Status     : patch available


Subject    : system doesn't come out of suspend  (CONFIG_NO_HZ)
References : http://lkml.org/lkml/2007/2/22/391
Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
             Soeren Sonnenburg <kernel@nn7.de>
Handled-By : Thomas Gleixner <tglx@linutronix.de>
             Ingo Molnar <mingo@elte.hu>
             Tejun Heo <htejun@gmail.com>
             Rafael J. Wysocki <rjw@sisk.pl>
Status     : problem is being debugged


Subject    : first disk access after resume takes several minutes
             ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
References : http://lkml.org/lkml/2007/3/8/117
Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
Handled-By : Thomas Gleixner <tglx@linutronix.de>
             Ingo Molnar <mingo@elte.hu>
Status     : problem is being debugged


Subject    : after resume: X hangs after drawing a couple of windows
References : http://lkml.org/lkml/2007/3/8/117
Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
Status     : unknown

[3/5] 2.6.21-rc4: known regressions (v2)

[Adrian Bunk]

This email lists some known regressions in Linus' tree compared to 2.6.20.

If you find your name in the Cc header, you are either submitter of one
of the bugs, maintainer of an affectected subsystem or driver, a patch
of you caused a breakage or I'm considering you in any other way
possibly involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.


Subject    : ThinkPad X60: resume no longer works  (PCI related?)
References : http://lkml.org/lkml/2007/3/13/3
Submitter  : Dave Jones <davej@redhat.com>
             Jeremy Fitzhardinge <jeremy@goop.org>
Caused-By  : PCI merge
             commit 78149df6d565c36675463352d0bfe0000b02b7a7
Handled-By : Eric W. Biederman <ebiederm@xmission.com>
             Rafael J. Wysocki <rjw@sisk.pl>
Status     : problem is being debugged


Subject    : MacMini: doesn't come out of suspend to ram
References : http://lkml.org/lkml/2007/3/21/374
Submitter  : Frédéric RISS <frederic.riss@gmail.com>
             Tino Keitel <tino.keitel@gmx.de>
Caused-By  : Bob Moore <robert.moore@intel.com>
             commit c5a7156959e89b32260ad6072bbf5077bcdfbeee
Status     : unknown


Subject    : Suspend to RAM doesn't work anymore  (ACPI?)
References : http://lkml.org/lkml/2007/3/19/128
             http://bugzilla.kernel.org/show_bug.cgi?id=8247
Submitter  : Tobias Doerffel <tobias.doerffel@gmail.com>
Handled-By : Rafael J. Wysocki <rjw@sisk.pl>
Status     : problem is being debugged


Subject    : s2ram autowake regression  (ACPI?)
References : http://lkml.org/lkml/2007/3/20/96
Submitter  : Pavel Machek <pavel@ucw.cz>
Handled-By : Len Brown <lenb@kernel.org>
Status     : submitter was asked to test a patch


Subject    : ThinkPad doesn't resume from suspend to RAM
References : http://lkml.org/lkml/2007/2/27/80
             http://lkml.org/lkml/2007/2/28/348
Submitter  : Jens Axboe <jens.axboe@oracle.com>
             Jeff Chua <jeff.chua.linux@gmail.com>
Status     : unknown


Subject    : suspend to disk hangs
References : http://lkml.org/lkml/2007/3/6/142
Submitter  : Jeff Chua <jeff.chua.linux@gmail.com>
Status     : unknown


Subject    : suspend to disk hangs
References : http://lkml.org/lkml/2007/3/16/126
Submitter  : Maxim Levitsky <maximlevitsky@gmail.com>
Caused-By  : Rafael J. Wysocki <rjw@sisk.pl>
             commit e3c7db621bed4afb8e231cb005057f2feb5db557
             commit ed746e3b18f4df18afa3763155972c5835f284c5
             commit 259130526c267550bc365d3015917d90667732f1
Status     : unknown


Subject    : suspend to disk hangs
References : http://bugzilla.kernel.org/show_bug.cgi?id=8224
Submitter  : Mike Harris <atarimike@wavecable.com>
Status     : unknown


Subject    : ThinkPad R60: suspend to disk broken
References : http://lkml.org/lkml/2007/3/23/74
Submitter  : Marcus Better <marcus@better.se>
Status     : submitter tries to bisect


Subject    : after resume: X hangs after drawing a couple of windows
References : http://lkml.org/lkml/2007/3/8/117
Submitter  : Michael S. Tsirkin <mst@mellanox.co.il>
Status     : unknown


Subject    : second suspend to disk in a row results in an oops  (libata?)
References : http://lkml.org/lkml/2007/3/17/43
Submitter  : Thomas Meyer <thomas@m3y3r.de>
Status     : unknown


Subject    : SATA breakage on resume
References : http://lkml.org/lkml/2007/3/7/233
Submitter  : Thomas Gleixner <tglx@linutronix.de>
             Soeren Sonnenburg <kernel@nn7.de>
Status     : unknown


-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [3/5] 2.6.21-rc4: known regressions (v2)

[Maxim]

On Friday 23 March 2007 20:50:22 Adrian Bunk wrote:

> Subject    : suspend to disk hangs
> References : http://lkml.org/lkml/2007/3/16/126
> Submitter  : Maxim Levitsky <maximlevitsky@gmail.com>
> Caused-By  : Rafael J. Wysocki <rjw@sisk.pl>
>              commit e3c7db621bed4afb8e231cb005057f2feb5db557
>              commit ed746e3b18f4df18afa3763155972c5835f284c5
>              commit 259130526c267550bc365d3015917d90667732f1
> Status     : unknown
> 
> 

Hello, 
	It is fixed

	The problem is that now cpu_up/cpu_down is called with tasks frozen,
	and this can lead to deadlock if some driver that registered cpu up/down notifier, sleeps,

	On my system it froze in two places, one in XFS due to freezable workqueues, and in
	microcode update driver that ask the "frozen" userspace for firmware.

	Fix for XFS is already in mainline, and Rafael J. Wysocki. already posted a patch that fixes microcode issue,
	I will test it.

	But I feel that there are more drivers that can deadlock system in same way, on my system S3/S4 works perfect :-)
	Even the weird  hang i had disappeared.

	Big thanks to Rafael J. Wysocki.


	Best regards,
		Maxim Levitsky

Re: [3/5] 2.6.21-rc4: known regressions (v2)

[Rafael J. Wysocki]

On Friday, 23 March 2007 19:50, Adrian Bunk wrote:
> This email lists some known regressions in Linus' tree compared to 2.6.20.
> 
> If you find your name in the Cc header, you are either submitter of one
> of the bugs, maintainer of an affectected subsystem or driver, a patch
> of you caused a breakage or I'm considering you in any other way
> possibly involved with one or more of these issues.
> 
> Due to the huge amount of recipients, please trim the Cc when answering.
> 
[--snip--]
> Subject    : suspend to disk hangs
> References : http://lkml.org/lkml/2007/3/16/126
> Submitter  : Maxim Levitsky <maximlevitsky@gmail.com>
> Caused-By  : Rafael J. Wysocki <rjw@sisk.pl>
>              commit e3c7db621bed4afb8e231cb005057f2feb5db557
>              commit ed746e3b18f4df18afa3763155972c5835f284c5
>              commit 259130526c267550bc365d3015917d90667732f1
> Status     : unknown

The problem has been identified as the known issue related to the XFS freezable
workqueues.  There is a patch available (http://lkml.org/lkml/2007/3/21/328),
that has been merged.

Still, there is a problem with the microcode update driver that's being worked
on.

The reporters of the resume problems who use the microcode driver, please
check if the problems go away if you unload the driver before the suspend.

Greetings,
Rafael

Re: [3/5] 2.6.21-rc4: known regressions (v2)

[Adrian Bunk]

On Mon, Mar 26, 2007 at 07:42:51PM +0200, Marcus Better wrote:
> Adrian Bunk wrote:
> > > > Subject    : ThinkPad R60: suspend to disk broken
> 
> > Does setting CONFIG_PCI_MSI=n make any difference?
> 
> Yes, it does. The hanging resume problem went away. 

Thanks for testing.

If you enable it again, does the patch from [1] also fix it?

> (The display corruption and the instant resume were not affected.)

Not a surprise - it's currently quite common that people run into 
several distinct suspend regressions...

> Marcus

cu
Adrian

[1] http://lkml.org/lkml/2007/3/24/136 [2]
[2] x86_64 uses arch/i386/pci/common.c

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [3/5] 2.6.21-rc4: known regressions (v2)

[Marcus Better]

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]

Adrian Bunk wrote:
> > > Does setting CONFIG_PCI_MSI=n make any difference?
> >
> > Yes, it does. The hanging resume problem went away.
>
> Thanks for testing.
>
> If you enable it again, does the patch from [1] also fix it?

Yes, it appears to fix it.

Marcus

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [linux-pm] [PATCH v2] Add suspend/resume for HPET

[Alan Stern]

On Saturday March 31, 2007, David Brownell wrote:

> I'm about ready to test the appended patch... a "move one device" call
> might be safest at this point in the release cycle though.
> 
> - Dave
> 
> ========================	SNIP!
> Change how the PM list is constructed, so that devices are added right
> after their parents (when they have one) rather than at the end of the
> list.  This preserves sequencing guarantees, but enables sequencing of
> suspend/resume operations by more important characteristics than "when
> device happened to enumerate" ... e.g. clocksources and clockevents
> at a clearly defined point during suspend and resume.
> 
> This patch has a potential downside for devices that have multiple
> power dependencies and which "just happened to work" before.

Dave:

Would you mind retracting this patch?  It will interfere with some work
I've been doing in USB, work that relies on exactly the sort of multiple 
power dependency you mention.

The problem is this: When a low- or full-speed USB device is resumed 
following a power loss (this is part of the "persistent USB" patch), it's 
necessary for the corresponding EHCI root hub to be resumed first so that 
the port's OWNER bit can be set properly.

It works fine as long as devices are resumed in order of registration,
because a low- or full-speed USB device can't be registered before the
high-speed root hub.  (If it was, it would be disconnected when the
high-speed root hub initialized and took control of the port.)  So this
isn't a "just happened to work" situation; it really is a critical
ordering dependency.

The clock source problem can be solved in other ways.  For instance, right 
now things are set up so that clock sources are registered at various 
random times and the clockevents code adapts to use the best available 
device, right?.  So simply arrange for a clock source to "depart" as it is 
suspended; then clockevents can fall back to using other lower-precision 
sources.  As long as devices suspend in reverse order of discovery, the 
system should always function properly.

Alan Stern