Dear Hannes,
In message 1391606231-24963-1-git-send-email-oe5hpm@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 oe5hpm@oevsv.at Cc: trini@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