Re: + rtc-disable-hpet-emulation-on-suspend.patch added to -mm tree

Re: + rtc-disable-hpet-emulation-on-suspend.patch added to -mm tree

[Ingo Molnar]

* akpm@linux-foundation.org <akpm@linux-foundation.org> wrote:

> ------------------------------------------------------
> Subject: rtc: disable hpet emulation on suspend
> From: Maxim Levitsky <maximlevitsky@gmail.com>
> 
> I noticed that rtc wont generate interrupts after a resume from disk.
> Here hpet rtc emulation is used.
> 
> Problem is that rtc hpet comparator, isn't reinitialized after resume.
> Easiest way to solve this, is always mask all hpet interrupts on suspend
> This is triggered, when suspending with alarm set.
> 
> 
> Otherwise, hpet driver will think it doesn't need to reinitialize
> the rtc comparator, thus rtc interrupts won't work.
> 
> This emulation isn't need for wakealarm.

> -		hpet_mask_rtc_irq_bit(mask);
>  
> +		/* shut down hpet emulation - we don't need it for alarm */
> +		hpet_mask_rtc_irq_bit(RTC_PIE|RTC_AIE|RTC_UIE);
>  		cmos_checkintr(cmos, tmp);

Would be nice to also unconditionally reinitialize the hpet/rtc after 
resume - regardless of what state we left it in.

Also, are you sure this does not break things like 
CONFIG_PM_TEST_SUSPEND=y which rely on wakealarm?

	Ingo

Re: + rtc-disable-hpet-emulation-on-suspend.patch added to -mm tree

[Maxim Levitsky]

On Wed, 2009-10-14 at 07:56 +0200, Ingo Molnar wrote:
> * akpm@linux-foundation.org <akpm@linux-foundation.org> wrote:
> 
> > ------------------------------------------------------
> > Subject: rtc: disable hpet emulation on suspend
> > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > 
> > I noticed that rtc wont generate interrupts after a resume from disk.
> > Here hpet rtc emulation is used.
> > 
> > Problem is that rtc hpet comparator, isn't reinitialized after resume.
> > Easiest way to solve this, is always mask all hpet interrupts on suspend
> > This is triggered, when suspending with alarm set.
> > 
> > 
> > Otherwise, hpet driver will think it doesn't need to reinitialize
> > the rtc comparator, thus rtc interrupts won't work.
> > 
> > This emulation isn't need for wakealarm.
> 
> > -		hpet_mask_rtc_irq_bit(mask);
> >  
> > +		/* shut down hpet emulation - we don't need it for alarm */
> > +		hpet_mask_rtc_irq_bit(RTC_PIE|RTC_AIE|RTC_UIE);
> >  		cmos_checkintr(cmos, tmp);
> 
> Would be nice to also unconditionally reinitialize the hpet/rtc after 
> resume - regardless of what state we left it in.

I think this is already done.
Problem was that rtc driver was leaving the RTC_AIE, thus breaking the
initialization.

If there are no users of hpet on suspend, (periodic,alarm,update), then
it won't be initialized on resume (only rtc comparator part, other parts
are initialized correctly nowadays) but as soon as anybody start using
it, it will be.
 

> 
> Also, are you sure this does not break things like 
> CONFIG_PM_TEST_SUSPEND=y which rely on wakealarm?


This shouldn't break anything. hpet irqs aren't used for the actual
alarm.

In fact I used a script to loop over real hibernation cycles, and it
works with or without this patch.

But with this patch, hwclock works after a resume.


Best regards,
	Maxim Levitsky

Re: + rtc-disable-hpet-emulation-on-suspend.patch added to -mm tree

[Ingo Molnar]

* Maxim Levitsky <maximlevitsky@gmail.com> wrote:

> On Wed, 2009-10-14 at 07:56 +0200, Ingo Molnar wrote:
> > * akpm@linux-foundation.org <akpm@linux-foundation.org> wrote:
> > 
> > > ------------------------------------------------------
> > > Subject: rtc: disable hpet emulation on suspend
> > > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > > 
> > > I noticed that rtc wont generate interrupts after a resume from disk.
> > > Here hpet rtc emulation is used.
> > > 
> > > Problem is that rtc hpet comparator, isn't reinitialized after resume.
> > > Easiest way to solve this, is always mask all hpet interrupts on suspend
> > > This is triggered, when suspending with alarm set.
> > > 
> > > 
> > > Otherwise, hpet driver will think it doesn't need to reinitialize
> > > the rtc comparator, thus rtc interrupts won't work.
> > > 
> > > This emulation isn't need for wakealarm.
> > 
> > > -		hpet_mask_rtc_irq_bit(mask);
> > >  
> > > +		/* shut down hpet emulation - we don't need it for alarm */
> > > +		hpet_mask_rtc_irq_bit(RTC_PIE|RTC_AIE|RTC_UIE);
> > >  		cmos_checkintr(cmos, tmp);
> > 
> > Would be nice to also unconditionally reinitialize the hpet/rtc after 
> > resume - regardless of what state we left it in.
> 
> I think this is already done.
> Problem was that rtc driver was leaving the RTC_AIE, thus breaking the
> initialization.
> 
> If there are no users of hpet on suspend, (periodic,alarm,update), then
> it won't be initialized on resume (only rtc comparator part, other parts
> are initialized correctly nowadays) but as soon as anybody start using
> it, it will be.
>  
> 
> > 
> > Also, are you sure this does not break things like 
> > CONFIG_PM_TEST_SUSPEND=y which rely on wakealarm?
> 
> 
> This shouldn't break anything. hpet irqs aren't used for the actual
> alarm.
> 
> In fact I used a script to loop over real hibernation cycles, and it 
> works with or without this patch.
> 
> But with this patch, hwclock works after a resume.

Excellent - thanks!

	Ingo