Re: [PATCH v2] clocksource: arm_global_timer: fix suspend resume

2015-11-20 Thread John Stultz 
On Fri, Nov 20, 2015 at 10:35 AM, Grygorii Strashko
 wrote:
> Hi Santosh,
>
> On 11/20/2015 07:23 PM, santosh shilimkar wrote:
>> + Thomas, Marc
>>
>> On 11/20/2015 5:57 AM, Grygorii Strashko wrote:
>>> Now the System stall is observed on TI AM437x based board
>>> (am437x-gp-evm) during resuming from System suspend when ARM Global
>>> timer is selected as clocksource device - SysRq are working, but
>>> nothing else. The reason of stall is that ARM Global timer loses its
>>> contexts.
>>>
>>> The reason of stall is that ARM Global timer loses its contexts during
>>> System suspend:
>>> GT_CONTROL.TIMER_ENABLE = 0 (unbanked)
>>> GT_COUNTERx = 0
>>>
>>> Hence, update ARM Global timer driver to reflect above behaviour
>>> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1
>>> - ensure clocksource and clockevent devices have coresponding flags
>>>(CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set
>>>depending on presence of "always-on" DT property.
>>>
>> Something which loses context in low power states can't be
>> called "always-on"
>
> Sry, it's kinda new area for me and I could make mistakes.
>
> While working on this patch I've:
>  - re-used implementation from ARM arch timer
> commit 82a5619410d4c4df65c04272db198eca5a867c18
> Author: Lorenzo Pieralisi 
> Date:   Tue Apr 8 10:04:32 2014 +0100
>
> clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection issue
>
>
> - and followed timekeeping.txt:
> "Typically the clock source is a monotonic, atomic counter which will provide
>  n bits which count from 0 to 2^(n-1) and then wraps around to 0 and start 
> over.
> It will ideally NEVER stop ticking as long as the system is running. It
> may stop during system suspend."
> ^^
>
> And that exactly my use-case: I'd like to use ARM GT as clocksource
> and with CPUIdle = n. But System suspend has to be allowed.
>
>
>>
>> Also if the clock-soucre can't tick in the low power states
>> then that device shouldn't be used as a clock-source.
>
> Agree. clocksource can't (except with suspend). Have I missed something?

I think the point Stantosh is making is that if the clocksource stops
in suspend (which is allowed) you should not be setting
CLOCK_SOURCE_SUSPEND_NONSTOP (which promises the clocksource doesn't
stop in suspend, so it can be used for suspend timing as well).

The contradictory part in your patch is that you're also setting the
clockevent logic as  CLOCK_EVT_FEAT_C3STOP, which flags that the
clockevent hardware might stop in low-power idle states (ie: not
suspend, but while the system is running).  Usually hardware that
halts in low-power mode (like the apic on some x86 machines) is not
also used for timekeeping (instead we use the hpet/acpi there).

So its unlikely that the hardware both stays running through suspend,
but also might halt in idle. That would be "unique".

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] clocksource: arm_global_timer: fix suspend resume

2015-11-20 Thread John Stultz 
On Fri, Nov 20, 2015 at 11:28 AM, Grygorii Strashko
<grygorii.stras...@ti.com> wrote:
> On 11/20/2015 09:09 PM, John Stultz wrote:
>> On Fri, Nov 20, 2015 at 10:35 AM, Grygorii Strashko
>> <grygorii.stras...@ti.com> wrote:
>>> Hi Santosh,
>>>
>>> On 11/20/2015 07:23 PM, santosh shilimkar wrote:
>>>> + Thomas, Marc
>>>>
>>>> On 11/20/2015 5:57 AM, Grygorii Strashko wrote:
>>>>> Now the System stall is observed on TI AM437x based board
>>>>> (am437x-gp-evm) during resuming from System suspend when ARM Global
>>>>> timer is selected as clocksource device - SysRq are working, but
>>>>> nothing else. The reason of stall is that ARM Global timer loses its
>>>>> contexts.
>>>>>
>>>>> The reason of stall is that ARM Global timer loses its contexts during
>>>>> System suspend:
>>>>>  GT_CONTROL.TIMER_ENABLE = 0 (unbanked)
>>>>>  GT_COUNTERx = 0
>>>>>
>>>>> Hence, update ARM Global timer driver to reflect above behaviour
>>>>> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1
>>>>> - ensure clocksource and clockevent devices have coresponding flags
>>>>> (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set
>>>>> depending on presence of "always-on" DT property.
>>>>>
>>>> Something which loses context in low power states can't be
>>>> called "always-on"
>>>
>>> Sry, it's kinda new area for me and I could make mistakes.
>>>
>>> While working on this patch I've:
>>>   - re-used implementation from ARM arch timer
>>> commit 82a5619410d4c4df65c04272db198eca5a867c18
>>> Author: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>
>>> Date:   Tue Apr 8 10:04:32 2014 +0100
>>>
>>>  clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection 
>>> issue
>>>
>>>
>>> - and followed timekeeping.txt:
>>> "Typically the clock source is a monotonic, atomic counter which will 
>>> provide
>>>   n bits which count from 0 to 2^(n-1) and then wraps around to 0 and start 
>>> over.
>>> It will ideally NEVER stop ticking as long as the system is running. It
>>> may stop during system suspend."
>>> ^^
>>>
>>> And that exactly my use-case: I'd like to use ARM GT as clocksource
>>> and with CPUIdle = n. But System suspend has to be allowed.
>>>
>>>
>>>>
>>>> Also if the clock-soucre can't tick in the low power states
>>>> then that device shouldn't be used as a clock-source.
>>>
>>> Agree. clocksource can't (except with suspend). Have I missed something?
>>
>> I think the point Stantosh is making is that if the clocksource stops
>> in suspend (which is allowed) you should not be setting
>> CLOCK_SOURCE_SUSPEND_NONSTOP (which promises the clocksource doesn't
>> stop in suspend, so it can be used for suspend timing as well).
>>
>
> Ok. Thanks. I got it now. Adding CLOCK_SOURCE_SUSPEND_NONSTOP is mistake.
>
>> The contradictory part in your patch is that you're also setting the
>> clockevent logic as  CLOCK_EVT_FEAT_C3STOP, which flags that the
>> clockevent hardware might stop in low-power idle states (ie: not
>> suspend, but while the system is running).  Usually hardware that
>> halts in low-power mode (like the apic on some x86 machines) is not
>> also used for timekeeping (instead we use the hpet/acpi there).
>
> Sry, I've set CLOCK_EVT_FEAT_C3STOP if "always-on" = *false*

You might also consider renaming that value from always_on to
something more descriptive, given the subtlety of the different states
here. Maybe instead use a flag called halts_in_idle or something?

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] clocksource: arm_global_timer: fix suspend resume

2015-11-20 Thread John Stultz 
On Fri, Nov 20, 2015 at 10:46 AM, Marc Zyngier  wrote:
>
> This patch has a very specific purpose: instructing the core code that
> this timer will never stop ticking, ever. It is really targeted at
> virtual machines, whose timer is backed by the host timer, even when the
> VM is not running.
>
> Using it on actual hardware is may not be the best idea, specially in
> the presence of PM.

And actually, the CLOCK_SOURCE_SUSPEND_NONSTOP flag was added to
support real hardware, so its not just limited to VMs. But that
hardware does have to keep the lights on for the counter backing the
clocksource, and I'm not sure how common it ever became.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 09/11] clocksource: add TI 32.768 Hz counter driver

2015-10-01 Thread John Stultz 
On Tue, Sep 29, 2015 at 1:44 PM, Felipe Balbi  wrote:
> Introduce a new clocksource driver for Texas
> Instruments 32.768 Hz device which is available
> on most OMAP-like devices.
>
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/clocksource/Kconfig|   8 +++
>  drivers/clocksource/Makefile   |   1 +
>  drivers/clocksource/timer-ti-32k.c | 121 
> +
>  3 files changed, 130 insertions(+)
>  create mode 100644 drivers/clocksource/timer-ti-32k.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a7726db13abb..0ccfd12a00a3 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -115,6 +115,14 @@ config CLKSRC_PISTACHIO
> bool
> select CLKSRC_OF
>
> +config CLKSRC_TI_32K
> +   bool "Texas Instruments 32.768 Hz Clocksource"
> +   depends on OF && ARCH_OMAP2PLUS
> +   select CLKSRC_OF
> +   help
> + This option enables support for Texas Instruments 32.768 Hz 
> clocksource
> + available on many OMAP-like platforms.
> +
>  config CLKSRC_STM32
> bool "Clocksource for STM32 SoCs" if !ARCH_STM32
> depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 5c00863c3e33..749abc3665b3 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_VF_PIT_TIMER)+= vf_pit_timer.o
>  obj-$(CONFIG_CLKSRC_QCOM)  += qcom-timer.o
>  obj-$(CONFIG_MTK_TIMER)+= mtk_timer.o
>  obj-$(CONFIG_CLKSRC_PISTACHIO) += time-pistachio.o
> +obj-$(CONFIG_CLKSRC_TI_32K)+= timer-ti-32k.o
>
>  obj-$(CONFIG_ARM_ARCH_TIMER)   += arm_arch_timer.o
>  obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o
> diff --git a/drivers/clocksource/timer-ti-32k.c 
> b/drivers/clocksource/timer-ti-32k.c
> new file mode 100644
> index ..10ccce2eb645
> --- /dev/null
> +++ b/drivers/clocksource/timer-ti-32k.c
> @@ -0,0 +1,121 @@
> +/**
> + * timer-ti-32k.c - OMAP2 32k Timer Support
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +/* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */
> +#define OMAP2_32KSYNCNT_REV_OFF0x0
> +#define OMAP2_32KSYNCNT_REV_SCHEME (0x3 << 30)
> +#define OMAP2_32KSYNCNT_CR_OFF_LOW 0x10
> +#define OMAP2_32KSYNCNT_CR_OFF_HIGH0x30
> +
> +/*
> + * 32KHz clocksource ... always available, on pretty most chips except
> + * OMAP 730 and 1510.  Other timers could be used as clocksources, with
> + * higher resolution in free-running counter modes (e.g. 12 MHz xtal),
> + * but systems won't necessarily want to spend resources that way.
> + */
> +static void __iomem *sync32k_cnt_reg;
> +
> +/**
> + * omap_read_persistent_clock64 -  Return time from a persistent clock.
> + *
> + * Reads the time from a source which isn't disabled during PM, the
> + * 32k sync timer.  Convert the cycles elapsed since last read into
> + * nsecs and adds to a monotonically increasing timespec64.
> + */
> +static struct timespec64 persistent_ts;
> +static cycles_t cycles;
> +static unsigned int persistent_mult, persistent_shift;
> +
> +static u64 notrace omap_32k_read_sched_clock(void)
> +{
> +   return readl_relaxed(sync32k_cnt_reg);
> +}
> +
> +static void omap_read_persistent_clock64(struct timespec64 *ts)
> +{
> +   unsigned long long nsecs;
> +   cycles_t last_cycles;
> +
> +   last_cycles = cycles;
> +   cycles = sync32k_cnt_reg ? readl_relaxed(sync32k_cnt_reg) : 0;
> +
> +   nsecs = clocksource_cyc2ns(cycles - last_cycles,
> +   persistent_mult, persistent_shift);
> +
> +   timespec64_add_ns(_ts, nsecs);
> +
> +   *ts = persistent_ts;
> +}

So this doesn't look to really be a persistent clock (like an RTC),
and is more just a clock that doesn't halt in suspend, right?
(I know some devices used this trick to provide suspend timing w/o an RTC).

That said, we do now support CLOCK_SOURCE_SUSPEND_NONSTOP, which
allows us to do suspend timing from the

Re: [RFC/PATCH 09/11] clocksource: add TI 32.768 Hz counter driver

2015-10-01 Thread John Stultz 
On Tue, Sep 29, 2015 at 1:44 PM, Felipe Balbi  wrote:
> Introduce a new clocksource driver for Texas
> Instruments 32.768 Hz device which is available
> on most OMAP-like devices.
>
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/clocksource/Kconfig|   8 +++
>  drivers/clocksource/Makefile   |   1 +
>  drivers/clocksource/timer-ti-32k.c | 121 
> +

Also isn't this somewhat duplicate of the code in
arch/arm/plat-omap/counter_32k.c ?

Was this really a rewrite or just a code migration? Should you
preserve the copyrights from counter_32k.c?

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] clocksource: Allow toggling between runtime and persistent clocksource for idle

2015-07-06 Thread John Stultz 
On Mon, Jul 6, 2015 at 12:12 AM, Tony Lindgren t...@atomide.com wrote:
 Some persistent clocksources can be on a slow external bus. For shorter
 latencies for RT use, let's allow toggling the clocksource during idle
 between a faster non-persistent runtime clocksource and a slower persistent
 clocksource.

So yea, as Thomas mentioned, this will cause problems for timekeeping
accuracy, since we end up resetting the ntp state when we change
clocksource (additionally you gain a bit of error each time you switch
clocksources). So you'll most likely see trouble w/ ntpd steering the
clock.

I'm not sure its quite clear in the description as to the need here.
Could you expand a bit as to the rational for why?  I assume it has to
do with you have a high-res clocksource that is good for fine-grained
clock_gettime() calls, but wraps quickly, making it poor for long idle
times. So you're trying to swap to a less fine grained clocksource
that won't wrap so fast?

The persistent-clock-like name muddies things further, since the
persistent-clock is used for suspend, while it looks like this is just
for idle times.

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] clocksource: Allow toggling between runtime and persistent clocksource for idle

2015-07-06 Thread John Stultz 
On Mon, Jul 6, 2015 at 10:34 AM, Thomas Gleixner t...@linutronix.de wrote:
 On Mon, 6 Jul 2015, John Stultz wrote:
 On Mon, Jul 6, 2015 at 12:12 AM, Tony Lindgren t...@atomide.com wrote:
  Some persistent clocksources can be on a slow external bus. For shorter
  latencies for RT use, let's allow toggling the clocksource during idle
  between a faster non-persistent runtime clocksource and a slower persistent
  clocksource.

 So yea, as Thomas mentioned, this will cause problems for timekeeping
 accuracy, since we end up resetting the ntp state when we change
 clocksource (additionally you gain a bit of error each time you switch
 clocksources). So you'll most likely see trouble w/ ntpd steering the
 clock.

 I'm not sure its quite clear in the description as to the need here.
 Could you expand a bit as to the rational for why?  I assume it has to
 do with you have a high-res clocksource that is good for fine-grained
 clock_gettime() calls, but wraps quickly, making it poor for long idle
 times. So you're trying to swap to a less fine grained clocksource
 that won't wrap so fast?

 The persistent-clock-like name muddies things further, since the
 persistent-clock is used for suspend, while it looks like this is just
 for idle times.

 Though that are idle states where the cpu is powered off so the fast
 clock source is powered off as well

 We all know how well that works from the x86/TSC horror. It's just the
 same thing again with a different arch prefix.

Right.. and it might be telling that on x86 systems with this issue,
we don't play games with it and that sort of hardware just has to use
the slower and less fine-grained clocksources all the time.

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] clocksource: Allow toggling between runtime and persistent clocksource for idle

2015-07-06 Thread John Stultz 
On Mon, Jul 6, 2015 at 8:46 AM, Thomas Gleixner t...@linutronix.de wrote:
 On Mon, 6 Jul 2015, Tony Lindgren wrote:
 * Thomas Gleixner t...@linutronix.de [150706 07:20]:
  On Mon, 6 Jul 2015, Tony Lindgren wrote:
 The timekeeping accuracy issue certainly needs some thinking, and
 also the resolution between the clocksources can be different.. In the
 test case I have the slow timer is always on and of a lower resolution
 than the ARM global timer being used during runtime.

 Got some handy timer test in mind you want me to run to provide data
 on the accuracy?

 John Stultz might have something.

You can turn on ntp stats in your ntp.conf and chart the loopstats
data w/ gnuplot:

set terminal png
set output loopstat.png
plot /var/log/ntpstats/loopstats.dateoftest using 2:3 with linespoints


I also have  the drift-log.py and graph-log.py scripts I use here:
https://github.com/johnstultz-work/timetests

But those aren't really ready for mass consumption, as they're
probably not very cross-distro compatible.

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] time: Add y2038 safe read_boot_clock64()

2015-03-18 Thread John Stultz 
On Wed, Mar 18, 2015 at 5:48 AM,  pang.xun...@zte.com.cn wrote:
 Ping ...

Thanks for the reminder. Since there's not been any objections, I'm
queuing this up.

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: convert arm/arm64 arch timer to use CLKSRC_OF init

2013-03-25 Thread John Stultz 

On 03/25/2013 03:36 PM, Arnd Bergmann wrote:

On Monday 25 March 2013, Rob Herring wrote:

I count integrator-cp, realview, versatile and non-DT VExpress that do
this (not surprisingly) and 25 platforms or timer implementations plus
arm64 that do sched_clock setup in time_init. What's broken by not
moving these earlier?

timekeeping_init() will leave the persistent_clock_exist variable as false,
which is read in rtc_suspend() and timekeeping_inject_sleeptime().


Are you mixing up the persistent_clock and sched_clock here? From a 
generic stand-point they have different requirements.



For all I can tell, you will get a little jitter every time you
do a suspend in that case. Or perhaps it means the system clock
will be forwarded by the amount of time spent in suspend twice
after wakeup, but I'm probably misreading the code for that case.


No, you shouldn't see timekeeping being incremented twice, we check in 
rtc_resume code if the persistent clock is present if so we won't inject 
any measured suspend time there. But you're probably right that we're 
being a little overly paranoid checking the same value twice.


As far as the benefit to the persistent clock: it is just a little 
better to use, since we can access it earlier in resume, prior to 
interrupts being enabled. So we should see less time error introduced 
each suspend.


thanks
-john

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minimum timing resolution in Ubuntu/Linaro on the PandaBoard ES

2012-02-08 Thread John Stultz 
On Tue, 2012-02-07 at 21:21 -0800, Dmitry Antipov wrote:
 BTW, I have no ideas why clock_getres(CLOCK_REALTIME,...) returns {0, 1}
 regardless of underlying clock source. I expect {0, 30517} for 32K timer
 and {0, 26} for MPU timer.

Yea. I had proposed to export the underlying clocksource's resolution
via clock_getres, but it was argued against. The concern is that
applications might not expect clock_getres to change while the
application is running.  Between any clock_getres() call and a time
read, the clocksources could change.

But if someone has a different reading of the posix spec, it might be
good to revisit this.

thanks
-john


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minimum timing resolution in Ubuntu/Linaro on the PandaBoard ES

2012-02-08 Thread John Stultz 
On Wed, 2012-02-08 at 04:32 -0500, Andrew Richardson wrote:
 Ah, very interesting.

  dmesg | grep clock
 [0.00] OMAP clockevent source: GPTIMER1 at 32768 Hz
 [0.00] sched_clock: 32 bits at 32kHz, resolution
 30517ns, wraps every 131071999ms

Hrm. So 30us is still much smaller then the 2.5ms you were seeing. So
that doesn't fully explain the behavior.

thanks
-john




--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minimum timing resolution in Ubuntu/Linaro on the PandaBoard ES

2012-02-08 Thread John Stultz 
On Wed, 2012-02-08 at 04:32 -0500, Andrew Richardson wrote:
 Ah, very interesting.

  dmesg | grep clock
 [0.00] OMAP clockevent source: GPTIMER1 at 32768 Hz
 [0.00] sched_clock: 32 bits at 32kHz, resolution
 30517ns, wraps every 131071999ms

Hrm. So 30us is still much smaller then the 2.5ms you were seeing. So
that doesn't fully explain the behavior.

thanks
-john





--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/5] arm:omap1/2/3/4:Convert 32k-Sync clocksource driver to platform_driver

2012-01-25 Thread john stultz 
On Wed, 2012-01-18 at 16:58 +0530, Vaibhav Hiremath wrote:
 +/**
 + * read_persistent_clock -  Return time from a persistent clock.
 + *
 + * Reads the time from a source which isn't disabled during PM, the
 + * 32k sync timer.  Convert the cycles elapsed since last read into
 + * nsecs and adds to a monotonically increasing timespec.
 + */
 +void read_persistent_clock(struct timespec *ts)
 +{
 + cycles_t delta;
 + struct timespec *tsp;
 + unsigned long long nsecs;
 + struct omap_counter_32k_device  *omap = cs;
 +
 + if (!omap) {
 + ts-tv_sec = 0;
 + ts-tv_nsec = 0;
 + return;
 + }
 + tsp = omap-persistent_ts;
 +
 + omap-last_cycles = omap-cycles;
 + omap-cycles = omap-cs.read(omap-cs);
 + delta = omap-cycles - omap-last_cycles;
 +
 + nsecs = clocksource_cyc2ns(delta,
 + omap-cs.mult, omap-cs.shift);
 +
 + timespec_add_ns(tsp, nsecs);
 + *ts = *tsp;
 +}

Hrm. So read_persistent_clock should probably be defined once per arch.
So I'm not sure if it makes sense to include this implementation into
the generic drivers/clocksource directory, as if some other arch tried
to include this clocksource (say if they had the same hardware) they
might have collisions w/ their read_persistent_clock implementation.


I'm all for being able to re-use clocksource drivers. But this is the
sort of thing that makes me worry we're maybe being too aggressive in
pushing clocksources that really are fairly arch/platform specific into
drivers/clocksource/

thanks
-john


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rtc: twl: Fix registration vs. init order

2011-08-12 Thread John Stultz 
On Wed, 2011-08-10 at 20:20 -0700, Todd Poynor wrote:
 Only register as an RTC device after the hardware has been
 successfully initialized.  The RTC class driver will call
 back to this driver to read a pending alarm, and other
 drivers watching for new devices on the RTC class may
 read the RTC time upon registration.  Such access might
 occur while the RTC is stopped, prior to clearing
 pending alarms, etc.
 
 The new ordering also avoids leaving the platform
 device drvdata set to an unregistered struct rtc_device *
 on probe errors.
 
 Signed-off-by: Todd Poynor toddpoy...@google.com

Thanks! Queued.
-john

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rtc-twl: Switch to using threaded irq

2011-08-10 Thread John Stultz 
On Mon, 2011-06-27 at 18:45 +0200, Sebastian Reichel wrote:
 On Tue, May 31, 2011 at 10:51:39AM +0200, Sebastian Reichel wrote:
   The driver is accessing to i2c bus in interrupt handler.
   Therefore, it should use threaded irq.
  I think the patch should also remove the local_irq_enable() call in
  twl_rtc_interrupt, since it's no longer needed with threaded irq. At
  least on the Pandaboard the RTC is still working with the appended
  patch.
 
 ping.
 
 Currently the kernel prints a stacktrace for each rtc-twl interrupt,
 because its interrupt handler enables interrupts.

I've queued both changes. Thanks for the reminder.

thanks
-john


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] rtc-twl: Switch to using threaded irq

2011-05-12 Thread John Stultz 
On Thu, 2011-05-05 at 07:51 +, ilkka.koski...@nokia.com wrote:
 Hi,
 
 Tony and John: What would be the appropriate path for this patch?

I'd probably push it through omap maintainer path, as its hardware
specific and can be better tested there.

thanks
-john


 On Apr 13, 2011 Krishnamoorthy, Balaji T  wrote:
 On Wed, Mar 16, 2011 at 9:37 PM, Ilkka Koskinen
 ilkka.koski...@nokia.com wrote:
 
  The driver is accessing to i2c bus in interrupt handler.
  Therefore, it should use threaded irq.
 
 Acked-by: Balaji T K balaj...@ti.com
 
 
  Signed-off-by: Ilkka Koskinen ilkka.koski...@nokia.com
  ---
   drivers/rtc/rtc-twl.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
  index ed1b868..2715b96 100644
  --- a/drivers/rtc/rtc-twl.c
  +++ b/drivers/rtc/rtc-twl.c
  @@ -475,7 +475,7 @@ static int __devinit twl_rtc_probe(struct 
  platform_device *pdev)
 if (ret  0)
 goto out1;
 
  -   ret = request_irq(irq, twl_rtc_interrupt,
  +   ret = request_threaded_irq(irq, NULL, twl_rtc_interrupt,
 IRQF_TRIGGER_RISING,
 dev_name(rtc-dev), rtc);
 if (ret  0) {
  --
  1.7.0.4
 


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [regression] 2.6.39-rc1 - Beagleboard usbnet broken

2011-04-04 Thread john stultz 
On Mon, Apr 4, 2011 at 2:22 AM, Mark Jackson mpfj-l...@mimc.co.uk wrote:
 Running 2.6.38 on my beagleboard, I can boot using nfs:-
[snip]
 I have just tried out 2.6.39-rc1, and the usbnet is no longer detected:-

Yea, I hit something similar, where on the beagleboard I only see the
root hubs, and not any of the devices (no usb, no keyboard, etc).

I bisected it down to:
commit 9e64bb1e9f0613093b3e34ac5402fcfef0dcc35a
Author: Keshava Munegowda keshava_mgo...@ti.com
Date:   Tue Mar 1 20:08:19 2011 +0530

arm: omap: usb: Invoke usbhs core device initialization

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Timekeeping issue on aggressive suspend/resume

2010-06-14 Thread john stultz 
On Mon, 2010-06-14 at 00:46 -0700, Suresh Rajashekara wrote:
 On Thu, Jun 10, 2010 at 12:52 PM, john stultz johns...@us.ibm.com wrote:
  I think Thomas was suggesting that you consider creating a option for
  where CLOCK_MONOTONIC included total_sleep_time.
 
  In that case the *hack* (and this is a hack, we'll need some more
  thoughtful discussion before anything like it could make it upstream)
  would be in timekeeping_resume() to comment out the lines that update
  wall_to_monotonic and total_sleep_time.
 
  It would be interesting to hear if that hack works for you, and we can
  try to come up with a better way to think about how to accommodate both
  views of how to account time over suspend.
 
 Thanks.
 
 I tried this fix. It seemed to help, however the accuracy of sleep
 time for the process was not quite right. A process thread which was
 supposed to wake up every (X) seconds, seemed to wake up every (X -
 delta X) seconds.

Ah, the sleep time is probably too coarse (seconds). We probably need to
increase the granularity from read_persistent_clock() and see if that
helps (although most persistent clocks aren't very fine grained).

 Also another side effect of this change was that the system time was
 no longer in sync with the wall time.

? This doesn't make much sense to me, as you shouldn't be manipulating
xtime differently.

Just to be clear, you mean the value from date doesn't match your
watch after resume? 

 These problems were more pronounced when the suspend/wakeup cycle time
 was brought down to 0.5 seconds from 4 seconds. The periodicity of
 most of the process threads were disturbed.
 
 I decided to NOT suspend/resume the timekeeping subsystem in the
 kernel and try. It seemed to work. Every application seems to work
 fine.
 
 Now my question is; Is it safe to disable suspend/resume of the
 timekeeping subsystem? Will it have an effect (on
 functionality/performance) which may not surface in my short
 experiments?

Well, the difficultly here is what folks actually mean by suspend. On
some hardware it means everything is powered off, and so on resume we
have to re-init hardware values.

It seems in your case that the hardware isn't completely powered off,
since the clocksource you're using seemed to continue counting while the
system was suspended. 

So in this case you might be ok. Your suspend seems closer to an deep
idle state on x86. So suspending timekeeping might not be necessary.

However, you're right that there may be lurking issues:

1) The suspend time would have to be limited to the clocksource's
max_idle_ns value, since after that amount of cycles have past, we might
overflow the accumulation function, or the clocksource may have wrapped.

2) If the hardware does reset the clocksource at some point during the
suspend, you'll have odd time issues.

3) You could run into some difficulty keeping close sync with an NTP
server, as the long delays between accumulation will probably cause an
oscillating over-shoot and over-correction.

I suspect these different definitions of suspend on all of the
different hardware types out there is going to be a growing problem in
the near term. Especially as deep idle states start to power off more
hardware and becomes closer to suspend in behavior.

thanks
-john

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Timekeeping issue on aggressive suspend/resume

2010-06-10 Thread john stultz 
On Wed, 2010-06-09 at 23:34 -0700, Suresh Rajashekara wrote:
 On Wed, Jun 9, 2010 at 1:22 PM, Thomas Gleixner t...@linutronix.de wrote:
  Though we could change that conditionally - the default would still be
  the freeze of jiffies and CLOCK_MONOTONIC for historical compability.
 
 If I were to change it only for our implementation, and make all the
 user space timers use CLOCK_REALTIME, then could you please point me
 in a direction as to what part of the kernel I should be touching to
 make that change?

I think Thomas was suggesting that you consider creating a option for
where CLOCK_MONOTONIC included total_sleep_time.

In that case the *hack* (and this is a hack, we'll need some more
thoughtful discussion before anything like it could make it upstream)
would be in timekeeping_resume() to comment out the lines that update
wall_to_monotonic and total_sleep_time.

It would be interesting to hear if that hack works for you, and we can
try to come up with a better way to think about how to accommodate both
views of how to account time over suspend.

Thomas, might this call for a new posix clock_id, CLOCK_BOOTTIME (ie:
CLOCK_MONOTONIC + total_sleep_time) or something that userland could use
to set timers on?

thanks
-john


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html