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

[Dragan Simic <dsimic@xxxxxxxxxxx>]

Hello Peter,

On 2024-12-11 12:26, Peter Geis wrote:
> On Wed, Dec 11, 2024 at 2:29 AM Dragan Simic <dsimic@xxxxxxxxxxx> 
> 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@xxxxxxxxx>

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@xxxxxxxxxxxxxx/
> > [2] 
> > https://lore.kernel.org/linux-rockchip/20241210013010.81257-7-pgwipeout@xxxxxxxxx/
> >
> > Cc: Peter Geis <pgwipeout@xxxxxxxxx>
> > Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>
> > ---
> >  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";