Skip to contentSkip to conversation

i40e/i40evf: Don't bother setting the CLEARPBA bit

PATCH New Alice Michael 2 participants 2 messages Dec 29, 2017 – Jan 5, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 10/14 i40e/i40evf: Don't bother setting the CLEARPBA bit

From: Alexander Duyck <[email protected]>

The CLEARPBA bit in the dynamic interrupt control register actually has no effect either way on the hardware. As per errata 28 in the XL710 specification update the interrupt is actually cleared any time the register is written with the INTENA_MSK bit set to 0. As such the act of toggling the enable bit actually will trigger the interrupt being cleared and could lead to potential lost events if auto-masking is not enabled.

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 11 ++++++++++-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_txrx.c +10 -1
index 4e661ef..628ec59 100644
@@ -2267,8 +2267,17 @@ static u32 i40e_buildreg_itr(const int type, const u16 itr)
 {
 	u32 val;
 
+	/* We don't bother with setting the CLEARPBA bit as the data sheet
+	 * points out doing so is "meaningless since it was already
+	 * auto-cleared". The auto-clearing happens when the interrupt is
+	 * asserted.
+	 *
+	 * Hardware errata 28 for also indicates that writing to a
+	 * xxINT_DYN_CTLx CSR with INTENA_MSK (bit 31) set to 0 will clear
+	 * an event in the PBA anyway so we need to rely on the automask
+	 * to hold pending events for us until the interrupt is re-enabled
+	 */
 	val = I40E_PFINT_DYN_CTLN_INTENA_MASK |
-	      I40E_PFINT_DYN_CTLN_CLEARPBA_MASK |
 	      (type << I40E_PFINT_DYN_CTLN_ITR_INDX_SHIFT) |
 	      (itr << I40E_PFINT_DYN_CTLN_INTERVAL_SHIFT);
 
drivers/net/ethernet/intel/i40evf/i40e_txrx.c +10 -1
index 3fd7e97..34d898f 100644
@@ -1464,8 +1464,17 @@ static u32 i40e_buildreg_itr(const int type, const u16 itr)
 {
 	u32 val;
 
+	/* We don't bother with setting the CLEARPBA bit as the data sheet
+	 * points out doing so is "meaningless since it was already
+	 * auto-cleared". The auto-clearing happens when the interrupt is
+	 * asserted.
+	 *
+	 * Hardware errata 28 for also indicates that writing to a
+	 * xxINT_DYN_CTLx CSR with INTENA_MSK (bit 31) set to 0 will clear
+	 * an event in the PBA anyway so we need to rely on the automask
+	 * to hold pending events for us until the interrupt is re-enabled
+	 */
 	val = I40E_VFINT_DYN_CTLN1_INTENA_MASK |
-	      I40E_VFINT_DYN_CTLN1_CLEARPBA_MASK |
 	      (type << I40E_VFINT_DYN_CTLN1_ITR_INDX_SHIFT) |
 	      (itr << I40E_VFINT_DYN_CTLN1_INTERVAL_SHIFT);
 
Signature · 1 line
2.9.5
#1 lore raw

PATCH 10/14 i40e/i40evf: Don't bother setting the CLEARPBA bit

Quoted 18 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, December 29, 2017 5:51 AM
To: Michael, Alice <[email protected]>; intel-wired- lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH S85-V1 10/14] i40e/i40evf: Don't bother setting the CLEARPBA bit

From: Alexander Duyck <[email protected]>

The CLEARPBA bit in the dynamic interrupt control register actually has no effect either way on the hardware. As per errata 28 in the XL710 specification update the interrupt is actually cleared any time the register is written with the INTENA_MSK bit set to 0. As such the act of toggling the enable bit actually will trigger the interrupt being cleared and could lead to potential lost events if auto-masking is not enabled.

Quoted 5 lines
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 11 ++++++++++-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)