[U-Boot] [PATCH v2 2/2] MarSBoard: add new board

[Stefan Roese <sr@denx.de>]

adding my 0.02$ to this discussion...

On 26.03.2014 22:31, Eric B?nard wrote:
> this board is produced by Embest/Element 14 and is based on i.MX6 Dual
> The following features are tested :
> - UART2 (console)
> - eMMC
> - uSDCard
> - Ethernet
> - USB Host (through 2 ports hub)
> - HDMI output
> - I2C 1/2
> - SPI NOR Flash
>
> Boot on SPI NOR and through USB loader are tested.
>
> For more informations on this board :
> http://www.embest-tech.com/shop/star/marsboard.html
>
> As this board shares a lot with RiOTboard, both boards are supported by
> the same code base which is renamed embest/mx6boards
>
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>   board/embest/{riotboard => mx6boards}/Makefile     |  2 +-
>   .../riotboard.c => mx6boards/mx6boards.c}          | 49 +++++++++++++++++-
>   boards.cfg                                         |  3 +-
>   include/configs/{riotboard.h => embestmx6boards.h} | 58 ++++++++++++++++++++++
>   4 files changed, 108 insertions(+), 4 deletions(-)
>   rename board/embest/{riotboard => mx6boards}/Makefile (87%)
>   rename board/embest/{riotboard/riotboard.c => mx6boards/mx6boards.c} (91%)
>   rename include/configs/{riotboard.h => embestmx6boards.h} (84%)
>
> diff --git a/board/embest/riotboard/Makefile b/board/embest/mx6boards/Makefile
> similarity index 87%
> rename from board/embest/riotboard/Makefile
> rename to board/embest/mx6boards/Makefile
> index 5f978c0..467fb50 100644
> --- a/board/embest/riotboard/Makefile
> +++ b/board/embest/mx6boards/Makefile
> @@ -6,4 +6,4 @@
>   # SPDX-License-Identifier:	GPL-2.0+
>   #
>
> -obj-y  := riotboard.o
> +obj-y  := mx6boards.o
> diff --git a/board/embest/riotboard/riotboard.c b/board/embest/mx6boards/mx6boards.c
> similarity index 91%
> rename from board/embest/riotboard/riotboard.c
> rename to board/embest/mx6boards/mx6boards.c
> index 15eaa1e..374c2ec 100644
> --- a/board/embest/riotboard/riotboard.c
> +++ b/board/embest/mx6boards/mx6boards.c
> @@ -60,6 +60,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   	PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | PAD_CTL_HYS |	\
>   	PAD_CTL_ODE | PAD_CTL_SRE_FAST)
>
> +#define SPI_PAD_CTRL (PAD_CTL_HYS | PAD_CTL_SPEED_MED | \
> +		      PAD_CTL_DSE_40ohm | PAD_CTL_SRE_FAST)
> +
>   int dram_init(void)
>   {
>   	gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
> @@ -153,8 +156,10 @@ iomux_v3_cfg_t const usdhc3_pads[] = {
>   	MX6_PAD_SD3_DAT1__SD3_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
>   	MX6_PAD_SD3_DAT2__SD3_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
>   	MX6_PAD_SD3_DAT3__SD3_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +#ifdef CONFIG_RIOTBOARD
>   	MX6_PAD_SD3_DAT4__GPIO7_IO01    | MUX_PAD_CTRL(NO_PAD_CTRL), /* WP */
>   	MX6_PAD_SD3_DAT5__GPIO7_IO00    | MUX_PAD_CTRL(NO_PAD_CTRL), /* CD */
> +#endif
>   };
>
>   iomux_v3_cfg_t const usdhc4_pads[] = {
> @@ -187,7 +192,12 @@ int board_mmc_getcd(struct mmc *mmc)
>   		ret = !gpio_get_value(USDHC2_CD_GPIO);
>   		break;
>   	case USDHC3_BASE_ADDR:
> +#ifdef CONFIG_RIOTBOARD
>   		ret = !gpio_get_value(USDHC3_CD_GPIO);
> +#endif
> +#ifdef CONFIG_MARSBOARD
> +		ret = 1; /* eMMC/uSDHC3 is always present */
> +#endif

Yes, I agree. #ifdef's are ugly. But code duplication is also a problem 
as Wolfgang has mentioned.

Isn't there a way to detect the board type at runtime somehow (via CPU 
type or GPIO input, ...)? And then dynamically configure the board 
either for "RiOT" or "MarS"? This would make the code a bit more complex 
of course. But there would be no #ifdef's and no code duplication. And 
you would only have to maintain one U-Boot binary / version for both boards.

So, what do you think? Is this possible?

Thanks,
Stefan