On 26/05/2025 6:24PM, Vadim Fedorenko wrote:
>On 23/05/2025 16:58, Milena Olech wrote:
>> Add cross timestamp support through virtchnl mailbox messages and directly,
>> through PCIe BAR registers. Cross timestamping assumes that both system
>> time and device clock time values are cached simultaneously, what is
>> triggered by HW. Feature is enabled for both ARM and x86 archs.
>> 
>> Signed-off-by: Milena Olech <[email protected]>
>> Reviewed-by: Karol Kolacinski <[email protected]>
>> ---
>>   drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 138 ++++++++++++++++++
>>   drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  19 ++-
>>   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  55 ++++++-
>>   3 files changed, 210 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c 
>> b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> index cb46185da749..f8d320875eb7 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> @@ -42,6 +42,13 @@ void idpf_ptp_get_features_access(const struct 
>> idpf_adapter *adapter)
>>                                                         direct,
>>                                                         mailbox);
>>   
>> +    /* Get the cross timestamp */
>> +    direct = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME;
>> +    mailbox = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB;
>> +    ptp->get_cross_tstamp_access = idpf_ptp_get_access(adapter,
>> +                                                       direct,
>> +                                                       mailbox);
>> +
>>      /* Set the device clock time */
>>      direct = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>>      mailbox = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>> @@ -171,6 +178,129 @@ static int idpf_ptp_read_src_clk_reg(struct 
>> idpf_adapter *adapter, u64 *src_clk,
>>      return 0;
>>   }
>>   
>> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER) || IS_ENABLED(CONFIG_X86)
>> +/**
>> + * idpf_ptp_get_sync_device_time_direct - Get the cross time stamp values
>> + *                                    directly
>> + * @adapter: Driver specific private structure
>> + * @dev_time: 64bit main timer value
>> + * @sys_time: 64bit system time value
>> + */
>> +static void idpf_ptp_get_sync_device_time_direct(struct idpf_adapter 
>> *adapter,
>> +                                             u64 *dev_time, u64 *sys_time)
>> +{
>> +    u32 dev_time_lo, dev_time_hi, sys_time_lo, sys_time_hi;
>> +    struct idpf_ptp *ptp = adapter->ptp;
>> +
>> +    spin_lock(&ptp->read_dev_clk_lock);
>> +
>> +    idpf_ptp_enable_shtime(adapter);
>> +
>> +    dev_time_lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
>> +    dev_time_hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
>> +
>> +    sys_time_lo = readl(ptp->dev_clk_regs.sys_time_ns_l);
>> +    sys_time_hi = readl(ptp->dev_clk_regs.sys_time_ns_h);
>> +
>> +    *dev_time = (u64)dev_time_hi << 32 | dev_time_lo;
>> +    *sys_time = (u64)sys_time_hi << 32 | sys_time_lo;
>> +
>> +    spin_unlock(&ptp->read_dev_clk_lock);
>
>nit: I would say the calc part should be outside of spinlock scope.
>For this part of code it's just several CPU cycles, but still looks
>cleaner.
>

Sure, makes sense.
Will fix it in v2

>> +}
>> +
>> +/**
>> + * idpf_ptp_get_sync_device_time_mailbox - Get the cross time stamp values
>> + *                                     through mailbox
>> + * @adapter: Driver specific private structure
>> + * @dev_time: 64bit main timer value expressed in nanoseconds
>> + * @sys_time: 64bit system time value expressed in nanoseconds
>> + *
>> + * Return: a pair of cross timestamp values on success, -errno otherwise.
>> + */
>> +static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter 
>> *adapter,
>> +                                             u64 *dev_time, u64 *sys_time)
>> +{
>> +    struct idpf_ptp_dev_timers cross_time;
>> +    int err;
>> +
>> +    err = idpf_ptp_get_cross_time(adapter, &cross_time);
>> +    if (err)
>> +            return err;
>> +
>> +    *dev_time = cross_time.dev_clk_time_ns;
>> +    *sys_time = cross_time.sys_time_ns;
>> +
>> +    return err;
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_sync_device_time - Get the cross time stamp info
>> + * @device: Current device time
>> + * @system: System counter value read synchronously with device time
>> + * @ctx: Context provided by timekeeping code
>> + *
>> + * Return: the device and the system clocks time read simultaneously on 
>> success,
>> + * -errno otherwise.
>> + */
>> +static int idpf_ptp_get_sync_device_time(ktime_t *device,
>> +                                     struct system_counterval_t *system,
>> +                                     void *ctx)
>> +{
>> +    struct idpf_adapter *adapter = ctx;
>> +    u64 ns_time_dev, ns_time_sys;
>> +    int err;
>> +
>> +    switch (adapter->ptp->get_cross_tstamp_access) {
>> +    case IDPF_PTP_NONE:
>> +            return -EOPNOTSUPP;
>> +    case IDPF_PTP_DIRECT:
>> +            idpf_ptp_get_sync_device_time_direct(adapter, &ns_time_dev,
>> +                                                 &ns_time_sys);
>> +            break;
>> +    case IDPF_PTP_MAILBOX:
>> +            err = idpf_ptp_get_sync_device_time_mailbox(adapter,
>> +                                                        &ns_time_dev,
>> +                                                        &ns_time_sys);
>> +            if (err)
>> +                    return err;
>> +            break;
>> +    default:
>> +            return -EOPNOTSUPP;
>> +    }
>> +
>> +    *device = ns_to_ktime(ns_time_dev);
>> +
>> +#if IS_ENABLED(CONFIG_X86)
>> +    system->cs_id = CSID_X86_ART;
>> +#else
>> +    system->cs_id = CSID_ARM_ARCH_COUNTER;
>> +#endif /* CONFIG_X86 */
>
>AFAIR, IS_ENABLED() can be used in normal code flow, so this code can be 
>written in one line (well, 2 lines actually) without explicit macro:

Ok!

>
>         system->cs_id = IS_ENABLED(CONFIG_X86) ? CSID_X86_ART
>                                                : CSID_ARM_ARCH_COUNTER
>
>> +    system->cycles = ns_time_sys;
>> +    system->use_nsecs = true;
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_crosststamp - Capture a device cross timestamp
>> + * @info: the driver's PTP info structure
>> + * @cts: The memory to fill the cross timestamp info
>> + *
>> + * Capture a cross timestamp between the system time and the device PTP 
>> hardware
>> + * clock.
>> + *
>> + * Return: cross timestamp value on success, -errno on failure.
>> + */
>> +static int idpf_ptp_get_crosststamp(struct ptp_clock_info *info,
>> +                                struct system_device_crosststamp *cts)
>> +{
>> +    struct idpf_adapter *adapter = idpf_ptp_info_to_adapter(info);
>> +
>> +    return get_device_system_crosststamp(idpf_ptp_get_sync_device_time,
>> +                                         adapter, NULL, cts);
>> +}
>> +#endif /* CONFIG_ARM_ARCH_TIMER || CONFIG_X86 */
>> +
>>   /**
>>    * idpf_ptp_gettimex64 - Get the time of the clock
>>    * @info: the driver's PTP info structure
>> @@ -664,6 +794,14 @@ static void idpf_ptp_set_caps(const struct idpf_adapter 
>> *adapter)
>>      info->verify = idpf_ptp_verify_pin;
>>      info->enable = idpf_ptp_gpio_enable;
>>      info->do_aux_work = idpf_ptp_do_aux_work;
>> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER)
>> +    info->getcrosststamp = idpf_ptp_get_crosststamp;
>> +#elif IS_ENABLED(CONFIG_X86)
>> +    if (pcie_ptm_enabled(adapter->pdev) &&
>> +        boot_cpu_has(X86_FEATURE_ART) &&
>> +        boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
>> +            info->getcrosststamp = idpf_ptp_get_crosststamp;
>> +#endif /* CONFIG_ARM_ARCH_TIMER */
>>   }
>>   
>>   /**
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h 
>> b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> index a876749d6116..cd19f65f9fff 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> @@ -21,6 +21,8 @@ struct idpf_ptp_cmd {
>>    * @dev_clk_ns_h: high part of the device clock register
>>    * @phy_clk_ns_l: low part of the PHY clock register
>>    * @phy_clk_ns_h: high part of the PHY clock register
>> + * @sys_time_ns_l: low part of the system time register
>> + * @sys_time_ns_h: high part of the system time register
>>    * @incval_l: low part of the increment value register
>>    * @incval_h: high part of the increment value register
>>    * @shadj_l: low part of the shadow adjust register
>> @@ -42,6 +44,10 @@ struct idpf_ptp_dev_clk_regs {
>>      void __iomem *phy_clk_ns_l;
>>      void __iomem *phy_clk_ns_h;
>>   
>> +    /* System time */
>> +    void __iomem *sys_time_ns_l;
>> +    void __iomem *sys_time_ns_h;
>> +
>>      /* Main timer adjustments */
>>      void __iomem *incval_l;
>>      void __iomem *incval_h;
>> @@ -162,6 +168,7 @@ struct idpf_ptp_vport_tx_tstamp_caps {
>>    * @dev_clk_regs: the set of registers to access the device clock
>>    * @caps: PTP capabilities negotiated with the Control Plane
>>    * @get_dev_clk_time_access: access type for getting the device clock time
>> + * @get_cross_tstamp_access: access type for the cross timestamping
>>    * @set_dev_clk_time_access: access type for setting the device clock time
>>    * @adj_dev_clk_time_access: access type for the adjusting the device clock
>>    * @tx_tstamp_access: access type for the Tx timestamp value read
>> @@ -182,10 +189,11 @@ struct idpf_ptp {
>>      struct idpf_ptp_dev_clk_regs dev_clk_regs;
>>      u32 caps;
>>      enum idpf_ptp_access get_dev_clk_time_access:2;
>> +    enum idpf_ptp_access get_cross_tstamp_access:2;
>>      enum idpf_ptp_access set_dev_clk_time_access:2;
>>      enum idpf_ptp_access adj_dev_clk_time_access:2;
>>      enum idpf_ptp_access tx_tstamp_access:2;
>> -    u8 rsv;
>> +    u8 rsv:6;
>>      struct idpf_ptp_secondary_mbx secondary_mbx;
>>      spinlock_t read_dev_clk_lock;
>>   };
>> @@ -264,6 +272,8 @@ void idpf_ptp_get_features_access(const struct 
>> idpf_adapter *adapter);
>>   bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
>>   int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>                            struct idpf_ptp_dev_timers *dev_clk_time);
>> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +                        struct idpf_ptp_dev_timers *cross_time);
>>   int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
>>   int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
>>   int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
>> @@ -305,6 +315,13 @@ idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>      return -EOPNOTSUPP;
>>   }
>>   
>> +static inline int
>> +idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +                    struct idpf_ptp_dev_timers *cross_time)
>> +{
>> +    return -EOPNOTSUPP;
>> +}
>> +
>>   static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
>>                                          u64 time)
>>   {
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c 
>> b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> index bdcc54a5fb56..4f1fb0cefe51 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> @@ -30,6 +30,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>              .send_buf.iov_len = sizeof(send_ptp_caps_msg),
>>              .timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>>      };
>> +    struct virtchnl2_ptp_cross_time_reg_offsets cross_tstamp_offsets;
>>      struct virtchnl2_ptp_clk_adj_reg_offsets clk_adj_offsets;
>>      struct virtchnl2_ptp_clk_reg_offsets clock_offsets;
>>      struct idpf_ptp_secondary_mbx *scnd_mbx;
>> @@ -71,7 +72,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   
>>      access_type = ptp->get_dev_clk_time_access;
>>      if (access_type != IDPF_PTP_DIRECT)
>> -            goto discipline_clock;
>> +            goto cross_tstamp;
>>   
>>      clock_offsets = recv_ptp_caps_msg->clk_offsets;
>>   
>> @@ -90,6 +91,22 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>      temp_offset = le32_to_cpu(clock_offsets.cmd_sync_trigger);
>>      ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>>   
>> +cross_tstamp:
>> +    access_type = ptp->get_cross_tstamp_access;
>> +    if (access_type != IDPF_PTP_DIRECT)
>> +            goto discipline_clock;
>> +
>> +    cross_tstamp_offsets = recv_ptp_caps_msg->cross_time_offsets;
>> +
>> +    temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_l);
>> +    ptp->dev_clk_regs.sys_time_ns_l = idpf_get_reg_addr(adapter,
>> +                                                        temp_offset);
>> +    temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_h);
>> +    ptp->dev_clk_regs.sys_time_ns_h = idpf_get_reg_addr(adapter,
>> +                                                        temp_offset);
>> +    temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
>> +    ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>> +
>>   discipline_clock:
>>      access_type = ptp->adj_dev_clk_time_access;
>>      if (access_type != IDPF_PTP_DIRECT)
>> @@ -162,6 +179,42 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter 
>> *adapter,
>>      return 0;
>>   }
>>   
>> +/**
>> + * idpf_ptp_get_cross_time - Send virtchnl get cross time message
>> + * @adapter: Driver specific private structure
>> + * @cross_time: Pointer to the device clock structure where the value is set
>> + *
>> + * Send virtchnl get cross time message to get the time of the clock and the
>> + * system time.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +                        struct idpf_ptp_dev_timers *cross_time)
>> +{
>> +    struct virtchnl2_ptp_get_cross_time cross_time_msg;
>> +    struct idpf_vc_xn_params xn_params = {
>> +            .vc_op = VIRTCHNL2_OP_PTP_GET_CROSS_TIME,
>> +            .send_buf.iov_base = &cross_time_msg,
>> +            .send_buf.iov_len = sizeof(cross_time_msg),
>> +            .recv_buf.iov_base = &cross_time_msg,
>> +            .recv_buf.iov_len = sizeof(cross_time_msg),
>> +            .timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>> +    };
>> +    int reply_sz;
>> +
>> +    reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
>> +    if (reply_sz < 0)
>> +            return reply_sz;
>> +    if (reply_sz != sizeof(cross_time_msg))
>> +            return -EIO;
>> +
>> +    cross_time->dev_clk_time_ns = le64_to_cpu(cross_time_msg.dev_time_ns);
>> +    cross_time->sys_time_ns = le64_to_cpu(cross_time_msg.sys_time_ns);
>> +
>> +    return 0;
>> +}
>> +
>>   /**
>>    * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
>>    * @adapter: Driver specific private structure

Reply via email to