Dear Hannes,
In message <1391606231-24963-1-git-send-email-oe5...@oevsv.at> you wrote:
> Adds support for Bernecker & Rainer Industrieelektronik GmbH T-Series
> Motherboard, using TI's AM3352 SoC.
>
> Most of code is derived from TI's AM335x_EVM
>
> Signed-off-by: Hannes Petermaier <oe5...@oevsv.at>
> Cc: tr...@ti.com
> ---
> Changes for v2:
> - added T-Series to boards.cfg (forgotten before)
>
> board/BuR/bur_tseries/Makefile | 13 +
> board/BuR/bur_tseries/board.c | 333 +++++++++++++++++++++++++
> board/BuR/bur_tseries/board.h | 18 ++
> board/BuR/bur_tseries/mux.c | 224 +++++++++++++++++
> board/BuR/bur_tseries/u-boot.lds | 101 ++++++++
> boards.cfg | 3 +
> include/configs/bur_tseries.h | 512
> ++++++++++++++++++++++++++++++++++++++
This is soemwhat redundant naming - you have the vendor name already
encoded in the vendor directory, so it is not necessary to repeat it
in the board directory name.
I think it would be better to have:
board/BuR/tseries/*
> +++ b/board/BuR/bur_tseries/board.c
...
> + * 04.02.2014 hpm very first board support for T-SERIES,
> + * based on board/ti/am335x/board.c
Please do not add any such changelog information to the source code.
We use git to track code modifications, which is much more powerful.
> +/*
> --------------------------------------------------------------------------*/
> +/* -- defines for GPIO -- */
> +#define ETHLED_ORANGE (96+16) /* GPIO3_16 */
> +#define REPSWITCH (0+20) /* GPIO0_20 */
Should these not rather go to the board config header file?
> diff --git a/board/BuR/bur_tseries/u-boot.lds
> b/board/BuR/bur_tseries/u-boot.lds
> new file mode 100644
> index 0000000..76db891
> --- /dev/null
> +++ b/board/BuR/bur_tseries/u-boot.lds
Are you sure you need a board specific linker script? Why?
> diff --git a/include/configs/bur_tseries.h b/include/configs/bur_tseries.h
> new file mode 100644
> index 0000000..a5e6dbc
> --- /dev/null
> +++ b/include/configs/bur_tseries.h
> @@ -0,0 +1,512 @@
...
> +/*#define DEBUG */
...
> +#if _HPMTEST
> +#define CONFIG_SILENT_CONSOLE
> +#define CONFIG_SILENT_U_BOOT_ONLY
> +#define CONFIG_SILENT_CONSOLE_UPDATE_ON_SET
> +#define CONFIG_SYS_DEVICE_NULLDEV
> +#endif /* _HPMTEST */
Please remove such dead code.
> +#define CONFIG_ENV_VARS_UBOOT_CONFIG /* Strongly encouraged */
Really? What exactly do you use these for?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Success is not the opposite of failure. Failure is the vital
ingredient in the recipe for success. - Mario Cortes
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
