From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <26154377.eZ6VsJ1LtB@diego> References: <20180122174628.15613-1-ayufan@ayufan.eu> <26154377.eZ6VsJ1LtB@diego> From: =?UTF-8?Q?Kamil_Trzci=C5=84ski?= Date: Wed, 24 Jan 2018 16:34:58 +0100 Message-ID: Subject: Re: [PATCH] rock64: dts: fix gmac2io stability issues Content-Type: multipart/alternative; boundary="f4f5e8075f34f435a6056387676a" To: =?UTF-8?Q?Heiko_St=C3=BCbner?= Cc: Rob Herring , Mark Rutland , Catalin Marinas , Will Deacon , David Wu , "David S. Miller" , Joseph Chen , Liang Chen , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org List-ID: --f4f5e8075f34f435a6056387676a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks. This is my first ever patch send to Kernel :) Kamil On Tue, Jan 23, 2018 at 1:52 PM, Heiko St=C3=BCbner wrote= : > Hi Kamil, > > Thanks for figuring out the source of the gmac instability :-) . > > I've applied the patch as fix for 4.16 but corrected some small issues > while > doing so: > - The subject should be something like > "arm64: dts: rockchip: fix rock64 gmac2io stability issues" > The prefixes to use vary per subsystem, so it's best to use git log to > look at previous commits > > Am Montag, 22. Januar 2018, 18:46:22 CET schrieb Kamil Trzci=C5=84ski: > > This commit enables thresh dma mode as this forces to disable > checksuming, > > and chooses delay values which make the interface stable. > > > > These changes are needed, because ROCK64 is faced with two problems: > > 1. tx checksuming does not work with packets larger than 1498, > > 2. the default delays for tx/rx are not stable when using 1Gbps > connection. > > > > Delays were found out with: > > https://github.com/ayufan-rock64/linux-build/tree/ > master/recipes/gmac-delays > > -test > > > > Change-Id: Ie894df4b52122988da683c02e3a05d635a5c7b84 > > That is some gerrit thingy which should not be part when sending patches > upstream. > > > Signed-off-by: Kamil Trzci=C5=84ski > > --- > > arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts index > > 5602ec01fed9..bcc0bb35d840 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > @@ -132,8 +132,6 @@ > > assigned-clocks =3D <&cru SCLK_MAC2IO>, <&cru SCLK_MAC2IO_EXT>; > > assigned-clock-parents =3D <&gmac_clkin>, <&gmac_clkin>; > > clock_in_out =3D "input"; > > - /* shows instability at 1GBit right now */ > > - max-speed =3D <100>; > > phy-supply =3D <&vcc_io>; > > phy-mode =3D "rgmii"; > > pinctrl-names =3D "default"; > > @@ -141,8 +139,9 @@ > > snps,reset-gpio =3D <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>; > > snps,reset-active-low; > > snps,reset-delays-us =3D <0 10000 50000>; > > - tx_delay =3D <0x26>; > > - rx_delay =3D <0x11>; > > + snps,force_thresh_dma_mode; > > With the exception of compatible, reg, interrupts and status, I try to ke= ep > other properties alphabetically sorted in Rockchip dts, so I've moved the > snps,force_thres_dma_mode up between pinctrl-0 and snps-reset-gpio. > > > Heiko > --f4f5e8075f34f435a6056387676a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks. This is my first ever patch send to Kernel :)
=
Kamil

<= br>
On Tue, Jan 23, 2018 at 1:52 PM, Heiko St=C3= =BCbner <heiko@sntech.de> wrote:
Hi Kamil,

Thanks for figuring out the source of the gmac instability :-) .

I've applied the patch as fix for 4.16 but corrected some small issues = while
doing so:
- The subject should be something like
=C2=A0 =C2=A0 =C2=A0 =C2=A0 "arm64: dts: rockchip: fix rock64 gmac2io = stability issues"
=C2=A0 The prefixes to use vary per subsystem, so it's best to use git = log to
=C2=A0 look at previous commits

Am Montag, 22. Januar 2018, 18:46:22 CET schrieb Kamil Trzci=C5=84ski:
> This commit enables thresh dma mode as this forces to disable checksum= ing,
> and chooses delay values which make the interface stable.
>
> These changes are needed, because ROCK64 is faced with two problems: > 1. tx checksuming does not work with packets larger than 1498,
> 2. the default delays for tx/rx are not stable when using 1Gbps connec= tion.
>
> Delays were found out with:
> https://github.com/= ayufan-rock64/linux-build/tree/master/recipes/gmac-delays
> -test
>
> Change-Id: Ie894df4b52122988da683c02e3a05d635a5c7b84

That is some gerrit thingy which should not be part when sending pat= ches
upstream.

> Signed-off-by: Kamil Trzci=C5=84ski <ayufan@ayufan.eu>
> ---
>=C2=A0 arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 7 +++----<= br> >=C2=A0 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts index
> 5602ec01fed9..bcc0bb35d840 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
> @@ -132,8 +132,6 @@
>=C2=A0 =C2=A0 =C2=A0 =C2=A0assigned-clocks =3D <&cru SCLK_MAC2IO= >, <&cru SCLK_MAC2IO_EXT>;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0assigned-clock-parents =3D <&gmac_clk= in>, <&gmac_clkin>;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0clock_in_out =3D "input";
> -=C2=A0 =C2=A0 =C2=A0/* shows instability at 1GBit right now */
> -=C2=A0 =C2=A0 =C2=A0max-speed =3D <100>;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0phy-supply =3D <&vcc_io>;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0phy-mode =3D "rgmii";
>=C2=A0 =C2=A0 =C2=A0 =C2=A0pinctrl-names =3D "default";
> @@ -141,8 +139,9 @@
>=C2=A0 =C2=A0 =C2=A0 =C2=A0snps,reset-gpio =3D <&gpio1 RK_PC2 GP= IO_ACTIVE_LOW>;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0snps,reset-active-low;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0snps,reset-delays-us =3D <0 10000 50000&g= t;;
> -=C2=A0 =C2=A0 =C2=A0tx_delay =3D <0x26>;
> -=C2=A0 =C2=A0 =C2=A0rx_delay =3D <0x11>;
> +=C2=A0 =C2=A0 =C2=A0snps,force_thresh_dma_mode;

With the exception of compatible, reg, interrupts and status, I= try to keep
other properties alphabetically sorted in Rockchip dts, so I've moved t= he
snps,force_thres_dma_mode up between pinctrl-0 and snps-reset-gpio.


Heiko

--f4f5e8075f34f435a6056387676a--