diff mbox series

Fix tx-queue-size on NBG6817 (allows MTU changes).

Message ID 20200915195655.13866-1-jacekowski@jacekowski.org
State Changes Requested
Delegated to: Petr Štetiar
Headers show
Series Fix tx-queue-size on NBG6817 (allows MTU changes). | expand

Commit Message

Jacek Milewicz Sept. 15, 2020, 7:56 p.m. UTC
From: Jacek Milewicz <jacekowski@jacekowski.org>

Fixes FS#3285 for NBG6817 only.

Signed-off-by: Jacek Milewicz <jacekowski@jacekowski.org>
---
 .../files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts       | 2 ++
 1 file changed, 2 insertions(+)

Comments

Paul Oranje Sept. 19, 2020, 2:06 p.m. UTC | #1
See below, regards,
Paul


> Op 15 sep. 2020, om 21:56 heeft jacekowski@jacekowski.org het volgende geschreven:
The subject does not to comply with the rules for that, it should at least start with the target, better would be:
	ipq806x: fix tx-queue-size on NBG6817

> 
> From: Jacek Milewicz <jacekowski@jacekowski.org>
> 
> Fixes FS#3285 for NBG6817 only.
Why is that ?

> 
> Signed-off-by: Jacek Milewicz <jacekowski@jacekowski.org>
> ---
> .../files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts       | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts b/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
> index 7cd1c7b567..5745040d2b 100644
> --- a/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
> +++ b/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
> @@ -289,6 +289,7 @@
> 			qcom,emulation = <0>;
> 			qcom,irq = <255>;
> 			mdiobus = <&mdio0>;
> +			tx-fifo-depth = <4096>;
> 
> 			pinctrl-0 = <&rgmii2_pins>;
> 			pinctrl-names = "default";
> @@ -310,6 +311,7 @@
> 			qcom,emulation = <0>;
> 			qcom,irq = <258>;
> 			mdiobus = <&mdio0>;
> +			tx-fifo-depth = <4096>;
> 
> 			fixed-link {
> 				speed = <1000>;
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Jacek Milewicz Sept. 21, 2020, 10:57 p.m. UTC | #2
On 19/09/2020 15:06, Paul Oranje wrote:
> See below, regards,
> Paul
>
>
>> Op 15 sep. 2020, om 21:56 heeft jacekowski@jacekowski.org het volgende geschreven:
> The subject does not to comply with the rules for that, it should at least start with the target, better would be:
> 	ipq806x: fix tx-queue-size on NBG6817

I was not aware of that.

>> From: Jacek Milewicz <jacekowski@jacekowski.org>
>>
>> Fixes FS#3285 for NBG6817 only.
> Why is that ?

Because on IPQ806x SoC it is impossible to detect correct value, 
therefore it has to be set for each device in device tree (which is what 
this patch is doing).

On most IPQ806x devices correct value has to be determined manually by 
trying few most common values and seeing what works (4k seems to be most 
common).

>
>> Signed-off-by: Jacek Milewicz <jacekowski@jacekowski.org>
>> ---
>> .../files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts       | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts b/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
>> index 7cd1c7b567..5745040d2b 100644
>> --- a/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
>> +++ b/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
>> @@ -289,6 +289,7 @@
>> 			qcom,emulation = <0>;
>> 			qcom,irq = <255>;
>> 			mdiobus = <&mdio0>;
>> +			tx-fifo-depth = <4096>;
>>
>> 			pinctrl-0 = <&rgmii2_pins>;
>> 			pinctrl-names = "default";
>> @@ -310,6 +311,7 @@
>> 			qcom,emulation = <0>;
>> 			qcom,irq = <258>;
>> 			mdiobus = <&mdio0>;
>> +			tx-fifo-depth = <4096>;
>>
>> 			fixed-link {
>> 				speed = <1000>;
>> -- 
>> 2.17.1
>>
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Christian Marangi Sept. 21, 2020, 11:06 p.m. UTC | #3
>
>
> On 19/09/2020 15:06, Paul Oranje wrote:
> > See below, regards,
> > Paul
> >
> >
> >> Op 15 sep. 2020, om 21:56 heeft jacekowski@jacekowski.org het volgende geschreven:
> > The subject does not to comply with the rules for that, it should at least start with the target, better would be:
> >       ipq806x: fix tx-queue-size on NBG6817
>
> I was not aware of that.
>
> >> From: Jacek Milewicz <jacekowski@jacekowski.org>
> >>
> >> Fixes FS#3285 for NBG6817 only.
> > Why is that ?
>
> Because on IPQ806x SoC it is impossible to detect correct value,
> therefore it has to be set for each device in device tree (which is what
> this patch is doing).
>
> On most IPQ806x devices correct value has to be determined manually by
> trying few most common values and seeing what works (4k seems to be most
> common).
>

Can I ask you how you found out and how to check if a bad value is set?
Is this set by default on the OEM firmware? Any test or something?
Jacek Milewicz Sept. 22, 2020, 12:02 a.m. UTC | #4
On 22/09/2020 00:06, Ansuel Smith wrote:
>>
>>
>> On 19/09/2020 15:06, Paul Oranje wrote:
>>> See below, regards,
>>> Paul
>>>
>>>
>>>> Op 15 sep. 2020, om 21:56 heeft jacekowski@jacekowski.org het volgende geschreven:
>>> The subject does not to comply with the rules for that, it should at least start with the target, better would be:
>>>        ipq806x: fix tx-queue-size on NBG6817
>>
>> I was not aware of that.
>>
>>>> From: Jacek Milewicz <jacekowski@jacekowski.org>
>>>>
>>>> Fixes FS#3285 for NBG6817 only.
>>> Why is that ?
>>
>> Because on IPQ806x SoC it is impossible to detect correct value,
>> therefore it has to be set for each device in device tree (which is what
>> this patch is doing).
>>
>> On most IPQ806x devices correct value has to be determined manually by
>> trying few most common values and seeing what works (4k seems to be most
>> common).
>>
> 
> Can I ask you how you found out and how to check if a bad value is set?
> Is this set by default on the OEM firmware? Any test or something?
> 

OEM firmware runs older kernel that does not check for tx-fifo-depth at 
all (https://patchwork.kernel.org/patch/11283121/ - this patch, dated 
December 2019 added the check). There is only few values that the driver 
will accept (256,512,1k,2k,4,8k,16k), so the easiest way to check is set 
it to highest (16k) and then try increasingly higher MTU to check at 
what point does it start dropping packets or crashing (and it seems like 
4k is fairly common value on this SoC).
Rich Brown Sept. 22, 2020, 11:04 a.m. UTC | #5
> On Sep 21, 2020, at 8:02 PM, Jacek Milewicz <jacekowski@jacekowski.org> wrote:
> 
> 
> 
> On 22/09/2020 00:06, Ansuel Smith wrote:
>>> 
>>> 
>>> On 19/09/2020 15:06, Paul Oranje wrote:
>>>> See below, regards,
>>>> Paul
>>>> 
>>>> 
>>>>> Op 15 sep. 2020, om 21:56 heeft jacekowski@jacekowski.org het volgende geschreven:
>>>> The subject does not to comply with the rules for that, it should at least start with the target, better would be:
>>>>       ipq806x: fix tx-queue-size on NBG6817
>>> 
>>> I was not aware of that.
>>> 
>>>>> From: Jacek Milewicz <jacekowski@jacekowski.org>
>>>>> 
>>>>> Fixes FS#3285 for NBG6817 only.
>>>> Why is that ?
>>> 
>>> Because on IPQ806x SoC it is impossible to detect correct value,
>>> therefore it has to be set for each device in device tree (which is what
>>> this patch is doing).
>>> 
>>> On most IPQ806x devices correct value has to be determined manually by
>>> trying few most common values and seeing what works (4k seems to be most
>>> common).
>>> 
>> Can I ask you how you found out and how to check if a bad value is set?
>> Is this set by default on the OEM firmware? Any test or something?
> 
> OEM firmware runs older kernel that does not check for tx-fifo-depth at all (https://patchwork.kernel.org/patch/11283121/ - this patch, dated December 2019 added the check). There is only few values that the driver will accept (256,512,1k,2k,4,8k,16k), so the easiest way to check is set it to highest (16k) and then try increasingly higher MTU to check at what point does it start dropping packets or crashing (and it seems like 4k is fairly common value on this SoC).

This all sounds like valuable, hard-won information that deserves to be documented in the code.

> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts b/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
index 7cd1c7b567..5745040d2b 100644
--- a/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
+++ b/target/linux/ipq806x/files-4.14/arch/arm/boot/dts/qcom-ipq8065-nbg6817.dts
@@ -289,6 +289,7 @@ 
 			qcom,emulation = <0>;
 			qcom,irq = <255>;
 			mdiobus = <&mdio0>;
+			tx-fifo-depth = <4096>;
 
 			pinctrl-0 = <&rgmii2_pins>;
 			pinctrl-names = "default";
@@ -310,6 +311,7 @@ 
 			qcom,emulation = <0>;
 			qcom,irq = <258>;
 			mdiobus = <&mdio0>;
+			tx-fifo-depth = <4096>;
 
 			fixed-link {
 				speed = <1000>;