Hi Paul,
Thanks for reviewing.
On 1/7/2024 8:42 pm, Paul Menzel wrote:
Dear Faizal,
Thank you for your patch.
Am 01.07.24 um 12:00 schrieb Faizal Rahim:
AVNU testing uncovered that even when the taprio gate is closed,
some packets still transmit.
What is AVNU? *some* would fit on the line above.
AVNU stands for "Avnu Alliance." AVNU (Audio Video Bridging Network
Alliance) is an industry consortium that promotes and certifies
interoperability of devices implementing IEEE 802.1 standards for
time-sensitive applications.
This AVNU test refers to AVNU certification test plan.
Should I add this information in the commit ?
A known i225/6 hardware errata states traffic might overflow the planned
Do you have an idea for that errata? Please document it. (I see you added
it at the end. Maybe use [1] notation for referencing it.)
Sure.
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@xxxxxxxxxxxxxxx>
As you Cc’ed stable@xxxxxxxxxxxxxxx, add a Fixes: tag?
Accidentally CC'ed stable@xxxxxxxxxxxxxxx.
Since it's a hardware bug, not software, probably Fixes: tag not needed ?
Not sure which Fixes: commit to point to hmm.
I'll remove stable kernel email and omit Fixes: tag, is that okay?
/* Returns the TSN specific registers to their default values after
* the adapter is reset.
*/
@@ -91,6 +100,9 @@ static int igc_tsn_disable_offload(struct igc_adapter
*adapter)
wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT);
wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT);
+ if (igc_is_device_id_i226(hw))
+ igc_tsn_restore_retx_default(adapter);
+
tqavctrl = rd32(IGC_TQAVCTRL);
tqavctrl &= ~(IGC_TQAVCTRL_TRANSMIT_MODE_TSN |
IGC_TQAVCTRL_ENHANCED_QAV | IGC_TQAVCTRL_FUTSCDDIS);
@@ -111,6 +123,25 @@ static int igc_tsn_disable_offload(struct
igc_adapter *adapter)
return 0;
}
+/* To partially fix i226 HW errata, reduce MAC internal buffering from
192 Bytes
+ * to 88 Bytes by setting RETX_CTL register using the recommendation from:
+ * a) Ethernet Controller I225/I22 Specification Update Rev 2.1
+ * Item 9: TSN: Packet Transmission Might Cross the Qbv Window
+ * b) I225/6 SW User Manual Rev 1.2.4: Section 8.11.5 Retry Buffer Control
+ */
+static void igc_tsn_set_retx_qbvfullth(struct igc_adapter *adapter)
It’d put threshold in the name.
My earlier thought is that it is easier to look for the keyword "qbvfullth"
in the i226 SW User Manual where you'll get a hit that brings you directly
to that register. "qbvfullthreshold" would not.
There are some comments in the new code that links 'th' to 'threshold' for
the reader.
But I'm okay to change it to "qbvfullthreshold".
Thoughts?
Regards,
Faizal