[U-Boot] [RFC PATCH v1 9/9] sparc: Add support for gr712rc-board

[Masahiro Yamada]

Hi Francois,



On Wed, 29 Oct 2014 10:53:07 +0200
Francois Retief <fgretief at spaceteq.co.za> wrote:

> Add support of the Aeroflex Gaisler GR712RC-BOARD Development Kit.
> 
> Signed-off-by: Francois Retief <fgretief at spaceteq.co.za>
> ---
> 

Thanks for your great job!


I am sorry for bothering you work, but please let me
do some cleanups before adding a new board
because you are following the old style coding.
http://patchwork.ozlabs.org/patch/404889/
http://patchwork.ozlabs.org/patch/404887/
http://patchwork.ozlabs.org/patch/404886/
http://patchwork.ozlabs.org/patch/404888/
http://patchwork.ozlabs.org/patch/404890/


> 
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 733334f..21bef85 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -22,6 +22,9 @@ config TARGET_GR_XC3S_1500
>  config TARGET_GRSIM
>         bool "Support grsim"
> 
> +config TARGET_GR712RC_BOARD
> +       bool "Support gr712rc_board"
> +
>  endchoice


Perhaps can we use better name?
The current prompts were automatically taken by a tool
and I have to admint they are not very good.

Like this?
http://patchwork.ozlabs.org/patch/404889/





>  source "board/gaisler/gr_cpci_ax2000/Kconfig"
> @@ -29,5 +32,6 @@ source "board/gaisler/gr_ep2s60/Kconfig"
>  source "board/gaisler/gr_xc3s_1500/Kconfig"
>  source "board/gaisler/grsim/Kconfig"
>  source "board/gaisler/grsim_leon2/Kconfig"
> +source "board/gaisler/gr712rc_board/Kconfig"
> 
>  endmenu
> diff --git a/board/gaisler/gr712rc_board/Kconfig b/board/gaisler/gr712rc_board/Kconfig
> new file mode 100644
> index 0000000..e0af97c
> --- /dev/null
> +++ b/board/gaisler/gr712rc_board/Kconfig
> @@ -0,0 +1,19 @@
> +if TARGET_GR712RC_BOARD
> +
> +config SYS_CPU
> +       string
> +       default "leon3"
> +
> +config SYS_BOARD
> +       string
> +       default "gr712rc_board"
> +
> +config SYS_VENDOR
> +       string
> +       default "gaisler"
> +
> +config SYS_CONFIG_NAME
> +       string
> +       default "gr712rc_board"
> +
> +endif


I think we can save more lines.
Please check this:
http://patchwork.ozlabs.org/patch/404888/



> diff --git a/board/gaisler/gr712rc_board/MAINTAINERS b/board/gaisler/gr712rc_board/MAINTAINERS
> new file mode 100644
> index 0000000..329fcf9
> --- /dev/null
> +++ b/board/gaisler/gr712rc_board/MAINTAINERS
> @@ -0,0 +1,6 @@
> +GR712RC BOARD
> +#M:    -
> +S:     New
> +F:     board/gaisler/gr712rc/
> +F:     include/configs/gr712rc.h
> +F:     configs/gr712rc_defconfig

Please use either "Maintained" or "Orphan" for the status field.
In this case "Maintained", of course.





> diff --git a/board/gaisler/gr712rc_board/config.mk b/board/gaisler/gr712rc_board/config.mk
> new file mode 100644
> index 0000000..8594a74
> --- /dev/null
> +++ b/board/gaisler/gr712rc_board/config.mk
> @@ -0,0 +1,15 @@
> +#
> +# (C) Copyright 2014, Denel Spaceteq
> +# Francois Retief <fgretief at spaceteq.co.za>
> +#
> +# SPDX-License-Identifier:     GPL-2.0+
> +#
> +
> +# U-BOOT IN FLASH
> +#CONFIG_SYS_TEXT_BASE = 0x00000000
> +
> +# U-BOOT IN SRAM
> +#CONFIG_SYS_TEXT_BASE = 0x40000000
> +
> +# U-BOOT IN SDRAM
> +CONFIG_SYS_TEXT_BASE = 0x60000000


Please do not define CONFIG_SYS_TEXT_BASE in config.mk; this is very old style.
Add it in the defconfig, like this:
http://patchwork.ozlabs.org/patch/404890/



Best Regards
Masahiro Yamada