All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Igor Grinberg <grinberg@compulab.co.il>
Cc: Kevin Hilman <khilman@ti.com>, Paul Walmsley <paul@pwsan.com>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jon Hunter <jon-hunter@ti.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Vaibhav Hiremath <hvaibhav@ti.com>
Subject: Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
Date: Thu, 8 Nov 2012 08:20:05 -0800	[thread overview]
Message-ID: <20121108162004.GA6801@atomide.com> (raw)
In-Reply-To: <509B5B84.50605@compulab.co.il>

* Igor Grinberg <grinberg@compulab.co.il> [121107 23:15]:
> On 11/07/12 19:33, Tony Lindgren wrote:
> > 
> > I think this should be the default for the timers as that counter
> > does not stop during deeper idle states.
> 
> Well, it is the default as you can see from the patch.
> The problem is that for boards that for some reason do not have
> the 32k wired and rely on MPU/GP timer source, the default will not work
> and currently there is no way for board to specify which timer source
> it can use.

Yes. I was just wondering if we can avoid patching all the board
files by doing it the other way around by introducing a new
omap_gp_timer rather than renaming all the existing ones?

> We have discussed this in San Diego (remember?) and you actually proposed
> this way as a solution. Well, may be I took it a bit further than you
> thought, but this is because the board code cannot know which timer source
> should be used at runtime and the fall back described below, does not work.

Yes thanks I agree we should get rid of that Kconfig option for sure. 

> >> --- a/arch/arm/mach-omap2/board-2430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> >> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
> >>  	.handle_irq	= omap2_intc_handle_irq,
> >>  	.init_machine	= omap_2430sdp_init,
> >>  	.init_late	= omap2430_init_late,
> >> -	.timer		= &omap2_timer,
> >> +	.timer		= &omap2_sync32k_timer,
> >>  	.restart	= omap_prcm_restart,
> >>  MACHINE_END
> >> --- a/arch/arm/mach-omap2/board-3430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> >> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
> >>  	.handle_irq	= omap3_intc_handle_irq,
> >>  	.init_machine	= omap_3430sdp_init,
> >>  	.init_late	= omap3430_init_late,
> >> -	.timer		= &omap3_timer,
> >> +	.timer		= &omap3_sync32k_timer,
> >>  	.restart	= omap_prcm_restart,
> >>  MACHINE_END
> > ...
> > 
> > Can't we assume that the default timer is omap[234]_sync32k_timer to
> > avoid renaming the timer entries in all the board files?
> 
> Hmmm...
> How will this work with the macros defining the sys_timer structure?
> I would also not want to hide the exact timer used under the default name.

Can't you just add a new sys_timer (or a new macro) for GP only setups? 
 
> > Then we just need a new timer entries for the hardware that does
> > not have the sycn32k_timer available?
> 
> Well, I tried to make it small patch just for the hardware that needs it,
> but I always found some corner case where, IMHO, this does not work/look good.

Can you explain a bit further?

I guess what I'm after is just to avoid renaming the existing
timers in the board-*.c files and only rename the ones that
need gp timer only.

Regards,

Tony

WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
Date: Thu, 8 Nov 2012 08:20:05 -0800	[thread overview]
Message-ID: <20121108162004.GA6801@atomide.com> (raw)
In-Reply-To: <509B5B84.50605@compulab.co.il>

* Igor Grinberg <grinberg@compulab.co.il> [121107 23:15]:
> On 11/07/12 19:33, Tony Lindgren wrote:
> > 
> > I think this should be the default for the timers as that counter
> > does not stop during deeper idle states.
> 
> Well, it is the default as you can see from the patch.
> The problem is that for boards that for some reason do not have
> the 32k wired and rely on MPU/GP timer source, the default will not work
> and currently there is no way for board to specify which timer source
> it can use.

Yes. I was just wondering if we can avoid patching all the board
files by doing it the other way around by introducing a new
omap_gp_timer rather than renaming all the existing ones?

> We have discussed this in San Diego (remember?) and you actually proposed
> this way as a solution. Well, may be I took it a bit further than you
> thought, but this is because the board code cannot know which timer source
> should be used at runtime and the fall back described below, does not work.

Yes thanks I agree we should get rid of that Kconfig option for sure. 

> >> --- a/arch/arm/mach-omap2/board-2430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> >> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
> >>  	.handle_irq	= omap2_intc_handle_irq,
> >>  	.init_machine	= omap_2430sdp_init,
> >>  	.init_late	= omap2430_init_late,
> >> -	.timer		= &omap2_timer,
> >> +	.timer		= &omap2_sync32k_timer,
> >>  	.restart	= omap_prcm_restart,
> >>  MACHINE_END
> >> --- a/arch/arm/mach-omap2/board-3430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> >> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
> >>  	.handle_irq	= omap3_intc_handle_irq,
> >>  	.init_machine	= omap_3430sdp_init,
> >>  	.init_late	= omap3430_init_late,
> >> -	.timer		= &omap3_timer,
> >> +	.timer		= &omap3_sync32k_timer,
> >>  	.restart	= omap_prcm_restart,
> >>  MACHINE_END
> > ...
> > 
> > Can't we assume that the default timer is omap[234]_sync32k_timer to
> > avoid renaming the timer entries in all the board files?
> 
> Hmmm...
> How will this work with the macros defining the sys_timer structure?
> I would also not want to hide the exact timer used under the default name.

Can't you just add a new sys_timer (or a new macro) for GP only setups? 
 
> > Then we just need a new timer entries for the hardware that does
> > not have the sycn32k_timer available?
> 
> Well, I tried to make it small patch just for the hardware that needs it,
> but I always found some corner case where, IMHO, this does not work/look good.

Can you explain a bit further?

I guess what I'm after is just to avoid renaming the existing
timers in the board-*.c files and only rename the ones that
need gp timer only.

Regards,

Tony

  reply	other threads:[~2012-11-08 16:20 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-07 14:42 [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER Igor Grinberg
2012-11-07 14:42 ` Igor Grinberg
2012-11-07 17:33 ` Tony Lindgren
2012-11-07 17:33   ` Tony Lindgren
2012-11-08  7:13   ` Igor Grinberg
2012-11-08  7:13     ` Igor Grinberg
2012-11-08 16:20     ` Tony Lindgren [this message]
2012-11-08 16:20       ` Tony Lindgren
2012-11-08 17:09       ` Hiremath, Vaibhav
2012-11-08 17:09         ` Hiremath, Vaibhav
2012-11-11  9:16       ` Igor Grinberg
2012-11-11  9:16         ` Igor Grinberg
2012-11-12 19:05         ` Jon Hunter
2012-11-12 19:05           ` Jon Hunter
2012-11-13  9:08           ` Igor Grinberg
2012-11-13  9:08             ` Igor Grinberg
2012-11-12 22:06         ` Tony Lindgren
2012-11-12 22:06           ` Tony Lindgren
2012-11-07 21:36 ` Jon Hunter
2012-11-07 21:36   ` Jon Hunter
2012-11-08  7:59   ` Igor Grinberg
2012-11-08  7:59     ` Igor Grinberg
2012-11-08 16:16     ` Jon Hunter
2012-11-08 16:16       ` Jon Hunter
2012-11-08 17:08       ` Hiremath, Vaibhav
2012-11-08 17:08         ` Hiremath, Vaibhav
2012-11-08 17:39         ` Jon Hunter
2012-11-08 17:39           ` Jon Hunter
2012-11-08 17:47           ` Hiremath, Vaibhav
2012-11-08 17:47             ` Hiremath, Vaibhav
2012-11-08 17:58             ` Jon Hunter
2012-11-08 17:58               ` Jon Hunter
2012-11-08 18:06               ` Hiremath, Vaibhav
2012-11-08 18:06                 ` Hiremath, Vaibhav
2012-11-08 18:13                 ` Jon Hunter
2012-11-08 18:13                   ` Jon Hunter
2012-11-08 18:28                   ` Hiremath, Vaibhav
2012-11-08 18:28                     ` Hiremath, Vaibhav
2012-11-08 18:47                     ` Jon Hunter
2012-11-08 18:47                       ` Jon Hunter
2012-11-09  0:55                 ` Jon Hunter
2012-11-09  0:55                   ` Jon Hunter
2012-11-12  6:42                   ` Hiremath, Vaibhav
2012-11-12  6:42                     ` Hiremath, Vaibhav
2012-11-08 17:58           ` Paul Walmsley
2012-11-08 17:58             ` Paul Walmsley
2012-11-08 18:08             ` Jon Hunter
2012-11-08 18:08               ` Jon Hunter
2012-11-08 18:17               ` Paul Walmsley
2012-11-08 18:17                 ` Paul Walmsley
2012-11-08 18:34                 ` Jon Hunter
2012-11-08 18:34                   ` Jon Hunter
2012-11-11 11:35                   ` Igor Grinberg
2012-11-11 11:35                     ` Igor Grinberg
2012-11-12  6:38                     ` Hiremath, Vaibhav
2012-11-12  6:38                       ` Hiremath, Vaibhav
2012-11-12  7:24                       ` Igor Grinberg
2012-11-12  7:24                         ` Igor Grinberg
2012-11-12 10:40                         ` Hiremath, Vaibhav
2012-11-12 10:40                           ` Hiremath, Vaibhav
2012-12-10 20:49                     ` Paul Walmsley
2012-12-10 20:49                       ` Paul Walmsley
2012-12-14 23:58         ` Russ Dill
2012-12-14 23:58           ` Russ Dill
2012-11-11 11:25       ` Igor Grinberg
2012-11-11 11:25         ` Igor Grinberg
2012-11-08 18:54     ` Jon Hunter
2012-11-08 18:54       ` Jon Hunter
2012-11-08 18:59       ` Hiremath, Vaibhav
2012-11-08 18:59         ` Hiremath, Vaibhav
2012-11-08 19:16         ` Jon Hunter
2012-11-08 19:16           ` Jon Hunter
2012-11-11 11:28           ` Igor Grinberg
2012-11-11 11:28             ` Igor Grinberg
2012-11-12 19:15             ` Jon Hunter
2012-11-12 19:15               ` Jon Hunter
2012-11-13  9:14               ` Igor Grinberg
2012-11-13  9:14                 ` Igor Grinberg
2012-11-13 16:13                 ` Jon Hunter
2012-11-13 16:13                   ` Jon Hunter
2012-11-14  7:23                   ` Igor Grinberg
2012-11-14  7:23                     ` Igor Grinberg
2012-11-12 10:38           ` Hiremath, Vaibhav
2012-11-12 10:38             ` Hiremath, Vaibhav
2012-11-12 11:01             ` Benoit Cousson
2012-11-12 11:01               ` Benoit Cousson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121108162004.GA6801@atomide.com \
    --to=tony@atomide.com \
    --cc=grinberg@compulab.co.il \
    --cc=hvaibhav@ti.com \
    --cc=jon-hunter@ti.com \
    --cc=khilman@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=santosh.shilimkar@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.