Hi Michael,
On Wed, 24 Feb 2021 at 17:51, Michael Walle michael@walle.cc wrote:
Am 2021-02-24 22:34, schrieb Vladimir Oltean:
On Wed, Feb 24, 2021 at 08:26:14PM +0100, Michael Walle wrote:
Wasn't the intention of David's patch, in fact, to have the new uclass name also match on "eth" aliases? If I'm correct, doesn't this mean we'll have to replace the strcmp with an actual stem check?
I guess it was intended the other way around, to rename the "ethN" aliases to "ethernetN". The latter are used way more in u-boot's device trees and linux' device trees just use ethernetN aliases, though I'm not sure where they are used (can't find any of_alias_get_id("ethernet") in linux). So this makes sense, no?
Bad phrasing, I meant to ask if it was intended to _still_ match on "eth" aliases.
By the way, Linux drivers use a combination of "ethernet" and "eth": https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/samsung/s... https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/broadcom/... https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/s...
I would not, under any circumstance, break compatibility with "eth" alias names.
Assuming that ethN is not used expect in u-boot device trees, how would we break backwards compatibility? Also it seems that the only users of the ethN aliases are the sandbox and u-boot's DSA.
I took another look, I was under the impression that fdtdec_set_ethernet_mac_address searches by "eth" in the /aliases node of the linux device tree, but it searches by "ethernet". Fair then.
fdtdec_set_ethernet_mac_address() seems to be "only" used by tegra?
Tough luck in that case for those who got it right, ugh I mean wrong, by actually looking at the code to see what it searches for?
So yeah, this patch is waiting for someone to update the existing aliases, and you seem to be a valid candidate. Although it's low impact, and it's all in-tree so it isn't breaking any interface, it is up to you if you value consistency more than correctness. I would probably fix just the device tree you care about to use "eth" instead of "ethernet", that seems the most localized solution to a concrete problem, then you could convert the uclass name to "ethernet" and therefore fix all the other users at once (and at the same time update the kontron boards). The reason why I would leave that at the end is because when you 'fix' stuff that isn't broken, nice surprises are always bound to happen, and I don't think you want your board to still remain broken at the end of some bug-canceling-another-bug-on-board-you-don't-have debugging headaches.
Fair enough. And I might fix em, the only user is the kontron board with the DSA support which wasn't picked up yet. But then yeah, I can't stand it that one sl28 variant has ethernetN aliases and the other as ethN aliases *g*
Btw another user might be https://patchwork.ozlabs.org/project/uboot/patch/20210224163044.17017-1-mich...
Thats actually why I noticed the inconsistency and found this patch.
If we have the choice, 'eth' is shorter = better.
Regards, Simon