Dear Vinicius,

Thank you for the patch.


Am 10.08.24 um 02:23 schrieb Vinicius Costa Gomes:
It was reported that 82580 NICs have a hardware bug that makes it
necessary to write into the TSICR (TimeSync Interrupt Cause) register
to clear it.

Were you able to verify that report by checking against some errata? Is Intel aware of the problem?

Add a conditional so only for 82580 we write into the TSICR register,
so we don't risk losing events for other models.

This (partially) reverts commit ee14cc9ea19b ("igb: Fix missing time sync 
events").

Fixes: ee14cc9ea19b ("igb: Fix missing time sync events")
Reported-by: Daiwei Li <daiwe...@gmail.com>
Closes: 
https://lore.kernel.org/intel-wired-lan/CAN0jFd1kO0MMtOh8N2Ztxn6f7vvDKp2h507sMryobkBKe=xk=w...@mail.gmail.com/
Signed-off-by: Vinicius Costa Gomes <vinicius.go...@intel.com>
---

@Daiwei Li, I don't have a 82580 handy, please confirm that the patch
fixes the issue you are having.

Please also add a description of the test case, and maybe the PCI vendor and device code of your network device.

  drivers/net/ethernet/intel/igb/igb_main.c | 27 ++++++++++++++++++-----
  1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index 11be39f435f3..edb34f67ae03 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6960,31 +6960,48 @@ static void igb_extts(struct igb_adapter *adapter, int 
tsintr_tt)
  static void igb_tsync_interrupt(struct igb_adapter *adapter)
  {
        struct e1000_hw *hw = &adapter->hw;
-       u32 tsicr = rd32(E1000_TSICR);
+       u32 ack = 0, tsicr = rd32(E1000_TSICR);
        struct ptp_clock_event event;
if (tsicr & TSINTR_SYS_WRAP) {
                event.type = PTP_CLOCK_PPS;
                if (adapter->ptp_caps.pps)
                        ptp_clock_event(adapter->ptp_clock, &event);
+               ack |= TSINTR_SYS_WRAP;
        }
if (tsicr & E1000_TSICR_TXTS) {
                /* retrieve hardware timestamp */
                schedule_work(&adapter->ptp_tx_work);
+               ack |= E1000_TSICR_TXTS;
        }
- if (tsicr & TSINTR_TT0)
+       if (tsicr & TSINTR_TT0) {
                igb_perout(adapter, 0);
+               ack |= TSINTR_TT0;
+       }
- if (tsicr & TSINTR_TT1)
+       if (tsicr & TSINTR_TT1) {
                igb_perout(adapter, 1);
+               ack |= TSINTR_TT1;
+       }
- if (tsicr & TSINTR_AUTT0)
+       if (tsicr & TSINTR_AUTT0) {
                igb_extts(adapter, 0);
+               ack |= TSINTR_AUTT0;
+       }
- if (tsicr & TSINTR_AUTT1)
+       if (tsicr & TSINTR_AUTT1) {
                igb_extts(adapter, 1);
+               ack |= TSINTR_AUTT1;
+       }
+
+       if (hw->mac.type == e1000_82580) {
+               /* 82580 has a hardware bug that requires a explicit

a*n*

+                * write to clear the TimeSync interrupt cause.
+                */
+               wr32(E1000_TSICR, ack);
+       }

Is there a nicer way to write this, so `ack` is only assigned in case for the 82580?

  }
static irqreturn_t igb_msix_other(int irq, void *data)


Kind regards,

Paul

Reply via email to