Re: [linux-sunxi] [PATCH 02/10] ARM: dts: sun6i: a31-hummingbird: Enable RGMII RX/TX delay on Ethernet PHY

[Icenowy Zheng]

于 2020年10月25日 GMT+08:00 上午2:30:35, "Jernej Škrabec" <jernej.skrabec@xxxxxxxx> 写到:
>Dne sobota, 24. oktober 2020 ob 19:51:06 CEST je Icenowy Zheng
>napisal(a):
>> 在 2020-10-25星期日的 00:25 +0800，Chen-Yu Tsai写道：
>> 
>> > From: Chen-Yu Tsai <wens@xxxxxxxx>
>> > 
>> > The Ethernet PHY on the A31 Hummingbird has the RX and TX delays
>> > enabled on the PHY, using pull-ups on the RXDLY and TXDLY pins.
>> > 
>> > Fix the phy-mode description to correct reflect this so that the
>> > implementation doesn't reconfigure the delays incorrectly. This
>> > happened with commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e
>> > rx/tx delay config").
>> 
>> Personally I think they should revert this commit, and consider other
>> solution.
>> 
>> This commit breaks everything.
>> 
>
>Previously broken driver allowed inproper DT to work, so you have to
>fix 
>everything eventually.

There is no "improper DT" if it's already shipped, DT should suffer driver
change, not changed to match driver.

I think at least Fedora tends to ship a DT with a system image that will
not get updated when kernel gets updated.

>
>Plus side, there is no need to have hack for Pine64 Plus ethernet
>anymore.
>
>Best regards,
>Jernej
>
>> (Although the patch on individual DT patches are technically correct)
>> 
>> > Fixes: c220aec2bb79 ("ARM: dts: sun6i: Add Merrii A31 Hummingbird
>> > support")
>> > Signed-off-by: Chen-Yu Tsai <wens@xxxxxxxx>
>> > ---
>> > 
>> >  arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> > b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> > index 049e6ab3cf56..73de34ae37fd 100644
>> > --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> > +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> > @@ -154,7 +154,7 @@ &gmac {
>> > 
>> >  	pinctrl-names = "default";
>> >  	pinctrl-0 = <&gmac_rgmii_pins>;
>> >  	phy-handle = <&phy1>;
>> > 
>> > -	phy-mode = "rgmii";
>> > +	phy-mode = "rgmii-id";
>> > 
>> >  	status = "okay";
>> >  
>> >  };