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

2015-09-06 Thread Nikita Kiryanov 
Introduce cl_eeprom_get_product_name() for obtaining product name
from the eeprom.

Cc: Stefano Babic 
Cc: Igor Grinberg 
Signed-off-by: Nikita Kiryanov 
---
Changes in V2:
- s/BOARD_PRODUCT_NAME_*/PRODUCT_NAME_*
- Added documentation
- rewrote cl_eeprom_get_product_name() to take a buffer parameter

 board/compulab/common/eeprom.c | 30 ++
 board/compulab/common/eeprom.h |  6 ++
 2 files changed, 36 insertions(+)

diff --git a/board/compulab/common/eeprom.c b/board/compulab/common/eeprom.c
index 9f18a3d..6304468 100644
--- a/board/compulab/common/eeprom.c
+++ b/board/compulab/common/eeprom.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include "eeprom.h"
 
 #ifndef CONFIG_SYS_I2C_EEPROM_ADDR
 # define CONFIG_SYS_I2C_EEPROM_ADDR0x50
@@ -25,6 +26,8 @@
 #define BOARD_REV_OFFSET   0
 #define BOARD_REV_OFFSET_LEGACY6
 #define BOARD_REV_SIZE 2
+#define PRODUCT_NAME_OFFSET128
+#define PRODUCT_NAME_SIZE  16
 #define MAC_ADDR_OFFSET4
 #define MAC_ADDR_OFFSET_LEGACY 0
 
@@ -151,3 +154,30 @@ u32 cl_eeprom_get_board_rev(uint eeprom_bus)
 
return board_rev;
 };
+
+/*
+ * Routine: cl_eeprom_get_board_rev
+ * Description: read system revision from eeprom
+ *
+ * @buf: buffer to store the product name
+ * @eeprom_bus: i2c bus num of the eeprom
+ *
+ * @return: 0 on success, < 0 on failure
+ */
+int cl_eeprom_get_product_name(uchar *buf, uint eeprom_bus)
+{
+   int err;
+
+   if (buf == NULL)
+   return -EINVAL;
+
+   err = cl_eeprom_setup(eeprom_bus);
+   if (err)
+   return err;
+
+   err = cl_eeprom_read(PRODUCT_NAME_OFFSET, buf, PRODUCT_NAME_SIZE);
+   if (!err) /* Protect ourselves from invalid data (unterminated str) */
+   buf[PRODUCT_NAME_SIZE - 1] = '\0';
+
+   return err;
+}
diff --git a/board/compulab/common/eeprom.h b/board/compulab/common/eeprom.h
index e74c379..c0b4739 100644
--- a/board/compulab/common/eeprom.h
+++ b/board/compulab/common/eeprom.h
@@ -9,10 +9,12 @@
 
 #ifndef _EEPROM_
 #define _EEPROM_
+#include 
 
 #ifdef CONFIG_SYS_I2C
 int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus);
 u32 cl_eeprom_get_board_rev(uint eeprom_bus);
+int cl_eeprom_get_product_name(uchar *buf, uint eeprom_bus);
 #else
 static inline int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus)
 {
@@ -22,6 +24,10 @@ static inline u32 cl_eeprom_get_board_rev(uint eeprom_bus)
 {
return 0;
 }
+static inline int cl_eeprom_get_product_name(uchar *buf, uint eeprom_bus)
+{
+   return -ENOSYS;
+}
 #endif
 
 #endif
-- 
1.9.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-09-06 Thread Igor Grinberg 
Hi Nikita,

On 09/06/15 11:48, Nikita Kiryanov wrote:
> Introduce cl_eeprom_get_product_name() for obtaining product name
> from the eeprom.
> 
> Cc: Stefano Babic 
> Cc: Igor Grinberg 
> Signed-off-by: Nikita Kiryanov 
> ---
> Changes in V2:
>   - s/BOARD_PRODUCT_NAME_*/PRODUCT_NAME_*
>   - Added documentation
>   - rewrote cl_eeprom_get_product_name() to take a buffer parameter
> 
>  board/compulab/common/eeprom.c | 30 ++
>  board/compulab/common/eeprom.h |  6 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/board/compulab/common/eeprom.c b/board/compulab/common/eeprom.c
> index 9f18a3d..6304468 100644
> --- a/board/compulab/common/eeprom.c
> +++ b/board/compulab/common/eeprom.c
> @@ -9,6 +9,7 @@
>  
>  #include 
>  #include 
> +#include "eeprom.h"
>  
>  #ifndef CONFIG_SYS_I2C_EEPROM_ADDR
>  # define CONFIG_SYS_I2C_EEPROM_ADDR  0x50
> @@ -25,6 +26,8 @@
>  #define BOARD_REV_OFFSET 0
>  #define BOARD_REV_OFFSET_LEGACY  6
>  #define BOARD_REV_SIZE   2
> +#define PRODUCT_NAME_OFFSET  128
> +#define PRODUCT_NAME_SIZE16
>  #define MAC_ADDR_OFFSET  4
>  #define MAC_ADDR_OFFSET_LEGACY   0

It seems that the time has come to move the above constants out of this file
into the eeprom.h, so they can be used by users of this "lib".
Don't you think?

>  
> @@ -151,3 +154,30 @@ u32 cl_eeprom_get_board_rev(uint eeprom_bus)
>  
>   return board_rev;
>  };
> +
> +/*
> + * Routine: cl_eeprom_get_board_rev
> + * Description: read system revision from eeprom

Seems like you have a successful copy/paste problem ;-)

> + *
> + * @buf: buffer to store the product name
> + * @eeprom_bus: i2c bus num of the eeprom
> + *
> + * @return: 0 on success, < 0 on failure
> + */
> +int cl_eeprom_get_product_name(uchar *buf, uint eeprom_bus)
> +{
> + int err;
> +
> + if (buf == NULL)
> + return -EINVAL;

I think that this check is not really necessary.
If someone passes NULL instead of buf - let it crash, no?

> +
> + err = cl_eeprom_setup(eeprom_bus);
> + if (err)
> + return err;
> +
> + err = cl_eeprom_read(PRODUCT_NAME_OFFSET, buf, PRODUCT_NAME_SIZE);
> + if (!err) /* Protect ourselves from invalid data (unterminated str) */

Why do you need the above check?
I think, you can just write '\0' anyway, no?

> + buf[PRODUCT_NAME_SIZE - 1] = '\0';
> +
> + return err;
> +}
> diff --git a/board/compulab/common/eeprom.h b/board/compulab/common/eeprom.h
> index e74c379..c0b4739 100644
> --- a/board/compulab/common/eeprom.h
> +++ b/board/compulab/common/eeprom.h
> @@ -9,10 +9,12 @@
>  
>  #ifndef _EEPROM_
>  #define _EEPROM_
> +#include 
>  
>  #ifdef CONFIG_SYS_I2C
>  int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus);
>  u32 cl_eeprom_get_board_rev(uint eeprom_bus);
> +int cl_eeprom_get_product_name(uchar *buf, uint eeprom_bus);
>  #else
>  static inline int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus)
>  {
> @@ -22,6 +24,10 @@ static inline u32 cl_eeprom_get_board_rev(uint eeprom_bus)
>  {
>   return 0;
>  }
> +static inline int cl_eeprom_get_product_name(uchar *buf, uint eeprom_bus)
> +{
> + return -ENOSYS;
> +}
>  #endif
>  
>  #endif
> 

-- 
Regards,
Igor.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot