On Tue, Oct 25, 2016 at 02:54:33PM +0200, Marcin Wojtas wrote:
Hi Leif,
Thank you for review.
- gMarvellTokenSpaceGuid.PcdPp2MacAddressPort0|{ 0x0a, 0x70, 0x40, 0x00, 0x00, 0x1 }
- gMarvellTokenSpaceGuid.PcdPp2MacAddressPort1|{ 0x0a, 0x70, 0x40, 0x00, 0x00, 0x2 }
- gMarvellTokenSpaceGuid.PcdPp2MacAddressPort2|{ 0x0a, 0x70, 0x40, 0x00, 0x00, 0x3 }
Hang on - where do these MAC address ranges come from? I cannot find this range allocated in the OUI. Were they just made up?
I'm really not a fan of hard-wired MAC addresses. While clearly I should have commented on that for 2/3, it didn't jump out at me. Can you make sure to put it behind an explicit build-time define called ENABLE_FALLBACK_MAC_ADDRESSES?
This was explicitly added feature to let MAC address be defined in dedicated PCD's, please see function: Pp2DxeConfigureMacAddress(). I'd prefer it to stay as-is, but if you insist, I'll remove it and set MAC as in the 'default' fallback of the function's switch-case. Please let know.
How are we expecting these devices to function on a network if they all have hardcoded MAC addresses to the same value?
I can go along with a quick and dirty hack for development purposes, which I why I suggested the -D build option. But that still relies on them not being in a range than may in the future get allocated to real devices that will then start seeing issues if located on networks with Armada 70x0 devices on.
But if they can't have device-unique MAC addresses, I would rather see them all set to zero and needing to be configured by the local user.
/ Leif