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

2007-03-31 Thread 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Ingo Molnar

* Linus Torvalds <[EMAIL PROTECTED]> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Ingo Molnar

* Linus Torvalds <[EMAIL PROTECTED]> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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 <[EMAIL PROTECTED]>
> >
> > +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= _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  % of cpu time (this is not a typo)
And other 'normal' programs that are running show same  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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Thomas Gleixner
On Sun, 2007-04-01 at 00:01 +0800, Jeff Chua wrote:
> On 3/31/07, Thomas Gleixner <[EMAIL PROTECTED]> 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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Jeff Chua

On 3/31/07, Thomas Gleixner <[EMAIL PROTECTED]> 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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 <[EMAIL PROTECTED]>
>
> +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= _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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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 [EMAIL PROTECTED]

 +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


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Jeff Chua

On 3/31/07, Thomas Gleixner [EMAIL PROTECTED] 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.



meConfirmed that suspend/resume disk/ram works on X60s with
meCONFIG_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.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Thomas Gleixner
On Sun, 2007-04-01 at 00:01 +0800, Jeff Chua wrote:
 On 3/31/07, Thomas Gleixner [EMAIL PROTECTED] 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.
 
 
 meConfirmed that suspend/resume disk/ram works on X60s with
 meCONFIG_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


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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 [EMAIL PROTECTED]
 
  +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  % of cpu time (this is not a typo)
And other 'normal' programs that are running show same  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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Ingo Molnar

* Linus Torvalds [EMAIL PROTECTED] 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread Ingo Molnar

* Linus Torvalds [EMAIL PROTECTED] 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-31 Thread 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

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread Andi Kleen
Ingo Molnar <[EMAIL PROTECTED]> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread Jeff Chua

On 3/29/07, Maxim Levitsky <[EMAIL PROTECTED]> wrote:

Subject: Add suspend/resume for HPET
This adds support of suspend/resume on i386 for HPET
Signed-off-by: Maxim Levitsky <[EMAIL PROTECTED]>


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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread Ingo Molnar

* Linus Torvalds <[EMAIL PROTECTED]> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread 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 <[EMAIL PROTECTED]>

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

Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
Signed-off-by: Maxim Levitsky <[EMAIL PROTECTED]>

---
 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 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -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= _class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+   int err;
+
+   if (!is_hpet_capable())
+   return 0;
+
+   err = sysdev_class_register(_class);
+
+   if (!err) {
+   err = sysdev_register(_device);
+   if (err)
+   sysdev_class_unregister(_class);
+   }
+
+   return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread 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 <[EMAIL PROTECTED]>
> > 
> > ---
> >  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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread 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 <[EMAIL PROTECTED]>
> 
> ---
>  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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread 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 [EMAIL PROTECTED]
 
 ---
  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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread 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 [EMAIL PROTECTED]
  
  ---
   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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread 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 [EMAIL PROTECTED]

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

Signed-off-by: Ingo Molnar [EMAIL PROTECTED]
Signed-off-by: Maxim Levitsky [EMAIL PROTECTED]

---
 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread Ingo Molnar

* Linus Torvalds [EMAIL PROTECTED] 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread Jeff Chua

On 3/29/07, Maxim Levitsky [EMAIL PROTECTED] wrote:

Subject: Add suspend/resume for HPET
This adds support of suspend/resume on i386 for HPET
Signed-off-by: Maxim Levitsky [EMAIL PROTECTED]


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.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-03-29 Thread Andi Kleen
Ingo Molnar [EMAIL PROTECTED] 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/