On 09/06/15 11:48, Nikita Kiryanov wrote:

cl_eeprom_read_mac_addr() doesn't differentiate between success case and inability to access eeprom. Fix this by propagating the return value of cl_eeprom_setup().

Cc: Stefano Babic sbabic@denx.de Cc: Igor Grinberg grinberg@compulab.co.il Signed-off-by: Nikita Kiryanov nikita@compulab.co.il

Acked-by: Igor Grinberg grinberg@compulab.co.il

---

Changes in V2:

• New patch

board/compulab/common/eeprom.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/compulab/common/eeprom.c b/board/compulab/common/eeprom.c index aaacd2e..9f18a3d 100644 --- a/board/compulab/common/eeprom.c +++ b/board/compulab/common/eeprom.c @@ -105,9 +105,11 @@ void get_board_serial(struct tag_serialnr *serialnr) int cl_eeprom_read_mac_addr(uchar *buf, uint eeprom_bus) { uint offset;

• int err;

• if (cl_eeprom_setup(eeprom_bus))

• return 0;
  

• err = cl_eeprom_setup(eeprom_bus);

• if (err)

• return err;
  
  

  offset = (cl_eeprom_layout != LAYOUT_LEGACY) ? MAC_ADDR_OFFSET : MAC_ADDR_OFFSET_LEGACY;

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 sbabic@denx.de Cc: Igor Grinberg grinberg@compulab.co.il Signed-off-by: Nikita Kiryanov nikita@compulab.co.il

---

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 <common.h> #include <i2c.h> +#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_SIZE 16 #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 <errno.h>

#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

On 09/06/15 11:48, Nikita Kiryanov wrote:

Old revisions of Utilite (a miniature PC based on cm-fx6) do not have a card detect for mmc, and thus the kernel needs to be told that there's a persistent storage on usdhc3 to force it to probe the mmc card.

Check the baseboard revision and modify the device tree accordingly if needed.

Cc: Stefano Babic sbabic@denx.de Cc: Igor Grinberg grinberg@compulab.co.il Signed-off-by: Nikita Kiryanov nikita@compulab.co.il

---

Changes in V2:

• #define USDHC3_PATH instead of const variable usdhc3_path
• Do not update device tree on eeprom read failures in a more explicit way

board/compulab/cm_fx6/cm_fx6.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 572111d..a21e7b0 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -580,9 +580,14 @@ int cm_fx6_setup_ecspi(void) { return 0; } #endif

#ifdef CONFIG_OF_BOARD_SETUP +#define USDHC3_PATH "/soc/aips-bus@02100000/usdhc@02198000/" int ft_board_setup(void *blob, bd_t *bd) {

• u32 baseboard_rev;
• int nodeoffset; uint8_t enetaddr[6];
• char baseboard_name[16];

That would probably have the actual size (macro).

• int err;

  /* MAC addr */ if (eth_getenv_enetaddr("ethaddr", enetaddr)) {

@@ -596,6 +601,21 @@ int ft_board_setup(void *blob, bd_t *bd) enetaddr, 6, 1); }

• baseboard_rev = cl_eeprom_get_board_rev(0);
• err = cl_eeprom_get_product_name((uchar *)baseboard_name, 0);

I would replace the 0 with a define explaining what that is.

• if (err || baseboard_rev == 0)

• return 0; /* Assume not an early revision SB-FX6m baseboard */
  

• if (!strncmp("SB-FX6m", baseboard_name, 7) && baseboard_rev <= 120) {

• fdt_shrink_to_minimum(blob); /* Make room for new properties */
  

• nodeoffset = fdt_path_offset(blob, USDHC3_PATH);
  

• fdt_delprop(blob, nodeoffset, "cd-gpios");
  

• fdt_find_and_setprop(blob, USDHC3_PATH, "non-removable",
  

• 		     NULL, 0, 1);
  

• fdt_find_and_setprop(blob, USDHC3_PATH, "keep-power-in-suspend",
  

• 		     NULL, 0, 1);
  

• }
• return 0;

} #endif