Robert Millan <[EMAIL PROTECTED]> writes: > On Tue, Oct 16, 2007 at 08:46:16PM +0200, Marco Gerards wrote: >> > >> > That's what grub_ticksleep does. grub_sleep() counts in seconds because >> > I tried to mimic POSIX which seems to be a trend for grub_* functions. I >> > think it can be used for menu timeout although I didn't have time to look. >> >> Right. Although I do not like setting the time in >> GRUB_TICKS_PER_SECOND for millisecond stuff, etc. In that case >> everyone has to implement the same functionality. > > Moved to grub_millisleep().
Good :) >> > OTOH, this wouldn't be the first place in grub where __i386__ is tested ;-) >> >> Oh? Perhaps that code is wrong? > > Actually now that I check it's only in one file. But the code is right > afaict. What I mean is that this might be a wrong approach in this other file as well. >> >> > + >> >> > +void >> >> > +grub_ticksleep (grub_uint32_t ticks) >> >> > +{ >> >> > + grub_uint32_t end_at; >> >> > + end_at = grub_get_rtc () + ticks; >> >> > + while (grub_get_rtc () < end_at) >> >> > + grub_cpu_idle (); >> >> > +} >> >> >> >> Why do you recreate this for every arch? This seems portable as long >> >> as you can sleep a bit from time to time. >> > >> > What if a platform provides a sleep-like mechanism, but not a get_rtc-like >> > one? You can implement sleep around get_rtc easily, but not the other way >> > around. This is the case for LB (simply because grub_get_rtc is not >> > implemented yet), but it could also happen on platforms that are designed >> > not to provide it or are just buggy. >> >> Well, I have no objections to this approach. > > Ok. I made it a bit better by implementing grub_millisleep_generic in > kern/misc.c and making each port just use that, having the option to do it > their way if preferred. Great. >> Are you sure init.c is >> the right place? > > Mostly. I've observed that for code that doesn't obviously belong somewhere > else, it tends to be in init.c if it's in C and startup.S if it's in asm (in > i386/pc/startup.S it actually gets to the extreme, since only a small part of > it is used for "startup" as such). > > So I think init.c is fine. Ok. > 2007-10-15 Robert Millan <[EMAIL PROTECTED]> > > * include/grub/time.h: New file. > * include/grub/i386/time.h: Likewise. > * include/grub/powerpc/time.h: Likewise. > * include/grub/sparc64/time.h: Likewise. > > * include/grub/i386/pc/time.h (KERNEL_TIME_HEADER): Rename all > instances to ... > (KERNEL_MACHINE_TIME_HEADER): ... this. > * include/grub/powerpc/ieee1275/time.h (KERNEL_TIME_HEADER): Rename all > instances to ... > (KERNEL_MACHINE_TIME_HEADER): ... this. > * include/grub/sparc64/ieee1275/time.h (KERNEL_TIME_HEADER): Rename all > instances to ... > (KERNEL_MACHINE_TIME_HEADER): ... this. > > * kern/i386/efi/init.c: Include `grub/time.h'. <grub/time.h> is preferred. [...] > +void > +grub_millisleep_generic (grub_uint32_t ms) > +{ > + grub_uint32_t time; > + int i; > + > + for (; ms > 0; ms -= TICK_DURATION_IN_MS) > + /* wait for the lowest fraction of milliseconds we can (rounded up) */ > + for (i = 0; i < TICK_DURATION_IN_MS; i++) > + { > + /* wait for next tick */ > + time = grub_get_rtc (); > + while (time == grub_get_rtc ()) > + grub_cpu_idle (); > + } > +} The problem with this is when TICK_DURATION_IN_MS is not very accurate. I think you can be more accurate if you use TICKS_PER_SECOND and use it to calculate the total amount of ticks to wait. This will avoid rounding problems. Accuracy is not always that important, but I prefer to have it, if we can. -- Marco _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel