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-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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

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

[PATCH v2] Add suspend/resume for HPET

2007-03-29 Thread Maxim Levitsky 
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 +++
 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

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

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

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

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-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2007-03-29 Thread 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-kernelm=117219531503973w=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

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

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

2007-03-29 Thread 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-kernelm=117219531503973w=2
 
 also:   http://marc.info/?l=linux-kernelm=117219531526317w=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
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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

---

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;

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

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

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

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

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

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





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

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

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

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

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

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

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

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

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

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

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

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

2007-03-23 Thread 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 [EMAIL PROTECTED]
 Caused-By  : Rafael J. Wysocki [EMAIL PROTECTED]
  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
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html