[U-Boot] [PATCH v1] board: ti: am57xx Add board detection logic

[Nishanth Menon]

On 10/28/2015 06:43 PM, Steve Kipisz wrote:
> Add board detection logic for the Beagleboard-x15. This patch
> takes advantage of recent infrastructure to allow common means
> of board detection.
> 
> - Read the EEPROM only once in MLO and store the board name and
>   revision in scratchpad memory so it is also available in u-boot.
> - Use the board name to detect the Beagleboard-x15 board and
>   set it up accordingly.
> 
> Signed-off-by: Steve Kipisz <s-kipisz2 at ti.com>
> ---
>  arch/arm/include/asm/omap_common.h | 10 +++++++++-
>  board/ti/am57xx/board.c            | 29 ++++++++++++++++++++++++++---
>  board/ti/am57xx/board.h            | 29 +++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+), 4 deletions(-)
>  create mode 100644 board/ti/am57xx/board.h
> 
> diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
> index 9a9b154..7e6ac0b 100644
> --- a/arch/arm/include/asm/omap_common.h
> +++ b/arch/arm/include/asm/omap_common.h
> @@ -765,7 +765,15 @@ static inline u8 is_dra72x(void)
>  #define OMAP_SRAM_SCRATCH_VCORES_PTR    (SRAM_SCRATCH_SPACE_ADDR + 0x1C)
>  #define OMAP_SRAM_SCRATCH_SYS_CTRL	(SRAM_SCRATCH_SPACE_ADDR + 0x20)
>  #define OMAP_SRAM_SCRATCH_BOOT_PARAMS	(SRAM_SCRATCH_SPACE_ADDR + 0x24)
> -#define OMAP5_SRAM_SCRATCH_SPACE_END	(SRAM_SCRATCH_SPACE_ADDR + 0x28)
> +#ifdef CONFIG_AM57XX
> +#define AM57XX_BOARD_NAME_START         (SRAM_SCRATCH_SPACE_ADDR + 0x28)
> +#define AM57XX_BOARD_NAME_END           (SRAM_SCRATCH_SPACE_ADDR + 0x34)
> +#define AM57XX_BOARD_VERSION_START      (SRAM_SCRATCH_SPACE_ADDR + 0x35)
> +#define AM57XX_BOARD_VERSION_END        (SRAM_SCRATCH_SPACE_ADDR + 0x49)
> +#define OMAP5_SRAM_SCRATCH_SPACE_END   (SRAM_SCRATCH_SPACE_ADDR + 0x4D)
> +#else
> +#define OMAP5_SRAM_SCRATCH_SPACE_END   (SRAM_SCRATCH_SPACE_ADDR + 0x28)
> +#endif
>  
>  /* Boot parameters */
>  #define DEVICE_DATA_OFFSET	0x18
> diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c
> index 042f9ab..869bbe7 100644
> --- a/board/ti/am57xx/board.c
> +++ b/board/ti/am57xx/board.c
> @@ -31,6 +31,7 @@
>  #include <ti-usb-phy-uboot.h>
>  
>  #include "mux_data.h"
> +#include "board.h"
>  
>  #ifdef CONFIG_DRIVER_TI_CPSW
>  #include <cpsw.h>
> @@ -270,6 +271,13 @@ int board_late_init(void)
>  	 * This is the POWERHOLD-in-Low behavior.
>  	 */
>  	palmas_i2c_write_u8(TPS65903X_CHIP_P1, 0xA0, 0x1);
> +
> +	/*
> +	 * Set board_name based on the name in the EEPROM.
> +	 */
> +	if (board_is_x15())
> +		setenv("board_name", "beagle_x15");
> +
>  	return 0;
>  }
>  
> @@ -282,9 +290,24 @@ void set_muxconf_regs_essential(void)
>  #ifdef CONFIG_IODELAY_RECALIBRATION
>  void recalibrate_iodelay(void)
>  {
> -	__recalibrate_iodelay(core_padconf_array_essential,
> -			      ARRAY_SIZE(core_padconf_array_essential),
> -			      iodelay_cfg_array, ARRAY_SIZE(iodelay_cfg_array));
> +	struct omap_eeprom ep;
> +
> +	/*
> +	 * omap_eeprom_init and omap_eeprom_read print a message
> +	 * to the console if they failed.
> +	 */
> +	if (!omap_eeprom_init(CONFIG_SYS_OMAP_I2C0, CONFIG_EEPROM_CHIP_ADDR)) {

This should have been posted as a series, since this patch uses a
function introduced by a previously posted patch .

> +		if (!omap_eeprom_read(CONFIG_EEPROM_CHIP_ADDR, 0, &ep)) {
> +			strncpy(am57xx_board_name, (char *)ep.name,
> +				sizeof(ep.name));
> +			am57xx_board_name[sizeof(ep.name)] = 0;
> +		}
> +	}
> +
This should probably be split out. the function is a
recalibrate_iodelay and eeprom detection is not really in it's purview.

> +	if (board_is_x15())
> +		__recalibrate_iodelay(core_padconf_array_essential,
> +			ARRAY_SIZE(core_padconf_array_essential),
> +			iodelay_cfg_array, ARRAY_SIZE(iodelay_cfg_array));

With this specific change, existing EVM boards are broken at this
patch since the eeprom detection is the basis for x15 configuration.
Further, you dont really need to differentiate between both boards.
the configuration is the same for both.

>  }
>  #endif
>  
> diff --git a/board/ti/am57xx/board.h b/board/ti/am57xx/board.h
> new file mode 100644
> index 0000000..55719ce
> --- /dev/null
> +++ b/board/ti/am57xx/board.h
> @@ -0,0 +1,29 @@
> +/*
> + * board.h
> + *
> + * TI AM57xx boards information header
> + *
> + * Copyright (C) 2015, Texas Instruments, Incorporated - http://www.ti.com/
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _BOARD_H_
> +#define _BOARD_H_
> +
> +#include <asm/arch/omap.h>
> +
> +static char *const am57xx_board_name = (char *)AM57XX_BOARD_NAME_START;
> +static char *const am57xx_board_rev = (char *)AM57XX_BOARD_VERSION_START;

I would rather have others comment on this change.

> +
> +/*
> + * TI AM57xx parts define a system EEPROM that defines certain sub-fields.
> + * We use these fields to in turn see what board we are on, and what
> + * that might require us to set or not set.
> + */
> +
> +static inline int board_is_x15(void)
> +{
> +	return !strncmp(am57xx_board_name, "BBRDX15_", HDR_NAME_LEN);
> +}
> +#endif
> 

NAK for the stated reasons.

-- 
Regards,
Nishanth Menon