* [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP
@ 2016-05-18 21:04 Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 2/5] igb: introduce IGB_PTP_OVERFLOW_CHECK flag Jacob Keller
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Jacob Keller @ 2016-05-18 21:04 UTC (permalink / raw)
To: intel-wired-lan
Upcoming patches will introduce new PTP specific flags. To avoid
cluttering the normal flags variable, introduce PTP specific "ptp_flags"
variable for this purpose, and move IGB_FLAG_PTP to become
IGB_PTP_ENABLED.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 5 ++++-
drivers/net/ethernet/intel/igb/igb_ptp.c | 8 ++++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index b9609afa5ca3..1e18a9eb16e0 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -445,6 +445,7 @@ struct igb_adapter {
unsigned long ptp_tx_start;
unsigned long last_rx_ptp_check;
unsigned long last_rx_timestamp;
+ unsigned int ptp_flags;
spinlock_t tmreg_lock;
struct cyclecounter cc;
struct timecounter tc;
@@ -474,12 +475,14 @@ struct igb_adapter {
u16 eee_advert;
};
+/* flags controlling PTP/1588 function */
+#define IGB_PTP_ENABLED BIT(0)
+
#define IGB_FLAG_HAS_MSI BIT(0)
#define IGB_FLAG_DCA_ENABLED BIT(1)
#define IGB_FLAG_QUAD_PORT_A BIT(2)
#define IGB_FLAG_QUEUE_PAIRS BIT(3)
#define IGB_FLAG_DMAC BIT(4)
-#define IGB_FLAG_PTP BIT(5)
#define IGB_FLAG_RSS_FIELD_IPV4_UDP BIT(6)
#define IGB_FLAG_RSS_FIELD_IPV6_UDP BIT(7)
#define IGB_FLAG_WOL_SUPPORTED BIT(8)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index f097c5a8ab93..88e3b40415a1 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -684,7 +684,7 @@ void igb_ptp_rx_hang(struct igb_adapter *adapter)
u32 tsyncrxctl = rd32(E1000_TSYNCRXCTL);
unsigned long rx_event;
- if (hw->mac.type != e1000_82576)
+ if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
return;
/* If we don't have a valid timestamp in the registers, just update the
@@ -1156,7 +1156,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
} else {
dev_info(&adapter->pdev->dev, "added PHC on %s\n",
adapter->netdev->name);
- adapter->flags |= IGB_FLAG_PTP;
+ adapter->ptp_flags |= IGB_PTP_ENABLED;
}
}
@@ -1194,7 +1194,7 @@ void igb_ptp_stop(struct igb_adapter *adapter)
ptp_clock_unregister(adapter->ptp_clock);
dev_info(&adapter->pdev->dev, "removed PHC on %s\n",
adapter->netdev->name);
- adapter->flags &= ~IGB_FLAG_PTP;
+ adapter->ptp_flags &= ~IGB_PTP_ENABLED;
}
}
@@ -1209,7 +1209,7 @@ void igb_ptp_reset(struct igb_adapter *adapter)
struct e1000_hw *hw = &adapter->hw;
unsigned long flags;
- if (!(adapter->flags & IGB_FLAG_PTP))
+ if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
return;
/* reset the tstamp_config */
--
2.8.2.820.gd1c5f70
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 2/5] igb: introduce IGB_PTP_OVERFLOW_CHECK flag
2016-05-18 21:04 [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Jacob Keller
@ 2016-05-18 21:04 ` Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in igb_ptp_init Jacob Keller
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Jacob Keller @ 2016-05-18 21:04 UTC (permalink / raw)
To: intel-wired-lan
Don't continue to use complex MAC type checks for handling various cases
where we have overflow check code. Make this code more obvious by
introducing a flag which is enabled for hardware that needs these
checks.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 1 +
drivers/net/ethernet/intel/igb/igb_ptp.c | 21 ++++++++-------------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 1e18a9eb16e0..38daaaba0cdb 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -477,6 +477,7 @@ struct igb_adapter {
/* flags controlling PTP/1588 function */
#define IGB_PTP_ENABLED BIT(0)
+#define IGB_PTP_OVERFLOW_CHECK BIT(1)
#define IGB_FLAG_HAS_MSI BIT(0)
#define IGB_FLAG_DCA_ENABLED BIT(1)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 88e3b40415a1..e2c494ad1be0 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1066,6 +1066,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
/* Dial the nominal frequency. */
wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
+ adapter->ptp_flags |= IGB_PTP_OVERFLOW_CHECK;
break;
case e1000_82580:
case e1000_i354:
@@ -1086,6 +1087,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->cc.shift = 0;
/* Enable the timer functions by clearing bit 31. */
wr32(E1000_TSAUXC, 0x0);
+ adapter->ptp_flags |= IGB_PTP_OVERFLOW_CHECK;
break;
case e1000_i210:
case e1000_i211:
@@ -1131,7 +1133,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
} else {
timecounter_init(&adapter->tc, &adapter->cc,
ktime_to_ns(ktime_get_real()));
+ }
+ if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK) {
INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
igb_ptp_overflow_check);
@@ -1168,20 +1172,11 @@ void igb_ptp_init(struct igb_adapter *adapter)
**/
void igb_ptp_stop(struct igb_adapter *adapter)
{
- switch (adapter->hw.mac.type) {
- case e1000_82576:
- case e1000_82580:
- case e1000_i354:
- case e1000_i350:
- cancel_delayed_work_sync(&adapter->ptp_overflow_work);
- break;
- case e1000_i210:
- case e1000_i211:
- /* No delayed work to cancel. */
- break;
- default:
+ if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
return;
- }
+
+ if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
+ cancel_delayed_work_sync(&adapter->ptp_overflow_work);
cancel_work_sync(&adapter->ptp_tx_work);
if (adapter->ptp_tx_skb) {
--
2.8.2.820.gd1c5f70
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in igb_ptp_init
2016-05-18 21:04 [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 2/5] igb: introduce IGB_PTP_OVERFLOW_CHECK flag Jacob Keller
@ 2016-05-18 21:04 ` Jacob Keller
2016-05-24 2:31 ` Brown, Aaron F
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 4/5] igb: implement igb_ptp_suspend Jacob Keller
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Jacob Keller @ 2016-05-18 21:04 UTC (permalink / raw)
To: intel-wired-lan
Modify igb_ptp_init to take advantage of igb_ptp_reset, and remove
duplicated work that was occurring in both igb_ptp_reset and
igb_ptp_init.
In total, resetting the TSAUXC register, and resetting the system time
both happen in igb_ptp_reset already. igb_ptp_reset now also takes care
of starting the delayed work item for overflow checks, as well.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/igb/igb_ptp.c | 45 ++++++++++++--------------------
1 file changed, 16 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index e2c494ad1be0..dae0dd0d7dc5 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1042,6 +1042,13 @@ int igb_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr)
-EFAULT : 0;
}
+/**
+ * igb_ptp_init - Initialize PTP functionality
+ * @adapter: Board private structure
+ *
+ * This function is called at device probe to initialize the PTP
+ * functionality.
+ */
void igb_ptp_init(struct igb_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
@@ -1064,8 +1071,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->cc.mask = CYCLECOUNTER_MASK(64);
adapter->cc.mult = 1;
adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
- /* Dial the nominal frequency. */
- wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
adapter->ptp_flags |= IGB_PTP_OVERFLOW_CHECK;
break;
case e1000_82580:
@@ -1085,8 +1090,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->cc.mask = CYCLECOUNTER_MASK(IGB_NBITS_82580);
adapter->cc.mult = 1;
adapter->cc.shift = 0;
- /* Enable the timer functions by clearing bit 31. */
- wr32(E1000_TSAUXC, 0x0);
adapter->ptp_flags |= IGB_PTP_OVERFLOW_CHECK;
break;
case e1000_i210:
@@ -1112,46 +1115,24 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->ptp_caps.settime64 = igb_ptp_settime_i210;
adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
adapter->ptp_caps.verify = igb_ptp_verify_pin;
- /* Enable the timer functions by clearing bit 31. */
- wr32(E1000_TSAUXC, 0x0);
break;
default:
adapter->ptp_clock = NULL;
return;
}
- wrfl();
-
spin_lock_init(&adapter->tmreg_lock);
INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
- /* Initialize the clock and overflow work for devices that need it. */
- if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211)) {
- struct timespec64 ts = ktime_to_timespec64(ktime_get_real());
-
- igb_ptp_settime_i210(&adapter->ptp_caps, &ts);
- } else {
- timecounter_init(&adapter->tc, &adapter->cc,
- ktime_to_ns(ktime_get_real()));
- }
-
- if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK) {
+ if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
igb_ptp_overflow_check);
- schedule_delayed_work(&adapter->ptp_overflow_work,
- IGB_SYSTIM_OVERFLOW_PERIOD);
- }
-
- /* Initialize the time sync interrupts for devices that support it. */
- if (hw->mac.type >= e1000_82580) {
- wr32(E1000_TSIM, TSYNC_INTERRUPTS);
- wr32(E1000_IMS, E1000_IMS_TS);
- }
-
adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
+ igb_ptp_reset(adapter);
+
adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
&adapter->pdev->dev);
if (IS_ERR(adapter->ptp_clock)) {
@@ -1243,4 +1224,10 @@ void igb_ptp_reset(struct igb_adapter *adapter)
}
out:
spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
+
+ wrfl();
+
+ if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
+ schedule_delayed_work(&adapter->ptp_overflow_work,
+ IGB_SYSTIM_OVERFLOW_PERIOD);
}
--
2.8.2.820.gd1c5f70
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 4/5] igb: implement igb_ptp_suspend
2016-05-18 21:04 [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 2/5] igb: introduce IGB_PTP_OVERFLOW_CHECK flag Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in igb_ptp_init Jacob Keller
@ 2016-05-18 21:04 ` Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 5/5] igb: call igb_ptp_suspend during suspend/resume cycle Jacob Keller
2016-05-24 2:30 ` [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Brown, Aaron F
4 siblings, 0 replies; 9+ messages in thread
From: Jacob Keller @ 2016-05-18 21:04 UTC (permalink / raw)
To: intel-wired-lan
Make igb_ptp_stop take advantage of this new function to reduce code
duplication.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 1 +
drivers/net/ethernet/intel/igb/igb_ptp.c | 22 +++++++++++++++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 38daaaba0cdb..5387b3a96489 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -550,6 +550,7 @@ void igb_set_fw_version(struct igb_adapter *);
void igb_ptp_init(struct igb_adapter *adapter);
void igb_ptp_stop(struct igb_adapter *adapter);
void igb_ptp_reset(struct igb_adapter *adapter);
+void igb_ptp_suspend(struct igb_adapter *adapter);
void igb_ptp_rx_hang(struct igb_adapter *adapter);
void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector, struct sk_buff *skb);
void igb_ptp_rx_pktstamp(struct igb_q_vector *q_vector, unsigned char *va,
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index dae0dd0d7dc5..e55f95e206be 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1146,12 +1146,13 @@ void igb_ptp_init(struct igb_adapter *adapter)
}
/**
- * igb_ptp_stop - Disable PTP device and stop the overflow check.
- * @adapter: Board private structure.
+ * igb_ptp_suspend - Disable PTP work items and prepare for suspend
+ * @adapter: Board private structure
*
- * This function stops the PTP support and cancels the delayed work.
- **/
-void igb_ptp_stop(struct igb_adapter *adapter)
+ * This function stops the overflow check work and PTP Tx timestamp work, and
+ * will prepare the device for OS suspend.
+ */
+void igb_ptp_suspend(struct igb_adapter *adapter)
{
if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
return;
@@ -1165,6 +1166,17 @@ void igb_ptp_stop(struct igb_adapter *adapter)
adapter->ptp_tx_skb = NULL;
clear_bit_unlock(__IGB_PTP_TX_IN_PROGRESS, &adapter->state);
}
+}
+
+/**
+ * igb_ptp_stop - Disable PTP device and stop the overflow check.
+ * @adapter: Board private structure.
+ *
+ * This function stops the PTP support and cancels the delayed work.
+ **/
+void igb_ptp_stop(struct igb_adapter *adapter)
+{
+ igb_ptp_suspend(adapter);
if (adapter->ptp_clock) {
ptp_clock_unregister(adapter->ptp_clock);
--
2.8.2.820.gd1c5f70
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 5/5] igb: call igb_ptp_suspend during suspend/resume cycle
2016-05-18 21:04 [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Jacob Keller
` (2 preceding siblings ...)
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 4/5] igb: implement igb_ptp_suspend Jacob Keller
@ 2016-05-18 21:04 ` Jacob Keller
2016-05-24 2:30 ` [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Brown, Aaron F
4 siblings, 0 replies; 9+ messages in thread
From: Jacob Keller @ 2016-05-18 21:04 UTC (permalink / raw)
To: intel-wired-lan
Properly stop the extra workqueue items and ensure that we resume
cleanly. This is better than using igb_ptp_init and igb_ptp_stop since
these functions destroy the PHC device, which will cause other problems
if we do so. Since igb_ptp_reset now re-schedules the work-queue item we
don't need an equivalent igb_ptp_resume in the resume workflow.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 21727692bef6..b7258402cb59 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7527,6 +7527,8 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake,
if (netif_running(netdev))
__igb_close(netdev, true);
+ igb_ptp_suspend(adapter);
+
igb_clear_interrupt_scheme(adapter);
#ifdef CONFIG_PM
--
2.8.2.820.gd1c5f70
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP
2016-05-18 21:04 [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Jacob Keller
` (3 preceding siblings ...)
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 5/5] igb: call igb_ptp_suspend during suspend/resume cycle Jacob Keller
@ 2016-05-24 2:30 ` Brown, Aaron F
2016-05-24 18:37 ` Keller, Jacob E
2016-05-24 20:50 ` Keller, Jacob E
4 siblings, 2 replies; 9+ messages in thread
From: Brown, Aaron F @ 2016-05-24 2:30 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Jacob Keller
> Sent: Wednesday, May 18, 2016 2:04 PM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Subject: [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable
> and use it to replace IGB_FLAG_PTP
>
> Upcoming patches will introduce new PTP specific flags. To avoid
> cluttering the normal flags variable, introduce PTP specific "ptp_flags"
> variable for this purpose, and move IGB_FLAG_PTP to become
> IGB_PTP_ENABLED.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/igb/igb.h | 5 ++++-
> drivers/net/ethernet/intel/igb/igb_ptp.c | 8 ++++----
> 2 files changed, 8 insertions(+), 5 deletions(-)
This series seemed completely fine for regression, but once I started testing ptp I quickly ran into several bugs. One showed up with this patch, the other (and more serious one) shows up in patch 3/5.
With just this patch from the series applied an i211 and a dual port 82580 [maybe more I only ran just this patch against those parts] running `ptp4l -m -I ethX` starts getting a number of "clearing Rx timestamp" messages intermixed with the timestamp display. This is very consistent, one RX timestamp hang message for every 6 updates to ptp shown.
Patch 3/5 causes more serious problems. On the i211 the
- Panic when ptp4l is started on dual port 82580 (Bartonville ~ either port) Not captured to logs and on system w/o usable serial port ~ but have snapshot of screen.
Having 3/5 igb: re-use igb_ptp_reset in igb_ptp_init, or later on top of queue causes more serious issues:
- The i211 stops getting / displaying timestamp info, rather it displays "received SYNC without timestamp" along with the frequent "clearing RX timestamp hang" messages introduced with 1/5.
- The dual port 82580, however, gets a panic on the screen the moment ptp4l is launched against it. Seems to be a spinlock causing a Kernel panic - not syncing: Fatal exception in interrupt. The trace does not get captured to logs and this particular box does not have a working serial port to capture the trace via a serial console. I do have a bitmap screen shot (I'll forward to Jacob separately) and will try to get and capture the trace on another machine.
With the entire series applied I get the same behavior as with 3/5. I have run that on a few more machines and it appears the Kernel Panic / freeze also occurs with i350 parts so I should be able to capture the trace on a serial console with one of the i350s tomorrow.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in igb_ptp_init
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in igb_ptp_init Jacob Keller
@ 2016-05-24 2:31 ` Brown, Aaron F
0 siblings, 0 replies; 9+ messages in thread
From: Brown, Aaron F @ 2016-05-24 2:31 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Jacob Keller
> Sent: Wednesday, May 18, 2016 2:04 PM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Subject: [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in
> igb_ptp_init
>
> Modify igb_ptp_init to take advantage of igb_ptp_reset, and remove
> duplicated work that was occurring in both igb_ptp_reset and
> igb_ptp_init.
>
> In total, resetting the TSAUXC register, and resetting the system time
> both happen in igb_ptp_reset already. igb_ptp_reset now also takes care
> of starting the delayed work item for overflow checks, as well.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/igb/igb_ptp.c | 45 ++++++++++++--------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
This is the same message I sent with 1/5 of this series, just adding it to this message to get the info captured with this patch in patchwork.
This series seemed completely fine for regression, but once I started testing ptp I quickly ran into several bugs. One showed up with this patch, the other (and more serious one) shows up in patch 3/5.
With just this patch from the series applied an i211 and a dual port 82580 [maybe more I only ran just this patch against those parts] running `ptp4l -m -I ethX` starts getting a number of "clearing Rx timestamp" messages intermixed with the timestamp display. This is very consistent, one RX timestamp hang message for every 6 updates to ptp shown.
Patch 3/5 causes more serious problems. On the i211 the
- Panic when ptp4l is started on dual port 82580 (Bartonville ~ either port) Not captured to logs and on system w/o usable serial port ~ but have snapshot of screen.
Having 3/5 igb: re-use igb_ptp_reset in igb_ptp_init, or later on top of queue causes more serious issues:
- The i211 stops getting / displaying timestamp info, rather it displays "received SYNC without timestamp" along with the frequent "clearing RX timestamp hang" messages introduced with 1/5.
- The dual port 82580, however, gets a panic on the screen the moment ptp4l is launched against it. Seems to be a spinlock causing a Kernel panic - not syncing: Fatal exception in interrupt. The trace does not get captured to logs and this particular box does not have a working serial port to capture the trace via a serial console. I do have a bitmap screen shot (I'll forward to Jacob separately) and will try to get and capture the trace on another machine.
With the entire series applied I get the same behavior as with 3/5. I have run that on a few more machines and it appears the Kernel Panic / freeze also occurs with i350 parts so I should be able to capture the trace on a serial console with one of the i350s tomorrow.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP
2016-05-24 2:30 ` [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Brown, Aaron F
@ 2016-05-24 18:37 ` Keller, Jacob E
2016-05-24 20:50 ` Keller, Jacob E
1 sibling, 0 replies; 9+ messages in thread
From: Keller, Jacob E @ 2016-05-24 18:37 UTC (permalink / raw)
To: intel-wired-lan
On Tue, 2016-05-24 at 02:30 +0000, Brown, Aaron F wrote:
> >
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.
> > org] On
> > Behalf Of Jacob Keller
> > Sent: Wednesday, May 18, 2016 2:04 PM
> > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> > Subject: [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags
> > variable
> > and use it to replace IGB_FLAG_PTP
> >
> > Upcoming patches will introduce new PTP specific flags. To avoid
> > cluttering the normal flags variable, introduce PTP specific
> > "ptp_flags"
> > variable for this purpose, and move IGB_FLAG_PTP to become
> > IGB_PTP_ENABLED.
> >
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> > ?drivers/net/ethernet/intel/igb/igb.h?????| 5 ++++-
> > ?drivers/net/ethernet/intel/igb/igb_ptp.c | 8 ++++----
> > ?2 files changed, 8 insertions(+), 5 deletions(-)
> This series seemed completely fine for regression, but once I started
> testing ptp I quickly ran into several bugs.??One showed up with this
> patch, the other (and more serious one) shows up in patch 3/5.
>
> With just this patch from the series applied an i211 and a dual port
> 82580 [maybe more I only ran just this patch against those parts]
> running `ptp4l -m -I ethX` starts getting a number of "clearing Rx
> timestamp" messages intermixed with the timestamp display.??This is
> very consistent, one RX timestamp hang??message for every 6 updates
> to ptp shown.
>
> Patch 3/5 causes more serious problems.??On the i211 the?
> - Panic when ptp4l is started on dual port 82580 (Bartonville ~
> either port)??Not captured to logs and on system w/o usable serial
> port ~ but have snapshot of screen.
>
> Having 3/5 igb: re-use igb_ptp_reset in igb_ptp_init, or later on top
> of queue causes more serious issues:
> - The i211 stops getting / displaying timestamp info, rather it
> displays "received SYNC without timestamp" along with the frequent
> "clearing RX timestamp hang" messages introduced with 1/5.
> - The dual port 82580, however, gets a panic on the screen the moment
> ptp4l is launched against it.??Seems to be a spinlock causing a
> Kernel panic - not syncing: Fatal exception in interrupt.??The trace
> does not get captured to logs and this particular box does not have a
> working serial port to capture the trace via a serial console.??I do
> have a bitmap screen shot (I'll forward to Jacob separately) and will
> try to get and capture the trace on another machine.
>
> With the entire series applied I get the same behavior as with
> 3/5.??I have run that on a few more machines and it appears the
> Kernel Panic / freeze also occurs with i350 parts so I should be able
> to capture the trace on a serial console with one of the i350s
> tomorrow.
Oops. Thanks. I'll take a look.
Regards,
Jake
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP
2016-05-24 2:30 ` [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Brown, Aaron F
2016-05-24 18:37 ` Keller, Jacob E
@ 2016-05-24 20:50 ` Keller, Jacob E
1 sibling, 0 replies; 9+ messages in thread
From: Keller, Jacob E @ 2016-05-24 20:50 UTC (permalink / raw)
To: intel-wired-lan
On Tue, 2016-05-24 at 02:30 +0000, Brown, Aaron F wrote:
> >
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.
> > org] On
> > Behalf Of Jacob Keller
> > Sent: Wednesday, May 18, 2016 2:04 PM
> > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> > Subject: [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags
> > variable
> > and use it to replace IGB_FLAG_PTP
> >
> > Upcoming patches will introduce new PTP specific flags. To avoid
> > cluttering the normal flags variable, introduce PTP specific
> > "ptp_flags"
> > variable for this purpose, and move IGB_FLAG_PTP to become
> > IGB_PTP_ENABLED.
> >
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> > ?drivers/net/ethernet/intel/igb/igb.h?????| 5 ++++-
> > ?drivers/net/ethernet/intel/igb/igb_ptp.c | 8 ++++----
> > ?2 files changed, 8 insertions(+), 5 deletions(-)
> This series seemed completely fine for regression, but once I started
> testing ptp I quickly ran into several bugs.??One showed up with this
> patch, the other (and more serious one) shows up in patch 3/5.
>
> With just this patch from the series applied an i211 and a dual port
> 82580 [maybe more I only ran just this patch against those parts]
> running `ptp4l -m -I ethX` starts getting a number of "clearing Rx
> timestamp" messages intermixed with the timestamp display.??This is
> very consistent, one RX timestamp hang??message for every 6 updates
> to ptp shown.
>
> Patch 3/5 causes more serious problems.??On the i211 the?
> - Panic when ptp4l is started on dual port 82580 (Bartonville ~
> either port)??Not captured to logs and on system w/o usable serial
> port ~ but have snapshot of screen.
>
> Having 3/5 igb: re-use igb_ptp_reset in igb_ptp_init, or later on top
> of queue causes more serious issues:
> - The i211 stops getting / displaying timestamp info, rather it
> displays "received SYNC without timestamp" along with the frequent
> "clearing RX timestamp hang" messages introduced with 1/5.
> - The dual port 82580, however, gets a panic on the screen the moment
> ptp4l is launched against it.??Seems to be a spinlock causing a
> Kernel panic - not syncing: Fatal exception in interrupt.??The trace
> does not get captured to logs and this particular box does not have a
> working serial port to capture the trace via a serial console.??I do
> have a bitmap screen shot (I'll forward to Jacob separately) and will
> try to get and capture the trace on another machine.
>
> With the entire series applied I get the same behavior as with
> 3/5.??I have run that on a few more machines and it appears the
> Kernel Panic / freeze also occurs with i350 parts so I should be able
> to capture the trace on a serial console with one of the i350s
> tomorrow.
I'll send a v4 shortly, I think I have it resolved.
Thanks,
Jake
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-05-24 20:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-18 21:04 [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 2/5] igb: introduce IGB_PTP_OVERFLOW_CHECK flag Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 3/5] igb: re-use igb_ptp_reset in igb_ptp_init Jacob Keller
2016-05-24 2:31 ` Brown, Aaron F
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 4/5] igb: implement igb_ptp_suspend Jacob Keller
2016-05-18 21:04 ` [Intel-wired-lan] [PATCH v3 5/5] igb: call igb_ptp_suspend during suspend/resume cycle Jacob Keller
2016-05-24 2:30 ` [Intel-wired-lan] [PATCH v3 1/5] igb: introduce ptp_flags variable and use it to replace IGB_FLAG_PTP Brown, Aaron F
2016-05-24 18:37 ` Keller, Jacob E
2016-05-24 20:50 ` Keller, Jacob E
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.