[U-Boot,4/5] mx6cuboxi: Differentiate Cubox-i and Hummingboard

From: Fabio Estevam <fabio.estevam@freescale.com>

Introduce is_hummingboard() function that reads GPIOs that can distinguish
between Cubox-i and Hummingboard.

Print the board name accordingly.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 41 +++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

Hi Fabio,

On 23/04/2015 05:57, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Introduce is_hummingboard() function that reads GPIOs that can distinguish
> between Cubox-i and Hummingboard.
> 
> Print the board name accordingly.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  board/solidrun/mx6cuboxi/mx6cuboxi.c | 41 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> index 1f240ae..83410b2 100644
> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -71,6 +71,12 @@ static iomux_v3_cfg_t const usdhc2_pads[] = {
>  	IOMUX_PADS(PAD_SD2_DAT3__SD2_DATA3	| MUX_PAD_CTRL(USDHC_PAD_CTRL)),
>  };
>  
> +static iomux_v3_cfg_t const hb_cbi_sense[] = {
> +	/* These pins are for sensing if it is a CuBox-i or a HummingBoard */
> +	IOMUX_PADS(PAD_KEY_ROW1__GPIO4_IO09  | MUX_PAD_CTRL(UART_PAD_CTRL)),
> +	IOMUX_PADS(PAD_EIM_DA4__GPIO3_IO04   | MUX_PAD_CTRL(UART_PAD_CTRL)),
> +};
> +
>  static void setup_iomux_uart(void)
>  {
>  	SETUP_IOMUX_PADS(uart1_pads);
> @@ -167,9 +173,42 @@ int board_init(void)
>  	return 0;
>  }
>  
> +static bool is_hummingboard(void)
> +{
> +	int val1, val2;
> +
> +	SETUP_IOMUX_PADS(hb_cbi_sense);
> +
> +	gpio_direction_input(IMX_GPIO_NR(4, 9));
> +	gpio_direction_input(IMX_GPIO_NR(3, 4));
> +
> +	val1 = gpio_get_value(IMX_GPIO_NR(4, 9));
> +	val2 = gpio_get_value(IMX_GPIO_NR(3, 4));
> +
> +	/*
> +	 * Machine selection -
> +	 * Machine        val1, val2
> +	 * -------------------------
> +	 * HB rev 3.x     x     0
> +	 * CBi            0     1
> +	 * HB             1     1
> +	 */
> +
> +	if (val2 == 0)
> +		return true;
> +	else if (val1 == 0)
> +		return false;
> +	else
> +		return true;
> +}
> +
>  int checkboard(void)
>  {
> -	puts("Board: MX6 Hummingboard\n");
> +	if (is_hummingboard())
> +		puts("Board: MX6 Hummingboard\n");
> +	else
> +		puts("Board: MX6 Cubox-i\n");
> +
>  	return 0;
>  }
>  
> 

Reviewed-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic

Hi Fabio, guys,

On 04/23/2015 06:57 AM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Introduce is_hummingboard() function that reads GPIOs that can distinguish
> between Cubox-i and Hummingboard.
>
> Print the board name accordingly.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>   board/solidrun/mx6cuboxi/mx6cuboxi.c | 41 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> index 1f240ae..83410b2 100644
> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -71,6 +71,12 @@ static iomux_v3_cfg_t const usdhc2_pads[] = {
>   	IOMUX_PADS(PAD_SD2_DAT3__SD2_DATA3	| MUX_PAD_CTRL(USDHC_PAD_CTRL)),
>   };
>
> +static iomux_v3_cfg_t const hb_cbi_sense[] = {
> +	/* These pins are for sensing if it is a CuBox-i or a HummingBoard */
> +	IOMUX_PADS(PAD_KEY_ROW1__GPIO4_IO09  | MUX_PAD_CTRL(UART_PAD_CTRL)),
> +	IOMUX_PADS(PAD_EIM_DA4__GPIO3_IO04   | MUX_PAD_CTRL(UART_PAD_CTRL)),
> +};
> +
>   static void setup_iomux_uart(void)
>   {
>   	SETUP_IOMUX_PADS(uart1_pads);
> @@ -167,9 +173,42 @@ int board_init(void)
>   	return 0;
>   }
>
> +static bool is_hummingboard(void)
> +{
> +	int val1, val2;
> +
> +	SETUP_IOMUX_PADS(hb_cbi_sense);
> +
> +	gpio_direction_input(IMX_GPIO_NR(4, 9));
> +	gpio_direction_input(IMX_GPIO_NR(3, 4));
> +
> +	val1 = gpio_get_value(IMX_GPIO_NR(4, 9));
> +	val2 = gpio_get_value(IMX_GPIO_NR(3, 4));
> +
> +	/*
> +	 * Machine selection -
> +	 * Machine        val1, val2
> +	 * -------------------------
> +	 * HB rev 3.x     x     0
> +	 * CBi            0     1
> +	 * HB             1     1
> +	 */
> +
> +	if (val2 == 0)
> +		return true;
> +	else if (val1 == 0)
> +		return false;
> +	else
> +		return true;
> +}

As more and more board variants are supported by unified source files,
functions like "is_specificboard()" are not scaling well - there's a
repetitive code for extracting hw-specific info, and then there's the
multiple functions that return true/false for each board variant.

Here's one proposal how this can be simplified a bit:

typedef enum
{
	BOARD_ALPHA,
	BOARD_BRAVO,
	BOARD_CHARLIE,
} model_t;

/* Can be also named is_board_variant() or something like this */
static bool is_board_model(model_t m)
{
	/* ... Extract HW info */

	switch (m)
	{
		case BOARD_ALPHA:
			if (check for this board model)
				return true;
			break;

		/* do same for the other board models */
		/* ... */
	}

	return false;
}

Not sure whether such code can be shared between different boards, but
can be a step towards unifying the handling of board models/variants.

I'm perfectly fine with Fabio's code here, just using the occasion to
share my idea.

> +
>   int checkboard(void)
>   {
> -	puts("Board: MX6 Hummingboard\n");
> +	if (is_hummingboard())
> +		puts("Board: MX6 Hummingboard\n");
> +	else
> +		puts("Board: MX6 Cubox-i\n");
> +
>   	return 0;
>   }
>
>

Kind regards,
Nikolay

Hi Nikolay,

On 23/04/2015 16:38, Nikolay Dimitrov wrote:
> 
> As more and more board variants are supported by unified source files,
> functions like "is_specificboard()" are not scaling well - there's a
> repetitive code for extracting hw-specific info, and then there's the
> multiple functions that return true/false for each board variant.

ok - let's see what we can do. I have not thinking about it, because how
to get model / version is quite always very specific. Some boards as
this one are reading GPIOs, some other are reading from I2C / SPI, some
other ones check if some hardware is present.

> 
> Here's one proposal how this can be simplified a bit:
> 
> typedef enum
> {
>     BOARD_ALPHA,
>     BOARD_BRAVO,
>     BOARD_CHARLIE,
> } model_t;
> 
> /* Can be also named is_board_variant() or something like this */
> static bool is_board_model(model_t m)
> {
>     /* ... Extract HW info */
> 
>     switch (m)
>     {
>         case BOARD_ALPHA:
>             if (check for this board model)
>                 return true;

The check must be done in board code, because only the board knows the
details.

>             break;
> 
>         /* do same for the other board models */
>         /* ... */
>     }
> 
>     return false;
> }
> 
> Not sure whether such code can be shared between different boards, but
> can be a step towards unifying the handling of board models/variants.
> 

Trying to get further and see if we can factorize it. Maybe not a lot.
We can also think that is_board_model() becomes get_board_model(), or we
generalize what was done in tqma6 with a sort of get_board_name().
Maybe we can factorize just a few code, but at least we could provide a
"standard" method without having all boards doing the same in different way.

> I'm perfectly fine with Fabio's code here, just using the occasion to
> share my idea.

Thanks - by sharing ideas we can improve code !

Best regards,
Stefano