> -----Original Message-----
> From: Miroslav Lichvar [mailto:mlich...@redhat.com]
> Sent: Friday, November 09, 2018 2:15 AM
> To: netdev@vger.kernel.org
> Cc: Richard Cochran <richardcoch...@gmail.com>; Keller, Jacob E
> <jacob.e.kel...@intel.com>; Miroslav Lichvar <mlich...@redhat.com>; Kirsher,
> Jeffrey T <jeffrey.t.kirs...@intel.com>
> Subject: [PATCH net-next 7/8] ixgbe: extend PTP gettime function to read
> system
> clock
>
> -static int ixgbe_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64
> *ts)
> +static int ixgbe_ptp_gettimex(struct ptp_clock_info *ptp,
> + struct timespec64 *ts,
> + struct ptp_system_timestamp *sts)
> {
> struct ixgbe_adapter *adapter =
> container_of(ptp, struct ixgbe_adapter, ptp_caps);
> + struct ixgbe_hw *hw = &adapter->hw;
> unsigned long flags;
> - u64 ns;
> + u64 ns, stamp;
>
> spin_lock_irqsave(&adapter->tmreg_lock, flags);
> - ns = timecounter_read(&adapter->hw_tc);
> +
> + switch (adapter->hw.mac.type) {
> + case ixgbe_mac_X550:
> + case ixgbe_mac_X550EM_x:
> + case ixgbe_mac_x550em_a:
> + /* Upper 32 bits represent billions of cycles, lower 32 bits
> + * represent cycles. However, we use timespec64_to_ns for the
> + * correct math even though the units haven't been corrected
> + * yet.
> + */
> + ptp_read_system_prets(sts);
> + IXGBE_READ_REG(hw, IXGBE_SYSTIMR);
> + ptp_read_system_postts(sts);
> + ts->tv_nsec = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
> + ts->tv_sec = IXGBE_READ_REG(hw, IXGBE_SYSTIMH);
> + stamp = timespec64_to_ns(ts);
> + break;
> + default:
> + ptp_read_system_prets(sts);
> + stamp = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
> + ptp_read_system_postts(sts);
> + stamp |= (u64)IXGBE_READ_REG(hw, IXGBE_SYSTIMH) << 32;
> + break;
> + }
> +
> + ns = timecounter_cyc2time(&adapter->hw_tc, stamp);
> +
At first, I was confused by this entire block of code, but then realized that
we can't update the timecounter_read method, so we instead have to break this
out so that our calls to ptp_read_system_prets() and ptp_read_system_postts()
can be added between the register reads.
Ok, that makes sense.
> spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
>
> *ts = ns_to_timespec64(ns);
> @@ -567,10 +597,14 @@ void ixgbe_ptp_overflow_check(struct ixgbe_adapter
> *adapter)
> {
> bool timeout = time_is_before_jiffies(adapter->last_overflow_check +
> IXGBE_OVERFLOW_PERIOD);
> - struct timespec64 ts;
> + unsigned long flags;
>
> if (timeout) {
> - ixgbe_ptp_gettime(&adapter->ptp_caps, &ts);
> + /* Update the timecounter */
> + spin_lock_irqsave(&adapter->tmreg_lock, flags);
> + timecounter_read(&adapter->hw_tc);
> + spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
> +
This also explains this change where we now have to update the timecounter
during the overflow check.
Ok, this makes sense to me.
Thanks,
Jake
