RE: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer

["Hiremath, Vaibhav" <hvaibhav@ti.com>]

On Wed, Apr 04, 2012 at 16:09:51, Hiremath, Vaibhav wrote:
> On Wed, Apr 04, 2012 at 14:34:09, Shilimkar, Santosh wrote:
> > On Tue, Apr 3, 2012 at 9:05 PM, Hiremath, Vaibhav <hvaibhav@ti.com> wrote:
> > > On Tue, Apr 03, 2012 at 00:05:38, Hilman, Kevin wrote:
> > >> "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> > >>
> > >> [...]
> > >>
> > >> > I don't personally like to add features which hardly anybody use and
> > >> > fundamentally broken with full kernel.
> > >>
> > >> Let's keep sane defaults, but not make it unreasonable to tweak eaither.
> > >>
> > >> I suggest what has already been mentioned.
> > >>
> > >> Register both timers, but have the sync timer have a higher rating.  On
> > >> AMxxx where there is no sync timer, GPtimer will be used.
> > >>
> > >
> > > This is another good option, I can change the rating of both the timers.
> > > With below description and given understanding/discussion/usability of both
> > > the timers, we can reverse the rating,
> > >
> > Btw, if we are going with this path, then there won't any need of commandline
> > option since clock-source can be switched from user space as well.
> > 
> 
> Yes, both the options will be open now and its upto user how he want to 
> choose it (either bootargs or sysfs).
> 

Santosh, Kevin and others (may be Russell can comment/conform),

Unfortunately, I am still struggling to get final patch out of this 
discussion,

Although now we concluded on changing the rates for 32ksync (rating = 300) 
and gptmier (rating = 250), and we thought kernel will choose better 
clocksource automatically, in default case it would be 32ksync_counter, and 
that is what we want; and user can use bootargs to override the clocksource 
to gptimer.

But we all missed on "setup_sched_clock()", which is dependent on clock 
Frequency, and based on that whole sched_clock() function works.

In case of 32ksync_counter:
--------------------------
cd.mult - 4000000000, cd.shift - 17
sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms

In case of gptimer:
--------------------------
cd.mult - 2581110154, cd.shift - 26
sched_clock: 32 bits at 26MHz, resolution 38ns, wraps every 165191ms

Also, in addition to this,

 - We do not have any API available in sched_clock code to update the 
   mult/shift factors.

 - You supposed to not call setup_sched_clock() multiple times, it dumps 
   whole stack with,

   WARN_ON(read_sched_clock != jiffy_sched_clock_read);

 - In case, user pass "clocksource="gp timer" in bootargs, still initially
   Kernel is going to use 32ksync_counter as a clocksource and the late in 
   the boot sequence it will switch to gptimer.

   [    0.637512] Switching to clocksource gp timer


There seems to be limitation for ARM architecture, it is restricted by
sched_clock implementation present in "arch/arm/kernel/sched_clock.c".
Natively, clocksource framework does support change in rate/frequency for
registered timer, using,

   - __clocksource_updatefreq_scale()
   - __clocksource_updatefreq_hz()
   - __clocksource_updatefreq_khz()


May be I am missing something here, so far this is what my understanding is, 
I am still digging through the code to understand how best we can handle 
this. And also, I wouldn't want to again create our own sched_clock 
interface.

May be Russell can help me to conform my understanding here.


Coming back to our actual problem of registering 2 clocksources, with all 
above things, I am slowly moving towards keeping CONFIG_OMAP_32K_TIMER 
config option and use compile time option for this (original approach) along 
with hwmod detection, so that it can be reused for devices like AM33xx.

Code implementation will be,

#ifdef CONFIG_OMAP_32K_TIMER
	oh = omap_hwmod_lookup(oh_name);
	if (oh && oh->slaves_cnt != 0) {
		u32 pbase;
		unsigned long size;
		struct resource mem_rsrc;
	
		res = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM,
					NULL, &mem_rsrc);
		if (!res) {
			pbase = mem_rsrc.start + 0x10;
			size = mem_rsrc.end - mem_rsrc.start;
			res = omap_init_clocksource_32k(pbase, size);
			if (!res)
				return;
	}
}
#endif

	/* Fall back to gp-timer code */
	res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source);
	BUG_ON(res);

	__omap_dm_timer_load_start(&clksrc,
			OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);

	setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);

	if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
		pr_err("Could not register clocksource %s\n",
						clocksource_gpt.name);
	else
		pr_info("OMAP clocksource: GPTIMER%d at %lu Hz\n",
						gptimer_id, clksrc.rate);


Thanks,
Vaibhav

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