| Message ID | 5c892a6a13d120193fdbd2dd62e9604ae0d5d404.1416265321.git.richardcochran@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2014-11-18 at 00:06 +0100, Richard Cochran wrote: > The TSAUXC register has a number of different bits, one of which disables > the main clock function. Previously, the clock was re-enabled by clearing > the entire register. This patch changes the code to preserve the values > of the other bits in that register. > This is a step in the right direction, but won't fully work due to this being called after a MAC reset. > Signed-off-by: Richard Cochran <richardcochran@gmail.com> > --- > drivers/net/ethernet/intel/igb/igb_ptp.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c > index 794c139..ce57128 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ptp.c > +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c > @@ -905,6 +905,8 @@ void igb_ptp_stop(struct igb_adapter *adapter) > void igb_ptp_reset(struct igb_adapter *adapter) > { > struct e1000_hw *hw = &adapter->hw; > + unsigned long flags; > + u32 tsauxc; > > if (!(adapter->flags & IGB_FLAG_PTP)) > return; > @@ -923,7 +925,11 @@ void igb_ptp_reset(struct igb_adapter *adapter) > case e1000_i210: > case e1000_i211: > /* Enable the timer functions and interrupts. */ > - wr32(E1000_TSAUXC, 0x0); > + spin_lock_irqsave(&adapter->tmreg_lock, flags); > + tsauxc = rd32(E1000_TSAUXC); > + tsauxc &= ~TSAUXC_DISABLE; > + wr32(E1000_TSAUXC, tsauxc); > + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); This won't work. We call igb_ptp_reset directly after a HW reset (MAC reset). TSAUXC data is not maintained. We probably need to store driver state of the TSAUXC bits, and then re-enable them in this reset path. This code today is fine, because there was no other state we cared about in the path. That being said, I don't think this change breaks anything. I just think it will not solve the problem. Thanks, Jake > wr32(E1000_TSIM, TSYNC_INTERRUPTS); > wr32(E1000_IMS, E1000_IMS_TS); > break;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 794c139..ce57128 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -905,6 +905,8 @@ void igb_ptp_stop(struct igb_adapter *adapter) void igb_ptp_reset(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; + unsigned long flags; + u32 tsauxc; if (!(adapter->flags & IGB_FLAG_PTP)) return; @@ -923,7 +925,11 @@ void igb_ptp_reset(struct igb_adapter *adapter) case e1000_i210: case e1000_i211: /* Enable the timer functions and interrupts. */ - wr32(E1000_TSAUXC, 0x0); + spin_lock_irqsave(&adapter->tmreg_lock, flags); + tsauxc = rd32(E1000_TSAUXC); + tsauxc &= ~TSAUXC_DISABLE; + wr32(E1000_TSAUXC, tsauxc); + spin_unlock_irqrestore(&adapter->tmreg_lock, flags); wr32(E1000_TSIM, TSYNC_INTERRUPTS); wr32(E1000_IMS, E1000_IMS_TS); break;
The TSAUXC register has a number of different bits, one of which disables the main clock function. Previously, the clock was re-enabled by clearing the entire register. This patch changes the code to preserve the values of the other bits in that register. Signed-off-by: Richard Cochran <richardcochran@gmail.com> --- drivers/net/ethernet/intel/igb/igb_ptp.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)