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

Groups > linux.kernel > #1532500 > unrolled thread

Re: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer clocksource

Started byGrygorii Strashko <grygorii.strashko@ti.com>
First post2016-11-29 17:50 +0100
Last post2016-12-02 21:10 +0100
Articles 5 — 3 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 2/2] ARM: omap: timers: reduce rating of gp_timer  clocksource Grygorii Strashko <grygorii.strashko@ti.com> - 2016-11-29 17:50 +0100
    Re: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer  clocksource Tony Lindgren <tony@atomide.com> - 2016-12-02 18:10 +0100
      Re: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer  clocksource Tony Lindgren <tony@atomide.com> - 2016-12-02 19:40 +0100
        Re: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer  clocksource Keerthy <j-keerthy@ti.com> - 2016-12-05 05:20 +0100
      Re: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer  clocksource Grygorii Strashko <grygorii.strashko@ti.com> - 2016-12-02 21:10 +0100

#1532500 — Re: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer clocksource

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-11-29 17:50 +0100
SubjectRe: [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer clocksource
Message-ID<sITLs-2xM-37@gated-at.bofh.it>

On 11/24/2016 12:19 AM, Keerthy wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> Now ARM Global timer (rating 300) will not be selected as clocksource,
> because it's initialized after OMAP GP Timer (rating 300) and
> Timekeeping core will not allow to replace clocksource with new one if
> both of them have the same rating.
>
> Reduce rating of OMAP GP Timer (300->290) when it's used as
> clocksource device - this will allow to select ARM Global timer (300)
> as clocksource when enabled.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Keerthy <j-keerthy@ti.com>

Unfortunately, this patch has dependency [1] and can't be used alone as
it will cause ARM Global timer to be selected as clocksource
always on am437x and this will kill cpuidle, because ARM Global timer
is not in always_on domain.

The intention of enabling ARM Global timer is only for non-pm aware use
cases for RT-kernel latency improvement - where deep cpuidle states are 
not enabled.

[1] https://patchwork.kernel.org/patch/8940051/

> ---
>  arch/arm/mach-omap2/timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index b2f2448..a0dbb0b 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -376,7 +376,7 @@ static cycle_t clocksource_read_cycles(struct clocksource *cs)
>  }
>
>  static struct clocksource clocksource_gpt = {
> -	.rating		= 300,
> +	.rating		= 290,
>  	.read		= clocksource_read_cycles,
>  	.mask		= CLOCKSOURCE_MASK(32),
>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
>

-- 
regards,
-grygorii

[toc] | [next] | [standalone]

#1535080

FromTony Lindgren <tony@atomide.com>
Date2016-12-02 18:10 +0100
Message-ID<sJZvs-6Fu-7@gated-at.bofh.it>
In reply to#1532500
* Grygorii Strashko <grygorii.strashko@ti.com> [161129 08:43]:
> 
> 
> On 11/24/2016 12:19 AM, Keerthy wrote:
> > From: Grygorii Strashko <grygorii.strashko@ti.com>
> > 
> > Now ARM Global timer (rating 300) will not be selected as clocksource,
> > because it's initialized after OMAP GP Timer (rating 300) and
> > Timekeeping core will not allow to replace clocksource with new one if
> > both of them have the same rating.
> > 
> > Reduce rating of OMAP GP Timer (300->290) when it's used as
> > clocksource device - this will allow to select ARM Global timer (300)
> > as clocksource when enabled.
> > 
> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Keerthy <j-keerthy@ti.com>
> 
> Unfortunately, this patch has dependency [1] and can't be used alone as
> it will cause ARM Global timer to be selected as clocksource
> always on am437x and this will kill cpuidle, because ARM Global timer
> is not in always_on domain.
> 
> The intention of enabling ARM Global timer is only for non-pm aware use
> cases for RT-kernel latency improvement - where deep cpuidle states are not
> enabled.

Yeah we need to fix up things to be able to change the clocksource
in addition to clockevent. However, currently only cpuidle_coupled
knows when the whole system is idle, so quite a bit of work is
needed to do that in a sane way.

What about the first patch in this series?

Regards,

Tony


> [1] https://patchwork.kernel.org/patch/8940051/
> 
> > ---
> >  arch/arm/mach-omap2/timer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> > index b2f2448..a0dbb0b 100644
> > --- a/arch/arm/mach-omap2/timer.c
> > +++ b/arch/arm/mach-omap2/timer.c
> > @@ -376,7 +376,7 @@ static cycle_t clocksource_read_cycles(struct clocksource *cs)
> >  }
> > 
> >  static struct clocksource clocksource_gpt = {
> > -	.rating		= 300,
> > +	.rating		= 290,
> >  	.read		= clocksource_read_cycles,
> >  	.mask		= CLOCKSOURCE_MASK(32),
> >  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
> > 
> 
> -- 
> regards,
> -grygorii

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

#1535117

FromTony Lindgren <tony@atomide.com>
Date2016-12-02 19:40 +0100
Message-ID<sK0Ux-7qD-5@gated-at.bofh.it>
In reply to#1535080
* Grygorii Strashko <grygorii.strashko@ti.com> [161202 10:02]:
> 
> 
> On 12/02/2016 10:47 AM, Tony Lindgren wrote:
> > * Grygorii Strashko <grygorii.strashko@ti.com> [161129 08:43]:
> > > 
> > > 
> > > On 11/24/2016 12:19 AM, Keerthy wrote:
> > > > From: Grygorii Strashko <grygorii.strashko@ti.com>
> > > > 
> > > > Now ARM Global timer (rating 300) will not be selected as clocksource,
> > > > because it's initialized after OMAP GP Timer (rating 300) and
> > > > Timekeeping core will not allow to replace clocksource with new one if
> > > > both of them have the same rating.
> > > > 
> > > > Reduce rating of OMAP GP Timer (300->290) when it's used as
> > > > clocksource device - this will allow to select ARM Global timer (300)
> > > > as clocksource when enabled.
> > > > 
> > > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > > > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > > 
> > > Unfortunately, this patch has dependency [1] and can't be used alone as
> > > it will cause ARM Global timer to be selected as clocksource
> > > always on am437x and this will kill cpuidle, because ARM Global timer
> > > is not in always_on domain.
> > > 
> > > The intention of enabling ARM Global timer is only for non-pm aware use
> > > cases for RT-kernel latency improvement - where deep cpuidle states are not
> > > enabled.
> > 
> > Yeah we need to fix up things to be able to change the clocksource
> > in addition to clockevent. However, currently only cpuidle_coupled
> > knows when the whole system is idle, so quite a bit of work is
> > needed to do that in a sane way.
> 
> Also sched_clock and timer_delay ;)

Yeah sched_clock would need something to save and restore it..
Not nice :)

> > What about the first patch in this series?
> > 
> 
> Fist one is ok. It was originally posted long time ago.

OK so can you please resend that one more time with proper
Fixes tag on it?

Regards,

Tony

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

#1535797

FromKeerthy <j-keerthy@ti.com>
Date2016-12-05 05:20 +0100
Message-ID<sKSUW-82v-5@gated-at.bofh.it>
In reply to#1535117

On Saturday 03 December 2016 12:01 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [161202 10:02]:
>>
>>
>> On 12/02/2016 10:47 AM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [161129 08:43]:
>>>>
>>>>
>>>> On 11/24/2016 12:19 AM, Keerthy wrote:
>>>>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>>
>>>>> Now ARM Global timer (rating 300) will not be selected as clocksource,
>>>>> because it's initialized after OMAP GP Timer (rating 300) and
>>>>> Timekeeping core will not allow to replace clocksource with new one if
>>>>> both of them have the same rating.
>>>>>
>>>>> Reduce rating of OMAP GP Timer (300->290) when it's used as
>>>>> clocksource device - this will allow to select ARM Global timer (300)
>>>>> as clocksource when enabled.
>>>>>
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>>
>>>> Unfortunately, this patch has dependency [1] and can't be used alone as
>>>> it will cause ARM Global timer to be selected as clocksource
>>>> always on am437x and this will kill cpuidle, because ARM Global timer
>>>> is not in always_on domain.
>>>>
>>>> The intention of enabling ARM Global timer is only for non-pm aware use
>>>> cases for RT-kernel latency improvement - where deep cpuidle states are not
>>>> enabled.
>>>
>>> Yeah we need to fix up things to be able to change the clocksource
>>> in addition to clockevent. However, currently only cpuidle_coupled
>>> knows when the whole system is idle, so quite a bit of work is
>>> needed to do that in a sane way.
>>
>> Also sched_clock and timer_delay ;)
>
> Yeah sched_clock would need something to save and restore it..
> Not nice :)
>
>>> What about the first patch in this series?
>>>
>>
>> Fist one is ok. It was originally posted long time ago.
>
> OK so can you please resend that one more time with proper
> Fixes tag on it?

Tony,

I have posted a v2 with the proper Fixes tag.

- Keerthy
>
> Regards,
>
> Tony
>

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

#1535187

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-12-02 21:10 +0100
Message-ID<sK0Ux-7qD-7@gated-at.bofh.it>
In reply to#1535080

On 12/02/2016 10:47 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [161129 08:43]:
>>
>>
>> On 11/24/2016 12:19 AM, Keerthy wrote:
>>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>>
>>> Now ARM Global timer (rating 300) will not be selected as clocksource,
>>> because it's initialized after OMAP GP Timer (rating 300) and
>>> Timekeeping core will not allow to replace clocksource with new one if
>>> both of them have the same rating.
>>>
>>> Reduce rating of OMAP GP Timer (300->290) when it's used as
>>> clocksource device - this will allow to select ARM Global timer (300)
>>> as clocksource when enabled.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>
>> Unfortunately, this patch has dependency [1] and can't be used alone as
>> it will cause ARM Global timer to be selected as clocksource
>> always on am437x and this will kill cpuidle, because ARM Global timer
>> is not in always_on domain.
>>
>> The intention of enabling ARM Global timer is only for non-pm aware use
>> cases for RT-kernel latency improvement - where deep cpuidle states are not
>> enabled.
>
> Yeah we need to fix up things to be able to change the clocksource
> in addition to clockevent. However, currently only cpuidle_coupled
> knows when the whole system is idle, so quite a bit of work is
> needed to do that in a sane way.

Also sched_clock and timer_delay ;)


>
> What about the first patch in this series?
>

Fist one is ok. It was originally posted long time ago.

-- 
regards,
-grygorii

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web