On Tue, Sep 08, 2020 at 10:14:15AM +0200, Andre Heider wrote:
On 08/09/2020 09:42, Pali Rohár wrote:
On Tuesday 08 September 2020 08:35:00 Andre Heider wrote:
The hardware does not provide a MAC address. Enable this so that network access works with just the default environment.
Well, this is not fully truth as MAC address is stored in SPI, just in non-standard format, in U-Boot env stored in env partition and it is hard to use outside of U-Boot, plus easy to erase / overwrite / lost.
True, but updating the bootloader usually implies wiping the env, so it's very easy to lose it.
It most certainly should NOT wipe out the existing environment, it should be using the same environment location as before.
I'm not a big fan of this change. This looks like a workaround / hack for boards where MAC address was erased (e.g. by broken U-Boot distro scripts) or for early boards where MAC address was not written at all (as I was told).
And on these boards this patch would cause that U-Boot would see on every boot different MAC address. This would cause another mess in network for U-Boot netboot as DHCP/TFTP server would see for one board every time different MAC address.
Is not really better to instruct user how to fix board where e.g. broken distro scripts erased MAC address? We have already paragraph in README.marvell about it.
Also this change affects "default" defconfig value. And based on above arguments I do not think that this change should be enabled by default.
I understand that for some situations it may be useful (e.g. mass board reparation process via netboot), but as this is config option, users in such situation can enable this option manually.
I think that for default behavior is not provide network access in U-Boot if for some reasons factory permanent MAC address was removed. User can easier and faster detect this issue and fix it.
It can be argued both ways I guess. If this option wouldn't make sense it wouldn't exist.
Out of the box working network access is probably what most users care about.
Changing this default means that you need to build the whole firmware yourself, and let's face it: building it for this board is a total clusterfuck. Most users will just download a binary. I don't care too much for this patch, but I would consider what fits most users.
Right now most users will probably run the downstream binaries provided by armbian, and as you know, that even has single hardcoded MAC, used for all boards. So this would already be an improvement ;)
Note that when CONFIG_NET_RANDOM_ETHADDR is set, we only use a random MAC address when we haven't found one either on the hardware or environment. It also prints a warning that you are using a random MAC, so if it's documented on how to recover the real MAC a user should see that warning and fix it.