Re: [PATCH 2/2] arm64: dts: renesas: Drop ethernet-phy-ieee802.3-c22 from PHY compatible string on all RZ boards

[Marek Vasut <marex@denx.de>]

On 7/2/24 10:38 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hi,

> On Sun, Jun 30, 2024 at 5:47 AM Marek Vasut <marex@denx.de> wrote:
>> The rtl82xx DT bindings do not require ethernet-phy-ieee802.3-c22
>> as the fallback compatible string. There are fewer users of the
>> Realtek PHY compatible string with fallback compatible string than
>> there are users without fallback compatible string, so drop the
>> fallback compatible string from the few remaining users:
>>
>> $ git grep -ho ethernet-phy-id001c....... | sort | uniq -c
>>        1 ethernet-phy-id001c.c816",
>>        2 ethernet-phy-id001c.c915",
>>        2 ethernet-phy-id001c.c915";
>>        5 ethernet-phy-id001c.c916",
>>       13 ethernet-phy-id001c.c916";
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202406290316.YvZdvLxu-lkp@intel.com/
>> Signed-off-by: Marek Vasut <marex@denx.de>
> 
> Thanks for your patch!
> 
>> Note: this closes only part of the report
> 
> In that case you should use a Link: instead of a Closes: tag?

But which patch would be the one that Closes that report then ?

>> --- a/arch/arm64/boot/dts/renesas/cat875.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/cat875.dtsi
>> @@ -22,8 +22,7 @@ &avb {
>>          status = "okay";
>>
>>          phy0: ethernet-phy@0 {
>> -               compatible = "ethernet-phy-id001c.c915",
>> -                            "ethernet-phy-ieee802.3-c22";
>> +               compatible = "ethernet-phy-id001c.c915";
>>                  reg = <0>;
>>                  interrupt-parent = <&gpio2>;
>>                  interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> 
> What about moving the PHYs inside an mdio subnode, and removing the
> compatible properties instead? That would protect against different
> board revisions using different PHYs or PHY revisions.
> 
> According to Niklas[1], using an mdio subnode cancels the original
> reason (failure to identify the PHY in reset state after unbind/rebind
> or kexec) for adding the compatible values[2].

My understanding is that the compatible string is necessary if the PHY 
needs clock/reset sequencing of any kind. Without the compatible string, 
it is not possible to select the correct PHY driver which would handle 
that sequencing according to the PHY requirements. This board here does 
use reset-gpio property in the PHY node (it is not visible in this diff 
context), so I believe a compatible string should be present here.

What would happen if this board got a revision with another PHY with 
different PHY reset sequencing requirements ? The MDIO node level reset 
handling might no longer be viable.