Re: [PATCH] arm64: dts: rockchip: Delete redundant RK3328 GMAC stability fixes

[Dragan Simic <dsimic@manjaro.org>]

Hello Peter,

On 2024-12-11 12:26, Peter Geis wrote:
> On Wed, Dec 11, 2024 at 2:29 AM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> Since the commit 8a469ee35606 ("arm64: dts: rockchip: Add txpbl node 
>> for
>> RK3399/RK3328"), having "snps,txpbl" properties defined as Ethernet 
>> stability
>> fixes in RK3328-based board dts(i) files is redundant, because that 
>> commit
>> added the required fix to the RK3328 SoC dtsi, so let's delete them.
>> 
>> It has been determined that the Ethernet stability fixes no longer 
>> require
>> the "snps,rxpbl" and "snps,aal" properties, [1] out of which the 
>> latter also
>> induces performance penalties, so let's delete these properties from 
>> the
>> relevant RK3328-based board dts(i) files.
> 
> You may want to include snps,force_thresh_dma_mode in this group such
> as on the rock64. It's even more limiting than snps,aal.
> 
> Otherwise,
> Acked-by: Peter Geis <pgwipeout@gmail.com>

Thanks for your suggestion and review!

I'll add "snps,force_thresh_dma_mode" to the set of redundant DT
properties deleted from RK3328-based board dts(i) files in the v2
of this patch.

>> This commit completes the removal of these redundant "snps,*" 
>> properties
>> that was started by a patch from Peter Geis. [2]
>> 
>> [1] 
>> https://lore.kernel.org/linux-rockchip/CAMdYzYpj3d7Rq0O0QjV4r6HEf_e07R0QAhPT2NheZdQV3TnQ6g@mail.gmail.com/
>> [2] 
>> https://lore.kernel.org/linux-rockchip/20241210013010.81257-7-pgwipeout@gmail.com/
>> 
>> Cc: Peter Geis <pgwipeout@gmail.com>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3328-a1.dts                | 1 -
>>  arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2.dtsi        | 1 -
>>  arch/arm64/boot/dts/rockchip/rk3328-orangepi-r1-plus.dtsi | 1 -
>>  arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts         | 3 ---
>>  4 files changed, 6 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-a1.dts 
>> b/arch/arm64/boot/dts/rockchip/rk3328-a1.dts
>> index 824183e515da..24baaa7f1d8c 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3328-a1.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3328-a1.dts
>> @@ -110,7 +110,6 @@ &gmac2io {
>>         phy-supply = <&vcc_io>;
>>         pinctrl-names = "default";
>>         pinctrl-0 = <&rgmiim1_pins>;
>> -       snps,aal;
>>         snps,pbl = <0x4>;
>>         tx_delay = <0x26>;
>>         rx_delay = <0x11>;
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2.dtsi
>> index f9fab35aed23..d5f129e304e5 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2.dtsi
>> @@ -142,7 +142,6 @@ &gmac2io {
>>         phy-supply = <&vcc_io_33>;
>>         pinctrl-0 = <&rgmiim1_pins>;
>>         pinctrl-names = "default";
>> -       snps,aal;
>> 
>>         mdio {
>>                 compatible = "snps,dwmac-mdio";
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-orangepi-r1-plus.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3328-orangepi-r1-plus.dtsi
>> index 181ec6de0019..9ec93f61433e 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3328-orangepi-r1-plus.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3328-orangepi-r1-plus.dtsi
>> @@ -113,7 +113,6 @@ &gmac2io {
>>         phy-supply = <&vcc_io>;
>>         pinctrl-0 = <&rgmiim1_pins>;
>>         pinctrl-names = "default";
>> -       snps,aal;
>> 
>>         mdio {
>>                 compatible = "snps,dwmac-mdio";
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts 
>> b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
>> index 3e08e2fd0a78..59dead1cc503 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
>> @@ -153,9 +153,6 @@ &gmac2io {
>>         phy-supply = <&vcc_io>;
>>         pinctrl-names = "default";
>>         pinctrl-0 = <&rgmiim1_pins>;
>> -       snps,aal;
>> -       snps,rxpbl = <0x4>;
>> -       snps,txpbl = <0x4>;
>>         tx_delay = <0x26>;
>>         rx_delay = <0x11>;
>>         status = "okay";