Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1270433 > unrolled thread

RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

Started byThomas Gleixner <tglx@linutronix.de>
First post2015-11-16 19:10 +0100
Last post2015-11-17 11:10 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system  resume from S3 Thomas Gleixner <tglx@linutronix.de> - 2015-11-16 19:10 +0100
    RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system  resume from S3 "Lee, Zhuo-hao" <zhuo-hao.lee@intel.com> - 2015-11-17 08:10 +0100
      RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system  resume from S3 Thomas Gleixner <tglx@linutronix.de> - 2015-11-17 11:10 +0100

#1270433 — RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

FromThomas Gleixner <tglx@linutronix.de>
Date2015-11-16 19:10 +0100
SubjectRE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3
Message-ID<qvwo2-2fL-13@gated-at.bofh.it>
On Sat, 14 Nov 2015, Lee, Zhuo-hao wrote:

> (1). alarmtimer create a rtc wake up timer however alarmtimer won't
>      remove that timer if the system wake up earlier

That's hardly a bug. That's a slight incorrectness which needs to be
fixed.

> (2). rtc wake up timer will trigger hpet_rtc_interrupt continuously
> until timer timeout.

> This patch only fixed (1). Fixing (1) can avoid (2).
> However, The (2) is another story which it is not covered by this patch.

And that's the real interesting question. Why is hpet_rtc_interrupt
continously triggered?

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/

[toc] | [next] | [standalone]


#1270926

From"Lee, Zhuo-hao" <zhuo-hao.lee@intel.com>
Date2015-11-17 08:10 +0100
Message-ID<qvIyR-1NN-3@gated-at.bofh.it>
In reply to#1270433
>> (1). alarmtimer create a rtc wake up timer however alarmtimer won't
>>      remove that timer if the system wake up earlier

>That's hardly a bug. That's a slight incorrectness which needs to be fixed.
I think this timer is useless after system resume. For the correctness, I think it should be fixed.
Do you agree? Or do you have any other suggestions? :)

>> (2). rtc wake up timer will trigger hpet_rtc_interrupt continuously 
>> until timer timeout.

>> This patch only fixed (1). Fixing (1) can avoid (2).
>> However, The (2) is another story which it is not covered by this patch.

>And that's the real interesting question. Why is hpet_rtc_interrupt continously triggered?

I think this is caused by driver implementation. 
My explanation for the continuous  hpet_rtc_interrupt():
1. In hpet_rtc_timer_init(), it will set "delta" to 1/64 seconds in AIE mode (about 16ms, because of the value DEFAULT_RTC_SHIFT), 
     that will cause hpet_rtc_interrupt() be triggered in every 16ms if the wakeup time (global variable hpet_alarm_time) is not reached.
2. When someone calls rtc_timer_start (ex: alarmtimer_suspend ), hpet driver will set timeout value (current time + delta) 
     to hpet_alarm_time, and then enable hpet timer.
3. In hpet_rtc_interrupt(), it will call hpet_rtc_timer_reinit() to reinit timer and add "delta" to 
     the hpet_t1_cmp for the next timeout value ( so we will receive an interrupt after 16ms).
     If hpet_alarm_time is reached, the irq_handler() will be called, the hpet_rtc_flags will be set to 0 and the hpet timer will be disable.
     Otherwise, we will receive an interrupt in each 16ms.
If the logic of 1,2,3 are correct, no bugs inside, maybe we can redesign the driver to reduce this periodic interrupt.

The following call path enable hpet timer when system go to suspend, just for you reference :)
alarmtimer_suspend
--rtc_timer_start
----rtc_timer_enqueue
------__rtc_set_alarm
--------cmos_set_alarm
----------cmos_irq_enable
------------hpet_set_rtc_irq_bit

Thanks,
Lee, Zhuo-hao
--
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/

[toc] | [prev] | [next] | [standalone]


#1271037

FromThomas Gleixner <tglx@linutronix.de>
Date2015-11-17 11:10 +0100
Message-ID<qvLn3-3DL-15@gated-at.bofh.it>
In reply to#1270926
On Tue, 17 Nov 2015, Lee, Zhuo-hao wrote:

Can you please fix your e-mail client to do proper line breaks around
80 char?

> >> (1). alarmtimer create a rtc wake up timer however alarmtimer won't
> >>      remove that timer if the system wake up earlier
> 
> > That's hardly a bug. That's a slight incorrectness which needs to be fixed.
> 
> I think this timer is useless after system resume. For the
> correctness, I think it should be fixed.  Do you agree? Or do you
> have any other suggestions? :)

Care to read what I wrote?

> >   ... That's a slight incorrectness which needs to be fixed.

> >> (2). rtc wake up timer will trigger hpet_rtc_interrupt continuously 
> >> until timer timeout.
> 
> >> This patch only fixed (1). Fixing (1) can avoid (2).
> >> However, The (2) is another story which it is not covered by this patch.
> 
> > And that's the real interesting question. Why is hpet_rtc_interrupt
> > continously triggered?

> I think this is caused by driver implementation. 
> My explanation for the continuous  hpet_rtc_interrupt():
> 1. In hpet_rtc_timer_init(), it will set "delta" to 1/64 seconds in
>    AIE mode (about 16ms, because of the value DEFAULT_RTC_SHIFT),
>    that will cause hpet_rtc_interrupt() be triggered in every 16ms
>    if the wakeup time (global variable hpet_alarm_time) is not
>    reached.
> 
> 2. When someone calls rtc_timer_start (ex: alarmtimer_suspend ),
>    hpet driver will set timeout value (current time + delta) to
>    hpet_alarm_time, and then enable hpet timer.
>
> 3. In hpet_rtc_interrupt(), it will call hpet_rtc_timer_reinit() to
>    reinit timer and add "delta" to the hpet_t1_cmp for the next
>    timeout value ( so we will receive an interrupt after 16ms).
>
>    If hpet_alarm_time is reached, the irq_handler() will be called, the
>    hpet_rtc_flags will be set to 0 and the hpet timer will be
>    disable.
>
>    Otherwise, we will receive an interrupt in each 16ms.

Right. We cannot do much about it. So the changelog of this patch
wants some explanation. Something like this:

  This was noticed because the HPET RTC emulation fires an interrupt
  every 16ms up to the point where the alarm time is reached.
  
Simply because you wouldn't have noticed if the interrupt happened
after 2 hours.

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/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web