Re: [PATCH] Implement grub_sleep() and grub_ticksleep()

[Robert Millan <rmh@aybabtu.com>]

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

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().

> > 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.

> >> > +
> >> > +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.

> 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.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: time.diff --]
[-- Type: text/x-diff, Size: 11850 bytes --]

2007-10-15  Robert Millan  <rmh@aybabtu.com>

	* 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_millisleep): New function.
	* kern/i386/pc/init.c: Include `grub/time.h'.
	(grub_millisleep): New function.
	* kern/powerpc/ieee1275/init.c: Include `grub/time.h'.
	Remove `grub/machine/time.h' include.
	(grub_millisleep): New function.
	* kern/sparc64/ieee1275/init.c: Include `grub/time.h'.
	Remove `grub/machine/time.h' include.
	(grub_millisleep): New function.

	* kern/misc.c: Include `grub/time.h'.
	(grub_millisleep_generic): New function.

diff -Nurp grub2/include/grub/i386/pc/time.h grub2.sleep/include/grub/i386/pc/time.h
--- grub2/include/grub/i386/pc/time.h	2007-07-22 01:32:24.000000000 +0200
+++ grub2.sleep/include/grub/i386/pc/time.h	2007-10-16 20:58:23.000000000 +0200
@@ -16,8 +16,8 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef KERNEL_TIME_HEADER
-#define KERNEL_TIME_HEADER	1
+#ifndef KERNEL_MACHINE_TIME_HEADER
+#define KERNEL_MACHINE_TIME_HEADER	1
 
 #include <grub/symbol.h>
 
@@ -26,4 +26,4 @@
 /* Return the real time in ticks.  */
 grub_uint32_t EXPORT_FUNC (grub_get_rtc) (void);
 
-#endif /* ! KERNEL_TIME_HEADER */
+#endif /* ! KERNEL_MACHINE_TIME_HEADER */
diff -Nurp grub2/include/grub/i386/time.h grub2.sleep/include/grub/i386/time.h
--- grub2/include/grub/i386/time.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.sleep/include/grub/i386/time.h	2007-10-16 21:00:04.000000000 +0200
@@ -0,0 +1,28 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB 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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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 GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef KERNEL_CPU_TIME_HEADER
+#define KERNEL_CPU_TIME_HEADER	1
+
+static __inline void
+grub_cpu_idle ()
+{
+  __asm__ __volatile__ ("hlt");
+}
+
+#endif /* ! KERNEL_CPU_TIME_HEADER */
diff -Nurp grub2/include/grub/powerpc/ieee1275/time.h grub2.sleep/include/grub/powerpc/ieee1275/time.h
--- grub2/include/grub/powerpc/ieee1275/time.h	2007-07-22 01:32:24.000000000 +0200
+++ grub2.sleep/include/grub/powerpc/ieee1275/time.h	2007-10-16 20:58:23.000000000 +0200
@@ -16,8 +16,8 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef KERNEL_TIME_HEADER
-#define KERNEL_TIME_HEADER	1
+#ifndef KERNEL_MACHINE_TIME_HEADER
+#define KERNEL_MACHINE_TIME_HEADER	1
 
 #include <grub/symbol.h>
 
@@ -26,4 +26,4 @@
 /* Return the real time in ticks.  */
 grub_uint32_t EXPORT_FUNC (grub_get_rtc) (void);
 
-#endif /* ! KERNEL_TIME_HEADER */
+#endif /* ! KERNEL_MACHINE_TIME_HEADER */
diff -Nurp grub2/include/grub/powerpc/time.h grub2.sleep/include/grub/powerpc/time.h
--- grub2/include/grub/powerpc/time.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.sleep/include/grub/powerpc/time.h	2007-10-16 21:00:48.000000000 +0200
@@ -0,0 +1,28 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB 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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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 GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef KERNEL_CPU_TIME_HEADER
+#define KERNEL_CPU_TIME_HEADER	1
+
+static __inline void
+grub_cpu_idle ()
+{
+  /* FIXME: not implemented */
+}
+
+#endif /* ! KERNEL_CPU_TIME_HEADER */
diff -Nurp grub2/include/grub/sparc64/ieee1275/time.h grub2.sleep/include/grub/sparc64/ieee1275/time.h
--- grub2/include/grub/sparc64/ieee1275/time.h	2007-07-22 01:32:25.000000000 +0200
+++ grub2.sleep/include/grub/sparc64/ieee1275/time.h	2007-10-16 20:58:23.000000000 +0200
@@ -16,8 +16,8 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef KERNEL_TIME_HEADER
-#define KERNEL_TIME_HEADER	1
+#ifndef KERNEL_MACHINE_TIME_HEADER
+#define KERNEL_MACHINE_TIME_HEADER	1
 
 #include <grub/symbol.h>
 
@@ -26,4 +26,4 @@
 /* Return the real time in ticks.  */
 grub_uint32_t EXPORT_FUNC (grub_get_rtc) (void);
 
-#endif /* ! KERNEL_TIME_HEADER */
+#endif /* ! KERNEL_MACHINE_TIME_HEADER */
diff -Nurp grub2/include/grub/sparc64/time.h grub2.sleep/include/grub/sparc64/time.h
--- grub2/include/grub/sparc64/time.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.sleep/include/grub/sparc64/time.h	2007-10-16 21:01:08.000000000 +0200
@@ -0,0 +1,28 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB 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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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 GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef KERNEL_CPU_TIME_HEADER
+#define KERNEL_CPU_TIME_HEADER	1
+
+static __inline void
+grub_cpu_idle ()
+{
+  /* FIXME: not implemented */
+}
+
+#endif /* ! KERNEL_CPU_TIME_HEADER */
diff -Nurp grub2/include/grub/time.h grub2.sleep/include/grub/time.h
--- grub2/include/grub/time.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.sleep/include/grub/time.h	2007-10-16 21:41:05.000000000 +0200
@@ -0,0 +1,38 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB 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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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 GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef KERNEL_TIME_HEADER
+#define KERNEL_TIME_HEADER	1
+
+#include <grub/symbol.h>
+#include <grub/machine/time.h>
+#include <grub/cpu/time.h>
+
+void EXPORT_FUNC(grub_millisleep) (grub_uint32_t ms);
+void EXPORT_FUNC(grub_millisleep_generic) (grub_uint32_t ms);
+
+static __inline void
+grub_sleep (grub_uint32_t s)
+{
+  grub_millisleep (1000 * s);
+}
+
+/* Duration of a tick in milliseconds, rounded up */
+#define TICK_DURATION_IN_MS ((1000 + GRUB_TICKS_PER_SECOND) / GRUB_TICKS_PER_SECOND)
+
+#endif /* ! KERNEL_TIME_HEADER */
diff -Nurp grub2/kern/i386/efi/init.c grub2.sleep/kern/i386/efi/init.c
--- grub2/kern/i386/efi/init.c	2007-07-22 01:32:27.000000000 +0200
+++ grub2.sleep/kern/i386/efi/init.c	2007-10-16 21:54:31.000000000 +0200
@@ -25,6 +25,13 @@
 #include <grub/cache.h>
 #include <grub/kernel.h>
 #include <grub/efi/efi.h>
+#include <grub/time.h>
+
+void
+grub_millisleep (grub_uint32_t ms)
+{
+  grub_millisleep_generic (ms);
+}
 
 void
 grub_machine_init (void)
diff -Nurp grub2/kern/i386/pc/init.c grub2.sleep/kern/i386/pc/init.c
--- grub2/kern/i386/pc/init.c	2007-09-07 23:55:26.000000000 +0200
+++ grub2.sleep/kern/i386/pc/init.c	2007-10-16 21:54:26.000000000 +0200
@@ -30,6 +30,7 @@
 #include <grub/loader.h>
 #include <grub/env.h>
 #include <grub/cache.h>
+#include <grub/time.h>
 
 struct mem_region
 {
@@ -46,6 +47,12 @@ grub_addr_t grub_os_area_addr;
 grub_size_t grub_os_area_size;
 grub_size_t grub_lower_mem, grub_upper_mem;
 
+void
+grub_millisleep (grub_uint32_t ms)
+{
+  grub_millisleep_generic (ms);
+}
+
 void 
 grub_arch_sync_caches (void *address __attribute__ ((unused)),
 		       grub_size_t len __attribute__ ((unused)))
diff -Nurp grub2/kern/misc.c grub2.sleep/kern/misc.c
--- grub2/kern/misc.c	2007-08-02 22:42:19.000000000 +0200
+++ grub2.sleep/kern/misc.c	2007-10-16 21:50:47.000000000 +0200
@@ -23,6 +23,7 @@
 #include <stdarg.h>
 #include <grub/term.h>
 #include <grub/env.h>
+#include <grub/time.h>
 
 void *
 grub_memmove (void *dest, const void *src, grub_size_t n)
@@ -1041,6 +1042,23 @@ grub_utf8_to_ucs4 (grub_uint32_t *dest, 
   return p - dest;
 }
 
+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 ();
+      }
+}
+
 /* Abort GRUB. This function does not return.  */
 void
 grub_abort (void)
diff -Nurp grub2/kern/powerpc/ieee1275/init.c grub2.sleep/kern/powerpc/ieee1275/init.c
--- grub2/kern/powerpc/ieee1275/init.c	2007-10-12 12:22:27.000000000 +0200
+++ grub2.sleep/kern/powerpc/ieee1275/init.c	2007-10-16 21:55:07.000000000 +0200
@@ -27,8 +27,8 @@
 #include <grub/setjmp.h>
 #include <grub/env.h>
 #include <grub/misc.h>
+#include <grub/time.h>
 #include <grub/machine/console.h>
-#include <grub/machine/time.h>
 #include <grub/machine/kernel.h>
 #include <grub/ieee1275/ofdisk.h>
 #include <grub/ieee1275/ieee1275.h>
@@ -47,6 +47,12 @@ extern char _start[];
 extern char _end[];
 
 void
+grub_millisleep (grub_uint32_t ms)
+{
+  grub_millisleep_generic (ms);
+}
+
+void
 grub_exit (void)
 {
   /* Trap to Open Firmware.  */
diff -Nurp grub2/kern/sparc64/ieee1275/init.c grub2.sleep/kern/sparc64/ieee1275/init.c
--- grub2/kern/sparc64/ieee1275/init.c	2007-07-22 01:32:28.000000000 +0200
+++ grub2.sleep/kern/sparc64/ieee1275/init.c	2007-10-16 22:02:33.000000000 +0200
@@ -27,8 +27,8 @@
 #include <grub/setjmp.h>
 #include <grub/env.h>
 #include <grub/misc.h>
+#include <grub/time.h>
 #include <grub/machine/console.h>
-#include <grub/machine/time.h>
 #include <grub/machine/kernel.h>
 #include <grub/ieee1275/ofdisk.h>
 #include <grub/ieee1275/ieee1275.h>
@@ -66,6 +66,12 @@ _start (uint64_t r0 __attribute__((unuse
   /* Never reached.  */
 }
 
+void
+grub_millisleep (grub_uint32_t ms)
+{
+  grub_millisleep_generic (ms);
+}
+
 int
 grub_ieee1275_test_flag (enum grub_ieee1275_flag flag)
 {