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@linux.intel.com
As you Cc’ed stable@vger.kernel.org, add a Fixes: tag?
Accidentally CC'ed stable@vger.kernel.org. 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