From mboxrd@z Thu Jan 1 00:00:00 1970 From: Srinivas KANDAGATLA Subject: Re: [PATCH v2 02/11] clocksource:global_timer: Add ARM global timer support. Date: Mon, 10 Jun 2013 14:41:58 +0100 Message-ID: <51B5D7A6.1020101@st.com> References: <1370855828-5318-1-git-send-email-srinivas.kandagatla@st.com> <1370856087-6452-1-git-send-email-srinivas.kandagatla@st.com> Reply-To: srinivas.kandagatla-qxv4g6HH51o@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Linus Walleij Cc: Mauro Carvalho Chehab , "linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Will Deacon , Mike Turquette , Samuel Ortiz , Stephen Gallimore , "linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Grant Likely , "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org" , Rob Herring , Stuart Menefy , Mark Brown , John Stultz , Russell King - ARM Linux , Thomas Gleixner , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Greg Kroah-Hartman , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Andrew List-Id: devicetree@vger.kernel.org On 10/06/13 14:13, Linus Walleij wrote: > On Mon, Jun 10, 2013 at 11:21 AM, Srinivas KANDAGATLA > wrote: >> >> Signed-off-by: Stuart Menefy >> Signed-off-by: Srinivas Kandagatla >> CC: Arnd Bergmann >> CC: Rob Herring >> CC: Linus Walleij >> CC: Will Deacon >> CC: Thomas Gleixner > > This is starting to look very good! > > (...) Thankyou. >> +static int __cpuinit gt_clockevents_init(struct clock_event_device *clk) >> +{ >> + struct clock_event_device **this_cpu_clk; >> + int cpu = smp_processor_id(); >> + >> + clk->name = "ARM global timer clock event"; >> + clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; >> + clk->set_mode = gt_clockevent_set_mode; >> + clk->set_next_event = gt_clockevent_set_next_event; >> + this_cpu_clk = __this_cpu_ptr(gt_evt); >> + *this_cpu_clk = clk; >> + clk->cpumask = cpumask_of(cpu); >> + clk->irq = gt_ppi; >> + clockevents_config_and_register(clk, gt_clk_rate, >> + 0, 0xffffffff); > > What do you mean with being able to set event on > 0? Its a bit of over do from me.. I will change it to what you suggested... > > This should most probably be: > > > clockevents_config_and_register(clk, gt_clk_rate, > 1, 0xffffffff); > > (...) >> +static struct clk *gt_get_clock(void) >> +{ >> + struct clk *clk; >> + int err; >> + >> + clk = clk_get_sys("gt", NULL); >> + if (IS_ERR(clk)) { >> + pr_err("global-timer: clock not found: %ld\n", PTR_ERR(clk)); >> + return clk; >> + } > (...) >> + clk = of_clk_get(np, 0); >> + if (!IS_ERR(clk)) >> + clk_register_clkdev(clk, NULL, "gt"); > > Well that was clever. > > Isn't it better to pass a struct device_node *np around and have that as > NULL in the non-DT boot path? I will try it and see how it will look. Thanks, srini > > (Maybe somebody in the community asked you to do this, then I > will live with it.) > > Yours, > Linus Walleij > >