| Message ID | 388385c50909f63bff239894c60dbb00abc65c93.1323744725.git.richardcochran@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
Le mardi 13 décembre 2011 à 04:00 +0100, Richard Cochran a écrit : > This commit removes the legacy timecompare code from the igb driver and > offers a tunable PHC instead. > > Signed-off-by: Richard Cochran <richardcochran@gmail.com> > --- > +void igb_systim_to_hwtstamp(struct igb_adapter *adapter, > + struct skb_shared_hwtstamps *hwtstamps, > + u64 systim) > +{ > + u64 ns; > + unsigned long flags; > + unsigned int shift; > + int msb_set; > + > + switch (adapter->hw.mac.type) { > + case e1000_i350: > + case e1000_82580: > + shift = OFL_SHIFT_82580; > + msb_set = (systim >> 32) & SYSTIMH_MSB_82580; > + break; > + case e1000_82576: > + shift = OFL_SHIFT_82576; > + msb_set = (systim >> 32) & SYSTIMH_MSB_82576; > + break; > + default: > + return; > + } > + > + spin_lock_irqsave(&adapter->tmreg_lock, flags); > + > + ns = igb_overflow_get(adapter, systim, msb_set, shift); > + > + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); > + > + memset(hwtstamps, 0, sizeof(*hwtstamps)); > + hwtstamps->hwtstamp = ns_to_ktime(ns); > +} Adding a (shared) spinlock on a multiqueue device is source of extra delay (because of extra cache line trafic), I guess. It seems current code doesnt need a spinlock, maybe it was a bug ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 13, 2011 at 04:28:52AM +0100, Eric Dumazet wrote: > > Adding a (shared) spinlock on a multiqueue device is source of extra > delay (because of extra cache line trafic), I guess. > > It seems current code doesnt need a spinlock, maybe it was a bug ? (I didn't think about the old code. I only deleted it. ;) The spinlock is needed because reading the 64 bit time value involves reading two 32 registers. The first read latches the value. Ditto for writing. In addition, here we have to watch the most significant bit for one-to-zero transistion, in order to keep count of the overflow. It is too bad that we have to take the spinlock for every time stamped packet, but it is the hardware's fault for not providing a 64 bit wide nanosecond time register. Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mardi 13 décembre 2011 à 04:41 +0100, Richard Cochran a écrit : > On Tue, Dec 13, 2011 at 04:28:52AM +0100, Eric Dumazet wrote: > > > > Adding a (shared) spinlock on a multiqueue device is source of extra > > delay (because of extra cache line trafic), I guess. > > > > It seems current code doesnt need a spinlock, maybe it was a bug ? > > (I didn't think about the old code. I only deleted it. ;) > > The spinlock is needed because reading the 64 bit time value involves > reading two 32 registers. The first read latches the value. Ditto for > writing. > > In addition, here we have to watch the most significant bit for > one-to-zero transistion, in order to keep count of the overflow. > > It is too bad that we have to take the spinlock for every time stamped > packet, but it is the hardware's fault for not providing a 64 bit wide > nanosecond time register. > Yes, probably. Are you sure a workaround is not possible, using a seqlock for synchronization of threads, and two hardware reads ? Or maybe it doesnt matter at all :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 13, 2011 at 04:52:29AM +0100, Eric Dumazet wrote: > > It is too bad that we have to take the spinlock for every time stamped > > packet, but it is the hardware's fault for not providing a 64 bit wide > > nanosecond time register. > > > > Yes, probably. Are you sure a workaround is not possible, using a > seqlock for synchronization of threads, and two hardware reads ? Many things are possible... > Or maybe it doesnt matter at all :) Yes, I think it not worth the effort. In general, the whole time stamping thing is at odds with network throughput. Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-12-12 at 19:00 -0800, Richard Cochran wrote: > This commit removes the legacy timecompare code from the igb driver and > offers a tunable PHC instead. > > Signed-off-by: Richard Cochran <richardcochran@gmail.com> Richard, first, thanks for this work, I have some feedback and request you make a V2. > - /* > - * The timestamp latches on lowest register read. For the 82580 > - * the lowest register is SYSTIMR instead of SYSTIML. However we never > - * adjusted TIMINCA so SYSTIMR will just read as all 0s so ignore it. > - */ Please keep this comment in your new igb_82580_systim_read, it explains a bit of *why* we are doing something. There were a lot of explanatory comments that you removed, please audit the "-" lines of your patch and add back the comments that are appropriate in your new code. > - if (hw->mac.type >= e1000_82580) { > - stamp = rd32(E1000_SYSTIMR) >> 8; > - shift = IGB_82580_TSYNC_SHIFT; > - } > - > - stamp |= (u64)rd32(E1000_SYSTIML) << shift; > - stamp |= (u64)rd32(E1000_SYSTIMH) << (shift + 32); > - return stamp; > -} > - > /** > * igb_get_hw_dev - return device > * used by hardware layer to print debugging information > @@ -2080,7 +2052,7 @@ static int __devinit igb_probe(struct pci_dev *pdev, > > #endif > /* do hw tstamp init after resetting */ > - igb_init_hw_timer(adapter); > + igb_ptp_init(adapter); > > dev_info(&pdev->dev, "Intel(R) Gigabit Ethernet Network Connection\n"); > /* print bus type/speed/width info */ > @@ -2150,6 +2122,8 @@ static void __devexit igb_remove(struct pci_dev *pdev) > struct igb_adapter *adapter = netdev_priv(netdev); > struct e1000_hw *hw = &adapter->hw; > > + igb_ptp_remove(adapter); > + > /* > * The watchdog timer may be rescheduled, so explicitly > * disable watchdog from being rescheduled. > @@ -2269,112 +2243,6 @@ out: > } > > /** > - * igb_init_hw_timer - Initialize hardware timer used with IEEE 1588 timestamp > - * @adapter: board private structure to initialize > - * > - * igb_init_hw_timer initializes the function pointer and values for the hw > - * timer found in hardware. > - **/ > -static void igb_init_hw_timer(struct igb_adapter *adapter) > -{ > - struct e1000_hw *hw = &adapter->hw; > - > - switch (hw->mac.type) { > - case e1000_i350: > - case e1000_82580: > - memset(&adapter->cycles, 0, sizeof(adapter->cycles)); > - adapter->cycles.read = igb_read_clock; > - adapter->cycles.mask = CLOCKSOURCE_MASK(64); > - adapter->cycles.mult = 1; > - /* > - * The 82580 timesync updates the system timer every 8ns by 8ns > - * and the value cannot be shifted. Instead we need to shift > - * the registers to generate a 64bit timer value. As a result > - * SYSTIMR/L/H, TXSTMPL/H, RXSTMPL/H all have to be shifted by > - * 24 in order to generate a larger value for synchronization. > - */ > - adapter->cycles.shift = IGB_82580_TSYNC_SHIFT; > - /* disable system timer temporarily by setting bit 31 */ > - wr32(E1000_TSAUXC, 0x80000000); > - wrfl(); > - > - /* Set registers so that rollover occurs soon to test this. */ > - wr32(E1000_SYSTIMR, 0x00000000); > - wr32(E1000_SYSTIML, 0x80000000); > - wr32(E1000_SYSTIMH, 0x000000FF); > - wrfl(); > - > - /* enable system timer by clearing bit 31 */ > - wr32(E1000_TSAUXC, 0x0); > - wrfl(); > - > - timecounter_init(&adapter->clock, > - &adapter->cycles, > - ktime_to_ns(ktime_get_real())); > - /* > - * Synchronize our NIC clock against system wall clock. NIC > - * time stamp reading requires ~3us per sample, each sample > - * was pretty stable even under load => only require 10 > - * samples for each offset comparison. > - */ > - memset(&adapter->compare, 0, sizeof(adapter->compare)); > - adapter->compare.source = &adapter->clock; > - adapter->compare.target = ktime_get_real; > - adapter->compare.num_samples = 10; > - timecompare_update(&adapter->compare, 0); > - break; > - case e1000_82576: > - /* > - * Initialize hardware timer: we keep it running just in case > - * that some program needs it later on. > - */ > - memset(&adapter->cycles, 0, sizeof(adapter->cycles)); > - adapter->cycles.read = igb_read_clock; > - adapter->cycles.mask = CLOCKSOURCE_MASK(64); > - adapter->cycles.mult = 1; > - /** > - * Scale the NIC clock cycle by a large factor so that > - * relatively small clock corrections can be added or > - * subtracted at each clock tick. The drawbacks of a large > - * factor are a) that the clock register overflows more quickly > - * (not such a big deal) and b) that the increment per tick has > - * to fit into 24 bits. As a result we need to use a shift of > - * 19 so we can fit a value of 16 into the TIMINCA register. > - */ > - adapter->cycles.shift = IGB_82576_TSYNC_SHIFT; > - wr32(E1000_TIMINCA, > - (1 << E1000_TIMINCA_16NS_SHIFT) | > - (16 << IGB_82576_TSYNC_SHIFT)); > - > - /* Set registers so that rollover occurs soon to test this. */ > - wr32(E1000_SYSTIML, 0x00000000); > - wr32(E1000_SYSTIMH, 0xFF800000); > - wrfl(); > - > - timecounter_init(&adapter->clock, > - &adapter->cycles, > - ktime_to_ns(ktime_get_real())); > - /* > - * Synchronize our NIC clock against system wall clock. NIC > - * time stamp reading requires ~3us per sample, each sample > - * was pretty stable even under load => only require 10 > - * samples for each offset comparison. > - */ > - memset(&adapter->compare, 0, sizeof(adapter->compare)); > - adapter->compare.source = &adapter->clock; > - adapter->compare.target = ktime_get_real; > - adapter->compare.num_samples = 10; > - timecompare_update(&adapter->compare, 0); > - break; > - case e1000_82575: > - /* 82575 does not support timesync */ > - default: > - break; > - } > - > -} > - > -/** > * igb_sw_init - Initialize general software structures (struct igb_adapter) > * @adapter: board private structure to initialize > * > @@ -5628,35 +5496,6 @@ static int igb_poll(struct napi_struct *napi, int budget) > } > > /** > - * igb_systim_to_hwtstamp - convert system time value to hw timestamp > - * @adapter: board private structure > - * @shhwtstamps: timestamp structure to update > - * @regval: unsigned 64bit system time value. > - * > - * We need to convert the system time value stored in the RX/TXSTMP registers > - * into a hwtstamp which can be used by the upper level timestamping functions > - */ > -static void igb_systim_to_hwtstamp(struct igb_adapter *adapter, > - struct skb_shared_hwtstamps *shhwtstamps, > - u64 regval) > -{ > - u64 ns; > - > - /* > - * The 82580 starts with 1ns at bit 0 in RX/TXSTMPL, shift this up to > - * 24 to match clock shift we setup earlier. > - */ > - if (adapter->hw.mac.type >= e1000_82580) > - regval <<= IGB_82580_TSYNC_SHIFT; > - > - ns = timecounter_cyc2time(&adapter->clock, regval); > - timecompare_update(&adapter->compare, ns); > - memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps)); > - shhwtstamps->hwtstamp = ns_to_ktime(ns); > - shhwtstamps->syststamp = timecompare_transform(&adapter->compare, ns); > -} > - > -/** > * igb_tx_hwtstamp - utility function which checks for TX time stamp > * @q_vector: pointer to q_vector containing needed info > * @buffer: pointer to igb_tx_buffer structure > diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c > index 7a9f6bc..7a62980 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ptp.c > +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c > @@ -406,3 +406,45 @@ void igb_ptp_remove(struct igb_adapter *adapter) > adapter->netdev->name); > } > } > + > +/** > + * igb_systim_to_hwtstamp - convert system time value to hw timestamp > + * @adapter: board private structure > + * @shhwtstamps: timestamp structure to update cut and paste typo? should match the arguments below. > + * @systim: unsigned 64bit system time value. > + * > + * We need to convert the system time value stored in the RX/TXSTMP registers > + * into a hwtstamp which can be used by the upper level timestamping functions > + */ > +void igb_systim_to_hwtstamp(struct igb_adapter *adapter, > + struct skb_shared_hwtstamps *hwtstamps, > + u64 systim) > +{ > + u64 ns; > + unsigned long flags; > + unsigned int shift; > + int msb_set; > + > + switch (adapter->hw.mac.type) { > + case e1000_i350: > + case e1000_82580: > + shift = OFL_SHIFT_82580; > + msb_set = (systim >> 32) & SYSTIMH_MSB_82580; > + break; > + case e1000_82576: > + shift = OFL_SHIFT_82576; > + msb_set = (systim >> 32) & SYSTIMH_MSB_82576; > + break; > + default: > + return; > + } > + > + spin_lock_irqsave(&adapter->tmreg_lock, flags); based on the discussion on the list with eric dumazet it should have a comment here why the spinlock is needed and about how it is expected to impact performance. > + > + ns = igb_overflow_get(adapter, systim, msb_set, shift); > + > + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); > + > + memset(hwtstamps, 0, sizeof(*hwtstamps)); > + hwtstamps->hwtstamp = ns_to_ktime(ns); > +} -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 14, 2011 at 02:33:09PM -0800, Jesse Brandeburg wrote: > On Mon, 2011-12-12 at 19:00 -0800, Richard Cochran wrote: > > This commit removes the legacy timecompare code from the igb driver and > > offers a tunable PHC instead. > > > > Signed-off-by: Richard Cochran <richardcochran@gmail.com> > > Richard, first, thanks for this work, I have some feedback and request > you make a V2. Thanks for your feedback, I'll will work your comments in. Any chance of getting the driver tested on a 82576? Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----Original Message----- >From: Richard Cochran [mailto:richardcochran@gmail.com] >Sent: Thursday, December 15, 2011 8:13 AM >To: Brandeburg, Jesse >Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; Keller, >Jacob E; Kirsher, Jeffrey T; Ronciak, John; John Stultz; Thomas >Gleixner; Wyborny, Carolyn >Subject: Re: [PATCH net-next 2/2] igb: offer a PTP Hardware Clock >instead of the timecompare method > >On Wed, Dec 14, 2011 at 02:33:09PM -0800, Jesse Brandeburg wrote: >> On Mon, 2011-12-12 at 19:00 -0800, Richard Cochran wrote: >> > This commit removes the legacy timecompare code from the igb driver >and >> > offers a tunable PHC instead. >> > >> > Signed-off-by: Richard Cochran <richardcochran@gmail.com> >> >> Richard, first, thanks for this work, I have some feedback and request >> you make a V2. > >Thanks for your feedback, I'll will work your comments in. > >Any chance of getting the driver tested on a 82576? > >Thanks, > >Richard I will request this testing and let you know. Thanks, Carolyn Carolyn Wyborny Linux Development LAN Access Division Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 15, 2011 at 08:46:55AM -0800, Wyborny, Carolyn wrote: > >-----Original Message----- > >From: Richard Cochran [mailto:richardcochran@gmail.com] ... > >Any chance of getting the driver tested on a 82576? > > > >Thanks, > > > >Richard > > I will request this testing and let you know. Never mind about testing this patch. I have already found a bug in the code path for the 82576. I will post a new, corrected patch later on this week. Thanks, and Merry Christmas to All, Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 13, 2011 at 04:00:35AM +0100, Richard Cochran wrote: > +void igb_systim_to_hwtstamp(struct igb_adapter *adapter, > + struct skb_shared_hwtstamps *hwtstamps, > + u64 systim) > +{ > + u64 ns; > + unsigned long flags; > + unsigned int shift; > + int msb_set; > + > + switch (adapter->hw.mac.type) { > + case e1000_i350: > + case e1000_82580: > + shift = OFL_SHIFT_82580; > + msb_set = (systim >> 32) & SYSTIMH_MSB_82580; > + break; > + case e1000_82576: > + shift = OFL_SHIFT_82576; > + msb_set = (systim >> 32) & SYSTIMH_MSB_82576; Should have converted systim to nanoseconds here. > + break; > + default: > + return; > + } > + > + spin_lock_irqsave(&adapter->tmreg_lock, flags); > + > + ns = igb_overflow_get(adapter, systim, msb_set, shift); > + > + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); > + > + memset(hwtstamps, 0, sizeof(*hwtstamps)); > + hwtstamps->hwtstamp = ns_to_ktime(ns); > +} > -- > 1.7.2.5 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/igb/Makefile b/drivers/net/ethernet/intel/igb/Makefile index c6e4621..42f0868 100644 --- a/drivers/net/ethernet/intel/igb/Makefile +++ b/drivers/net/ethernet/intel/igb/Makefile @@ -32,6 +32,6 @@ obj-$(CONFIG_IGB) += igb.o -igb-objs := igb_main.o igb_ethtool.o e1000_82575.o \ +igb-objs := igb_main.o igb_ethtool.o igb_ptp.o e1000_82575.o \ e1000_mac.o e1000_nvm.o e1000_phy.o e1000_mbx.o diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 001f95d..2ea5e91 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -34,8 +34,6 @@ #include "e1000_mac.h" #include "e1000_82575.h" -#include <linux/clocksource.h> -#include <linux/timecompare.h> #include <linux/net_tstamp.h> #include <linux/ptp_clock_kernel.h> #include <linux/bitops.h> @@ -329,9 +327,6 @@ struct igb_adapter { /* OS defined structs */ struct pci_dev *pdev; - struct cyclecounter cycles; - struct timecounter clock; - struct timecompare compare; struct hwtstamp_config hwtstamp_config; spinlock_t stats64_lock; @@ -386,7 +381,6 @@ struct igb_adapter { #define IGB_DMCTLX_DCFLUSH_DIS 0x80000000 /* Disable DMA Coal Flush */ #define IGB_82576_TSYNC_SHIFT 19 -#define IGB_82580_TSYNC_SHIFT 24 #define IGB_TS_HDR_LEN 16 enum e1000_state_t { __IGB_TESTING, @@ -423,6 +417,13 @@ extern bool igb_has_link(struct igb_adapter *adapter); extern void igb_set_ethtool_ops(struct net_device *); extern void igb_power_up_link(struct igb_adapter *); +extern void igb_ptp_init(struct igb_adapter *adapter); +extern void igb_ptp_remove(struct igb_adapter *adapter); + +extern void igb_systim_to_hwtstamp(struct igb_adapter *adapter, + struct skb_shared_hwtstamps *hwtstamps, + u64 systim); + static inline s32 igb_reset_phy(struct e1000_hw *hw) { if (hw->phy.ops.reset) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 89d576c..0ee04b9 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -113,7 +113,6 @@ static void igb_free_all_rx_resources(struct igb_adapter *); static void igb_setup_mrqc(struct igb_adapter *); static int igb_probe(struct pci_dev *, const struct pci_device_id *); static void __devexit igb_remove(struct pci_dev *pdev); -static void igb_init_hw_timer(struct igb_adapter *adapter); static int igb_sw_init(struct igb_adapter *); static int igb_open(struct net_device *); static int igb_close(struct net_device *); @@ -549,33 +548,6 @@ exit: return; } - -/** - * igb_read_clock - read raw cycle counter (to be used by time counter) - */ -static cycle_t igb_read_clock(const struct cyclecounter *tc) -{ - struct igb_adapter *adapter = - container_of(tc, struct igb_adapter, cycles); - struct e1000_hw *hw = &adapter->hw; - u64 stamp = 0; - int shift = 0; - - /* - * The timestamp latches on lowest register read. For the 82580 - * the lowest register is SYSTIMR instead of SYSTIML. However we never - * adjusted TIMINCA so SYSTIMR will just read as all 0s so ignore it. - */ - if (hw->mac.type >= e1000_82580) { - stamp = rd32(E1000_SYSTIMR) >> 8; - shift = IGB_82580_TSYNC_SHIFT; - } - - stamp |= (u64)rd32(E1000_SYSTIML) << shift; - stamp |= (u64)rd32(E1000_SYSTIMH) << (shift + 32); - return stamp; -} - /** * igb_get_hw_dev - return device * used by hardware layer to print debugging information @@ -2080,7 +2052,7 @@ static int __devinit igb_probe(struct pci_dev *pdev, #endif /* do hw tstamp init after resetting */ - igb_init_hw_timer(adapter); + igb_ptp_init(adapter); dev_info(&pdev->dev, "Intel(R) Gigabit Ethernet Network Connection\n"); /* print bus type/speed/width info */ @@ -2150,6 +2122,8 @@ static void __devexit igb_remove(struct pci_dev *pdev) struct igb_adapter *adapter = netdev_priv(netdev); struct e1000_hw *hw = &adapter->hw; + igb_ptp_remove(adapter); + /* * The watchdog timer may be rescheduled, so explicitly * disable watchdog from being rescheduled. @@ -2269,112 +2243,6 @@ out: } /** - * igb_init_hw_timer - Initialize hardware timer used with IEEE 1588 timestamp - * @adapter: board private structure to initialize - * - * igb_init_hw_timer initializes the function pointer and values for the hw - * timer found in hardware. - **/ -static void igb_init_hw_timer(struct igb_adapter *adapter) -{ - struct e1000_hw *hw = &adapter->hw; - - switch (hw->mac.type) { - case e1000_i350: - case e1000_82580: - memset(&adapter->cycles, 0, sizeof(adapter->cycles)); - adapter->cycles.read = igb_read_clock; - adapter->cycles.mask = CLOCKSOURCE_MASK(64); - adapter->cycles.mult = 1; - /* - * The 82580 timesync updates the system timer every 8ns by 8ns - * and the value cannot be shifted. Instead we need to shift - * the registers to generate a 64bit timer value. As a result - * SYSTIMR/L/H, TXSTMPL/H, RXSTMPL/H all have to be shifted by - * 24 in order to generate a larger value for synchronization. - */ - adapter->cycles.shift = IGB_82580_TSYNC_SHIFT; - /* disable system timer temporarily by setting bit 31 */ - wr32(E1000_TSAUXC, 0x80000000); - wrfl(); - - /* Set registers so that rollover occurs soon to test this. */ - wr32(E1000_SYSTIMR, 0x00000000); - wr32(E1000_SYSTIML, 0x80000000); - wr32(E1000_SYSTIMH, 0x000000FF); - wrfl(); - - /* enable system timer by clearing bit 31 */ - wr32(E1000_TSAUXC, 0x0); - wrfl(); - - timecounter_init(&adapter->clock, - &adapter->cycles, - ktime_to_ns(ktime_get_real())); - /* - * Synchronize our NIC clock against system wall clock. NIC - * time stamp reading requires ~3us per sample, each sample - * was pretty stable even under load => only require 10 - * samples for each offset comparison. - */ - memset(&adapter->compare, 0, sizeof(adapter->compare)); - adapter->compare.source = &adapter->clock; - adapter->compare.target = ktime_get_real; - adapter->compare.num_samples = 10; - timecompare_update(&adapter->compare, 0); - break; - case e1000_82576: - /* - * Initialize hardware timer: we keep it running just in case - * that some program needs it later on. - */ - memset(&adapter->cycles, 0, sizeof(adapter->cycles)); - adapter->cycles.read = igb_read_clock; - adapter->cycles.mask = CLOCKSOURCE_MASK(64); - adapter->cycles.mult = 1; - /** - * Scale the NIC clock cycle by a large factor so that - * relatively small clock corrections can be added or - * subtracted at each clock tick. The drawbacks of a large - * factor are a) that the clock register overflows more quickly - * (not such a big deal) and b) that the increment per tick has - * to fit into 24 bits. As a result we need to use a shift of - * 19 so we can fit a value of 16 into the TIMINCA register. - */ - adapter->cycles.shift = IGB_82576_TSYNC_SHIFT; - wr32(E1000_TIMINCA, - (1 << E1000_TIMINCA_16NS_SHIFT) | - (16 << IGB_82576_TSYNC_SHIFT)); - - /* Set registers so that rollover occurs soon to test this. */ - wr32(E1000_SYSTIML, 0x00000000); - wr32(E1000_SYSTIMH, 0xFF800000); - wrfl(); - - timecounter_init(&adapter->clock, - &adapter->cycles, - ktime_to_ns(ktime_get_real())); - /* - * Synchronize our NIC clock against system wall clock. NIC - * time stamp reading requires ~3us per sample, each sample - * was pretty stable even under load => only require 10 - * samples for each offset comparison. - */ - memset(&adapter->compare, 0, sizeof(adapter->compare)); - adapter->compare.source = &adapter->clock; - adapter->compare.target = ktime_get_real; - adapter->compare.num_samples = 10; - timecompare_update(&adapter->compare, 0); - break; - case e1000_82575: - /* 82575 does not support timesync */ - default: - break; - } - -} - -/** * igb_sw_init - Initialize general software structures (struct igb_adapter) * @adapter: board private structure to initialize * @@ -5628,35 +5496,6 @@ static int igb_poll(struct napi_struct *napi, int budget) } /** - * igb_systim_to_hwtstamp - convert system time value to hw timestamp - * @adapter: board private structure - * @shhwtstamps: timestamp structure to update - * @regval: unsigned 64bit system time value. - * - * We need to convert the system time value stored in the RX/TXSTMP registers - * into a hwtstamp which can be used by the upper level timestamping functions - */ -static void igb_systim_to_hwtstamp(struct igb_adapter *adapter, - struct skb_shared_hwtstamps *shhwtstamps, - u64 regval) -{ - u64 ns; - - /* - * The 82580 starts with 1ns at bit 0 in RX/TXSTMPL, shift this up to - * 24 to match clock shift we setup earlier. - */ - if (adapter->hw.mac.type >= e1000_82580) - regval <<= IGB_82580_TSYNC_SHIFT; - - ns = timecounter_cyc2time(&adapter->clock, regval); - timecompare_update(&adapter->compare, ns); - memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps)); - shhwtstamps->hwtstamp = ns_to_ktime(ns); - shhwtstamps->syststamp = timecompare_transform(&adapter->compare, ns); -} - -/** * igb_tx_hwtstamp - utility function which checks for TX time stamp * @q_vector: pointer to q_vector containing needed info * @buffer: pointer to igb_tx_buffer structure diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 7a9f6bc..7a62980 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -406,3 +406,45 @@ void igb_ptp_remove(struct igb_adapter *adapter) adapter->netdev->name); } } + +/** + * igb_systim_to_hwtstamp - convert system time value to hw timestamp + * @adapter: board private structure + * @shhwtstamps: timestamp structure to update + * @systim: unsigned 64bit system time value. + * + * We need to convert the system time value stored in the RX/TXSTMP registers + * into a hwtstamp which can be used by the upper level timestamping functions + */ +void igb_systim_to_hwtstamp(struct igb_adapter *adapter, + struct skb_shared_hwtstamps *hwtstamps, + u64 systim) +{ + u64 ns; + unsigned long flags; + unsigned int shift; + int msb_set; + + switch (adapter->hw.mac.type) { + case e1000_i350: + case e1000_82580: + shift = OFL_SHIFT_82580; + msb_set = (systim >> 32) & SYSTIMH_MSB_82580; + break; + case e1000_82576: + shift = OFL_SHIFT_82576; + msb_set = (systim >> 32) & SYSTIMH_MSB_82576; + break; + default: + return; + } + + spin_lock_irqsave(&adapter->tmreg_lock, flags); + + ns = igb_overflow_get(adapter, systim, msb_set, shift); + + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); + + memset(hwtstamps, 0, sizeof(*hwtstamps)); + hwtstamps->hwtstamp = ns_to_ktime(ns); +}
This commit removes the legacy timecompare code from the igb driver and offers a tunable PHC instead. Signed-off-by: Richard Cochran <richardcochran@gmail.com> --- drivers/net/ethernet/intel/igb/Makefile | 2 +- drivers/net/ethernet/intel/igb/igb.h | 13 ++- drivers/net/ethernet/intel/igb/igb_main.c | 167 +---------------------------- drivers/net/ethernet/intel/igb/igb_ptp.c | 42 +++++++ 4 files changed, 53 insertions(+), 171 deletions(-)