* [PATCH net-next v3 0/2] ptp: add control over HW timestamp latch point
@ 2024-11-06 1:07 Arkadiusz Kubalewski
2024-11-06 1:07 ` [PATCH net-next v3 1/2] " Arkadiusz Kubalewski
2024-11-06 1:07 ` [PATCH net-next v3 2/2] ice: " Arkadiusz Kubalewski
0 siblings, 2 replies; 8+ messages in thread
From: Arkadiusz Kubalewski @ 2024-11-06 1:07 UTC (permalink / raw)
To: netdev, linux-kernel, intel-wired-lan
Cc: anthony.l.nguyen, przemyslaw.kitszel, davem, edumazet, kuba,
pabeni, richardcochran, Arkadiusz Kubalewski
HW support of ptp/timesync solutions in network PHY chips can be
achieved with two different approaches, the timestamp maybe latched
either in the beginning or after the Start of Frame Delimiter (SFD) [1].
Allow ptp device drivers to provide user with control over the timestamp
latch point.
[1] https://www.ieee802.org/3/cx/public/april20/tse_3cx_01_0420.pdf
v3:
- move new enum ptp_ts_point to uapi ptp_clock.h and add NONE value to
indicate that the timestamp latch point was not provided by the HW,
allow further changes to ethtool netlink interface exstensions.
Arkadiusz Kubalewski (2):
ptp: add control over HW timestamp latch point
ice: ptp: add control over HW timestamp latch point
Documentation/ABI/testing/sysfs-ptp | 12 +++++
drivers/net/ethernet/intel/ice/ice_ptp.c | 44 +++++++++++++++
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 60 +++++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 2 +
drivers/ptp/ptp_sysfs.c | 44 +++++++++++++++
include/linux/ptp_clock_kernel.h | 12 +++++
include/uapi/linux/ptp_clock.h | 18 +++++++
7 files changed, 192 insertions(+)
base-commit: 0452a2d8b8b98a5b1a9139c1a9ed98bccee356cc
--
2.38.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v3 1/2] ptp: add control over HW timestamp latch point
2024-11-06 1:07 [PATCH net-next v3 0/2] ptp: add control over HW timestamp latch point Arkadiusz Kubalewski
@ 2024-11-06 1:07 ` Arkadiusz Kubalewski
2024-11-06 2:04 ` Jakub Kicinski
2024-11-06 17:45 ` Andrew Lunn
2024-11-06 1:07 ` [PATCH net-next v3 2/2] ice: " Arkadiusz Kubalewski
1 sibling, 2 replies; 8+ messages in thread
From: Arkadiusz Kubalewski @ 2024-11-06 1:07 UTC (permalink / raw)
To: netdev, linux-kernel, intel-wired-lan
Cc: anthony.l.nguyen, przemyslaw.kitszel, davem, edumazet, kuba,
pabeni, richardcochran, Arkadiusz Kubalewski, Aleksandr Loktionov
Currently HW support of ptp/timesync solutions in network PHY chips can be
implemented with two different approaches, the timestamp maybe latched
either at the beginning or after the Start of Frame Delimiter (SFD) [1].
Allow ptp device drivers to provide user with control over the HW
timestamp latch point with ptp sysfs ABI. Provide a new file under sysfs
ptp device (/sys/class/ptp/ptp<N>/ts_point). The file is available for the
user, if the device driver implements at least one of newly provided
callbacks. If the file is not provided the user shall find a PHY timestamp
latch point within the HW vendor specification.
The file is designed for root user/group access only, as the read for
regular user could impact performance of the ptp device.
Usage, examples:
** Obtain current state:
$ cat /sys/class/ptp/ptp<N>/ts_point
Command returns enum/integer:
* 1 - timestamp latched by PHY at the beginning of SFD,
* 2 - timestamp latched by PHY after the SFD,
* None - callback returns error to the user.
** Configure timestamp latch point at the beginning of SFD:
$ echo 1 > /sys/class/ptp/ptp<N>/ts_point
** Configure timestamp latch point after the SFD:
$ echo 2 > /sys/class/ptp/ptp<N>/ts_point
[1] https://www.ieee802.org/3/cx/public/april20/tse_3cx_01_0420.pdf
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
v3:
- max value of enum ptp_ts_point is also enumerated,
- move enum ptp_ts_point to uapi,
- add NONE value to enum ptp_ts_point, to make clear that value was
not provided, thus allow further extension of ethtool netlink.
---
Documentation/ABI/testing/sysfs-ptp | 12 ++++++++
drivers/ptp/ptp_sysfs.c | 44 +++++++++++++++++++++++++++++
include/linux/ptp_clock_kernel.h | 12 ++++++++
include/uapi/linux/ptp_clock.h | 18 ++++++++++++
4 files changed, 86 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index 9c317ac7c47a..063b3e20386e 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -140,3 +140,15 @@ Description:
PPS events to the Linux PPS subsystem. To enable PPS
events, write a "1" into the file. To disable events,
write a "0" into the file.
+
+What: /sys/class/ptp/ptp<N>/ts_point
+Date: October 2024
+Contact: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
+Description:
+ This file provides control over the point in time in
+ which the HW timestamp is latched. As specified in IEEE
+ 802.3cx, the latch point can be either at the beginning
+ or after the end of Start of Frame Delimiter (SFD).
+ Value "1" means the timestamp is latched at the
+ beginning of the SFD. Value "2" means that timestamp is
+ latched after the end of SFD.
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 6b1b8f57cd95..2f3f28edbbfd 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -28,6 +28,46 @@ static ssize_t max_phase_adjustment_show(struct device *dev,
}
static DEVICE_ATTR_RO(max_phase_adjustment);
+static ssize_t ts_point_show(struct device *dev, struct device_attribute *attr,
+ char *page)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ enum ptp_ts_point point;
+ int err;
+
+ if (!ptp->info->get_ts_point)
+ return -EOPNOTSUPP;
+ err = ptp->info->get_ts_point(ptp->info, &point);
+ if (err)
+ return err;
+
+ return sysfs_emit(page, "%d\n", point);
+}
+
+static ssize_t ts_point_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ enum ptp_ts_point point;
+ int err;
+ u8 val;
+
+ if (!ptp->info->set_ts_point)
+ return -EOPNOTSUPP;
+ if (kstrtou8(buf, 0, &val))
+ return -EINVAL;
+ if (val <= PTP_TS_POINT_NONE || val > PTP_TS_POINT_MAX)
+ return -EINVAL;
+ point = val;
+
+ err = ptp->info->set_ts_point(ptp->info, point);
+ if (err)
+ return err;
+
+ return count;
+}
+static DEVICE_ATTR(ts_point, 0660, ts_point_show, ts_point_store);
+
#define PTP_SHOW_INT(name, var) \
static ssize_t var##_show(struct device *dev, \
struct device_attribute *attr, char *page) \
@@ -335,6 +375,7 @@ static struct attribute *ptp_attrs[] = {
&dev_attr_pps_enable.attr,
&dev_attr_n_vclocks.attr,
&dev_attr_max_vclocks.attr,
+ &dev_attr_ts_point.attr,
NULL
};
@@ -363,6 +404,9 @@ static umode_t ptp_is_attribute_visible(struct kobject *kobj,
} else if (attr == &dev_attr_max_phase_adjustment.attr) {
if (!info->adjphase || !info->getmaxphase)
mode = 0;
+ } else if (attr == &dev_attr_ts_point.attr) {
+ if (!info->get_ts_point && !info->set_ts_point)
+ mode = 0;
}
return mode;
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index c892d22ce0a7..d48619c7c60a 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -159,6 +159,14 @@ struct ptp_system_timestamp {
* scheduling time (>=0) or negative value in case further
* scheduling is not required.
*
+ * @set_ts_point: Request change of timestamp latch point, as the timestamp
+ * could be latched at the beginning or after the end of start
+ * frame delimiter (SFD), as described in IEEE 802.3cx
+ * specification.
+ *
+ * @get_ts_point: Obtain the timestamp measurement latch point, counterpart of
+ * .set_ts_point() for getting currently configured value.
+ *
* Drivers should embed their ptp_clock_info within a private
* structure, obtaining a reference to it using container_of().
*
@@ -195,6 +203,10 @@ struct ptp_clock_info {
int (*verify)(struct ptp_clock_info *ptp, unsigned int pin,
enum ptp_pin_function func, unsigned int chan);
long (*do_aux_work)(struct ptp_clock_info *ptp);
+ int (*set_ts_point)(struct ptp_clock_info *ptp,
+ enum ptp_ts_point point);
+ int (*get_ts_point)(struct ptp_clock_info *ptp,
+ enum ptp_ts_point *point);
};
struct ptp_clock;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 18eefa6d93d6..11a9dad9db00 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -253,4 +253,22 @@ struct ptp_extts_event {
unsigned int rsv[2]; /* Reserved for future use. */
};
+/**
+ * enum ptp_ts_point - possible timestamp latch points (IEEE 802.3cx)
+ *
+ * @PTP_TS_POINT_NONE: no timestamp latch point was provided
+ * @PTP_TS_POINT_SFD: timestamp latched at the beginning of sending Start
+ * of Frame Delimiter (SFD)
+ * @PTP_TS_POINT_POST_SFD: timestamp latched after the end of sending Start
+ * of Frame Delimiter (SFD)
+ */
+enum ptp_ts_point {
+ PTP_TS_POINT_NONE = 0,
+ PTP_TS_POINT_SFD,
+ PTP_TS_POINT_POST_SFD,
+
+ /* private: */
+ PTP_TS_POINT_MAX
+};
+
#endif
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v3 2/2] ice: ptp: add control over HW timestamp latch point
2024-11-06 1:07 [PATCH net-next v3 0/2] ptp: add control over HW timestamp latch point Arkadiusz Kubalewski
2024-11-06 1:07 ` [PATCH net-next v3 1/2] " Arkadiusz Kubalewski
@ 2024-11-06 1:07 ` Arkadiusz Kubalewski
1 sibling, 0 replies; 8+ messages in thread
From: Arkadiusz Kubalewski @ 2024-11-06 1:07 UTC (permalink / raw)
To: netdev, linux-kernel, intel-wired-lan
Cc: anthony.l.nguyen, przemyslaw.kitszel, davem, edumazet, kuba,
pabeni, richardcochran, Arkadiusz Kubalewski, Aleksandr Loktionov
Allow user to control the latch point of ptp HW timestamps in E825
devices.
Usage, examples:
** Obtain current state:
$ cat /sys/class/net/eth<N>/device/ptp/ts_point
Command returns enum/integer:
* 1 - timestamp latched by PHY at the beginning of SFD,
* 2 - timestamp latched by PHY after the SFD,
* None - callback returns error to the user.
** Configure timestamp latch point at the beginning of SFD:
$ echo 1 > /sys/class/net/eth<N>/device/ptp/ts_point
** Configure timestamp latch point after the SFD:
$ echo 2 > /sys/class/net/eth<N>/device/ptp/ts_point
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
v3:
- improve readability, for "nothing to do" logic
- /s/PTP/ptp
- remove 'tx' from docs description
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 44 +++++++++++++++
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 60 +++++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 2 +
3 files changed, 106 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index a999fface272..c351c9707394 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2509,6 +2509,48 @@ static int ice_ptp_parse_sdp_entries(struct ice_pf *pf, __le16 *entries,
return 0;
}
+/**
+ * ice_get_ts_point - get the timestamp latch point
+ * @info: the driver's ptp info structure
+ * @point: returns the configured timestamp latch point
+ *
+ * Return: 0 on success, negative on failure.
+ */
+static int ice_get_ts_point(struct ptp_clock_info *info,
+ enum ptp_ts_point *point)
+{
+ struct ice_pf *pf = ptp_info_to_pf(info);
+ struct ice_hw *hw = &pf->hw;
+ int ret;
+
+ ice_ptp_lock(hw);
+ ret = ice_ptp_hw_ts_point_get(hw, point);
+ ice_ptp_unlock(hw);
+
+ return ret;
+}
+
+/**
+ * ice_set_ts_point - set the timestamp latch point
+ * @info: the driver's ptp info structure
+ * @point: requested timestamp latch point
+ *
+ * Return: 0 on success, negative on failure.
+ */
+static int ice_set_ts_point(struct ptp_clock_info *info,
+ enum ptp_ts_point point)
+{
+ struct ice_pf *pf = ptp_info_to_pf(info);
+ struct ice_hw *hw = &pf->hw;
+ int ret;
+
+ ice_ptp_lock(hw);
+ ret = ice_ptp_hw_ts_point_set(hw, point);
+ ice_ptp_unlock(hw);
+
+ return ret;
+}
+
/**
* ice_ptp_set_funcs_e82x - Set specialized functions for E82X support
* @pf: Board private structure
@@ -2529,6 +2571,8 @@ static void ice_ptp_set_funcs_e82x(struct ice_pf *pf)
if (ice_is_e825c(&pf->hw)) {
pf->ptp.ice_pin_desc = ice_pin_desc_e825c;
pf->ptp.info.n_pins = ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e825c);
+ pf->ptp.info.set_ts_point = ice_set_ts_point;
+ pf->ptp.info.get_ts_point = ice_get_ts_point;
} else {
pf->ptp.ice_pin_desc = ice_pin_desc_e82x;
pf->ptp.info.n_pins = ICE_PIN_DESC_ARR_LEN(ice_pin_desc_e82x);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index dfd49732bd5b..06c32f180932 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -6320,3 +6320,63 @@ int ice_cgu_get_output_pin_state_caps(struct ice_hw *hw, u8 pin_id,
return 0;
}
+
+/**
+ * ice_ptp_hw_ts_point_get - check if timestamps are latched on/post SFD
+ * @hw: pointer to the HW struct
+ * @point: return the configured timestamp latch point
+ *
+ * Verify if HW timestamping point is configured to latch at the beginning or
+ * post of SFD (Start of Frame Delimiter)
+ *
+ * Return: 0 on success, negative on error
+ */
+int ice_ptp_hw_ts_point_get(struct ice_hw *hw, enum ptp_ts_point *point)
+{
+ u8 port = hw->port_info->lport;
+ u32 val;
+ int err;
+
+ err = ice_read_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE, &val);
+ if (err)
+ return err;
+ if (val & PHY_MAC_XIF_TS_SFD_ENA_M)
+ *point = PTP_TS_POINT_SFD;
+ else
+ *point = PTP_TS_POINT_POST_SFD;
+
+ return err;
+}
+
+/**
+ * ice_ptp_hw_ts_point_set - configure timestamping on/post SFD
+ * @hw: pointer to the HW struct
+ * @point: requested timestamp latch point
+ *
+ * Configure timestamping to measure at the beginning/post SFD (Start of Frame
+ * Delimiter)
+ *
+ * Return: 0 on success, negative on error
+ */
+int ice_ptp_hw_ts_point_set(struct ice_hw *hw, enum ptp_ts_point point)
+{
+ u8 port = hw->port_info->lport;
+ int err, val;
+
+ err = ice_read_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE, &val);
+ if (err)
+ return err;
+ if ((val & PHY_MAC_XIF_TS_SFD_ENA_M) && point == PTP_TS_POINT_SFD)
+ return -EINVAL;
+ if (!(val & PHY_MAC_XIF_TS_SFD_ENA_M) &&
+ point == PTP_TS_POINT_POST_SFD)
+ return -EINVAL;
+ if (point == PTP_TS_POINT_SFD)
+ val |= PHY_MAC_XIF_TS_SFD_ENA_M;
+ else if (point == PTP_TS_POINT_POST_SFD)
+ val &= ~PHY_MAC_XIF_TS_SFD_ENA_M;
+ else
+ return -EINVAL;
+
+ return ice_write_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE, val);
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 47af7c5c79b8..5e4edaee063e 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -348,6 +348,8 @@ void ice_ptp_init_hw(struct ice_hw *hw);
int ice_get_phy_tx_tstamp_ready(struct ice_hw *hw, u8 block, u64 *tstamp_ready);
int ice_ptp_one_port_cmd(struct ice_hw *hw, u8 configured_port,
enum ice_ptp_tmr_cmd configured_cmd);
+int ice_ptp_hw_ts_point_get(struct ice_hw *hw, enum ptp_ts_point *point);
+int ice_ptp_hw_ts_point_set(struct ice_hw *hw, enum ptp_ts_point point);
/* E822 family functions */
int ice_read_quad_reg_e82x(struct ice_hw *hw, u8 quad, u16 offset, u32 *val);
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 1/2] ptp: add control over HW timestamp latch point
2024-11-06 1:07 ` [PATCH net-next v3 1/2] " Arkadiusz Kubalewski
@ 2024-11-06 2:04 ` Jakub Kicinski
2024-11-07 9:01 ` Kubalewski, Arkadiusz
2024-11-06 17:45 ` Andrew Lunn
1 sibling, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2024-11-06 2:04 UTC (permalink / raw)
To: Arkadiusz Kubalewski
Cc: netdev, linux-kernel, intel-wired-lan, anthony.l.nguyen,
przemyslaw.kitszel, davem, edumazet, pabeni, richardcochran,
Aleksandr Loktionov
On Wed, 6 Nov 2024 02:07:55 +0100 Arkadiusz Kubalewski wrote:
> +What: /sys/class/ptp/ptp<N>/ts_point
> +Date: October 2024
> +Contact: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> +Description:
> + This file provides control over the point in time in
> + which the HW timestamp is latched. As specified in IEEE
> + 802.3cx, the latch point can be either at the beginning
> + or after the end of Start of Frame Delimiter (SFD).
> + Value "1" means the timestamp is latched at the
> + beginning of the SFD. Value "2" means that timestamp is
> + latched after the end of SFD.
Richard has the final say but IMO packet timestamping config does not
belong to the PHC, rather ndo_hwtstamp_set.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 1/2] ptp: add control over HW timestamp latch point
2024-11-06 1:07 ` [PATCH net-next v3 1/2] " Arkadiusz Kubalewski
2024-11-06 2:04 ` Jakub Kicinski
@ 2024-11-06 17:45 ` Andrew Lunn
2024-11-07 9:14 ` [Intel-wired-lan] " Kubalewski, Arkadiusz
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2024-11-06 17:45 UTC (permalink / raw)
To: Arkadiusz Kubalewski
Cc: netdev, linux-kernel, intel-wired-lan, anthony.l.nguyen,
przemyslaw.kitszel, davem, edumazet, kuba, pabeni, richardcochran,
Aleksandr Loktionov
On Wed, Nov 06, 2024 at 02:07:55AM +0100, Arkadiusz Kubalewski wrote:
> Currently HW support of ptp/timesync solutions in network PHY chips can be
> implemented with two different approaches, the timestamp maybe latched
> either at the beginning or after the Start of Frame Delimiter (SFD) [1].
>
> Allow ptp device drivers to provide user with control over the HW
> timestamp latch point with ptp sysfs ABI. Provide a new file under sysfs
> ptp device (/sys/class/ptp/ptp<N>/ts_point). The file is available for the
> user, if the device driver implements at least one of newly provided
> callbacks. If the file is not provided the user shall find a PHY timestamp
> latch point within the HW vendor specification.
>
> The file is designed for root user/group access only, as the read for
> regular user could impact performance of the ptp device.
>
> Usage, examples:
>
> ** Obtain current state:
> $ cat /sys/class/ptp/ptp<N>/ts_point
> Command returns enum/integer:
> * 1 - timestamp latched by PHY at the beginning of SFD,
> * 2 - timestamp latched by PHY after the SFD,
> * None - callback returns error to the user.
-EOPNOTSUPP would be more conventional, not None.
>
> ** Configure timestamp latch point at the beginning of SFD:
> $ echo 1 > /sys/class/ptp/ptp<N>/ts_point
>
> ** Configure timestamp latch point after the SFD:
> $ echo 2 > /sys/class/ptp/ptp<N>/ts_point
and i assume these also return -EOPNOTSUPP if it is not supported.
And a dumb question, why should i care where the latch point is? Why
would i want to change it? Richard always says that you cannot trust
the kernel to have the same latency from kernel to kernel version
because driver developers like to tweak the latency, invalidating all
measurements the user has done when setting up their ptp4l
daemon. This just seems like yet another way to break my ptp4l
configuration.
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net-next v3 1/2] ptp: add control over HW timestamp latch point
2024-11-06 2:04 ` Jakub Kicinski
@ 2024-11-07 9:01 ` Kubalewski, Arkadiusz
2024-11-07 17:54 ` Richard Cochran
0 siblings, 1 reply; 8+ messages in thread
From: Kubalewski, Arkadiusz @ 2024-11-07 9:01 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, Nguyen, Anthony L,
Kitszel, Przemyslaw, davem@davemloft.net, edumazet@google.com,
pabeni@redhat.com, richardcochran@gmail.com, Loktionov, Aleksandr
>From: Jakub Kicinski <kuba@kernel.org>
>Sent: Wednesday, November 6, 2024 3:05 AM
>
>On Wed, 6 Nov 2024 02:07:55 +0100 Arkadiusz Kubalewski wrote:
>> +What: /sys/class/ptp/ptp<N>/ts_point
>> +Date: October 2024
>> +Contact: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>> +Description:
>> + This file provides control over the point in time in
>> + which the HW timestamp is latched. As specified in IEEE
>> + 802.3cx, the latch point can be either at the beginning
>> + or after the end of Start of Frame Delimiter (SFD).
>> + Value "1" means the timestamp is latched at the
>> + beginning of the SFD. Value "2" means that timestamp is
>> + latched after the end of SFD.
>
>Richard has the final say but IMO packet timestamping config does not
>belong to the PHC, rather ndo_hwtstamp_set.
Ok, thank you for feedback.
Richard do you agree with Kuba?
Thank you!
Arkadiusz
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [Intel-wired-lan] [PATCH net-next v3 1/2] ptp: add control over HW timestamp latch point
2024-11-06 17:45 ` Andrew Lunn
@ 2024-11-07 9:14 ` Kubalewski, Arkadiusz
0 siblings, 0 replies; 8+ messages in thread
From: Kubalewski, Arkadiusz @ 2024-11-07 9:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, Nguyen, Anthony L,
Kitszel, Przemyslaw, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, richardcochran@gmail.com,
Loktionov, Aleksandr
>From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
>Andrew Lunn
>Sent: Wednesday, November 6, 2024 6:45 PM
>
>On Wed, Nov 06, 2024 at 02:07:55AM +0100, Arkadiusz Kubalewski wrote:
>> Currently HW support of ptp/timesync solutions in network PHY chips can
>>be
>> implemented with two different approaches, the timestamp maybe latched
>> either at the beginning or after the Start of Frame Delimiter (SFD) [1].
>>
>> Allow ptp device drivers to provide user with control over the HW
>> timestamp latch point with ptp sysfs ABI. Provide a new file under sysfs
>> ptp device (/sys/class/ptp/ptp<N>/ts_point). The file is available for
>>the
>> user, if the device driver implements at least one of newly provided
>> callbacks. If the file is not provided the user shall find a PHY
>>timestamp
>> latch point within the HW vendor specification.
>>
>> The file is designed for root user/group access only, as the read for
>> regular user could impact performance of the ptp device.
>>
>> Usage, examples:
>>
>> ** Obtain current state:
>> $ cat /sys/class/ptp/ptp<N>/ts_point
>> Command returns enum/integer:
>> * 1 - timestamp latched by PHY at the beginning of SFD,
>> * 2 - timestamp latched by PHY after the SFD,
>> * None - callback returns error to the user.
>
>-EOPNOTSUPP would be more conventional, not None.
>
Sure, I can change it if new version is needed (Kuba's other thread on this)
>>
>> ** Configure timestamp latch point at the beginning of SFD:
>> $ echo 1 > /sys/class/ptp/ptp<N>/ts_point
>>
>> ** Configure timestamp latch point after the SFD:
>> $ echo 2 > /sys/class/ptp/ptp<N>/ts_point
>
>and i assume these also return -EOPNOTSUPP if it is not supported.
>
>And a dumb question, why should i care where the latch point is? Why
>would i want to change it? Richard always says that you cannot trust
>the kernel to have the same latency from kernel to kernel version
>because driver developers like to tweak the latency, invalidating all
>measurements the user has done when setting up their ptp4l
>daemon. This just seems like yet another way to break my ptp4l
>configuration.
>
> Andrew
Well, making control knob available to the users.
The explanation/rationale/problem statement is available under provided
Link, and patch allows part of IEEE 802_3cx standard to be controlled.
Thank you!
Arkadiusz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 1/2] ptp: add control over HW timestamp latch point
2024-11-07 9:01 ` Kubalewski, Arkadiusz
@ 2024-11-07 17:54 ` Richard Cochran
0 siblings, 0 replies; 8+ messages in thread
From: Richard Cochran @ 2024-11-07 17:54 UTC (permalink / raw)
To: Kubalewski, Arkadiusz
Cc: Jakub Kicinski, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
Nguyen, Anthony L, Kitszel, Przemyslaw, davem@davemloft.net,
edumazet@google.com, pabeni@redhat.com, Loktionov, Aleksandr
On Thu, Nov 07, 2024 at 09:01:41AM +0000, Kubalewski, Arkadiusz wrote:
> >From: Jakub Kicinski <kuba@kernel.org>
> >Sent: Wednesday, November 6, 2024 3:05 AM
> >Richard has the final say but IMO packet timestamping config does not
> >belong to the PHC, rather ndo_hwtstamp_set.
Right, PHC means the hardware clock. That is distinct from the time
stamping settings. These belong to the network device, and are
contolled by using the SIOCSHWTSTAMP ioctl.
> Ok, thank you for feedback.
>
> Richard do you agree with Kuba?
IMO, setting the time stamp point should be with an ioctl in a way
similar way to SIOCSHWTSTAMP.
Thanks,
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-11-07 17:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-06 1:07 [PATCH net-next v3 0/2] ptp: add control over HW timestamp latch point Arkadiusz Kubalewski
2024-11-06 1:07 ` [PATCH net-next v3 1/2] " Arkadiusz Kubalewski
2024-11-06 2:04 ` Jakub Kicinski
2024-11-07 9:01 ` Kubalewski, Arkadiusz
2024-11-07 17:54 ` Richard Cochran
2024-11-06 17:45 ` Andrew Lunn
2024-11-07 9:14 ` [Intel-wired-lan] " Kubalewski, Arkadiusz
2024-11-06 1:07 ` [PATCH net-next v3 2/2] ice: " Arkadiusz Kubalewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).