[PATCH] arm64: dts: rockchip: rk356x: add ethernet aliases

[Dragan Simic]

On 2024-07-02 17:14, Philipp Puschmann wrote:
> Am 02.07.24 um 16:41 schrieb Dragan Simic:
>> On 2024-07-02 16:25, Philipp Puschmann wrote:
>>>> On 2024-07-02 14:46, Philipp Puschmann wrote:
>>>>> Providing ethernet aliases solves a subtle problem for the rk3568. 
>>>>> The
>>>>> bus_id used for the sysfs directory name of the mdio. Without 
>>>>> ethernet
>>>>> alias the bus_id is always 0 and so creating the sysfs directory 
>>>>> for the
>>>>> second mdio fails with a duplicate filename error and by this the 
>>>>> setup
>>>>> of the second ethernet port fails too.
>>>>> 
>>>>> Note: The alias numbering is inverted as gmac1 comes from more 
>>>>> generic
>>>>> rk356x.dtsi but gmac0 comes from specialised rk3568.
>>>> 
>>>> Please see the following commits and the discussions on the 
>>>> rockchip-linux
>>>> mailing list that are linked in them:
>>>> 
>>>> - b0140a1b3b1d ("arm64: dts: rockchip: Add ethernet0 alias to the 
>>>> dts
>>>>   for RK3588(S) boards")
>>>> - 36d9b3ae708e ("arm64: dts: rockchip: Add ethernet0 alias to the 
>>>> dts
>>>>   for RK3566 boards")
>>>> - 5d90cb1edcf7 ("arm64: dts: rockchip: Remove ethernet0 alias from 
>>>> the
>>>>   SoC dtsi for RK3399")
>>>> - c900fef5deff ("arm64: dts: rockchip: Remove ethernet0 alias from 
>>>> the
>>>>   SoC dtsi for RK3368")
>>>>> To sum it up, ethernetX aliases are considered board-level 
>>>>> features,
>>>> because not all boards/devices actually expose the Ethernet 
>>>> interfaces
>>>> built into the SoCs.  Thus, adding ethernetX aliases to the SoC dtsi
>>>> files, unfortunately, isn't an acceptable option.
>>> 
>>> Are ethernet aliases are handled differently than i2c, serial and spi 
>>> aliases?
>>> There are aliases for each of them, without doing any harm. And even 
>>> if the gmac
>>> nodes are disabled with status property?
>> 
>> In a word, yes; they are handled a bit differently, which I already 
>> tried
>> to sum up.  As Krzysztof already noted, please read the discussions 
>> linked
>> in the patches listed above.
>> 
>>> On my rk3568-based custom board i had no ethernet aliases and 
>>> networking was
>>> enabled normally with the status properties of the gmac nodes. Either 
>>> one or
>>> the other or both network devices were initialized. Would be strange 
>>> if an
>>> alias would be used without regard to initializing a device driver.
>> 
>> It's also about following the TRMs and the aliases (or common names) 
>> defined
>> in them, as described in the above-mentioned discussions.
> 
> Ok. I understand the point why the ethernet alias belongs to the board
> dts instead of the SoC dtis.
> But on the quick i found no reference in Documentation/ or in 
> drivers/net or the
> mentioned
> that and why ethernet aliases aren't optional (and it appears in many 
> cases they
> are). From my years of board bring-up my understanding of aliases was, 
> that they
> are in general are optional and for some subsystems they are used to 
> hard-code
> sysfs paths and device names in /dev to solve the problem of randomness 
> in
> initialization order.

Sure, ethernetX aliases are optional.  Basically, if a board exposes 
none of
the Ethernet interfaces built into the SoC, there should be no ethernetX 
aliases
and, of course, no GMACs enabled in the board dts file.

>>>> The sysfs issue that you've discovered should be instead solved in 
>>>> some
>>>> other, more systemic way.
>>> 
>>> The bus_id value comes from stmmac_platform.c and of_alias_get_id() 
>>> with
>>> "ethernet" as parameter is used, what is a common way in the kernel. 
>>> It
>>> delivers unique ints starting with 0. stmmac_mdio then uses the 
>>> bus_id to
>>> create a mdio bus id string stmmac-${bus_id} to register a mdio_bus.
>>> From my understanding this kind of bus id is commonly used to name 
>>> devices
>>> and paths in the sysfs. Viewed only this problem it would be possible
>>> to use other information like the node address or some unique
>>> information to use it as unique part of the mdio bus id. But doesn't 
>>> break
>>> things too, at least some kind of convention?
>>> 
>>> Another hack i tried first, was to use a static increasing int to get
>>> the bus_id. This would keep the resulting sysfs tree probably 
>>> unchanged
>>> but would drop the connection between dts and bus numbering in sysfs.
>> 
>> Wouldn't those issues be solved by simply defining the needed 
>> ethernetX
>> aliases in the dts file for your board?
> 
> Yes. But for me it wasn't obviously that and why i would need that 
> aliases
> to make a mdio work that is not even mentioned in the dts file (in my 
> case).
> So in a perfect would would like to have a solution that comes without 
> some
> hidden magic or the need to dig through the code.

Frankly, I'm not sure that defining aliases is some hidden magic, :) but
I think that I understand your point.  Thus, I'd still suggest that you 
try
to improve the code in the areas you found troublesome, making it more 
robust
in a systemic way.