[U-Boot] [PATCH 2/3] compulab: eeprom: add support for obtaining product name

[Igor Grinberg]

Hi Nikita,

On 09/02/15 19:05, Nikita Kiryanov wrote:
> Introduce cl_eeprom_get_product_name() for obtaining product name
> from the eeprom.
> 
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Igor Grinberg <grinberg at compulab.co.il>
> Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> ---
>  board/compulab/common/eeprom.c | 19 +++++++++++++++++++
>  board/compulab/common/eeprom.h |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/board/compulab/common/eeprom.c b/board/compulab/common/eeprom.c
> index aaacd2e..a48da93 100644
> --- a/board/compulab/common/eeprom.c
> +++ b/board/compulab/common/eeprom.c
> @@ -25,6 +25,8 @@
>  #define BOARD_REV_OFFSET		0
>  #define BOARD_REV_OFFSET_LEGACY		6
>  #define BOARD_REV_SIZE			2
> +#define BOARD_PRODUCT_NAME_OFFSET	128
> +#define BOARD_PRODUCT_NAME_SIZE		16

Please, can we make it shorter like with the MAC_ADDR?
e.g. PRODUCT_NAME_*

>  #define MAC_ADDR_OFFSET			4
>  #define MAC_ADDR_OFFSET_LEGACY		0
>  
> @@ -149,3 +151,20 @@ u32 cl_eeprom_get_board_rev(uint eeprom_bus)
>  
>  	return board_rev;
>  };
> +
> +static char product_name[BOARD_PRODUCT_NAME_SIZE];

That variable does not serve any good purpose...

> +char *cl_eeprom_get_product_name(uint eeprom_bus)
> +{
> +	if (cl_eeprom_setup(eeprom_bus))
> +		return NULL;
> +
> +	if (cl_eeprom_read(BOARD_PRODUCT_NAME_OFFSET,
> +			   (uchar *)product_name, BOARD_PRODUCT_NAME_SIZE)) {
> +		return NULL;
> +	}
> +
> +	/* Protect ourselves from invalid data (unterminated string) */
> +	product_name[BOARD_PRODUCT_NAME_SIZE - 1] = '\0';
> +
> +	return product_name;
> +}

I'd like to see this function implemented as the
cl_eeprom_read_mac_addr() function is.

> diff --git a/board/compulab/common/eeprom.h b/board/compulab/common/eeprom.h
> index e74c379..fceb5df 100644
> --- a/board/compulab/common/eeprom.h
> +++ b/board/compulab/common/eeprom.h
> @@ -13,6 +13,7 @@
>  #ifdef CONFIG_SYS_I2C
>  int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus);
>  u32 cl_eeprom_get_board_rev(uint eeprom_bus);
> +char *cl_eeprom_get_product_name(uint eeprom_bus);
>  #else
>  static inline int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus)
>  {
> @@ -22,6 +23,10 @@ static inline u32 cl_eeprom_get_board_rev(uint eeprom_bus)
>  {
>  	return 0;
>  }
> +static inline char *cl_eeprom_get_product_name(uint eeprom_bus)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  #endif
> 

-- 
Regards,
Igor.