Re: [PATCH v3 4/4] ARM: stm32: DH: Use common MAC address functions

Hi,

On 7/25/22 10:19, Philip Oberfichtner wrote:

To reduce code duplication, let the stm32 based DH boards use the common code for setting up their MAC addresses.

Signed-off-by: Philip Oberfichtner pro@denx.de Tested-by: Marek Vasut marex@denx.de Reviewed-by: Marek Vasut marex@denx.de

---

Changes in v3: - Reviewed by Marek

Changes in v2: - convert to livetree (rebase on commit 5a605b7c86152) - Tested-by Marek

board/dhelectronics/dh_stm32mp1/board.c | 102 +++++++++++------------- 1 file changed, 45 insertions(+), 57 deletions(-)

diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronics/dh_stm32mp1/board.c index 7a4c08cb7f..ab354e3e33 100644 --- a/board/dhelectronics/dh_stm32mp1/board.c +++ b/board/dhelectronics/dh_stm32mp1/board.c @@ -42,6 +42,7 @@ #include <usb/dwc2_udc.h> #include <watchdog.h> #include <dm/ofnode.h> +#include "../common/dh_common.h" #include "../../st/common/stpmic1.h"

/* SYSCFG registers */ @@ -84,36 +85,17 @@ #define KS_CIDER 0xC0 #define CIDER_ID 0x8870

-int setup_mac_address(void) +static bool dh_stm32_mac_is_in_ks8851(void) {

• unsigned char enetaddr[6];
• bool skip_eth0 = false;
• bool skip_eth1 = false;
• struct udevice *dev;
• int ret; ofnode node;
• ret = eth_env_get_enetaddr("ethaddr", enetaddr);
• if (ret) /* ethaddr is already set */

• skip_eth0 = true;
  

• u32 reg, cider, ccr;

  node = ofnode_path("ethernet1");

• if (!ofnode_valid(node)) {

• /* ethernet1 is not present in the system */
  

• skip_eth1 = true;
  

• goto out_set_ethaddr;
  

• }

• if (!ofnode_valid(node))

• return false;
  
  

• ret = eth_env_get_enetaddr("eth1addr", enetaddr);
• if (ret) {

• /* eth1addr is already set */
  

• skip_eth1 = true;
  

• goto out_set_ethaddr;
  

• }
• ret = ofnode_device_is_compatible(node, "micrel,ks8851-mll");
• if (ret)

• goto out_set_ethaddr;
  

• if (ofnode_device_is_compatible(node, "micrel,ks8851-mll"))

• return false;
  
  

  /*

  • KS8851 with EEPROM may use custom MAC from EEPROM, read

@@ -121,56 +103,62 @@ int setup_mac_address(void) * is present. If EEPROM is present, it must contain valid * MAC address. */

• u32 reg, cider, ccr; reg = ofnode_get_addr(node); if (!reg)

• goto out_set_ethaddr;
  

• return false;
  
  

  writew(KS_BE0 | KS_BE1 | KS_CIDER, reg + 2); cider = readw(reg);

• if ((cider & 0xfff0) != CIDER_ID) {

• skip_eth1 = true;
  

• goto out_set_ethaddr;
  

• }

• if ((cider & 0xfff0) != CIDER_ID)

• return true;
  
  

  writew(KS_BE0 | KS_BE1 | KS_CCR, reg + 2); ccr = readw(reg);

• if (ccr & KS_CCR_EEPROM) {

• skip_eth1 = true;
  

• goto out_set_ethaddr;
  

• }

• if (ccr & KS_CCR_EEPROM)

• return true;
  

• return false;

+}

-out_set_ethaddr:

• if (skip_eth0 && skip_eth1)

+static int dh_stm32_setup_ethaddr(void) +{

• unsigned char enetaddr[6];
• if (dh_mac_is_in_env("ethaddr")) return 0;

• node = ofnode_path("eeprom0");
• if (!ofnode_valid(node)) {

• printf("%s: No eeprom0 path offset\n", __func__);
  

• return -ENOENT;
  

• }

• if (!dh_get_mac_from_eeprom(enetaddr, "eeprom0"))

• return eth_env_set_enetaddr("ethaddr", enetaddr);
  
  

• ret = uclass_get_device_by_ofnode(UCLASS_I2C_EEPROM, node, &dev);
• if (ret) {

• printf("Cannot find EEPROM!\n");
  

• return ret;
  

• }

• return -ENXIO;

+}

• ret = i2c_eeprom_read(dev, 0xfa, enetaddr, 0x6);
• if (ret) {

• printf("Error reading configuration EEPROM!\n");
  

• return ret;
  

• }

+static int dh_stm32_setup_eth1addr(void) +{

• unsigned char enetaddr[6];

• if (is_valid_ethaddr(enetaddr)) {

• if (!skip_eth0)
  

• 	eth_env_set_enetaddr("ethaddr", enetaddr);
  

• if (dh_mac_is_in_env("eth1addr"))

• return 0;
  
  

• if (dh_stm32_mac_is_in_ks8851())

• return 0;
  

• if (!dh_get_mac_from_eeprom(enetaddr, "eeprom0")) { enetaddr[5]++;

• if (!skip_eth1)
  

• 	eth_env_set_enetaddr("eth1addr", enetaddr);
  

• return eth_env_set_enetaddr("eth1addr", enetaddr);
  

  }

• return -ENXIO;

+}

+int setup_mac_address(void) +{

• if (dh_stm32_setup_ethaddr())

• printf("%s: Unable to setup ethaddr!\n", __func__);
  

• if (dh_stm32_setup_eth1addr())

• printf("%s: Unable to setup eth1addr!\n", __func__);
  

• return 0; }

Minor remarks on the last function:

the 2 'printf' calls can be replaced by log_error() call (or log_debug ?)

to correctly handle this trace with CONFIG_LOG_LEVEL

=> printf should avoid except in command result to handle log features during initialization

Anyway

Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com

Thanks Patrick