Re: [PATCH net 0/2] allwinner: a523: Rename emac0 to gmac0

On Tue, Jul 8, 2025 at 1:15 AM Andre Przywara [off-list ref] wrote:

On Sun, 6 Jul 2025 21:14:09 +0800
Chen-Yu Tsai [off-list ref] wrote:

Hi,

quoted

On Sun, Jul 6, 2025 at 7:23 AM Andre Przywara [off-list ref] wrote:

quoted

On Sat, 5 Jul 2025 17:53:17 +0200
Andrew Lunn [off-list ref] wrote:

Hi Andrew,

quoted

quoted

So it's really whatever Allwinner wants to call it. I would rather have
the names follow the datasheet than us making some scheme up.

Are the datasheets publicly available?

We collect them in the sunxi wiki (see the links below), but just to
make sure:
I am not disputing that GMAC is the name mentioned in the A523 manual,
and would have probably been the right name to use originally - even
though it's not very consistent, as the same IP is called EMAC in the
older SoCs' manuals. I am also not against renaming identifiers or even
(internal) DT labels. But the problem here is that the renaming affects
the DT compatible string and the pinctrl function name, both of which
are used as an interface between the devicetree and its users, which is
not only the Linux kernel, but also U-Boot and other OSes like the BSDs.

I reiterate my position: they are not stable until they actually hit a
release. This provides some time to fix mistakes before they are set in
stone.

quoted

In this particular case we would probably get away with it, because
it's indeed very early in the development cycle for this SoC, but for
instance the "emac0" function name is already used in some U-Boot
patch series on the list:
https://lore.kernel.org/linux-sunxi/20250323113544.7933-18-andre.przywara@arm.com/ (local)

If we REALLY need to rename this, it wouldn't be the end of the world,
but would create some churn on the U-Boot side.

I just wanted to point out that any changes to the DT bindings have
some impact to other projects, even if they are proposed as a coherent
series on the Linux side. Hence my question if this is really necessary.

For the compatible string, I can live with having a comment in the binding
stating the name used in the datasheet for reference.

For the compatible string going with a fallback name, I can live with
renaming it ;-)

quoted

For the pinctrl stuff, which is the contentious bit here, I thought the
whole idea of the newer pinctrl bindings is that the driver uses
"allwinner,pinmux" instead of "function". I think having both being valid
is confusing, and likely to cause conflicts later on. If we're going to
use the hardware register values in the device tree, I'd really like them
to be the only source of truth. The commit message for the binding also
sort of suggests that "allwinner,pinmux" is the part that matters.

Yes, it should be, but it turns out to be convenient for U-Boot to
continue using the old method - until someone writes a driver for the new
schema. And a function name is still mandatory, it's just mostly for
reference now (so would indeed be fine to rename).

Hmm. Could we somehow make it clear in the binding that the function name
is for reference only?

So if you really insist on this: please go ahead and merge it, so that
the 6.16 release contains the new name.

I'm afraid I insist.

I think we still need an ack from the DT maintainers though, and then
the binding change would go through the net tree?

But please note my silent protest about those cosmetic name changes in the
DT realm, which establishes an interface reaching beyond just the Linux
kernel. When it comes to new boards, the U-Boot DT sync process is rather
slow, so I really am eager to cherry-pick -rc1 DTs already, instead of
waiting another 7 weeks - which bites me in this case. It's not merged
yet, so is fine in this case, but I would really like to avoid similar
hiccups in the future.

Noted. I will try to be more vigilant during the review process.


Thanks
ChenYu