drivers: ethernet: Use NVMEM cell to get MAC address #96598
Conversation
pdgendt
requested review from
henrikbrixandersen,
jukkar,
maass-hamburg,
nandojve and
talih0
pdgendt
force-pushed
the
nvmem-mac-address
branch
from
1a5832d to
bd8b9b6
Compare
pdgendt
force-pushed
the
nvmem-mac-address
branch
2 times, most recently
from
9afd50b to
8241eb8
Compare
pdgendt
changed the title
pdgendt
force-pushed
the
nvmem-mac-address
branch
from
8241eb8 to
8ac0a2f
Compare
| /* 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; | ||
| } |
There was a problem hiding this comment.
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.
Can you explain a bit what you mean here? I don't fully understand.
This static MAC address is when you have the following:
eth-driver {
local-mac-address = [02 02 03 04 05 06];
};
This should only ever be used during testing. An alternative is have the OUI in the static part:
eth-driver {
local-mac-address = [00 04 25];
zephyr,random-mac-address;
};
/* or */
eth-driver {
local-mac-address = [00 04 25];
nvmem-cells = <&mac_address>;
nvmem-cell-names = "mac-address";
};
Maybe the naming should be changed in that case to something like mac-address-prefix?
There was a problem hiding this comment.
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.
Yes, I'd refactor those to use the struct net_eth_mac_config instead.
drivers/ethernet/eth_sam_gmac.c
Outdated
|
|
||
| #include <zephyr/kernel.h> | ||
| #include <zephyr/device.h> | ||
| #include <zephyr/nvmem.h> |
There was a problem hiding this comment.
should be added to eth.h instead
| config NVMEM | ||
| default y |
There was a problem hiding this comment.
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)
pdgendt
force-pushed
the
nvmem-mac-address
branch
2 times, most recently
from
9e377d9 to
da6b6b0
Compare
|
I think there are some other users that may be interested in the fuse framework. It's probably applicable to your #98140 P-R as well. |
include/zephyr/net/ethernet.h
Outdated
| * @param node_id Node identifier. | ||
| */ | ||
| #define Z_NET_ETH_MAC_DEV_CONFIG_NAME(node_id) \ | ||
| _CONCAT(__net_eth_mac_dev_config, DEVICE_DT_NAME_GET(node_id)) |
There was a problem hiding this comment.
Maybe a _ between the prefix and the device name?
| select NOCACHE_MEMORY if ARCH_HAS_NOCACHE_MEMORY_SUPPORT | ||
| select ETH_DSA_SUPPORT_DEPRECATED | ||
| select PINCTRL | ||
| imply NVMEM if ($(dt_compat_any_has_prop,$(DT_COMPAT_ATMEL_SAM_GMAC),nvmem-cells) || \ |
There was a problem hiding this comment.
I prefer imply as it isn't a hard requirement, and could be turned off, for example for testing purposes.
pdgendt
force-pushed
the
nvmem-mac-address
branch
2 times, most recently
from
1793285 to
44acb16
Compare
pdgendt
changed the title
zephyrbot
added
area: Samples
zephyrbot
requested review from
ArunMCHP,
ClaCodes,
decsny,
lmajewski,
mbolivar,
rlubos,
rruuaanng and
stephanosio
etienne-lms
left a comment
etienne-lms
left a comment
There was a problem hiding this comment.
LGTM.
Could you fill the commit message body of commit
"include: zephyr: net: ethernet.h: Add MAC address configuration"?
I don't understand the CI build test failures.
Add MAC address configuration struct and macros. Signed-off-by: Pieter De Gendt <[email protected]>
Given an ethernet MAC configuration struct, load a MAC address into a provided array. Signed-off-by: Pieter De Gendt <[email protected]>
Turn ethernet controller devicetree nodes into NVMEM consumers. Allow passing mac-address cells. Signed-off-by: Pieter De Gendt <[email protected]>
Rework the Atmel SAM GMAC driver to read a MAC address from the provided NVMEM cell instead of using I2C commands directly. Signed-off-by: Pieter De Gendt <[email protected]>
pdgendt
force-pushed
the
nvmem-mac-address
branch
from
44acb16 to
468c869
Compare
|
5 participants
A proof-of-concept using PR #96379 asan alternative for #74835.Note that this loads the MAC address during driver initialization. A more complete (but complex) solution would be to move it to the network interface initialization.