netdev - Re: [PATCH net-next 1/4] igb: refactor and simplify time sync interrupt handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CDCA19C9.1E862%matthew.vick@intel.com>
Date:	Tue, 28 May 2013 15:24:08 +0000
From:	"Vick, Matthew" <matthew.vick@...el.com>
To:	Richard Cochran <richardcochran@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	David Miller <davem@...emloft.net>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Keller, Jacob E" <jacob.e.keller@...el.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
Subject: Re: [PATCH net-next 1/4] igb: refactor and simplify time sync
 interrupt handling

On 5/27/13 2:21 AM, "Richard Cochran" <richardcochran@...il.com> wrote:

>The code that handles the time sync interrupt is repeated in three
>different places. This patch refactors the identical code blocks into
>a single helper function. Also, reading the TSICR register already
>acknowledges the time sync interrupts, and so there is no need to
>write that register at all.
>
>Signed-off-by: Richard Cochran <richardcochran@...il.com>
>---
> drivers/net/ethernet/intel/igb/igb_main.c |   47
>+++++++++++------------------
> 1 file changed, 17 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
>b/drivers/net/ethernet/intel/igb/igb_main.c
>index 6a0c1b6..d25a965 100644
>--- a/drivers/net/ethernet/intel/igb/igb_main.c
>+++ b/drivers/net/ethernet/intel/igb/igb_main.c
>@@ -5035,6 +5035,17 @@ void igb_update_stats(struct igb_adapter *adapter,
> 	}
> }
> 
>+static void igb_tsync_interrupt(struct igb_adapter *adapter)
>+{
>+	struct e1000_hw *hw = &adapter->hw;
>+	u32 tsicr = rd32(E1000_TSICR);
>+
>+	if (tsicr & E1000_TSICR_TXTS) {
>+		/* retrieve hardware timestamp */
>+		schedule_work(&adapter->ptp_tx_work);
>+	}
>+}
>+
> static irqreturn_t igb_msix_other(int irq, void *data)
> {
> 	struct igb_adapter *adapter = data;
>@@ -5066,16 +5077,8 @@ static irqreturn_t igb_msix_other(int irq, void
>*data)
> 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
> 	}
> 
>-	if (icr & E1000_ICR_TS) {
>-		u32 tsicr = rd32(E1000_TSICR);
>-
>-		if (tsicr & E1000_TSICR_TXTS) {
>-			/* acknowledge the interrupt */
>-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
>-			/* retrieve hardware timestamp */
>-			schedule_work(&adapter->ptp_tx_work);
>-		}
>-	}
>+	if (icr & E1000_ICR_TS)
>+		igb_tsync_interrupt(adapter);
> 
> 	wr32(E1000_EIMS, adapter->eims_other);
> 
>@@ -5850,16 +5853,8 @@ static irqreturn_t igb_intr_msi(int irq, void
>*data)
> 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
> 	}
> 
>-	if (icr & E1000_ICR_TS) {
>-		u32 tsicr = rd32(E1000_TSICR);
>-
>-		if (tsicr & E1000_TSICR_TXTS) {
>-			/* acknowledge the interrupt */
>-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
>-			/* retrieve hardware timestamp */
>-			schedule_work(&adapter->ptp_tx_work);
>-		}
>-	}
>+	if (icr & E1000_ICR_TS)
>+		igb_tsync_interrupt(adapter);
> 
> 	napi_schedule(&q_vector->napi);
> 
>@@ -5904,16 +5899,8 @@ static irqreturn_t igb_intr(int irq, void *data)
> 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
> 	}
> 
>-	if (icr & E1000_ICR_TS) {
>-		u32 tsicr = rd32(E1000_TSICR);
>-
>-		if (tsicr & E1000_TSICR_TXTS) {
>-			/* acknowledge the interrupt */
>-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
>-			/* retrieve hardware timestamp */
>-			schedule_work(&adapter->ptp_tx_work);
>-		}
>-	}
>+	if (icr & E1000_ICR_TS)
>+		igb_tsync_interrupt(adapter);
> 
> 	napi_schedule(&q_vector->napi);
> 
>-- 
>1.7.10.4

NAK due to the question you raised in the other thread--this will break
1588 on the 82580, but there was really no way for you to know that. As I
mentioned before, I'm getting the internal discussion going to confirm
that.

Matthew

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html