diff mbox

clocksource: timer-atmel-pit: don't suspend/resume if unused

Message ID 1418911523-28492-1-git-send-email-nicolas.ferre@atmel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nicolas Ferre Dec. 18, 2014, 2:05 p.m. UTC
From: Sylvain Rochet <sylvain.rochet@finsecur.com>

Waiting for PIT to stop counting takes a long time:
  1/(Master clock/prescaler/PIVR)
= 1/(133 MHz     /16       /2^20)
= 126 ms

Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
of the unused PIT device reduce (suspend time + resume time) from ~140 ms
to ~17 ms.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
[nicolas.ferre@atmel.com: move to newer clocksource driver]
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
---
Hi Sylvain,

I re-worked (and "Acked") your patch so it can be applied on the newer Mainline
kernels. Beware, I changed the "subject line" as well. The PIT driver moved
recently (3.18).

Daniel,
Can you take this patch in your tree?

Thanks, bye.
  Nicolas.

 drivers/clocksource/timer-atmel-pit.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Nicolas Ferre Jan. 12, 2015, 2:04 p.m. UTC | #1
Le 18/12/2014 15:05, Nicolas Ferre a écrit :
> From: Sylvain Rochet <sylvain.rochet@finsecur.com>
> 
> Waiting for PIT to stop counting takes a long time:
>   1/(Master clock/prescaler/PIVR)
> = 1/(133 MHz     /16       /2^20)
> = 126 ms
> 
> Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
> of the unused PIT device reduce (suspend time + resume time) from ~140 ms
> to ~17 ms.
> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> [nicolas.ferre@atmel.com: move to newer clocksource driver]
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> Hi Sylvain,
> 
> I re-worked (and "Acked") your patch so it can be applied on the newer Mainline
> kernels. Beware, I changed the "subject line" as well. The PIT driver moved
> recently (3.18).
> 
> Daniel,
> Can you take this patch in your tree?

Hi Daniel,

Anything prevents this patch from being merged (aka ping ;-))?

Bye,


> Thanks, bye.
>   Nicolas.
> 
>  drivers/clocksource/timer-atmel-pit.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/clocksource/timer-atmel-pit.c b/drivers/clocksource/timer-atmel-pit.c
> index b5b4d4585c9a..5408bc7c68d8 100644
> --- a/drivers/clocksource/timer-atmel-pit.c
> +++ b/drivers/clocksource/timer-atmel-pit.c
> @@ -123,6 +123,10 @@ static void at91sam926x_pit_suspend(struct clock_event_device *cedev)
>  {
>  	struct pit_data *data = clkevt_to_pit_data(cedev);
>  
> +	/* Don't suspend PIT if unused */
> +	if (cedev->mode == CLOCK_EVT_MODE_UNUSED)
> +		return;
> +
>  	/* Disable timer */
>  	pit_write(data->base, AT91_PIT_MR, 0);
>  }
> @@ -145,6 +149,10 @@ static void at91sam926x_pit_resume(struct clock_event_device *cedev)
>  {
>  	struct pit_data *data = clkevt_to_pit_data(cedev);
>  
> +	/* Don't resume PIT if unused */
> +	if (cedev->mode == CLOCK_EVT_MODE_UNUSED)
> +		return;
> +
>  	at91sam926x_pit_reset(data);
>  }
>  
>
Daniel Lezcano Jan. 12, 2015, 2:22 p.m. UTC | #2
On 01/12/2015 03:04 PM, Nicolas Ferre wrote:
> Le 18/12/2014 15:05, Nicolas Ferre a écrit :
>> From: Sylvain Rochet <sylvain.rochet@finsecur.com>
>>
>> Waiting for PIT to stop counting takes a long time:
>>    1/(Master clock/prescaler/PIVR)
>> = 1/(133 MHz     /16       /2^20)
>> = 126 ms
>>
>> Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
>> of the unused PIT device reduce (suspend time + resume time) from ~140 ms
>> to ~17 ms.
>>
>> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
>> [nicolas.ferre@atmel.com: move to newer clocksource driver]
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>> Hi Sylvain,
>>
>> I re-worked (and "Acked") your patch so it can be applied on the newer Mainline
>> kernels. Beware, I changed the "subject line" as well. The PIT driver moved
>> recently (3.18).
>>
>> Daniel,
>> Can you take this patch in your tree?
>
> Hi Daniel,
>
> Anything prevents this patch from being merged (aka ping ;-))?

[Cc'ed tglx].

Hi Nico,

thanks for the head up.

Nothing prevents it but I am wondering if this change shouldn't be in 
the generic framework (kernel/time/clocksource.c and 
kernel/time/clockevents.c), so all drivers will benefit this change ?


>> Thanks, bye.
>>    Nicolas.
>>
>>   drivers/clocksource/timer-atmel-pit.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/clocksource/timer-atmel-pit.c b/drivers/clocksource/timer-atmel-pit.c
>> index b5b4d4585c9a..5408bc7c68d8 100644
>> --- a/drivers/clocksource/timer-atmel-pit.c
>> +++ b/drivers/clocksource/timer-atmel-pit.c
>> @@ -123,6 +123,10 @@ static void at91sam926x_pit_suspend(struct clock_event_device *cedev)
>>   {
>>   	struct pit_data *data = clkevt_to_pit_data(cedev);
>>
>> +	/* Don't suspend PIT if unused */
>> +	if (cedev->mode == CLOCK_EVT_MODE_UNUSED)
>> +		return;
>> +
>>   	/* Disable timer */
>>   	pit_write(data->base, AT91_PIT_MR, 0);
>>   }
>> @@ -145,6 +149,10 @@ static void at91sam926x_pit_resume(struct clock_event_device *cedev)
>>   {
>>   	struct pit_data *data = clkevt_to_pit_data(cedev);
>>
>> +	/* Don't resume PIT if unused */
>> +	if (cedev->mode == CLOCK_EVT_MODE_UNUSED)
>> +		return;
>> +
>>   	at91sam926x_pit_reset(data);
>>   }
>>
>>
>
>
Thomas Gleixner Jan. 13, 2015, 10:47 a.m. UTC | #3
On Mon, 12 Jan 2015, Daniel Lezcano wrote:

> On 01/12/2015 03:04 PM, Nicolas Ferre wrote:
> > Le 18/12/2014 15:05, Nicolas Ferre a écrit :
> > > From: Sylvain Rochet <sylvain.rochet@finsecur.com>
> > > 
> > > Waiting for PIT to stop counting takes a long time:
> > >    1/(Master clock/prescaler/PIVR)
> > > = 1/(133 MHz     /16       /2^20)
> > > = 126 ms
> > > 
> > > Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
> > > of the unused PIT device reduce (suspend time + resume time) from ~140 ms
> > > to ~17 ms.
> > > 
> > > Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> > > [nicolas.ferre@atmel.com: move to newer clocksource driver]
> > > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > ---
> > > Hi Sylvain,
> > > 
> > > I re-worked (and "Acked") your patch so it can be applied on the newer
> > > Mainline
> > > kernels. Beware, I changed the "subject line" as well. The PIT driver
> > > moved
> > > recently (3.18).
> > > 
> > > Daniel,
> > > Can you take this patch in your tree?
> > 
> > Hi Daniel,
> > 
> > Anything prevents this patch from being merged (aka ping ;-))?
> 
> [Cc'ed tglx].
> 
> Hi Nico,
> 
> thanks for the head up.
> 
> Nothing prevents it but I am wondering if this change shouldn't be in the
> generic framework (kernel/time/clocksource.c and kernel/time/clockevents.c),
> so all drivers will benefit this change ?

Indeed. There is no point in calling suspend/resume for unused
clockevents. They should be stopped and disabled already.

Now with clocksources this might be different. We have no explicit
state for this, but its trivial to add one at least for those
clocksources which have enable/disable callbacks. For the other ones
not so much.

Thanks,

	tglx
Daniel Lezcano Jan. 15, 2015, 11:41 a.m. UTC | #4
On 01/13/2015 11:47 AM, Thomas Gleixner wrote:
> On Mon, 12 Jan 2015, Daniel Lezcano wrote:
>
>> On 01/12/2015 03:04 PM, Nicolas Ferre wrote:
>>> Le 18/12/2014 15:05, Nicolas Ferre a écrit :
>>>> From: Sylvain Rochet <sylvain.rochet@finsecur.com>
>>>>
>>>> Waiting for PIT to stop counting takes a long time:
>>>>     1/(Master clock/prescaler/PIVR)
>>>> = 1/(133 MHz     /16       /2^20)
>>>> = 126 ms
>>>>
>>>> Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
>>>> of the unused PIT device reduce (suspend time + resume time) from ~140 ms
>>>> to ~17 ms.
>>>>
>>>> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
>>>> [nicolas.ferre@atmel.com: move to newer clocksource driver]
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> ---
>>>> Hi Sylvain,
>>>>
>>>> I re-worked (and "Acked") your patch so it can be applied on the newer
>>>> Mainline
>>>> kernels. Beware, I changed the "subject line" as well. The PIT driver
>>>> moved
>>>> recently (3.18).
>>>>
>>>> Daniel,
>>>> Can you take this patch in your tree?
>>>
>>> Hi Daniel,
>>>
>>> Anything prevents this patch from being merged (aka ping ;-))?
>>
>> [Cc'ed tglx].
>>
>> Hi Nico,
>>
>> thanks for the head up.
>>
>> Nothing prevents it but I am wondering if this change shouldn't be in the
>> generic framework (kernel/time/clocksource.c and kernel/time/clockevents.c),
>> so all drivers will benefit this change ?
>
> Indeed. There is no point in calling suspend/resume for unused
> clockevents. They should be stopped and disabled already.

Hi Nico,

are you planning to do the change in the generic framework ?

Thanks
   -- Daniel

> Now with clocksources this might be different. We have no explicit
> state for this, but its trivial to add one at least for those
> clocksources which have enable/disable callbacks. For the other ones
> not so much.
Alexandre Belloni Jan. 15, 2015, 11:52 a.m. UTC | #5
Hi,

I'll do it but only if you review my other series :)

On 15/01/2015 at 12:41:27 +0100, Daniel Lezcano wrote :
> On 01/13/2015 11:47 AM, Thomas Gleixner wrote:
> >On Mon, 12 Jan 2015, Daniel Lezcano wrote:
> >
> >>On 01/12/2015 03:04 PM, Nicolas Ferre wrote:
> >>>Le 18/12/2014 15:05, Nicolas Ferre a écrit :
> >>>>From: Sylvain Rochet <sylvain.rochet@finsecur.com>
> >>>>
> >>>>Waiting for PIT to stop counting takes a long time:
> >>>>    1/(Master clock/prescaler/PIVR)
> >>>>= 1/(133 MHz     /16       /2^20)
> >>>>= 126 ms
> >>>>
> >>>>Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
> >>>>of the unused PIT device reduce (suspend time + resume time) from ~140 ms
> >>>>to ~17 ms.
> >>>>
> >>>>Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> >>>>[nicolas.ferre@atmel.com: move to newer clocksource driver]
> >>>>Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> >>>>Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> >>>>---
> >>>>Hi Sylvain,
> >>>>
> >>>>I re-worked (and "Acked") your patch so it can be applied on the newer
> >>>>Mainline
> >>>>kernels. Beware, I changed the "subject line" as well. The PIT driver
> >>>>moved
> >>>>recently (3.18).
> >>>>
> >>>>Daniel,
> >>>>Can you take this patch in your tree?
> >>>
> >>>Hi Daniel,
> >>>
> >>>Anything prevents this patch from being merged (aka ping ;-))?
> >>
> >>[Cc'ed tglx].
> >>
> >>Hi Nico,
> >>
> >>thanks for the head up.
> >>
> >>Nothing prevents it but I am wondering if this change shouldn't be in the
> >>generic framework (kernel/time/clocksource.c and kernel/time/clockevents.c),
> >>so all drivers will benefit this change ?
> >
> >Indeed. There is no point in calling suspend/resume for unused
> >clockevents. They should be stopped and disabled already.
> 
> Hi Nico,
> 
> are you planning to do the change in the generic framework ?
> 
> Thanks
>   -- Daniel
> 
> >Now with clocksources this might be different. We have no explicit
> >state for this, but its trivial to add one at least for those
> >clocksources which have enable/disable callbacks. For the other ones
> >not so much.
> 
> 
> 
> 
> 
> -- 
>  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
Daniel Lezcano Jan. 15, 2015, 12:23 p.m. UTC | #6
On 01/15/2015 12:52 PM, Alexandre Belloni wrote:
> Hi,
>
> I'll do it but only if you review my other series :)

deal ! ;)

   -- Daniel


>
> On 15/01/2015 at 12:41:27 +0100, Daniel Lezcano wrote :
>> On 01/13/2015 11:47 AM, Thomas Gleixner wrote:
>>> On Mon, 12 Jan 2015, Daniel Lezcano wrote:
>>>
>>>> On 01/12/2015 03:04 PM, Nicolas Ferre wrote:
>>>>> Le 18/12/2014 15:05, Nicolas Ferre a écrit :
>>>>>> From: Sylvain Rochet <sylvain.rochet@finsecur.com>
>>>>>>
>>>>>> Waiting for PIT to stop counting takes a long time:
>>>>>>     1/(Master clock/prescaler/PIVR)
>>>>>> = 1/(133 MHz     /16       /2^20)
>>>>>> = 126 ms
>>>>>>
>>>>>> Up to 126 ms if master clock is set to 133 MHz, skipping suspend/resume
>>>>>> of the unused PIT device reduce (suspend time + resume time) from ~140 ms
>>>>>> to ~17 ms.
>>>>>>
>>>>>> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
>>>>>> [nicolas.ferre@atmel.com: move to newer clocksource driver]
>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>>>> ---
>>>>>> Hi Sylvain,
>>>>>>
>>>>>> I re-worked (and "Acked") your patch so it can be applied on the newer
>>>>>> Mainline
>>>>>> kernels. Beware, I changed the "subject line" as well. The PIT driver
>>>>>> moved
>>>>>> recently (3.18).
>>>>>>
>>>>>> Daniel,
>>>>>> Can you take this patch in your tree?
>>>>>
>>>>> Hi Daniel,
>>>>>
>>>>> Anything prevents this patch from being merged (aka ping ;-))?
>>>>
>>>> [Cc'ed tglx].
>>>>
>>>> Hi Nico,
>>>>
>>>> thanks for the head up.
>>>>
>>>> Nothing prevents it but I am wondering if this change shouldn't be in the
>>>> generic framework (kernel/time/clocksource.c and kernel/time/clockevents.c),
>>>> so all drivers will benefit this change ?
>>>
>>> Indeed. There is no point in calling suspend/resume for unused
>>> clockevents. They should be stopped and disabled already.
>>
>> Hi Nico,
>>
>> are you planning to do the change in the generic framework ?
>>
>> Thanks
>>    -- Daniel
>>
>>> Now with clocksources this might be different. We have no explicit
>>> state for this, but its trivial to add one at least for those
>>> clocksources which have enable/disable callbacks. For the other ones
>>> not so much.
>>
>>
>>
>>
>>
>> --
>>   <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>
>
diff mbox

Patch

diff --git a/drivers/clocksource/timer-atmel-pit.c b/drivers/clocksource/timer-atmel-pit.c
index b5b4d4585c9a..5408bc7c68d8 100644
--- a/drivers/clocksource/timer-atmel-pit.c
+++ b/drivers/clocksource/timer-atmel-pit.c
@@ -123,6 +123,10 @@  static void at91sam926x_pit_suspend(struct clock_event_device *cedev)
 {
 	struct pit_data *data = clkevt_to_pit_data(cedev);
 
+	/* Don't suspend PIT if unused */
+	if (cedev->mode == CLOCK_EVT_MODE_UNUSED)
+		return;
+
 	/* Disable timer */
 	pit_write(data->base, AT91_PIT_MR, 0);
 }
@@ -145,6 +149,10 @@  static void at91sam926x_pit_resume(struct clock_event_device *cedev)
 {
 	struct pit_data *data = clkevt_to_pit_data(cedev);
 
+	/* Don't resume PIT if unused */
+	if (cedev->mode == CLOCK_EVT_MODE_UNUSED)
+		return;
+
 	at91sam926x_pit_reset(data);
 }