From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1IhrFq-0004p5-Lz for mharc-grub-devel@gnu.org; Tue, 16 Oct 2007 14:34:58 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IhrFo-0004m3-If for grub-devel@gnu.org; Tue, 16 Oct 2007 14:34:56 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IhrFm-0004kB-Tc for grub-devel@gnu.org; Tue, 16 Oct 2007 14:34:55 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IhrFm-0004jt-Lr for grub-devel@gnu.org; Tue, 16 Oct 2007 14:34:54 -0400 Received: from aybabtu.com ([69.60.117.155]) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1IhrFl-00045t-NV for grub-devel@gnu.org; Tue, 16 Oct 2007 14:34:54 -0400 Received: from [192.168.10.6] (helo=thorin) by aybabtu.com with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1IhrFh-0004rb-W5 for grub-devel@gnu.org; Tue, 16 Oct 2007 20:34:51 +0200 Received: from rmh by thorin with local (Exim 4.63) (envelope-from ) id 1IhrFE-0001Dm-Up for grub-devel@gnu.org; Tue, 16 Oct 2007 20:34:20 +0200 Date: Tue, 16 Oct 2007 20:34:20 +0200 From: Robert Millan To: The development of GRUB 2 Message-ID: <20071016183420.GA4335@thorin> References: <20071015143857.GA26571@thorin> <87wstnqhdb.fsf@xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87wstnqhdb.fsf@xs4all.nl> Organization: free as in freedom X-Message-Flag: Microsoft discourages use of Outlook. X-Debbugs-No-Ack: true User-Agent: Mutt/1.5.13 (2006-08-11) X-detected-kernel: by monty-python.gnu.org: Genre and OS details not recognized. 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:34:57 -0000 On Tue, Oct 16, 2007 at 04:11:28PM +0200, Marco Gerards wrote: > Robert Millan writes: > > > This patch implements grub_sleep() and grub_ticksleep(). > > Great! > > > 2007-10-15 Robert Millan > > > > * include/grub/time.h: New file. > > > > * include/grub/i386/pc/time.h (KERNEL_TIME_HEADER): Rename all > > instances to ... > > (KERNEL_MACHINE_TIME_HEADER): ... this. > > * include/grub/powerpc/ieee1275/time.h: Likewise. > > * include/grub/sparc64/ieee1275/time.h: Likewise. > > Better just repeat the same process for the last two lines. > > > * kern/i386/efi/init.c: Include `grub/time.h'. > > (grub_ticksleep): New function. > > * kern/i386/pc/init.c: Likewise. > > * kern/powerpc/ieee1275/init.c: Likewise. > > * kern/sparc64/ieee1275/init.c: Likewise. > > Please repeat is. Only use likewise when the change to one function > is the same. Better do too much instead of not enough. Ok > > +#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. > > +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. OTOH, this wouldn't be the first place in grub where __i386__ is tested ;-) > > +#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. -- Robert Millan I know my rights; I want my phone call! What use is a phone call, if you are unable to speak? (as seen on /.)