From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1IhrPp-0002yC-Fu for mharc-grub-devel@gnu.org; Tue, 16 Oct 2007 14:45:17 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IhrPm-0002xE-Pe for grub-devel@gnu.org; Tue, 16 Oct 2007 14:45:14 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IhrPl-0002w8-Bw for grub-devel@gnu.org; Tue, 16 Oct 2007 14:45:13 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IhrPl-0002w4-6g for grub-devel@gnu.org; Tue, 16 Oct 2007 14:45:13 -0400 Received: from smtp-vbr1.xs4all.nl ([194.109.24.21]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IhrPk-0006bB-Nz for grub-devel@gnu.org; Tue, 16 Oct 2007 14:45:13 -0400 Received: from localhost.localdomain (249-174.surfsnel.dsl.internl.net [145.99.174.249]) by smtp-vbr1.xs4all.nl (8.13.8/8.13.8) with ESMTP id l9GIjBCD074349 for ; Tue, 16 Oct 2007 20:45:12 +0200 (CEST) (envelope-from mgerards@xs4all.nl) From: Marco Gerards To: The development of GRUB 2 References: <20071015143857.GA26571@thorin> <87wstnqhdb.fsf@xs4all.nl> <20071016183420.GA4335@thorin> Mail-Copies-To: mgerards@xs4all.nl Date: Tue, 16 Oct 2007 20:46:16 +0200 In-Reply-To: <20071016183420.GA4335@thorin> (Robert Millan's message of "Tue, 16 Oct 2007 20:34:20 +0200") Message-ID: <87odeyrj7r.fsf@xs4all.nl> User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Virus-Scanned: by XS4ALL Virus Scanner X-detected-kernel: by monty-python.gnu.org: FreeBSD 4.6-4.9 Subject: Re: [PATCH] Implement grub_sleep() and grub_ticksleep() X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2007 18:45:15 -0000 Robert Millan writes: Hi, [...] >> > +#include >> > +#include >> > + >> > +void EXPORT_FUNC(grub_ticksleep) (grub_uint32_t ticks); >> > + >> > +static __inline void >> > +grub_sleep (grub_uint32_t s) >> > +{ >> > + grub_ticksleep (s * GRUB_TICKS_PER_SECOND); >> > +} >> >> Sleeping entire seconds is a bit much. Can you also add this for >> smaller time instances? > > 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. >> > +static __inline void >> > +grub_cpu_idle () >> > +{ >> > +#if defined(__i386__) >> > + __asm__ __volatile__ ("hlt"); >> > + /* FIXME: add other CPUs here */ >> > +#endif >> > +} >> >> This should go into a arch specific headerfile. > > Is this really necessary? It simplifies things a lot, since every cpu would > need a time.h just for that, whereas currently non-i386 gets a dummy stub for > free. Most of the time we use the arch specific header files. That is what they are for. > OTOH, this wouldn't be the first place in grub where __i386__ is tested ;-) Oh? Perhaps that code is wrong? >> > +#endif /* ! KERNEL_TIME_HEADER */ >> > diff -Nur grub2/kern/i386/efi/init.c grub2.ticks/kern/i386/efi/init.c >> > --- grub2/kern/i386/efi/init.c 2007-07-22 01:32:27.000000000 +0200 >> > +++ grub2.ticks/kern/i386/efi/init.c 2007-10-15 16:28:06.000000000 +0200 >> > @@ -25,6 +25,16 @@ >> > #include >> > #include >> > #include >> > +#include >> > + >> > +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. Are you sure init.c is the right place? -- Marco