[U-Boot] [PATCH 3/9] x86: Allow excluding reset handling code from u-boot.

[Graeme Russ]

Hi Simon,

At first I thought this patch dealt with the 'board reset' code but
then realised it deals with the 'reset vector' - Can you fix the patch
subject please

On Thu, Oct 4, 2012 at 10:39 AM, Simon Glass <sjg at chromium.org> wrote:
> From: Gabe Black <gabeblack at chromium.org>
>
> When running from coreboot we don't want this code.
>
> This version works by ifdef-ing out all of the code that would go
> into those sections and all the code that refers to it. The sections are
> then empty, and the linker will either leave them empty for the loader
> to ignore or remove them entirely.

Could this be done by #ifdef'ing the section in the linker script?

> Signed-off-by: Gabe Black <gabeblack at chromium.org>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>  arch/x86/cpu/resetvec.S                          |    6 ++++++
>  arch/x86/cpu/start16.S                           |    4 ++++
>  arch/x86/cpu/u-boot.lds                          |    3 +++
>  board/chromebook-x86/coreboot/coreboot_start16.S |    6 ++++++
>  board/eNET/eNET_start16.S                        |    4 ++++
>  5 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
> index 44aee5f..5b359ff 100644
> --- a/arch/x86/cpu/resetvec.S
> +++ b/arch/x86/cpu/resetvec.S
> @@ -25,6 +25,10 @@
>
>  /* Reset vector, jumps to start16.S */
>
> +#include <config.h>
> +
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .extern start16
>
>  .section .resetvec, "ax"
> @@ -36,3 +40,5 @@ reset_vector:
>
>         .org 0xf
>         nop
> +
> +#endif

Condition it out in the Makefile instead

> diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
> index cc393ff..d944840 100644
> --- a/arch/x86/cpu/start16.S
> +++ b/arch/x86/cpu/start16.S
> @@ -28,11 +28,14 @@
>
>  #include <asm/global_data.h>
>  #include <asm/processor-flags.h>
> +#include <config.h>
>
>  #define BOOT_SEG       0xffff0000      /* linear segment of boot code */
>  #define a32            .byte 0x67;
>  #define o32            .byte 0x66;
>
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .section .start16, "ax"
>  .code16
>  .globl start16
> @@ -141,3 +144,4 @@ gdt:
>         .byte   0x93            /* access */
>         .byte   0xcf            /* flags + limit_high */
>         .byte   0x00            /* base_high */
> +#endif

Ditto

> diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
> index fe28030..2a90a01 100644
> --- a/arch/x86/cpu/u-boot.lds
> +++ b/arch/x86/cpu/u-boot.lds
> @@ -85,6 +85,8 @@ SECTIONS
>         __bios_start = LOADADDR(.bios);
>         __bios_size = SIZEOF(.bios);
>
> +#ifndef CONFIG_NO_RESET_CODE
> +
>         /*
>          * The following expressions place the 16-bit Real-Mode code and
>          * Reset Vector at the end of the Flash ROM
> @@ -94,4 +96,5 @@ SECTIONS
>
>         . = RESET_VEC_LOC;
>         .resetvec : AT (CONFIG_SYS_TEXT_BASE + (CONFIG_SYS_MONITOR_LEN - RESET_SEG_SIZE + RESET_VEC_LOC)) { KEEP(*(.resetvec)); }
> +#endif
>  }

The commit comment states "and the linker will either leave them empty
for the loader to ignore or remove them entirely" but you are actually
explicitly remove them anyway - If they are not compiled, is this
necessary?

> diff --git a/board/chromebook-x86/coreboot/coreboot_start16.S b/board/chromebook-x86/coreboot/coreboot_start16.S
> index 9ad06df..6fac3d6 100644
> --- a/board/chromebook-x86/coreboot/coreboot_start16.S
> +++ b/board/chromebook-x86/coreboot/coreboot_start16.S
> @@ -28,6 +28,10 @@
>   * that is used by U-boot to its final destination.
>   */
>
> +#include <config.h>
> +
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .text
>  .section .start16, "ax"
>  .code16
> @@ -35,6 +39,8 @@
>  board_init16:
>         jmp     board_init16_ret
>
> +#endif
> +
>  .section .bios, "ax"
>  .code16
>  .globl realmode_reset

Hmm, I doubt coreboot really need a board level start16.S and (quite
frankly) the whole 'realmode reset' code (i.e. BIOS reset) is crap and
should be globally tossed (no one will ever call it)

> diff --git a/board/eNET/eNET_start16.S b/board/eNET/eNET_start16.S
> index 5e3f44c..43dda2f 100644
> --- a/board/eNET/eNET_start16.S
> +++ b/board/eNET/eNET_start16.S
> @@ -32,6 +32,8 @@
>  #include <asm/arch/sc520.h>
>  #include <generated/asm-offsets.h>
>
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .text
>  .section .start16, "ax"
>  .code16
> @@ -63,6 +65,8 @@ board_init16:
>
>         jmp     board_init16_ret
>
> +#endif
> +
>  .section .bios, "ax"
>  .code16
>  .globl realmode_reset

All the above should mean there is no reason to touch the eNET code

Regards,

Graeme