Re: [PATCH] arm64: dts: rockchip: Add txpbl node for RK3399/RK3328

From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Carlos de Paula <me@carlosedp.com>
Cc: papadakospan@gmail.com, jose.abreu@synopsys.com,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Robin Murphy <robin.murphy@arm.com>,
	Jonas Karlman <jonas@kwiboo.se>, Peter Geis <pgwipeout@gmail.com>,
	Katsuhiro Suzuki <katsuhiro@katsuster.net>,
	Johan Jonker <jbx6244@gmail.com>,
	Philipp Tomsich <philipp.tomsich@theobroma-systems.com>,
	Christoph Muellner <christoph.muellner@theobroma-systems.com>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: dts: rockchip: Add txpbl node for RK3399/RK3328
Date: Sun, 1 Mar 2020 10:02:57 +0100	[thread overview]
Message-ID: <92930c13-3538-32a7-b016-4150bcbc0e56@linaro.org> (raw)
In-Reply-To: <20200218221040.10955-1-me@carlosedp.com>

On 18/02/2020 23:10, Carlos de Paula wrote:
> Some rockchip SoCs like the RK3399 and RK3328 exhibit an issue
> where tx checksumming does not work with packets larger than 1498.
> 
> The default Programmable Buffer Length for TX in these GMAC's is
> not suitable for MTUs higher than 1498. The workaround is to disable
> TX offloading with 'ethtool -K eth0 tx off rx off' causing performance
> impacts as it disables hardware checksumming.
> 
> This patch sets snps,txpbl to 0x4 which is a safe number tested ok for
> the most popular MTU value of 1500.

What about using something like:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt#n16

or

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-controller.yaml#n37

?

> For reference, see https://lkml.org/lkml/2019/4/1/1382.
> 
> Signed-off-by: Carlos de Paula <me@carlosedp.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 2 ++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> index 1f53ead52c7f..b7f1de4b7fd0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> @@ -906,6 +906,7 @@
>  		resets = <&cru SRST_GMAC2IO_A>;
>  		reset-names = "stmmaceth";
>  		rockchip,grf = <&grf>;
> +		snps,txpbl = <0x4>;
>  		status = "disabled";
>  	};
>  
> @@ -913,6 +914,7 @@
>  		compatible = "rockchip,rk3328-gmac";
>  		reg = <0x0 0xff550000 0x0 0x10000>;
>  		rockchip,grf = <&grf>;
> +		snps,txpbl = <0x4>;
>  		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
>  		interrupt-names = "macirq";
>  		clocks = <&cru SCLK_MAC2PHY_SRC>, <&cru SCLK_MAC2PHY_RXTX>,
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 33cc21fcf4c1..cd5415d7e559 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -288,6 +288,7 @@
>  		resets = <&cru SRST_A_GMAC>;
>  		reset-names = "stmmaceth";
>  		rockchip,grf = <&grf>;
> +		snps,txpbl = <0x4>;
>  		status = "disabled";
>  	};
>  
> 

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog