[PATCH] split i386-generic part of i386/pc/loader.h

[PATCH] split i386-generic part of i386/pc/loader.h

[Robert Millan]

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


This patch splits the i386-generic part of i386/pc/loader.h into i386/loader.h.

Comments?

-- 
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: loader_h.diff --]
[-- Type: text/x-diff, Size: 5529 bytes --]

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

	* include/grub/i386/loader.h: New file.

	* include/grub/i386/pc/loader.h (grub_linux_prot_size): Moved to ...
	* include/grub/i386/loader.h (grub_linux_prot_size): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_tmp_addr): Moved to ...
	* include/grub/i386/loader.h (grub_linux_tmp_addr): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_real_addr): Moved to ...
	* include/grub/i386/loader.h (grub_linux_real_addr): ... here.
	* include/grub/i386/pc/loader.h (grub_os_area_addr): Moved to ...
	* include/grub/i386/loader.h (grub_os_area_addr): ... here.
	* include/grub/i386/pc/loader.h (grub_os_area_size): Moved to ...
	* include/grub/i386/loader.h (grub_os_area_size): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_boot_zimage): Moved to ...
	* include/grub/i386/loader.h (grub_linux_boot_zimage): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_boot_bzimage): Moved to ...
	* include/grub/i386/loader.h (grub_linux_boot_bzimage): ... here.
	* include/grub/i386/pc/loader.h (grub_multiboot_real_boot): Moved to ...
	* include/grub/i386/loader.h (grub_multiboot_real_boot): ... here.
	* include/grub/i386/pc/loader.h (grub_multiboot2_real_boot): Moved to ...
	* include/grub/i386/loader.h (grub_multiboot2_real_boot): ... here.
	* include/grub/i386/pc/loader.h (grub_rescue_cmd_linux): Moved to ...
	* include/grub/i386/loader.h (grub_rescue_cmd_linux): ... here.
	* include/grub/i386/pc/loader.h (grub_rescue_cmd_initrd): Moved to ...
	* include/grub/i386/loader.h (grub_rescue_cmd_initrd): ... here.

	* include/grub/i386/pc/loader.h: Include `grub/cpu/loader.h'.

diff -Nurp grub2/include/grub/i386/loader.h grub2.loader_h/include/grub/i386/loader.h
--- grub2/include/grub/i386/loader.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.loader_h/include/grub/i386/loader.h	2007-10-17 10:27:32.000000000 +0200
@@ -0,0 +1,48 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2002,2003,2004,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 GRUB_LOADER_CPU_HEADER
+#define GRUB_LOADER_CPU_HEADER	1
+
+#include <grub/types.h>
+#include <grub/symbol.h>
+#include <grub/multiboot.h>
+
+extern grub_uint32_t EXPORT_VAR(grub_linux_prot_size);
+extern char *EXPORT_VAR(grub_linux_tmp_addr);
+extern char *EXPORT_VAR(grub_linux_real_addr);
+extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
+extern grub_size_t EXPORT_VAR(grub_os_area_size);
+
+void EXPORT_FUNC(grub_linux_boot_zimage) (void) __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_linux_boot_bzimage) (void) __attribute__ ((noreturn));
+
+/* The asm part of the multiboot loader.  */
+void EXPORT_FUNC(grub_multiboot_real_boot) (grub_addr_t entry, 
+					    struct grub_multiboot_info *mbi) 
+     __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_multiboot2_real_boot) (grub_addr_t entry,
+                                             struct grub_multiboot_info *mbi)
+     __attribute__ ((noreturn));
+
+/* It is necessary to export these functions, because normal mode commands
+   reuse rescue mode commands.  */
+void grub_rescue_cmd_linux (int argc, char *argv[]);
+void grub_rescue_cmd_initrd (int argc, char *argv[]);
+
+#endif /* ! GRUB_LOADER_CPU_HEADER */
diff -Nurp grub2/include/grub/i386/pc/loader.h grub2.loader_h/include/grub/i386/pc/loader.h
--- grub2/include/grub/i386/pc/loader.h	2007-07-25 21:29:23.000000000 +0200
+++ grub2.loader_h/include/grub/i386/pc/loader.h	2007-10-17 10:31:06.000000000 +0200
@@ -19,33 +19,10 @@
 #ifndef GRUB_LOADER_MACHINE_HEADER
 #define GRUB_LOADER_MACHINE_HEADER	1
 
-#include <grub/types.h>
 #include <grub/symbol.h>
-#include <grub/multiboot.h>
-
-extern grub_uint32_t EXPORT_VAR(grub_linux_prot_size);
-extern char *EXPORT_VAR(grub_linux_tmp_addr);
-extern char *EXPORT_VAR(grub_linux_real_addr);
-extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
-extern grub_size_t EXPORT_VAR(grub_os_area_size);
-
-void EXPORT_FUNC(grub_linux_boot_zimage) (void) __attribute__ ((noreturn));
-void EXPORT_FUNC(grub_linux_boot_bzimage) (void) __attribute__ ((noreturn));
+#include <grub/cpu/loader.h>
 
 /* This is an asm part of the chainloader.  */
 void EXPORT_FUNC(grub_chainloader_real_boot) (int drive, void *part_addr) __attribute__ ((noreturn));
 
-/* The asm part of the multiboot loader.  */
-void EXPORT_FUNC(grub_multiboot_real_boot) (grub_addr_t entry, 
-					    struct grub_multiboot_info *mbi) 
-     __attribute__ ((noreturn));
-void EXPORT_FUNC(grub_multiboot2_real_boot) (grub_addr_t entry,
-                                             struct grub_multiboot_info *mbi)
-     __attribute__ ((noreturn));
-
-/* It is necessary to export these functions, because normal mode commands
-   reuse rescue mode commands.  */
-void grub_rescue_cmd_linux (int argc, char *argv[]);
-void grub_rescue_cmd_initrd (int argc, char *argv[]);
-
 #endif /* ! GRUB_LOADER_MACHINE_HEADER */

Re: [PATCH] split i386-generic part of i386/pc/loader.h

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

Hi,

[...]

> 2007-10-17  Robert Millan  <rmh@aybabtu.com>
>
> 	* include/grub/i386/loader.h: New file.
>
> 	* include/grub/i386/pc/loader.h (grub_linux_prot_size): Moved to ...
> 	* include/grub/i386/loader.h (grub_linux_prot_size): ... here.
> 	* include/grub/i386/pc/loader.h (grub_linux_tmp_addr): Moved to ...
> 	* include/grub/i386/loader.h (grub_linux_tmp_addr): ... here.
> 	* include/grub/i386/pc/loader.h (grub_linux_real_addr): Moved to ...
> 	* include/grub/i386/loader.h (grub_linux_real_addr): ... here.
> 	* include/grub/i386/pc/loader.h (grub_os_area_addr): Moved to ...
> 	* include/grub/i386/loader.h (grub_os_area_addr): ... here.
> 	* include/grub/i386/pc/loader.h (grub_os_area_size): Moved to ...
> 	* include/grub/i386/loader.h (grub_os_area_size): ... here.
> 	* include/grub/i386/pc/loader.h (grub_linux_boot_zimage): Moved to ...
> 	* include/grub/i386/loader.h (grub_linux_boot_zimage): ... here.
> 	* include/grub/i386/pc/loader.h (grub_linux_boot_bzimage): Moved to ...
> 	* include/grub/i386/loader.h (grub_linux_boot_bzimage): ... here.
> 	* include/grub/i386/pc/loader.h (grub_multiboot_real_boot): Moved to ...
> 	* include/grub/i386/loader.h (grub_multiboot_real_boot): ... here.
> 	* include/grub/i386/pc/loader.h (grub_multiboot2_real_boot): Moved to ...
> 	* include/grub/i386/loader.h (grub_multiboot2_real_boot): ... here.
> 	* include/grub/i386/pc/loader.h (grub_rescue_cmd_linux): Moved to ...
> 	* include/grub/i386/loader.h (grub_rescue_cmd_linux): ... here.
> 	* include/grub/i386/pc/loader.h (grub_rescue_cmd_initrd): Moved to ...
> 	* include/grub/i386/loader.h (grub_rescue_cmd_initrd): ... here.

Same as the last patch:

(...): Move from here...
(...): ... to here.

btw, it is easier to do:

 	* include/grub/i386/pc/loader.h (grub_linux_prot_size)
 	(grub_linux_tmp_addr, grub_linux_real_addr): Moved from here...
 	* include/grub/i386/loader.h  (grub_linux_prot_size)
 	(grub_linux_tmp_addr, grub_linux_real_addr): ...to here.

Please have a close look how things like this were done in older
patches, to get a feeling on how to write this down.

--
Marco

Re: [PATCH] split i386-generic part of i386/pc/loader.h

[Robert Millan]

Committed.

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

Re: [PATCH] split i386-generic part of i386/pc/loader.h

[Vesa Jääskeläinen]

Robert Millan wrote:
> This patch splits the i386-generic part of i386/pc/loader.h into i386/loader.h.
> 
> Comments?

Why did you move stuff from i386/pc to i386 ?

I do see a point on running grub2 on non-pc i386...

Re: [PATCH] split i386-generic part of i386/pc/loader.h

[Robert Millan]

On Wed, Oct 17, 2007 at 09:17:54PM +0300, Vesa Jääskeläinen wrote:
> Robert Millan wrote:
> > This patch splits the i386-generic part of i386/pc/loader.h into i386/loader.h.
> > 
> > Comments?
> 
> Why did you move stuff from i386/pc to i386 ?
> 
> I do see a point on running grub2 on non-pc i386...

It's for LinuxBIOS.

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

Re: [PATCH] split i386-generic part of i386/pc/loader.h

[Vesa Jääskeläinen]

Robert Millan wrote:
> On Wed, Oct 17, 2007 at 09:17:54PM +0300, Vesa Jääskeläinen wrote:
>> Robert Millan wrote:
>>> This patch splits the i386-generic part of i386/pc/loader.h into i386/loader.h.
>>>
>>> Comments?
>> Why did you move stuff from i386/pc to i386 ?
>>
>> I do see a point on running grub2 on non-pc i386...
> 
> It's for LinuxBIOS.
> 

If this is some new arch that will be supported on i386 then I would
suggest i386/linuxbios/.

I think the question to be asked here is that does all i386
architectures need this functionality that you extracted. I have nothing
against modularizing code, in example if you compare contents of the efi
and pc directories you will notice that efi does not need most of the
stuff that is present on pc.

Actually I think better name for pc would be pcbios...

If you want to share code with LinuxBIOS why not just share those files
located on i386/pc/.

Re: [PATCH] split i386-generic part of i386/pc/loader.h

[Robert Millan]

On Thu, Oct 18, 2007 at 05:58:17PM +0300, Vesa Jääskeläinen wrote:
> 
> If this is some new arch that will be supported on i386 then I would
> suggest i386/linuxbios/.

AFAICT, the declarations that I moved are not LinuxBIOS-specific in nature.
Sure, EFI wasn't using them, but AFAICT that's just because EFI is the
firmware equivalent of a control freak :-)  (unless I missed something of
course..)

> Actually I think better name for pc would be pcbios...

Probably a good idea; then we could reuse "pc" for real PC-specific stuff
that is not specific to BIOS neither x86. but why not just `bios' ?  I think
that term already refers to the PC firmware.

> If you want to share code with LinuxBIOS why not just share those files
> located on i386/pc/.

In general it's not a problem for me to do that, but the file wasn't just
moved, the parts that were specific to bios were left in i386/pc/loader.h.

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