Hi Graeme,
On Sat, Jul 9, 2011 at 10:24 PM, Graeme Russ graeme.russ@gmail.com wrote:
Hi Simon,
On 06/07/11 02:49, Simon Glass wrote:
These functions provide access to the high resolution microsecond timer and tidy up a global variable in the code.
Excellent - Good to see microsecond timers making their way in already
/* reset time, capture current incrementer value time */
gd->lastinc = readl(&timer_base->cntr_1us) /(TIMER_CLK/CONFIG_SYS_HZ);
gd->lastinc = timer_get_us() / (TIMER_CLK/CONFIG_SYS_HZ);CONFIG_SYS_HZ is always supposed to be 1000 and in future I think it should be removed entirely - Can you clean this up to not us CONFIG_SYS_HZ?
I will take a look.
gd->tbl = 0; /* start "advancing" time stamp from 0 */}
@@ -91,7 +90,7 @@ ulong get_timer_masked(void) ulong now;
/* current tick value */
now = readl(&timer_base->cntr_1us) / (TIMER_CLK / CONFIG_SYS_HZ);
now = timer_get_us() / (TIMER_CLK / CONFIG_SYS_HZ);And here
if (now >= gd->lastinc) /* normal mode (non roll) */ /* move stamp forward with absolute diff ticks */@@ -120,3 +119,17 @@ ulong get_tbclk(void) { return CONFIG_SYS_HZ; }
Hmmm, maybe all of the above should use get_tbclk()?
+unsigned long timer_get_us(void) +{
struct timerus *timer_base = (struct timerus *)NV_PA_TMRUS_BASE;return readl(&timer_base->cntr_1us);+}
timer_get_us needs to be u64 (unsigned long long). Also, the new timer API will define this as time_now_us, would be great if you could use this naming convention now to save me doing a mass of replaces later
Yes will do.
+unsigned long timer_get_future_us(u32 delay) +{
return timer_get_us() + delay;+}
C'mon - We've been here before - This is timer API stuff. Where are you going to use this? Why can't the proposed API be used instead?
I know you don't like the 'time since' implementation, but this has been discussed to death and it appears to me that the majority decision was to go that route rather than the 'future time' route. It is a completely pointless exercise and a complete waste of my time to re-write the timer API just to have someone that doesn't like a particular aspect go off and write a one-off function.
Well this code pre-dates this and I haven't revised it. I will take another look and sort this out. In fact from memory the return value isn't even used!
diff --git a/arch/arm/include/asm/arch-tegra2/timer.h
b/arch/arm/include/asm/arch-tegra2/timer.h
new file mode 100644 index 0000000..5d5445e --- /dev/null +++ b/arch/arm/include/asm/arch-tegra2/timer.h @@ -0,0 +1,34 @@ +/*
- Copyright (c) 2011 The Chromium OS Authors.
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+/* Tegra2 timer functions */
+#ifndef _TEGRA2_TIMER_H +#define _TEGRA2_TIMER_H
+/* returns the current monotonic timer value in microseconds */ +unsigned long timer_get_us(void);
+/* returns what the time will likely be some microseconds into the
future */
+unsigned long timer_get_future_us(u32 delay);
'likely' meaning it may or may not - no guarantee though. The new timer API is designed specifically designed to resolve this - 'At least x ms/us have passed' or 'at most x ms/us have passed'. No more 'x ms/us _might_ have passed'
Yes, watch this space.
BTW I have come across another problem where initialization must be done which has long delays in it (LCD display power-up sequence). It's starting to feel like we should have a central place for registering a timer which calls back when a time has expired. That way we can continue with other tings while we wait for the time to elapse. Something like:
/* Move to the next state */ static int next_state(void *my_data) { /* do some things, and then if you want to be called again... */ timer_register(timer_now_ms() + 40, next_state, my_data) }
void start_lcd_init() { // do first thing ... // set a timer to do the next thing later timer_register(timer_now_ms() + 200, next_state, my_data) }
...
Every now and then code (or a timer wait function) can call
timer_check()
which will call any expired timers on the list and remove them. This allows LCD init to continue while downloading the kernel, for example.
I haven't thought too hard about this, but apart from LCD I know that USB has some big delays. Obviously we can't make U-Boot multi-threaded but we can perhaps do something simple like the above. What do you think?
Also given that your timer API stuff seems to have a good reception and people are very happy, is there any impediment to getting this in sooner rather than later?
Regards, Simon
+#endif
Regards,
Graeme