[PATCH v1] net: eth-uclass: Change uclass driver name to ethernet

[Tom Rini]

On Thu, Feb 25, 2021 at 08:57:37AM +0100, Michael Walle wrote:
> Hi Simon,
> 
> Am 2021-02-25 03:31, schrieb Simon Glass:
> > On Wed, 24 Feb 2021 at 17:51, Michael Walle <michael at 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/sxgbe/sxgbe_platform.c#L43
> > > > https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/broadcom/genet/bcmmii.c#L441
> > > > https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#L436
> > > >
> > > >> > 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-michael@walle.cc/
> > > 
> > > Thats actually why I noticed the inconsistency and found this patch.
> > 
> > If we have the choice, 'eth' is shorter  = better.
> 
> This is how it looks in the linux tree:
> 
> $ grep "ethernet[0-9].*=.*&" arch/**/*dts{,i}|wc -l
> 633
> $ grep "eth[0-9].*=.*&" arch/**/*dts{,i}|wc -l
> 0
> 
> This is in the u-boot tree:
> $ grep "ethernet[0-9].*=.*&" arch/**/*dts{,i}|wc -l
> 261
> $ grep "eth[0-9].*=.*&" arch/**/*dts{,i}|wc -l
> 13

So "ethernet" it is.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210225/c3747b26/attachment.sig>