-
Notifications
You must be signed in to change notification settings - Fork 8k
[PoC] drivers: ethernet/wifi: Use NVMEM cell to get MAC address #96598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7d0d45e
9df0f74
8a40281
be54d83
ae532f4
8ac0a2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -8,6 +8,7 @@ | |||||||
#define ZEPHYR_DRIVERS_ETHERNET_ETH_H_ | ||||||||
|
||||||||
#include <zephyr/types.h> | ||||||||
#include <zephyr/net/ethernet.h> | ||||||||
#include <zephyr/random/random.h> | ||||||||
|
||||||||
/* helper macro to return mac address octet from local_mac_address prop */ | ||||||||
|
@@ -42,4 +43,37 @@ static inline void gen_random_mac(uint8_t *mac_addr, uint8_t b0, uint8_t b1, uin | |||||||
sys_rand_get(&mac_addr[3], 3U); | ||||||||
} | ||||||||
|
||||||||
static inline int net_eth_mac_load(const struct net_eth_mac_config *cfg, uint8_t *mac_addr) | ||||||||
{ | ||||||||
if (cfg == NULL || cfg->type == NET_ETH_MAC_DEFAULT) { | ||||||||
return -ENODATA; | ||||||||
} | ||||||||
|
||||||||
/* Copy the static part */ | ||||||||
memcpy(mac_addr, cfg->addr, cfg->addr_len); | ||||||||
|
||||||||
if (cfg->type == NET_ETH_MAC_STATIC && cfg->addr_len == NET_ETH_ADDR_LEN) { | ||||||||
/* Complete static address */ | ||||||||
return 0; | ||||||||
} | ||||||||
Comment on lines
+52
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to have no difference between the static mac address and the default. as most drivers set the mac address from the dt as the initial value of the mac address in the data struct There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain a bit what you mean here? I don't fully understand. This static MAC address is when you have the following:
This should only ever be used during testing. An alternative is have the OUI in the static part:
Maybe the naming should be changed in that case to something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. most instance based drivers use something like: zephyr/drivers/ethernet/eth_litex_liteeth.c Lines 333 to 335 in 491498a
saving that also in the config struct seems not nessesary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'd refactor those to use the |
||||||||
|
||||||||
if (cfg->type == NET_ETH_MAC_RANDOM) { | ||||||||
sys_rand_get(&mac_addr[cfg->addr_len], NET_ETH_ADDR_LEN - cfg->addr_len); | ||||||||
|
||||||||
/* Set MAC address locally administered, unicast (LAA) */ | ||||||||
mac_addr[0] |= 0x02; | ||||||||
|
||||||||
return 0; | ||||||||
} | ||||||||
|
||||||||
#if defined(CONFIG_NVMEM) | ||||||||
if (cfg->type == NET_ETH_MAC_NVMEM) { | ||||||||
return nvmem_cell_read(&cfg->cell, &mac_addr[cfg->addr_len], 0, | ||||||||
NET_ETH_ADDR_LEN - cfg->addr_len); | ||||||||
} | ||||||||
#endif | ||||||||
|
||||||||
return -ENODATA; | ||||||||
} | ||||||||
|
||||||||
#endif /* ZEPHYR_DRIVERS_ETHERNET_ETH_H_ */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME); | |
|
||
#include <zephyr/kernel.h> | ||
#include <zephyr/device.h> | ||
#include <zephyr/nvmem.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be added to eth.h instead |
||
#include <zephyr/sys/__assert.h> | ||
#include <zephyr/sys/barrier.h> | ||
#include <zephyr/sys/util.h> | ||
|
@@ -1729,38 +1730,6 @@ static int eth_initialize(const struct device *dev) | |
return retval; | ||
} | ||
|
||
#if DT_INST_NODE_HAS_PROP(0, mac_eeprom) | ||
static void get_mac_addr_from_i2c_eeprom(uint8_t mac_addr[6]) | ||
{ | ||
uint32_t iaddr = CONFIG_ETH_SAM_GMAC_MAC_I2C_INT_ADDRESS; | ||
int ret; | ||
const struct i2c_dt_spec i2c = I2C_DT_SPEC_GET(DT_INST_PHANDLE(0, mac_eeprom)); | ||
|
||
if (!device_is_ready(i2c.bus)) { | ||
LOG_ERR("Bus device is not ready"); | ||
return; | ||
} | ||
|
||
ret = i2c_write_read_dt(&i2c, | ||
&iaddr, CONFIG_ETH_SAM_GMAC_MAC_I2C_INT_ADDRESS_SIZE, | ||
mac_addr, 6); | ||
|
||
if (ret != 0) { | ||
LOG_ERR("I2C: failed to read MAC addr"); | ||
return; | ||
} | ||
} | ||
#endif | ||
|
||
static void generate_mac(uint8_t mac_addr[6]) | ||
{ | ||
#if DT_INST_NODE_HAS_PROP(0, mac_eeprom) | ||
get_mac_addr_from_i2c_eeprom(mac_addr); | ||
#elif DT_INST_PROP(0, zephyr_random_mac_address) | ||
gen_random_mac(mac_addr, ATMEL_OUI_B0, ATMEL_OUI_B1, ATMEL_OUI_B2); | ||
#endif | ||
} | ||
|
||
static void phy_link_state_changed(const struct device *pdev, | ||
struct phy_link_state *state, | ||
void *user_data) | ||
|
@@ -1836,7 +1805,11 @@ static void eth0_iface_init(struct net_if *iface) | |
return; | ||
} | ||
|
||
generate_mac(dev_data->mac_addr); | ||
result = net_eth_mac_load(cfg->mcfg, dev_data->mac_addr); | ||
if (result < 0) { | ||
LOG_ERR("Failed to load MAC (%d)", result); | ||
return; | ||
} | ||
|
||
LOG_INF("MAC: %02x:%02x:%02x:%02x:%02x:%02x", | ||
dev_data->mac_addr[0], dev_data->mac_addr[1], | ||
|
@@ -2153,6 +2126,7 @@ static void eth0_irq_config(void) | |
} | ||
|
||
PINCTRL_DT_INST_DEFINE(0); | ||
NET_ETH_MAC_DT_INST_DEFINE(0); | ||
|
||
static const struct eth_sam_dev_cfg eth0_config = { | ||
.regs = (Gmac *)DT_REG_ADDR(DT_INST_PARENT(0)), | ||
|
@@ -2161,7 +2135,8 @@ static const struct eth_sam_dev_cfg eth0_config = { | |
.clock_cfg = SAM_DT_CLOCK_PMC_CFG(0, DT_INST_PARENT(0)), | ||
#endif | ||
.config_func = eth0_irq_config, | ||
.phy_dev = DEVICE_DT_GET(DT_INST_PHANDLE(0, phy_handle)) | ||
.phy_dev = DEVICE_DT_GET(DT_INST_PHANDLE(0, phy_handle)), | ||
.mcfg = NET_ETH_MAC_DT_INST_DEV_CONFIG_GET(0), | ||
}; | ||
|
||
static struct eth_sam_dev_data eth0_data = { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NVMEM, should be selected by the ethernet driver, if it has nvmem-cell is set, similar to the gpios being activated by the ethernet phys, that have a reset or irq gpio. (or at least implied)