On 23-08-08, Ahmad Fatoum wrote: > Hello Marco, > > On 08.08.23 11:46, Marco Felsch wrote: > > Hi Ahmad, > > > > On 23-08-08, Ahmad Fatoum wrote: > >> Hello Marco, > >> > >> On 07.08.23 19:07, Marco Felsch wrote: > >>> Some vendors like Polyhex store the MAC address ASCII encoded instead of > >>> using the plain 6-byte MAC address. This commit adds the support to > >>> decode the 12-byte ASCII encoded MAC addresses. > >>> > >>> Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx> > >> > >> FYI, the upstream device tree binding for this is NVMEM layout, which was only > >> recently added to Linux and for which barebox has no support yet. > > > > I know that, thanks for the info :) I thought that this is no "layout" > > it's just the mac-address stored in ASCII instead of plain 6-byte > > storage. > > Sequential big-endian 6 bytes is the normal format. Anything else (ASCII > with nothing between it, ASCII with :, ASCII with -) is a different layout IMO. You're right. > >> I can understand that porting NVMEM layouts, just to get a MAC address assigned > >> might not be an attractive proposition, but I don't think that adding a new > >> barebox-specific binding is the right way here. > > > > Me neither therefore I dropped the barebox specific binding and did just > > do some heuristic. > > It's a binding, whether you use a boolean property, the size in the reg field > or add a nvmem-layout subnode. To be clear, the binding under "Documentation/devicetree/bindings/net/ethernet-controller.yaml" just says that the nvmem-cells is <1> and the nvmem-cells-names must be set to "mac-address". The binding don't say how much space the mac-address storage occupy. > >> I'd suggest, you get the nvmem cell in board code and assign it there. > >> There's readily available API for that. If you are interested in a > >> generic solution, NVMEM layouts are the way to go IMO. > > > > Thought about that too but went this way because it's much less code > > than doing it in the board code. Also it allows to share the code with > > others. > > How widespread is it to store MAC address that way? If it's just Debix doing > it this way, you are effectively adding a binding that's only useful to Debix > into common code. Debix is the first I'm aware of. I know that this is common code. As said above the binding don't stop us from having a larger nvmem-cell size. > > As said, I don't think that this is a layout. Of course there are more > > ASCII strings to store the production test result but this is not > > relevant. I really need to check which is more effort > > board-code vs. layout-support if you think that this is layout. > > I'd be more amenable to this patch if there exists no way in upstream bindings > to represent this, which was for a long time the case. That's not the case any > more, so we should not add any new barebox-specific bindings for MAC addresses > that duplicate what's achievable by the upstream binding. > > For an example of how to do this in board code, see rdu_eth_register_ethaddr(). I will move it to the board code, thanks for the link. Regards, Marco
