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

Groups > linux.kernel > #1690620 > unrolled thread

Re: [PATCH v5 2/5] tick: Add freeze timer events

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 v5 2/5] tick: Add freeze timer events "dbasehore ." <dbasehore@chromium.org> - 2017-07-18 22:10 +0200
    Re: [PATCH v5 2/5] tick: Add freeze timer events Thomas Gleixner <tglx@linutronix.de> - 2017-07-19 00:00 +0200
      Re: [PATCH v5 2/5] tick: Add freeze timer events "dbasehore ." <dbasehore@chromium.org> - 2017-07-19 00:10 +0200
        Re: [PATCH v5 2/5] tick: Add freeze timer events Thomas Gleixner <tglx@linutronix.de> - 2017-07-19 00:30 +0200
          Re: [PATCH v5 2/5] tick: Add freeze timer events Thomas Gleixner <tglx@linutronix.de> - 2017-07-19 00:40 +0200
          Re: [PATCH v5 2/5] tick: Add freeze timer events "dbasehore ." <dbasehore@chromium.org> - 2017-07-19 00:40 +0200

#1690620 — Re: [PATCH v5 2/5] tick: Add freeze timer events

On Mon, Jul 17, 2017 at 11:40 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 17 Jul 2017, dbasehore . wrote:
>> On Mon, Jul 17, 2017 at 6:33 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> I could make a patch to try it out. I would probably add a flag to rtc
>> timers to indicate whether it wakes the system (default true). We
>> would have to add a sync with the rtc irq and the rtc irqwork. I would
>> probably add a rtc_timer_sync function that would flush the rtc irq
>> and flush the irqwork. I would call this after the freeze_ops sync
>> function since the sci irq needs to finish before syncing with the rtc
>> irq. Also, pm_wakeup_irq seems racy with the current implementation of
>> s2idle_loop since the RTC irq could be mistakenly set as pm_wakeup_irq
>> when something else actually triggered the full wakeup. Fortunately, I
>> don't think pm_wakeup_irq is used for anything except debugging, but
>> we might change that.
>
> There is another option which you might consider. We can reserve one of the
> HPET comparators for that purpose and have a special interrupt handler for
> it. Checking the HPET for expiry from the low level code should be trivial.
>

Does that handle setting up new timers properly or does the timer sync
code still need to be written?

> Thanks,
>
>         tglx
>
>

[toc] | [next] | [standalone]

#1690724

On Tue, 18 Jul 2017, dbasehore . wrote:
> On Mon, Jul 17, 2017 at 11:40 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Mon, 17 Jul 2017, dbasehore . wrote:
> >> On Mon, Jul 17, 2017 at 6:33 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> I could make a patch to try it out. I would probably add a flag to rtc
> >> timers to indicate whether it wakes the system (default true). We
> >> would have to add a sync with the rtc irq and the rtc irqwork. I would
> >> probably add a rtc_timer_sync function that would flush the rtc irq
> >> and flush the irqwork. I would call this after the freeze_ops sync
> >> function since the sci irq needs to finish before syncing with the rtc
> >> irq. Also, pm_wakeup_irq seems racy with the current implementation of
> >> s2idle_loop since the RTC irq could be mistakenly set as pm_wakeup_irq
> >> when something else actually triggered the full wakeup. Fortunately, I
> >> don't think pm_wakeup_irq is used for anything except debugging, but
> >> we might change that.
> >
> > There is another option which you might consider. We can reserve one of the
> > HPET comparators for that purpose and have a special interrupt handler for
> > it. Checking the HPET for expiry from the low level code should be trivial.
> >
> Does that handle setting up new timers properly or does the timer sync
> code still need to be written?

Sorry, I don't understand the question. What is timer sync code?

Thanks,

	tglx

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

#1690751

On Tue, Jul 18, 2017 at 2:53 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 18 Jul 2017, dbasehore . wrote:
>> On Mon, Jul 17, 2017 at 11:40 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Mon, 17 Jul 2017, dbasehore . wrote:
>> >> On Mon, Jul 17, 2017 at 6:33 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> >> I could make a patch to try it out. I would probably add a flag to rtc
>> >> timers to indicate whether it wakes the system (default true). We
>> >> would have to add a sync with the rtc irq and the rtc irqwork. I would
>> >> probably add a rtc_timer_sync function that would flush the rtc irq
>> >> and flush the irqwork. I would call this after the freeze_ops sync
>> >> function since the sci irq needs to finish before syncing with the rtc
>> >> irq. Also, pm_wakeup_irq seems racy with the current implementation of
>> >> s2idle_loop since the RTC irq could be mistakenly set as pm_wakeup_irq
>> >> when something else actually triggered the full wakeup. Fortunately, I
>> >> don't think pm_wakeup_irq is used for anything except debugging, but
>> >> we might change that.
>> >
>> > There is another option which you might consider. We can reserve one of the
>> > HPET comparators for that purpose and have a special interrupt handler for
>> > it. Checking the HPET for expiry from the low level code should be trivial.
>> >
>> Does that handle setting up new timers properly or does the timer sync
>> code still need to be written?
>
> Sorry, I don't understand the question. What is timer sync code?
>

Does the comparator allow you to have a completely separate alarm set
in the hardware? If there's another timer setup (say some user
specified wake alarm), we need to program that alarm after the current
one goes off if we aren't able to program multiple alarms at the same
time.

> Thanks,
>
>         tglx

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

#1690781

On Tue, 18 Jul 2017, dbasehore . wrote:
> On Tue, Jul 18, 2017 at 2:53 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Tue, 18 Jul 2017, dbasehore . wrote:
> >> On Mon, Jul 17, 2017 at 11:40 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > On Mon, 17 Jul 2017, dbasehore . wrote:
> >> >> On Mon, Jul 17, 2017 at 6:33 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> >> I could make a patch to try it out. I would probably add a flag to rtc
> >> >> timers to indicate whether it wakes the system (default true). We
> >> >> would have to add a sync with the rtc irq and the rtc irqwork. I would
> >> >> probably add a rtc_timer_sync function that would flush the rtc irq
> >> >> and flush the irqwork. I would call this after the freeze_ops sync
> >> >> function since the sci irq needs to finish before syncing with the rtc
> >> >> irq. Also, pm_wakeup_irq seems racy with the current implementation of
> >> >> s2idle_loop since the RTC irq could be mistakenly set as pm_wakeup_irq
> >> >> when something else actually triggered the full wakeup. Fortunately, I
> >> >> don't think pm_wakeup_irq is used for anything except debugging, but
> >> >> we might change that.
> >> >
> >> > There is another option which you might consider. We can reserve one of the
> >> > HPET comparators for that purpose and have a special interrupt handler for
> >> > it. Checking the HPET for expiry from the low level code should be trivial.
> >> >
> >> Does that handle setting up new timers properly or does the timer sync
> >> code still need to be written?
> >
> > Sorry, I don't understand the question. What is timer sync code?
> >
> 
> Does the comparator allow you to have a completely separate alarm set
> in the hardware? If there's another timer setup (say some user
> specified wake alarm), we need to program that alarm after the current
> one goes off if we aren't able to program multiple alarms at the same
> time.

The HPET consists of several independent comparator units. The kernel uses
only a limited set of them. We can reserve one or more for that purpose, so
it does not require any multiplexing or whatever. How many of them do you
need?

Thanks,

	tglx

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

#1690785

On Tue, 18 Jul 2017, dbasehore . wrote:
> On Tue, Jul 18, 2017 at 3:22 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > The HPET consists of several independent comparator units. The kernel uses
> > only a limited set of them. We can reserve one or more for that purpose, so
> > it does not require any multiplexing or whatever. How many of them do you
> > need?
> >
> 
> I just need one. Is it a different irq than the RTC? It would be nice
> if we could avoid going through the s2idle_loop code.

Yes, it is a different IRQ.

Thanks,

	tglx

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

#1690790

On Tue, Jul 18, 2017 at 3:22 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 18 Jul 2017, dbasehore . wrote:
>> On Tue, Jul 18, 2017 at 2:53 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Tue, 18 Jul 2017, dbasehore . wrote:
>> >> On Mon, Jul 17, 2017 at 11:40 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> > On Mon, 17 Jul 2017, dbasehore . wrote:
>> >> >> On Mon, Jul 17, 2017 at 6:33 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> >> >> I could make a patch to try it out. I would probably add a flag to rtc
>> >> >> timers to indicate whether it wakes the system (default true). We
>> >> >> would have to add a sync with the rtc irq and the rtc irqwork. I would
>> >> >> probably add a rtc_timer_sync function that would flush the rtc irq
>> >> >> and flush the irqwork. I would call this after the freeze_ops sync
>> >> >> function since the sci irq needs to finish before syncing with the rtc
>> >> >> irq. Also, pm_wakeup_irq seems racy with the current implementation of
>> >> >> s2idle_loop since the RTC irq could be mistakenly set as pm_wakeup_irq
>> >> >> when something else actually triggered the full wakeup. Fortunately, I
>> >> >> don't think pm_wakeup_irq is used for anything except debugging, but
>> >> >> we might change that.
>> >> >
>> >> > There is another option which you might consider. We can reserve one of the
>> >> > HPET comparators for that purpose and have a special interrupt handler for
>> >> > it. Checking the HPET for expiry from the low level code should be trivial.
>> >> >
>> >> Does that handle setting up new timers properly or does the timer sync
>> >> code still need to be written?
>> >
>> > Sorry, I don't understand the question. What is timer sync code?
>> >
>>
>> Does the comparator allow you to have a completely separate alarm set
>> in the hardware? If there's another timer setup (say some user
>> specified wake alarm), we need to program that alarm after the current
>> one goes off if we aren't able to program multiple alarms at the same
>> time.
>
> The HPET consists of several independent comparator units. The kernel uses
> only a limited set of them. We can reserve one or more for that purpose, so
> it does not require any multiplexing or whatever. How many of them do you
> need?
>

I just need one. Is it a different irq than the RTC? It would be nice
if we could avoid going through the s2idle_loop code.

> Thanks,
>
>         tglx
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web