All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock
@ 2019-06-03 12:12 Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 1/9] mlxsw: cmd: Free running clock PCI BAR and offsets via query firmware Ido Schimmel
                   ` (9 more replies)
  0 siblings, 10 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

This is the first of about four patchsets that add PTP support in mlxsw
for the Spectrum-1 ASIC.

This patchset exposes the physical hardware clock, while subsequent
patchsets will add timestamping support and mlxsw tracepoints for
debugging and testing.

Shalom says:

This patchset adds support for physical hardware clock for Spectrum-1
ASIC only.

Patches #1, #2 and #3 add the ability to query the free running clock
PCI address.

Patches #4 and #5 add two new register, the Management UTC Register and
the Management Pulse Per Second Register.

Patch #6 publishes scaled_ppm_to_ppb() to allow drivers to use it.

Patch #7 adds the physical hardware clock operations.

Patch #8 initializes the physical hardware clock.

Patch #9 adds a selftest for testing the PTP physical hardware clock.

Shalom Toledo (9):
  mlxsw: cmd: Free running clock PCI BAR and offsets via query firmware
  mlxsw: core: Add a new interface for reading the hardware free running
    clock
  mlxsw: pci: Query free running clock PCI BAR and offsets
  mlxsw: reg: Add Management UTC Register
  mlxsw: reg: Add Management Pulse Per Second Register
  ptp: ptp_clock: Publish scaled_ppm_to_ppb
  mlxsw: spectrum_ptp: Add implementation for physical hardware clock
    operations
  mlxsw: spectrum: PTP physical hardware clock initialization
  selftests: ptp: Add Physical Hardware Clock test

 drivers/net/ethernet/mellanox/mlxsw/Makefile  |   1 +
 drivers/net/ethernet/mellanox/mlxsw/cmd.h     |  12 +
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  12 +
 drivers/net/ethernet/mellanox/mlxsw/core.h    |   8 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c     |  32 +++
 drivers/net/ethernet/mellanox/mlxsw/pci_hw.h  |   3 +
 drivers/net/ethernet/mellanox/mlxsw/reg.h     | 103 +++++++
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  36 +++
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |   3 +
 .../ethernet/mellanox/mlxsw/spectrum_ptp.c    | 267 ++++++++++++++++++
 .../ethernet/mellanox/mlxsw/spectrum_ptp.h    |  44 +++
 drivers/ptp/ptp_clock.c                       |   5 +-
 include/linux/ptp_clock_kernel.h              |   8 +
 tools/testing/selftests/ptp/phc.sh            | 166 +++++++++++
 14 files changed, 697 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
 create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
 create mode 100755 tools/testing/selftests/ptp/phc.sh

-- 
2.20.1


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH net-next 1/9] mlxsw: cmd: Free running clock PCI BAR and offsets via query firmware
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 2/9] mlxsw: core: Add a new interface for reading the hardware free running clock Ido Schimmel
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Add free running clock PCI BAR and offset to query firmware command.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/cmd.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/cmd.h b/drivers/net/ethernet/mellanox/mlxsw/cmd.h
index 0772e4339b33..5ffdfb532cb7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/cmd.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/cmd.h
@@ -317,6 +317,18 @@ MLXSW_ITEM64(cmd_mbox, query_fw, doorbell_page_offset, 0x40, 0, 64);
  */
 MLXSW_ITEM32(cmd_mbox, query_fw, doorbell_page_bar, 0x48, 30, 2);
 
+/* cmd_mbox_query_fw_free_running_clock_offset
+ * The offset of the free running clock page
+ */
+MLXSW_ITEM64(cmd_mbox, query_fw, free_running_clock_offset, 0x50, 0, 64);
+
+/* cmd_mbox_query_fw_fr_rn_clk_bar
+ * PCI base address register (BAR) of the free running clock page
+ * 0: BAR 0
+ * 1: 64 bit BAR
+ */
+MLXSW_ITEM32(cmd_mbox, query_fw, fr_rn_clk_bar, 0x58, 30, 2);
+
 /* QUERY_BOARDINFO - Query Board Information
  * -----------------------------------------
  * OpMod == 0 (N/A), INMmod == 0 (N/A)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 2/9] mlxsw: core: Add a new interface for reading the hardware free running clock
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 1/9] mlxsw: cmd: Free running clock PCI BAR and offsets via query firmware Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 3/9] mlxsw: pci: Query free running clock PCI BAR and offsets Ido Schimmel
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Add two new bus operations for reading the hardware free running clock.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 12 ++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h |  8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 182762898361..b49a6520386c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2011,6 +2011,18 @@ int mlxsw_core_resources_query(struct mlxsw_core *mlxsw_core, char *mbox,
 }
 EXPORT_SYMBOL(mlxsw_core_resources_query);
 
+u32 mlxsw_core_read_frc_h(struct mlxsw_core *mlxsw_core)
+{
+	return mlxsw_core->bus->read_frc_h(mlxsw_core->bus_priv);
+}
+EXPORT_SYMBOL(mlxsw_core_read_frc_h);
+
+u32 mlxsw_core_read_frc_l(struct mlxsw_core *mlxsw_core)
+{
+	return mlxsw_core->bus->read_frc_l(mlxsw_core->bus_priv);
+}
+EXPORT_SYMBOL(mlxsw_core_read_frc_l);
+
 static int __init mlxsw_core_module_init(void)
 {
 	int err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e3832cb5bdda..02d1e580e44f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -306,6 +306,9 @@ int mlxsw_core_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
 void mlxsw_core_fw_flash_start(struct mlxsw_core *mlxsw_core);
 void mlxsw_core_fw_flash_end(struct mlxsw_core *mlxsw_core);
 
+u32 mlxsw_core_read_frc_h(struct mlxsw_core *mlxsw_core);
+u32 mlxsw_core_read_frc_l(struct mlxsw_core *mlxsw_core);
+
 bool mlxsw_core_res_valid(struct mlxsw_core *mlxsw_core,
 			  enum mlxsw_res_id res_id);
 
@@ -336,6 +339,8 @@ struct mlxsw_bus {
 			char *in_mbox, size_t in_mbox_size,
 			char *out_mbox, size_t out_mbox_size,
 			u8 *p_status);
+	u32 (*read_frc_h)(void *bus_priv);
+	u32 (*read_frc_l)(void *bus_priv);
 	u8 features;
 };
 
@@ -353,7 +358,8 @@ struct mlxsw_bus_info {
 	struct mlxsw_fw_rev fw_rev;
 	u8 vsd[MLXSW_CMD_BOARDINFO_VSD_LEN];
 	u8 psid[MLXSW_CMD_BOARDINFO_PSID_LEN];
-	u8 low_frequency;
+	u8 low_frequency:1,
+	   read_frc_capable:1;
 };
 
 struct mlxsw_hwmon;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 3/9] mlxsw: pci: Query free running clock PCI BAR and offsets
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 1/9] mlxsw: cmd: Free running clock PCI BAR and offsets via query firmware Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 2/9] mlxsw: core: Add a new interface for reading the hardware free running clock Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register Ido Schimmel
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Query free running clock PCI BAR and offsets during the pci_init.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c    | 32 ++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/pci_hw.h |  3 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index b40455f8293d..6acb9bbfdf89 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -102,6 +102,7 @@ struct mlxsw_pci_queue_type_group {
 struct mlxsw_pci {
 	struct pci_dev *pdev;
 	u8 __iomem *hw_addr;
+	u64 free_running_clock_offset;
 	struct mlxsw_pci_queue_type_group queues[MLXSW_PCI_QUEUE_TYPE_COUNT];
 	u32 doorbell_offset;
 	struct mlxsw_core *core;
@@ -1414,6 +1415,15 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
 	mlxsw_pci->doorbell_offset =
 		mlxsw_cmd_mbox_query_fw_doorbell_page_offset_get(mbox);
 
+	if (mlxsw_cmd_mbox_query_fw_fr_rn_clk_bar_get(mbox) != 0) {
+		dev_err(&pdev->dev, "Unsupported free running clock BAR queried from hw\n");
+		err = -EINVAL;
+		goto err_fr_rn_clk_bar;
+	}
+
+	mlxsw_pci->free_running_clock_offset =
+		mlxsw_cmd_mbox_query_fw_free_running_clock_offset_get(mbox);
+
 	num_pages = mlxsw_cmd_mbox_query_fw_fw_pages_get(mbox);
 	err = mlxsw_pci_fw_area_init(mlxsw_pci, mbox, num_pages);
 	if (err)
@@ -1469,6 +1479,7 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
 err_boardinfo:
 	mlxsw_pci_fw_area_fini(mlxsw_pci);
 err_fw_area_init:
+err_fr_rn_clk_bar:
 err_doorbell_page_bar:
 err_iface_rev:
 err_query_fw:
@@ -1672,6 +1683,24 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 	return err;
 }
 
+static u32 mlxsw_pci_read_frc_h(void *bus_priv)
+{
+	struct mlxsw_pci *mlxsw_pci = bus_priv;
+	u64 frc_offset;
+
+	frc_offset = mlxsw_pci->free_running_clock_offset;
+	return mlxsw_pci_read32(mlxsw_pci, FREE_RUNNING_CLOCK_H(frc_offset));
+}
+
+static u32 mlxsw_pci_read_frc_l(void *bus_priv)
+{
+	struct mlxsw_pci *mlxsw_pci = bus_priv;
+	u64 frc_offset;
+
+	frc_offset = mlxsw_pci->free_running_clock_offset;
+	return mlxsw_pci_read32(mlxsw_pci, FREE_RUNNING_CLOCK_L(frc_offset));
+}
+
 static const struct mlxsw_bus mlxsw_pci_bus = {
 	.kind			= "pci",
 	.init			= mlxsw_pci_init,
@@ -1679,6 +1708,8 @@ static const struct mlxsw_bus mlxsw_pci_bus = {
 	.skb_transmit_busy	= mlxsw_pci_skb_transmit_busy,
 	.skb_transmit		= mlxsw_pci_skb_transmit,
 	.cmd_exec		= mlxsw_pci_cmd_exec,
+	.read_frc_h		= mlxsw_pci_read_frc_h,
+	.read_frc_l		= mlxsw_pci_read_frc_l,
 	.features		= MLXSW_BUS_F_TXRX | MLXSW_BUS_F_RESET,
 };
 
@@ -1740,6 +1771,7 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mlxsw_pci->bus_info.device_kind = driver_name;
 	mlxsw_pci->bus_info.device_name = pci_name(mlxsw_pci->pdev);
 	mlxsw_pci->bus_info.dev = &pdev->dev;
+	mlxsw_pci->bus_info.read_frc_capable = true;
 	mlxsw_pci->id = id;
 
 	err = mlxsw_core_bus_device_register(&mlxsw_pci->bus_info,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h b/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h
index 8648ca171254..e57e42e2d2b2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h
@@ -43,6 +43,9 @@
 #define MLXSW_PCI_DOORBELL(offset, type_offset, num)	\
 	((offset) + (type_offset) + (num) * 4)
 
+#define MLXSW_PCI_FREE_RUNNING_CLOCK_H(offset)	(offset)
+#define MLXSW_PCI_FREE_RUNNING_CLOCK_L(offset)	((offset) + 4)
+
 #define MLXSW_PCI_CQS_MAX	96
 #define MLXSW_PCI_EQS_COUNT	2
 #define MLXSW_PCI_EQ_ASYNC_NUM	0
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (2 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 3/9] mlxsw: pci: Query free running clock PCI BAR and offsets Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-04 14:17   ` Richard Cochran
  2019-06-03 12:12 ` [PATCH net-next 5/9] mlxsw: reg: Add Management Pulse Per Second Register Ido Schimmel
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

The MTUTC register configures the HW UTC counter.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h | 45 +++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 7348c5a5ad6a..9ec154975cb2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -8691,6 +8691,50 @@ static inline void mlxsw_reg_mlcr_pack(char *payload, u8 local_port,
 					   MLXSW_REG_MLCR_DURATION_MAX : 0);
 }
 
+/* MTUTC - Management UTC Register
+ * -------------------------------
+ * Configures the HW UTC counter.
+ */
+#define MLXSW_REG_MTUTC_ID 0x9055
+#define MLXSW_REG_MTUTC_LEN 0x1C
+
+MLXSW_REG_DEFINE(mtutc, MLXSW_REG_MTUTC_ID, MLXSW_REG_MTUTC_LEN);
+
+enum mlxsw_reg_mtutc_operation {
+	MLXSW_REG_MTUTC_OPERATION_SET_TIME_AT_NEXT_SEC = 0,
+	MLXSW_REG_MTUTC_OPERATION_ADJUST_FREQ = 3,
+};
+
+/* reg_mtutc_operation
+ * Operation.
+ * Access: OP
+ */
+MLXSW_ITEM32(reg, mtutc, operation, 0x00, 0, 4);
+
+/* reg_mtutc_freq_adjustment
+ * Frequency adjustment: Every PPS the HW frequency will be
+ * adjusted by this value. Units of HW clock, where HW counts
+ * 10^9 HW clocks for 1 HW second.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mtutc, freq_adjustment, 0x04, 0, 32);
+
+/* reg_mtutc_utc_sec
+ * UTC seconds.
+ * Access: WO
+ */
+MLXSW_ITEM32(reg, mtutc, utc_sec, 0x10, 0, 32);
+
+static inline void
+mlxsw_reg_mtutc_pack(char *payload, enum mlxsw_reg_mtutc_operation oper,
+		     u32 freq_adj, u32 utc_sec)
+{
+	MLXSW_REG_ZERO(mtutc, payload);
+	mlxsw_reg_mtutc_operation_set(payload, oper);
+	mlxsw_reg_mtutc_freq_adjustment_set(payload, freq_adj);
+	mlxsw_reg_mtutc_utc_sec_set(payload, utc_sec);
+}
+
 /* MCQI - Management Component Query Information
  * ---------------------------------------------
  * This register allows querying information about firmware components.
@@ -10105,6 +10149,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
 	MLXSW_REG(mgir),
 	MLXSW_REG(mrsr),
 	MLXSW_REG(mlcr),
+	MLXSW_REG(mtutc),
 	MLXSW_REG(mpsc),
 	MLXSW_REG(mcqi),
 	MLXSW_REG(mcc),
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 5/9] mlxsw: reg: Add Management Pulse Per Second Register
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (3 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb Ido Schimmel
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

The MTPPS register provides the device PPS capabilities, configure the PPS
in and out modules and holds the PPS in time stamp.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h | 58 +++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 9ec154975cb2..d8eb9ef01646 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -8691,6 +8691,63 @@ static inline void mlxsw_reg_mlcr_pack(char *payload, u8 local_port,
 					   MLXSW_REG_MLCR_DURATION_MAX : 0);
 }
 
+/* MTPPS - Management Pulse Per Second Register
+ * --------------------------------------------
+ * This register provides the device PPS capabilities, configure the PPS in and
+ * out modules and holds the PPS in time stamp.
+ */
+#define MLXSW_REG_MTPPS_ID 0x9053
+#define MLXSW_REG_MTPPS_LEN 0x3C
+
+MLXSW_REG_DEFINE(mtpps, MLXSW_REG_MTPPS_ID, MLXSW_REG_MTPPS_LEN);
+
+/* reg_mtpps_enable
+ * Enables the PPS functionality the specific pin.
+ * A boolean variable.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mtpps, enable, 0x20, 31, 1);
+
+enum mlxsw_reg_mtpps_pin_mode {
+	MLXSW_REG_MTPPS_PIN_MODE_VIRTUAL_PIN = 0x2,
+};
+
+/* reg_mtpps_pin_mode
+ * Pin mode to be used. The mode must comply with the supported modes of the
+ * requested pin.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mtpps, pin_mode, 0x20, 8, 4);
+
+#define MLXSW_REG_MTPPS_PIN_SP_VIRTUAL_PIN	7
+
+/* reg_mtpps_pin
+ * Pin to be configured or queried out of the supported pins.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mtpps, pin, 0x20, 0, 8);
+
+/* reg_mtpps_time_stamp
+ * When pin_mode = pps_in, the latched device time when it was triggered from
+ * the external GPIO pin.
+ * When pin_mode = pps_out or virtual_pin or pps_out_and_virtual_pin, the target
+ * time to generate next output signal.
+ * Time is in units of device clock.
+ * Access: RW
+ */
+MLXSW_ITEM64(reg, mtpps, time_stamp, 0x28, 0, 64);
+
+static inline void
+mlxsw_reg_mtpps_vpin_pack(char *payload, u64 time_stamp)
+{
+	MLXSW_REG_ZERO(mtpps, payload);
+	mlxsw_reg_mtpps_pin_set(payload, MLXSW_REG_MTPPS_PIN_SP_VIRTUAL_PIN);
+	mlxsw_reg_mtpps_pin_mode_set(payload,
+				     MLXSW_REG_MTPPS_PIN_MODE_VIRTUAL_PIN);
+	mlxsw_reg_mtpps_enable_set(payload, true);
+	mlxsw_reg_mtpps_time_stamp_set(payload, time_stamp);
+}
+
 /* MTUTC - Management UTC Register
  * -------------------------------
  * Configures the HW UTC counter.
@@ -10149,6 +10206,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
 	MLXSW_REG(mgir),
 	MLXSW_REG(mrsr),
 	MLXSW_REG(mlcr),
+	MLXSW_REG(mtpps),
 	MLXSW_REG(mtutc),
 	MLXSW_REG(mpsc),
 	MLXSW_REG(mcqi),
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (4 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 5/9] mlxsw: reg: Add Management Pulse Per Second Register Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-04 14:21   ` Richard Cochran
  2019-06-03 12:12 ` [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations Ido Schimmel
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Publish scaled_ppm_to_ppb to allow drivers to use it.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/ptp/ptp_clock.c          | 5 +++--
 include/linux/ptp_clock_kernel.h | 8 ++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index e189fa1be21e..f0893953b503 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -63,7 +63,7 @@ static void enqueue_external_timestamp(struct timestamp_event_queue *queue,
 	spin_unlock_irqrestore(&queue->lock, flags);
 }
 
-static s32 scaled_ppm_to_ppb(long ppm)
+s32 ptp_clock_scaled_ppm_to_ppb(long ppm)
 {
 	/*
 	 * The 'freq' field in the 'struct timex' is in parts per
@@ -82,6 +82,7 @@ static s32 scaled_ppm_to_ppb(long ppm)
 	ppb >>= 13;
 	return (s32) ppb;
 }
+EXPORT_SYMBOL(ptp_clock_scaled_ppm_to_ppb);
 
 /* posix clock implementation */
 
@@ -137,7 +138,7 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
 		delta = ktime_to_ns(kt);
 		err = ops->adjtime(ops, delta);
 	} else if (tx->modes & ADJ_FREQUENCY) {
-		s32 ppb = scaled_ppm_to_ppb(tx->freq);
+		s32 ppb = ptp_clock_scaled_ppm_to_ppb(tx->freq);
 		if (ppb > ops->max_adj || ppb < -ops->max_adj)
 			return -ERANGE;
 		if (ops->adjfine)
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 28eb9c792522..fc59d57640a5 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -212,6 +212,14 @@ extern void ptp_clock_event(struct ptp_clock *ptp,
 
 extern int ptp_clock_index(struct ptp_clock *ptp);
 
+/**
+ * ptp_clock_scaled_ppm_to_ppb() - convert scaled ppm to ppb
+ *
+ * @ppm:    Parts per million, but with a 16 bit binary fractional field
+ */
+
+extern s32 ptp_clock_scaled_ppm_to_ppb(long ppm);
+
 /**
  * ptp_find_pin() - obtain the pin index of a given auxiliary function
  *
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (5 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-04 14:28   ` Richard Cochran
  2019-06-04 17:03   ` Richard Cochran
  2019-06-03 12:12 ` [PATCH net-next 8/9] mlxsw: spectrum: PTP physical hardware clock initialization Ido Schimmel
                   ` (2 subsequent siblings)
  9 siblings, 2 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Implement physical hardware clock operations.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/Makefile  |   1 +
 .../ethernet/mellanox/mlxsw/spectrum_ptp.c    | 267 ++++++++++++++++++
 .../ethernet/mellanox/mlxsw/spectrum_ptp.h    |  44 +++
 3 files changed, 312 insertions(+)
 create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
 create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h

diff --git a/drivers/net/ethernet/mellanox/mlxsw/Makefile b/drivers/net/ethernet/mellanox/mlxsw/Makefile
index c4dc72e1ce63..171b36bd8a4e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Makefile
+++ b/drivers/net/ethernet/mellanox/mlxsw/Makefile
@@ -31,5 +31,6 @@ mlxsw_spectrum-objs		:= spectrum.o spectrum_buffers.o \
 				   spectrum_nve.o spectrum_nve_vxlan.o \
 				   spectrum_dpipe.o
 mlxsw_spectrum-$(CONFIG_MLXSW_SPECTRUM_DCB)	+= spectrum_dcb.o
+mlxsw_spectrum-$(CONFIG_PTP_1588_CLOCK)		+= spectrum_ptp.o
 obj-$(CONFIG_MLXSW_MINIMAL)	+= mlxsw_minimal.o
 mlxsw_minimal-objs		:= minimal.o
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
new file mode 100644
index 000000000000..9bdbbba29400
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2019 Mellanox Technologies. All rights reserved */
+
+#include <linux/ptp_clock_kernel.h>
+#include <linux/clocksource.h>
+#include <linux/timecounter.h>
+#include <linux/spinlock.h>
+#include <linux/device.h>
+
+#include "spectrum_ptp.h"
+#include "core.h"
+
+#define MLXSW_SP1_PTP_CLOCK_CYCLES_SHIFT	29
+#define MLXSW_SP1_PTP_CLOCK_FREQ_KHZ		156257 /* 6.4nSec */
+#define MLXSW_SP1_PTP_CLOCK_MASK		64
+
+struct mlxsw_sp_ptp_clock {
+	struct mlxsw_core *core;
+	spinlock_t lock; /* protect this structure */
+	struct cyclecounter cycles;
+	struct timecounter tc;
+	u32 nominal_c_mult;
+	struct ptp_clock *ptp;
+	struct ptp_clock_info ptp_info;
+	unsigned long overflow_period;
+	struct delayed_work overflow_work;
+};
+
+static u64 __mlxsw_sp1_ptp_read_frc(struct mlxsw_sp_ptp_clock *clock,
+				    struct ptp_system_timestamp *sts)
+{
+	struct mlxsw_core *mlxsw_core = clock->core;
+	u32 frc_h1, frc_h2, frc_l;
+
+	frc_h1 = mlxsw_core_read_frc_h(mlxsw_core);
+	ptp_read_system_prets(sts);
+	frc_l = mlxsw_core_read_frc_l(mlxsw_core);
+	ptp_read_system_postts(sts);
+	frc_h2 = mlxsw_core_read_frc_h(mlxsw_core);
+
+	if (frc_h1 != frc_h2) {
+		/* wrap around */
+		ptp_read_system_prets(sts);
+		frc_l = mlxsw_core_read_frc_l(mlxsw_core);
+		ptp_read_system_postts(sts);
+	}
+
+	return (u64) frc_l | (u64) frc_h2 << 32;
+}
+
+static u64 mlxsw_sp1_ptp_read_frc(const struct cyclecounter *cc)
+{
+	struct mlxsw_sp_ptp_clock *clock =
+		container_of(cc, struct mlxsw_sp_ptp_clock, cycles);
+
+	return __mlxsw_sp1_ptp_read_frc(clock, NULL) & cc->mask;
+}
+
+static int
+mlxsw_sp1_ptp_update_phc_adjfreq(struct mlxsw_sp_ptp_clock *clock, int freq_adj)
+{
+	struct mlxsw_core *mlxsw_core = clock->core;
+	char mtutc_pl[MLXSW_REG_MTUTC_LEN];
+
+	mlxsw_reg_mtutc_pack(mtutc_pl, MLXSW_REG_MTUTC_OPERATION_ADJUST_FREQ,
+			     freq_adj, 0);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtutc), mtutc_pl);
+}
+
+static u64 mlxsw_sp1_ptp_ns2cycles(const struct timecounter *tc, u64 nsec)
+{
+	u64 cycles = (u64) nsec;
+
+	cycles <<= tc->cc->shift;
+	cycles = div_u64(cycles, tc->cc->mult);
+
+	return cycles;
+}
+
+static int
+mlxsw_sp1_ptp_update_phc_settime(struct mlxsw_sp_ptp_clock *clock, u64 nsec)
+{
+	struct mlxsw_core *mlxsw_core = clock->core;
+	char mtutc_pl[MLXSW_REG_MTUTC_LEN];
+	char mtpps_pl[MLXSW_REG_MTPPS_LEN];
+	u64 next_sec_in_nsec, cycles;
+	u32 next_sec;
+	int err;
+
+	next_sec = nsec / NSEC_PER_SEC + 1;
+	next_sec_in_nsec = next_sec * NSEC_PER_SEC;
+
+	spin_lock(&clock->lock);
+	cycles = mlxsw_sp1_ptp_ns2cycles(&clock->tc, next_sec_in_nsec);
+	spin_unlock(&clock->lock);
+
+	mlxsw_reg_mtpps_vpin_pack(mtpps_pl, cycles);
+	err = mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtpps), mtpps_pl);
+	if (err)
+		return err;
+
+	mlxsw_reg_mtutc_pack(mtutc_pl,
+			     MLXSW_REG_MTUTC_OPERATION_SET_TIME_AT_NEXT_SEC,
+			     0, next_sec);
+	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtutc), mtutc_pl);
+}
+
+static int mlxsw_sp1_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct mlxsw_sp_ptp_clock *clock =
+		container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
+	int neg_adj = 0;
+	u32 diff;
+	u64 adj;
+	s32 ppb;
+
+	ppb = ptp_clock_scaled_ppm_to_ppb(scaled_ppm);
+
+	if (ppb < 0) {
+		neg_adj = 1;
+		ppb = -ppb;
+	}
+
+	adj = clock->nominal_c_mult;
+	adj *= ppb;
+	diff = div_u64(adj, NSEC_PER_SEC);
+
+	spin_lock(&clock->lock);
+	timecounter_read(&clock->tc);
+	clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
+				       clock->nominal_c_mult + diff;
+	spin_unlock(&clock->lock);
+
+	return mlxsw_sp1_ptp_update_phc_adjfreq(clock, neg_adj ? -ppb : ppb);
+}
+
+static int mlxsw_sp1_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct mlxsw_sp_ptp_clock *clock =
+		container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
+	u64 nsec;
+
+	spin_lock(&clock->lock);
+	timecounter_adjtime(&clock->tc, delta);
+	nsec = timecounter_read(&clock->tc);
+	spin_unlock(&clock->lock);
+
+	return mlxsw_sp1_ptp_update_phc_settime(clock, nsec);
+}
+
+static int mlxsw_sp1_ptp_gettimex(struct ptp_clock_info *ptp,
+				  struct timespec64 *ts,
+				  struct ptp_system_timestamp *sts)
+{
+	struct mlxsw_sp_ptp_clock *clock =
+		container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
+	u64 cycles, nsec;
+
+	spin_lock(&clock->lock);
+	cycles = __mlxsw_sp1_ptp_read_frc(clock, sts);
+	nsec = timecounter_cyc2time(&clock->tc, cycles);
+	spin_unlock(&clock->lock);
+
+	*ts = ns_to_timespec64(nsec);
+
+	return 0;
+}
+
+static int mlxsw_sp1_ptp_settime(struct ptp_clock_info *ptp,
+				 const struct timespec64 *ts)
+{
+	struct mlxsw_sp_ptp_clock *clock =
+		container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
+	u64 nsec = timespec64_to_ns(ts);
+
+	spin_lock(&clock->lock);
+	timecounter_init(&clock->tc, &clock->cycles, nsec);
+	nsec = timecounter_read(&clock->tc);
+	spin_unlock(&clock->lock);
+
+	return mlxsw_sp1_ptp_update_phc_settime(clock, nsec);
+}
+
+static const struct ptp_clock_info mlxsw_sp1_ptp_clock_info = {
+	.owner		= THIS_MODULE,
+	.name		= "mlxsw_sp_clock",
+	.max_adj	= 100000000,
+	.adjfine	= mlxsw_sp1_ptp_adjfine,
+	.adjtime	= mlxsw_sp1_ptp_adjtime,
+	.gettimex64	= mlxsw_sp1_ptp_gettimex,
+	.settime64	= mlxsw_sp1_ptp_settime,
+};
+
+static void mlxsw_sp1_ptp_clock_overflow(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct mlxsw_sp_ptp_clock *clock;
+
+	clock = container_of(dwork, struct mlxsw_sp_ptp_clock, overflow_work);
+
+	spin_lock(&clock->lock);
+	timecounter_read(&clock->tc);
+	spin_unlock(&clock->lock);
+	mlxsw_core_schedule_dw(&clock->overflow_work, clock->overflow_period);
+}
+
+struct mlxsw_sp_ptp_clock *
+mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev)
+{
+	u64 overflow_cycles, nsec, frac = 0;
+	struct mlxsw_sp_ptp_clock *clock;
+	int err;
+
+	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
+	if (!clock)
+		return ERR_PTR(-ENOMEM);
+
+	spin_lock_init(&clock->lock);
+	clock->cycles.read = mlxsw_sp1_ptp_read_frc;
+	clock->cycles.shift = MLXSW_SP1_PTP_CLOCK_CYCLES_SHIFT;
+	clock->cycles.mult = clocksource_khz2mult(MLXSW_SP1_PTP_CLOCK_FREQ_KHZ,
+						  clock->cycles.shift);
+	clock->nominal_c_mult = clock->cycles.mult;
+	clock->cycles.mask = CLOCKSOURCE_MASK(MLXSW_SP1_PTP_CLOCK_MASK);
+	clock->core = mlxsw_sp->core;
+
+	timecounter_init(&clock->tc, &clock->cycles,
+			 ktime_to_ns(ktime_get_real()));
+
+	/* Calculate period in seconds to call the overflow watchdog - to make
+	 * sure counter is checked at least twice every wrap around.
+	 * The period is calculated as the minimum between max HW cycles count
+	 * (The clock source mask) and max amount of cycles that can be
+	 * multiplied by clock multiplier where the result doesn't exceed
+	 * 64bits.
+	 */
+	overflow_cycles = div64_u64(~0ULL >> 1, clock->cycles.mult);
+	overflow_cycles = min(overflow_cycles, div_u64(clock->cycles.mask, 3));
+
+	nsec = cyclecounter_cyc2ns(&clock->cycles, overflow_cycles, 0, &frac);
+	clock->overflow_period = nsecs_to_jiffies(nsec);
+
+	INIT_DELAYED_WORK(&clock->overflow_work, mlxsw_sp1_ptp_clock_overflow);
+	mlxsw_core_schedule_dw(&clock->overflow_work, 0);
+
+	clock->ptp_info = mlxsw_sp1_ptp_clock_info;
+	clock->ptp = ptp_clock_register(&clock->ptp_info, dev);
+	if (IS_ERR(clock->ptp)) {
+		err = PTR_ERR(clock->ptp);
+		dev_err(dev, "ptp_clock_register failed %d\n", err);
+		goto err_ptp_clock_register;
+	}
+
+	return clock;
+
+err_ptp_clock_register:
+	cancel_delayed_work_sync(&clock->overflow_work);
+	kfree(clock);
+	return ERR_PTR(err);
+}
+
+void mlxsw_sp1_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock)
+{
+	ptp_clock_unregister(clock->ptp);
+	cancel_delayed_work_sync(&clock->overflow_work);
+	kfree(clock);
+}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
new file mode 100644
index 000000000000..76fa00a4be75
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2019 Mellanox Technologies. All rights reserved */
+
+#ifndef _MLXSW_SPECTRUM_PTP_H
+#define _MLXSW_SPECTRUM_PTP_H
+
+#include <linux/device.h>
+
+#include "spectrum.h"
+
+struct mlxsw_sp_ptp_clock;
+
+#if IS_REACHABLE(CONFIG_PTP_1588_CLOCK)
+
+struct mlxsw_sp_ptp_clock *
+mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev);
+
+void mlxsw_sp1_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock);
+
+#else
+
+static inline struct mlxsw_sp_ptp_clock *
+mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev)
+{
+	return NULL;
+}
+
+static inline void mlxsw_sp1_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock)
+{
+}
+
+#endif
+
+static inline struct mlxsw_sp_ptp_clock *
+mlxsw_sp2_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev)
+{
+	return NULL;
+}
+
+static inline void mlxsw_sp2_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock)
+{
+}
+
+#endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 8/9] mlxsw: spectrum: PTP physical hardware clock initialization
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (6 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-03 12:12 ` [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test Ido Schimmel
  2019-06-03 17:35 ` [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock David Miller
  9 siblings, 0 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Initialize the PTP physical hardware clock.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 36 +++++++++++++++++++
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  3 ++
 2 files changed, 39 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index dfe6b44baf63..ef86418bb217 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -41,6 +41,7 @@
 #include "spectrum_dpipe.h"
 #include "spectrum_acl_flex_actions.h"
 #include "spectrum_span.h"
+#include "spectrum_ptp.h"
 #include "../mlxfw/mlxfw.h"
 
 #define MLXSW_SP_FWREV_MINOR_TO_BRANCH(minor) ((minor) / 100)
@@ -4329,6 +4330,22 @@ static int mlxsw_sp_basic_trap_groups_set(struct mlxsw_core *mlxsw_core)
 	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(htgt), htgt_pl);
 }
 
+struct mlxsw_sp_ptp_ops {
+	struct mlxsw_sp_ptp_clock *
+		(*clock_init)(struct mlxsw_sp *mlxsw_sp, struct device *dev);
+	void (*clock_fini)(struct mlxsw_sp_ptp_clock *clock);
+};
+
+static const struct mlxsw_sp_ptp_ops mlxsw_sp1_ptp_ops = {
+	.clock_init	= mlxsw_sp1_ptp_clock_init,
+	.clock_fini	= mlxsw_sp1_ptp_clock_fini,
+};
+
+static const struct mlxsw_sp_ptp_ops mlxsw_sp2_ptp_ops = {
+	.clock_init	= mlxsw_sp2_ptp_clock_init,
+	.clock_fini	= mlxsw_sp2_ptp_clock_fini,
+};
+
 static int mlxsw_sp_netdevice_event(struct notifier_block *unused,
 				    unsigned long event, void *ptr);
 
@@ -4426,6 +4443,18 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 		goto err_router_init;
 	}
 
+	if (mlxsw_sp->bus_info->read_frc_capable) {
+		/* NULL is a valid return value from clock_init */
+		mlxsw_sp->clock =
+			mlxsw_sp->ptp_ops->clock_init(mlxsw_sp,
+						      mlxsw_sp->bus_info->dev);
+		if (IS_ERR(mlxsw_sp->clock)) {
+			err = PTR_ERR(mlxsw_sp->clock);
+			dev_err(mlxsw_sp->bus_info->dev, "Failed to init ptp clock\n");
+			goto err_ptp_clock_init;
+		}
+	}
+
 	/* Initialize netdevice notifier after router and SPAN is initialized,
 	 * so that the event handler can use router structures and call SPAN
 	 * respin.
@@ -4456,6 +4485,9 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 err_dpipe_init:
 	unregister_netdevice_notifier(&mlxsw_sp->netdevice_nb);
 err_netdev_notifier:
+	if (mlxsw_sp->clock)
+		mlxsw_sp->ptp_ops->clock_fini(mlxsw_sp->clock);
+err_ptp_clock_init:
 	mlxsw_sp_router_fini(mlxsw_sp);
 err_router_init:
 	mlxsw_sp_acl_fini(mlxsw_sp);
@@ -4499,6 +4531,7 @@ static int mlxsw_sp1_init(struct mlxsw_core *mlxsw_core,
 	mlxsw_sp->rif_ops_arr = mlxsw_sp1_rif_ops_arr;
 	mlxsw_sp->sb_vals = &mlxsw_sp1_sb_vals;
 	mlxsw_sp->port_type_speed_ops = &mlxsw_sp1_port_type_speed_ops;
+	mlxsw_sp->ptp_ops = &mlxsw_sp1_ptp_ops;
 
 	return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info);
 }
@@ -4518,6 +4551,7 @@ static int mlxsw_sp2_init(struct mlxsw_core *mlxsw_core,
 	mlxsw_sp->rif_ops_arr = mlxsw_sp2_rif_ops_arr;
 	mlxsw_sp->sb_vals = &mlxsw_sp2_sb_vals;
 	mlxsw_sp->port_type_speed_ops = &mlxsw_sp2_port_type_speed_ops;
+	mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
 
 	return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info);
 }
@@ -4529,6 +4563,8 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
 	mlxsw_sp_ports_remove(mlxsw_sp);
 	mlxsw_sp_dpipe_fini(mlxsw_sp);
 	unregister_netdevice_notifier(&mlxsw_sp->netdevice_nb);
+	if (mlxsw_sp->clock)
+		mlxsw_sp->ptp_ops->clock_fini(mlxsw_sp->clock);
 	mlxsw_sp_router_fini(mlxsw_sp);
 	mlxsw_sp_acl_fini(mlxsw_sp);
 	mlxsw_sp_nve_fini(mlxsw_sp);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 8601b3041acd..ea4d56486ea3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -136,6 +136,7 @@ struct mlxsw_sp_acl_tcam_ops;
 struct mlxsw_sp_nve_ops;
 struct mlxsw_sp_sb_vals;
 struct mlxsw_sp_port_type_speed_ops;
+struct mlxsw_sp_ptp_ops;
 
 struct mlxsw_sp {
 	struct mlxsw_sp_port **ports;
@@ -155,6 +156,7 @@ struct mlxsw_sp {
 	struct mlxsw_sp_kvdl *kvdl;
 	struct mlxsw_sp_nve *nve;
 	struct notifier_block netdevice_nb;
+	struct mlxsw_sp_ptp_clock *clock;
 
 	struct mlxsw_sp_counter_pool *counter_pool;
 	struct {
@@ -172,6 +174,7 @@ struct mlxsw_sp {
 	const struct mlxsw_sp_rif_ops **rif_ops_arr;
 	const struct mlxsw_sp_sb_vals *sb_vals;
 	const struct mlxsw_sp_port_type_speed_ops *port_type_speed_ops;
+	const struct mlxsw_sp_ptp_ops *ptp_ops;
 };
 
 static inline struct mlxsw_sp_upper *
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (7 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 8/9] mlxsw: spectrum: PTP physical hardware clock initialization Ido Schimmel
@ 2019-06-03 12:12 ` Ido Schimmel
  2019-06-04 17:05   ` Richard Cochran
  2019-06-07 11:15   ` Vladimir Oltean
  2019-06-03 17:35 ` [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock David Miller
  9 siblings, 2 replies; 37+ messages in thread
From: Ido Schimmel @ 2019-06-03 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, richardcochran, jiri, shalomt, petrm, mlxsw, Ido Schimmel

From: Shalom Toledo <shalomt@mellanox.com>

Test the PTP Physical Hardware Clock functionality using the "phc_ctl" (a
part of "linuxptp").

The test contains three sub-tests:
  * "settime" test
  * "adjtime" test
  * "adjfreq" test

"settime" test:
  * set the PHC time to 0 seconds.
  * wait for 120.5 seconds.
  * check if PHC time equal to 120.XX seconds.

"adjtime" test:
  * set the PHC time to 0 seconds.
  * adjust the time by 10 seconds.
  * check if PHC time equal to 10.XX seconds.

"adjfreq" test:
  * adjust the PHC frequency to be 1% faster.
  * set the PHC time to 0 seconds.
  * wait for 100.5 seconds.
  * check if PHC time equal to 101.XX seconds.

Usage:
  $ ./phc.sh /dev/ptp<X>

  It is possible to run a subset of the tests, for example:
    * To run only the "settime" test:
      $ TESTS="settime" ./phc.sh /dev/ptp<X>

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 tools/testing/selftests/ptp/phc.sh | 166 +++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)
 create mode 100755 tools/testing/selftests/ptp/phc.sh

diff --git a/tools/testing/selftests/ptp/phc.sh b/tools/testing/selftests/ptp/phc.sh
new file mode 100755
index 000000000000..ac6e5a6e1d3a
--- /dev/null
+++ b/tools/testing/selftests/ptp/phc.sh
@@ -0,0 +1,166 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="
+	settime
+	adjtime
+	adjfreq
+"
+DEV=$1
+
+##############################################################################
+# Sanity checks
+
+if [[ "$(id -u)" -ne 0 ]]; then
+	echo "SKIP: need root privileges"
+	exit 0
+fi
+
+if [[ "$DEV" == "" ]]; then
+	echo "SKIP: PTP device not provided"
+	exit 0
+fi
+
+require_command()
+{
+	local cmd=$1; shift
+
+	if [[ ! -x "$(command -v "$cmd")" ]]; then
+		echo "SKIP: $cmd not installed"
+		exit 1
+	fi
+}
+
+phc_sanity()
+{
+	phc_ctl $DEV get &> /dev/null
+
+	if [ $? != 0 ]; then
+		echo "SKIP: unknown clock $DEV: No such device"
+		exit 1
+	fi
+}
+
+require_command phc_ctl
+phc_sanity
+
+##############################################################################
+# Helpers
+
+# Exit status to return at the end. Set in case one of the tests fails.
+EXIT_STATUS=0
+# Per-test return value. Clear at the beginning of each test.
+RET=0
+
+check_err()
+{
+	local err=$1
+
+	if [[ $RET -eq 0 && $err -ne 0 ]]; then
+		RET=$err
+	fi
+}
+
+log_test()
+{
+	local test_name=$1
+
+	if [[ $RET -ne 0 ]]; then
+		EXIT_STATUS=1
+		printf "TEST: %-60s  [FAIL]\n" "$test_name"
+		return 1
+	fi
+
+	printf "TEST: %-60s  [ OK ]\n" "$test_name"
+	return 0
+}
+
+tests_run()
+{
+	local current_test
+
+	for current_test in ${TESTS:-$ALL_TESTS}; do
+		$current_test
+	done
+}
+
+##############################################################################
+# Tests
+
+settime_do()
+{
+	local res
+
+	res=$(phc_ctl $DEV set 0 wait 120.5 get 2> /dev/null \
+		| awk '/clock time is/{print $5}' \
+		| awk -F. '{print $1}')
+
+	(( res == 120 ))
+}
+
+adjtime_do()
+{
+	local res
+
+	res=$(phc_ctl $DEV set 0 adj 10 get 2> /dev/null \
+		| awk '/clock time is/{print $5}' \
+		| awk -F. '{print $1}')
+
+	(( res == 10 ))
+}
+
+adjfreq_do()
+{
+	local res
+
+	# Set the clock to be 1% faster
+	res=$(phc_ctl $DEV freq 10000000 set 0 wait 100.5 get 2> /dev/null \
+		| awk '/clock time is/{print $5}' \
+		| awk -F. '{print $1}')
+
+	(( res == 101 ))
+}
+
+##############################################################################
+
+cleanup()
+{
+	phc_ctl $DEV freq 0.0 &> /dev/null
+	phc_ctl $DEV set &> /dev/null
+}
+
+settime()
+{
+	RET=0
+
+	settime_do
+	check_err $?
+	log_test "settime"
+	cleanup
+}
+
+adjtime()
+{
+	RET=0
+
+	adjtime_do
+	check_err $?
+	log_test "adjtime"
+	cleanup
+}
+
+adjfreq()
+{
+	RET=0
+
+	adjfreq_do
+	check_err $?
+	log_test "adjfreq"
+	cleanup
+}
+
+trap cleanup EXIT
+
+tests_run
+
+exit $EXIT_STATUS
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock
  2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
                   ` (8 preceding siblings ...)
  2019-06-03 12:12 ` [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test Ido Schimmel
@ 2019-06-03 17:35 ` David Miller
  9 siblings, 0 replies; 37+ messages in thread
From: David Miller @ 2019-06-03 17:35 UTC (permalink / raw)
  To: idosch; +Cc: netdev, richardcochran, jiri, shalomt, petrm, mlxsw, idosch

From: Ido Schimmel <idosch@idosch.org>
Date: Mon,  3 Jun 2019 15:12:35 +0300

> This is the first of about four patchsets that add PTP support in mlxsw
> for the Spectrum-1 ASIC.

Richard, could you give this a quick review?

Thank you.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-03 12:12 ` [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register Ido Schimmel
@ 2019-06-04 14:17   ` Richard Cochran
  2019-06-05 11:30     ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-04 14:17 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

On Mon, Jun 03, 2019 at 03:12:39PM +0300, Ido Schimmel wrote:
> From: Shalom Toledo <shalomt@mellanox.com>
> 
> The MTUTC register configures the HW UTC counter.

Why is this called the "UTC" counter?

The PTP time scale is TAI.  Is this counter intended to reflect the
Linux CLOCK_REALTIME system time?

> +/* MTUTC - Management UTC Register
> + * -------------------------------
> + * Configures the HW UTC counter.
> + */
> +#define MLXSW_REG_MTUTC_ID 0x9055
> +#define MLXSW_REG_MTUTC_LEN 0x1C

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb
  2019-06-03 12:12 ` [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb Ido Schimmel
@ 2019-06-04 14:21   ` Richard Cochran
  2019-06-05 11:46     ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-04 14:21 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

On Mon, Jun 03, 2019 at 03:12:41PM +0300, Ido Schimmel wrote:
> From: Shalom Toledo <shalomt@mellanox.com>
> 
> Publish scaled_ppm_to_ppb to allow drivers to use it.

But why?

> @@ -63,7 +63,7 @@ static void enqueue_external_timestamp(struct timestamp_event_queue *queue,
>  	spin_unlock_irqrestore(&queue->lock, flags);
>  }
>  
> -static s32 scaled_ppm_to_ppb(long ppm)
> +s32 ptp_clock_scaled_ppm_to_ppb(long ppm)

Six words is a little bit much for the name of a function.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-03 12:12 ` [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations Ido Schimmel
@ 2019-06-04 14:28   ` Richard Cochran
  2019-06-05  6:30     ` Jiri Pirko
  2019-06-05 11:44     ` Shalom Toledo
  2019-06-04 17:03   ` Richard Cochran
  1 sibling, 2 replies; 37+ messages in thread
From: Richard Cochran @ 2019-06-04 14:28 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:

> +static int
> +mlxsw_sp1_ptp_update_phc_settime(struct mlxsw_sp_ptp_clock *clock, u64 nsec)

Six words ^^^

What is wrong with "mlxsw_phc_settime" ?

> +{
> +	struct mlxsw_core *mlxsw_core = clock->core;
> +	char mtutc_pl[MLXSW_REG_MTUTC_LEN];
> +	char mtpps_pl[MLXSW_REG_MTPPS_LEN];
> +	u64 next_sec_in_nsec, cycles;
> +	u32 next_sec;
> +	int err;
> +
> +	next_sec = nsec / NSEC_PER_SEC + 1;
> +	next_sec_in_nsec = next_sec * NSEC_PER_SEC;
> +
> +	spin_lock(&clock->lock);
> +	cycles = mlxsw_sp1_ptp_ns2cycles(&clock->tc, next_sec_in_nsec);
> +	spin_unlock(&clock->lock);
> +
> +	mlxsw_reg_mtpps_vpin_pack(mtpps_pl, cycles);
> +	err = mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtpps), mtpps_pl);
> +	if (err)
> +		return err;
> +
> +	mlxsw_reg_mtutc_pack(mtutc_pl,
> +			     MLXSW_REG_MTUTC_OPERATION_SET_TIME_AT_NEXT_SEC,
> +			     0, next_sec);
> +	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtutc), mtutc_pl);
> +}
> +
> +static int mlxsw_sp1_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> +	struct mlxsw_sp_ptp_clock *clock =
> +		container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
> +	int neg_adj = 0;
> +	u32 diff;
> +	u64 adj;
> +	s32 ppb;
> +
> +	ppb = ptp_clock_scaled_ppm_to_ppb(scaled_ppm);

Now I see why you did this.  Nice try.

The 'scaled_ppm' has a finer resolution than ppb.  Please make use of
the finer resolution in your calculation.  It does make a difference.

> +
> +	if (ppb < 0) {
> +		neg_adj = 1;
> +		ppb = -ppb;
> +	}
> +
> +	adj = clock->nominal_c_mult;
> +	adj *= ppb;
> +	diff = div_u64(adj, NSEC_PER_SEC);
> +
> +	spin_lock(&clock->lock);
> +	timecounter_read(&clock->tc);
> +	clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
> +				       clock->nominal_c_mult + diff;
> +	spin_unlock(&clock->lock);
> +
> +	return mlxsw_sp1_ptp_update_phc_adjfreq(clock, neg_adj ? -ppb : ppb);
> +}

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-03 12:12 ` [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations Ido Schimmel
  2019-06-04 14:28   ` Richard Cochran
@ 2019-06-04 17:03   ` Richard Cochran
  2019-06-05  9:00     ` Petr Machata
  1 sibling, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-04 17:03 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
> +struct mlxsw_sp_ptp_clock *
> +mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev)
> +{
> +	u64 overflow_cycles, nsec, frac = 0;
> +	struct mlxsw_sp_ptp_clock *clock;
> +	int err;
> +
> +	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
> +	if (!clock)
> +		return ERR_PTR(-ENOMEM);
> +
> +	spin_lock_init(&clock->lock);
> +	clock->cycles.read = mlxsw_sp1_ptp_read_frc;
> +	clock->cycles.shift = MLXSW_SP1_PTP_CLOCK_CYCLES_SHIFT;
> +	clock->cycles.mult = clocksource_khz2mult(MLXSW_SP1_PTP_CLOCK_FREQ_KHZ,
> +						  clock->cycles.shift);
> +	clock->nominal_c_mult = clock->cycles.mult;
> +	clock->cycles.mask = CLOCKSOURCE_MASK(MLXSW_SP1_PTP_CLOCK_MASK);
> +	clock->core = mlxsw_sp->core;
> +
> +	timecounter_init(&clock->tc, &clock->cycles,
> +			 ktime_to_ns(ktime_get_real()));
> +
> +	/* Calculate period in seconds to call the overflow watchdog - to make
> +	 * sure counter is checked at least twice every wrap around.
> +	 * The period is calculated as the minimum between max HW cycles count
> +	 * (The clock source mask) and max amount of cycles that can be
> +	 * multiplied by clock multiplier where the result doesn't exceed
> +	 * 64bits.
> +	 */
> +	overflow_cycles = div64_u64(~0ULL >> 1, clock->cycles.mult);
> +	overflow_cycles = min(overflow_cycles, div_u64(clock->cycles.mask, 3));
> +
> +	nsec = cyclecounter_cyc2ns(&clock->cycles, overflow_cycles, 0, &frac);
> +	clock->overflow_period = nsecs_to_jiffies(nsec);
> +
> +	INIT_DELAYED_WORK(&clock->overflow_work, mlxsw_sp1_ptp_clock_overflow);
> +	mlxsw_core_schedule_dw(&clock->overflow_work, 0);
> +
> +	clock->ptp_info = mlxsw_sp1_ptp_clock_info;
> +	clock->ptp = ptp_clock_register(&clock->ptp_info, dev);
> +	if (IS_ERR(clock->ptp)) {
> +		err = PTR_ERR(clock->ptp);
> +		dev_err(dev, "ptp_clock_register failed %d\n", err);
> +		goto err_ptp_clock_register;
> +	}
> +
> +	return clock;

You need to handle the case where ptp_clock_register() returns NULL...

/**
 * ptp_clock_register() - register a PTP hardware clock driver
 *
 * @info:   Structure describing the new clock.
 * @parent: Pointer to the parent device of the new clock.
 *
 * Returns a valid pointer on success or PTR_ERR on failure.  If PHC
 * support is missing at the configuration level, this function
 * returns NULL, and drivers are expected to gracefully handle that
 * case separately.
 */

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test
  2019-06-03 12:12 ` [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test Ido Schimmel
@ 2019-06-04 17:05   ` Richard Cochran
  2019-06-07 11:15   ` Vladimir Oltean
  1 sibling, 0 replies; 37+ messages in thread
From: Richard Cochran @ 2019-06-04 17:05 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

On Mon, Jun 03, 2019 at 03:12:44PM +0300, Ido Schimmel wrote:
> From: Shalom Toledo <shalomt@mellanox.com>
> 
> Test the PTP Physical Hardware Clock functionality using the "phc_ctl" (a
> part of "linuxptp").
> 
> The test contains three sub-tests:
>   * "settime" test
>   * "adjtime" test
>   * "adjfreq" test

This looks useful!

Acked-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-04 14:28   ` Richard Cochran
@ 2019-06-05  6:30     ` Jiri Pirko
  2019-06-05 17:24       ` Richard Cochran
  2019-06-05 11:44     ` Shalom Toledo
  1 sibling, 1 reply; 37+ messages in thread
From: Jiri Pirko @ 2019-06-05  6:30 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

Tue, Jun 04, 2019 at 04:28:19PM CEST, richardcochran@gmail.com wrote:
>On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
>
>> +static int
>> +mlxsw_sp1_ptp_update_phc_settime(struct mlxsw_sp_ptp_clock *clock, u64 nsec)
>
>Six words ^^^

It is aligned with the rest of mlxsw code.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-04 17:03   ` Richard Cochran
@ 2019-06-05  9:00     ` Petr Machata
  2019-06-05 17:31       ` Richard Cochran
  0 siblings, 1 reply; 37+ messages in thread
From: Petr Machata @ 2019-06-05  9:00 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Shalom Toledo, mlxsw,
	Ido Schimmel


Richard Cochran <richardcochran@gmail.com> writes:

> On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
>> +struct mlxsw_sp_ptp_clock *
>> +mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev)

[...]

>> +	clock->ptp_info = mlxsw_sp1_ptp_clock_info;
>> +	clock->ptp = ptp_clock_register(&clock->ptp_info, dev);
>> +	if (IS_ERR(clock->ptp)) {
>> +		err = PTR_ERR(clock->ptp);
>> +		dev_err(dev, "ptp_clock_register failed %d\n", err);
>> +		goto err_ptp_clock_register;
>> +	}
>> +
>> +	return clock;
>
> You need to handle the case where ptp_clock_register() returns NULL...
>
> /**
>  * ptp_clock_register() - register a PTP hardware clock driver
>  *
>  * @info:   Structure describing the new clock.
>  * @parent: Pointer to the parent device of the new clock.
>  *
>  * Returns a valid pointer on success or PTR_ERR on failure.  If PHC
>  * support is missing at the configuration level, this function
>  * returns NULL, and drivers are expected to gracefully handle that
>  * case separately.
>  */

We don't build the PTP module at all unless CONFIG_PTP_1588_CLOCK is
enabled, and fall back to inline stubs unless it IS_REACHABLE. I believe
this should be OK.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-04 14:17   ` Richard Cochran
@ 2019-06-05 11:30     ` Shalom Toledo
  2019-06-05 17:23       ` Richard Cochran
  0 siblings, 1 reply; 37+ messages in thread
From: Shalom Toledo @ 2019-06-05 11:30 UTC (permalink / raw)
  To: Richard Cochran, Ido Schimmel
  Cc: netdev, davem, Jiri Pirko, Petr Machata, mlxsw, Ido Schimmel

On 04/06/2019 17:17, Richard Cochran wrote:
> On Mon, Jun 03, 2019 at 03:12:39PM +0300, Ido Schimmel wrote:
>> From: Shalom Toledo <shalomt@mellanox.com>
>>
>> The MTUTC register configures the HW UTC counter.
> 
> Why is this called the "UTC" counter?
> 
> The PTP time scale is TAI.  Is this counter intended to reflect the
> Linux CLOCK_REALTIME system time?

Exactly. The hardware doesn't have the ability to convert the FRC to UTC, so
we, as a driver, need to do it and align the hardware with this value.

> 
>> +/* MTUTC - Management UTC Register
>> + * -------------------------------
>> + * Configures the HW UTC counter.
>> + */
>> +#define MLXSW_REG_MTUTC_ID 0x9055
>> +#define MLXSW_REG_MTUTC_LEN 0x1C
> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-04 14:28   ` Richard Cochran
  2019-06-05  6:30     ` Jiri Pirko
@ 2019-06-05 11:44     ` Shalom Toledo
  2019-06-05 17:40       ` Richard Cochran
  1 sibling, 1 reply; 37+ messages in thread
From: Shalom Toledo @ 2019-06-05 11:44 UTC (permalink / raw)
  To: Richard Cochran, Ido Schimmel
  Cc: netdev, davem, Jiri Pirko, Petr Machata, mlxsw, Ido Schimmel

On 04/06/2019 17:28, Richard Cochran wrote:
> On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
> 
>> +static int
>> +mlxsw_sp1_ptp_update_phc_settime(struct mlxsw_sp_ptp_clock *clock, u64 nsec)
> 
> Six words ^^^
> 
> What is wrong with "mlxsw_phc_settime" ?

I can drop the "update". But as Jiri mentioned, it is aligned with the rest of
mlxsw code.

> 
>> +{
>> +	struct mlxsw_core *mlxsw_core = clock->core;
>> +	char mtutc_pl[MLXSW_REG_MTUTC_LEN];
>> +	char mtpps_pl[MLXSW_REG_MTPPS_LEN];
>> +	u64 next_sec_in_nsec, cycles;
>> +	u32 next_sec;
>> +	int err;
>> +
>> +	next_sec = nsec / NSEC_PER_SEC + 1;
>> +	next_sec_in_nsec = next_sec * NSEC_PER_SEC;
>> +
>> +	spin_lock(&clock->lock);
>> +	cycles = mlxsw_sp1_ptp_ns2cycles(&clock->tc, next_sec_in_nsec);
>> +	spin_unlock(&clock->lock);
>> +
>> +	mlxsw_reg_mtpps_vpin_pack(mtpps_pl, cycles);
>> +	err = mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtpps), mtpps_pl);
>> +	if (err)
>> +		return err;
>> +
>> +	mlxsw_reg_mtutc_pack(mtutc_pl,
>> +			     MLXSW_REG_MTUTC_OPERATION_SET_TIME_AT_NEXT_SEC,
>> +			     0, next_sec);
>> +	return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtutc), mtutc_pl);
>> +}
>> +
>> +static int mlxsw_sp1_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
>> +{
>> +	struct mlxsw_sp_ptp_clock *clock =
>> +		container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
>> +	int neg_adj = 0;
>> +	u32 diff;
>> +	u64 adj;
>> +	s32 ppb;
>> +
>> +	ppb = ptp_clock_scaled_ppm_to_ppb(scaled_ppm);
> 
> Now I see why you did this.  Nice try.

I didn't try anything.

The reason is that the hardware units is in ppb and not in scaled_ppm(or ppm),
so I just converted to ppb in order to set the hardware.

But I got your point, I will change my calculation to use scaled_ppm (to get a
more finer resolution) and not ppb, and convert to ppb just before setting the
hardware. Is that make sense?

But I'm still need to expose scaled_ppm_to_ppb.

> 
> The 'scaled_ppm' has a finer resolution than ppb.  Please make use of
> the finer resolution in your calculation.  It does make a difference.

Will change, thanks for that!

> 
>> +
>> +	if (ppb < 0) {
>> +		neg_adj = 1;
>> +		ppb = -ppb;
>> +	}
>> +
>> +	adj = clock->nominal_c_mult;
>> +	adj *= ppb;
>> +	diff = div_u64(adj, NSEC_PER_SEC);
>> +
>> +	spin_lock(&clock->lock);
>> +	timecounter_read(&clock->tc);
>> +	clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
>> +				       clock->nominal_c_mult + diff;
>> +	spin_unlock(&clock->lock);
>> +
>> +	return mlxsw_sp1_ptp_update_phc_adjfreq(clock, neg_adj ? -ppb : ppb);
>> +}
> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb
  2019-06-04 14:21   ` Richard Cochran
@ 2019-06-05 11:46     ` Shalom Toledo
  0 siblings, 0 replies; 37+ messages in thread
From: Shalom Toledo @ 2019-06-05 11:46 UTC (permalink / raw)
  To: Richard Cochran, Ido Schimmel
  Cc: netdev, davem, Jiri Pirko, Petr Machata, mlxsw, Ido Schimmel

On 04/06/2019 17:21, Richard Cochran wrote:
> On Mon, Jun 03, 2019 at 03:12:41PM +0300, Ido Schimmel wrote:
>> From: Shalom Toledo <shalomt@mellanox.com>
>>
>> Publish scaled_ppm_to_ppb to allow drivers to use it.
> 
> But why?

See my comment in patch #7

> 
>> @@ -63,7 +63,7 @@ static void enqueue_external_timestamp(struct timestamp_event_queue *queue,
>>  	spin_unlock_irqrestore(&queue->lock, flags);
>>  }
>>  
>> -static s32 scaled_ppm_to_ppb(long ppm)
>> +s32 ptp_clock_scaled_ppm_to_ppb(long ppm)
> 
> Six words is a little bit much for the name of a function.

I can make it scaled_ppm_to_ppb() as before. Is that ok? or maybe you have
something else in your mind?

> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-05 11:30     ` Shalom Toledo
@ 2019-06-05 17:23       ` Richard Cochran
  2019-06-05 18:55         ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-05 17:23 UTC (permalink / raw)
  To: Shalom Toledo
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On Wed, Jun 05, 2019 at 11:30:06AM +0000, Shalom Toledo wrote:
> On 04/06/2019 17:17, Richard Cochran wrote:
> > On Mon, Jun 03, 2019 at 03:12:39PM +0300, Ido Schimmel wrote:
> >> From: Shalom Toledo <shalomt@mellanox.com>
> >>
> >> The MTUTC register configures the HW UTC counter.
> > 
> > Why is this called the "UTC" counter?
> > 
> > The PTP time scale is TAI.  Is this counter intended to reflect the
> > Linux CLOCK_REALTIME system time?
> 
> Exactly. The hardware doesn't have the ability to convert the FRC to UTC, so
> we, as a driver, need to do it and align the hardware with this value.

What does "FRC" mean?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-05  6:30     ` Jiri Pirko
@ 2019-06-05 17:24       ` Richard Cochran
  0 siblings, 0 replies; 37+ messages in thread
From: Richard Cochran @ 2019-06-05 17:24 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ido Schimmel, netdev, davem, jiri, shalomt, petrm, mlxsw, Ido Schimmel

On Wed, Jun 05, 2019 at 08:30:21AM +0200, Jiri Pirko wrote:
> Tue, Jun 04, 2019 at 04:28:19PM CEST, richardcochran@gmail.com wrote:
> >On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
> >
> >> +static int
> >> +mlxsw_sp1_ptp_update_phc_settime(struct mlxsw_sp_ptp_clock *clock, u64 nsec)
> >
> >Six words ^^^
> 
> It is aligned with the rest of mlxsw code.

Those can be fixed, too.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-05  9:00     ` Petr Machata
@ 2019-06-05 17:31       ` Richard Cochran
  2019-06-06 10:21         ` Petr Machata
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-05 17:31 UTC (permalink / raw)
  To: Petr Machata
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Shalom Toledo, mlxsw,
	Ido Schimmel

On Wed, Jun 05, 2019 at 09:00:09AM +0000, Petr Machata wrote:
> We don't build the PTP module at all unless CONFIG_PTP_1588_CLOCK is
> enabled, and fall back to inline stubs unless it IS_REACHABLE. I believe
> this should be OK.

Please use "imply PTP_1588_CLOCK" in your kconfig, just like the other
PTP drivers do.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-05 11:44     ` Shalom Toledo
@ 2019-06-05 17:40       ` Richard Cochran
  2019-06-05 19:28         ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-05 17:40 UTC (permalink / raw)
  To: Shalom Toledo
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On Wed, Jun 05, 2019 at 11:44:21AM +0000, Shalom Toledo wrote:
> On 04/06/2019 17:28, Richard Cochran wrote:
> > Now I see why you did this.  Nice try.
> 
> I didn't try anything.
> 
> The reason is that the hardware units is in ppb and not in scaled_ppm(or ppm),
> so I just converted to ppb in order to set the hardware.

Oh, I thought you were adapting code for the deprecated .adjfreq method.
 
> But I got your point, I will change my calculation to use scaled_ppm (to get a
> more finer resolution) and not ppb, and convert to ppb just before setting the
> hardware. Is that make sense?

So the HW actually accepts ppb adjustment values?  Fine.

But I don't understand this:

> >> +	if (ppb < 0) {
> >> +		neg_adj = 1;
> >> +		ppb = -ppb;
> >> +	}
> >> +
> >> +	adj = clock->nominal_c_mult;
> >> +	adj *= ppb;
> >> +	diff = div_u64(adj, NSEC_PER_SEC);
> >> +
> >> +	spin_lock(&clock->lock);
> >> +	timecounter_read(&clock->tc);
> >> +	clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
> >> +				       clock->nominal_c_mult + diff;
> >> +	spin_unlock(&clock->lock);

You have a SW time counter here

> >> +	return mlxsw_sp1_ptp_update_phc_adjfreq(clock, neg_adj ? -ppb : ppb);

and a hardware method here?

Why not choose one or the other?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-05 17:23       ` Richard Cochran
@ 2019-06-05 18:55         ` Shalom Toledo
  2019-06-06  2:37           ` Richard Cochran
  0 siblings, 1 reply; 37+ messages in thread
From: Shalom Toledo @ 2019-06-05 18:55 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On 05/06/2019 20:23, Richard Cochran wrote:
> On Wed, Jun 05, 2019 at 11:30:06AM +0000, Shalom Toledo wrote:
>> On 04/06/2019 17:17, Richard Cochran wrote:
>>> On Mon, Jun 03, 2019 at 03:12:39PM +0300, Ido Schimmel wrote:
>>>> From: Shalom Toledo <shalomt@mellanox.com>
>>>>
>>>> The MTUTC register configures the HW UTC counter.
>>>
>>> Why is this called the "UTC" counter?
>>>
>>> The PTP time scale is TAI.  Is this counter intended to reflect the
>>> Linux CLOCK_REALTIME system time?
>>
>> Exactly. The hardware doesn't have the ability to convert the FRC to UTC, so
>> we, as a driver, need to do it and align the hardware with this value.
> 
> What does "FRC" mean?

Free Running Counter

> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-05 17:40       ` Richard Cochran
@ 2019-06-05 19:28         ` Shalom Toledo
  2019-06-06  2:43           ` Richard Cochran
  0 siblings, 1 reply; 37+ messages in thread
From: Shalom Toledo @ 2019-06-05 19:28 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On 05/06/2019 20:40, Richard Cochran wrote:
> On Wed, Jun 05, 2019 at 11:44:21AM +0000, Shalom Toledo wrote:
>> On 04/06/2019 17:28, Richard Cochran wrote:
>>> Now I see why you did this.  Nice try.
>>
>> I didn't try anything.
>>
>> The reason is that the hardware units is in ppb and not in scaled_ppm(or ppm),
>> so I just converted to ppb in order to set the hardware.
> 
> Oh, I thought you were adapting code for the deprecated .adjfreq method.
>  
>> But I got your point, I will change my calculation to use scaled_ppm (to get a
>> more finer resolution) and not ppb, and convert to ppb just before setting the
>> hardware. Is that make sense?
> 
> So the HW actually accepts ppb adjustment values?  Fine.

So I leave the code as is.

> 
> But I don't understand this:
> 
>>>> +	if (ppb < 0) {
>>>> +		neg_adj = 1;
>>>> +		ppb = -ppb;
>>>> +	}
>>>> +
>>>> +	adj = clock->nominal_c_mult;
>>>> +	adj *= ppb;
>>>> +	diff = div_u64(adj, NSEC_PER_SEC);
>>>> +
>>>> +	spin_lock(&clock->lock);
>>>> +	timecounter_read(&clock->tc);
>>>> +	clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
>>>> +				       clock->nominal_c_mult + diff;
>>>> +	spin_unlock(&clock->lock);
> 
> You have a SW time counter here

Yep.

> 
>>>> +	return mlxsw_sp1_ptp_update_phc_adjfreq(clock, neg_adj ? -ppb : ppb);
> 
> and a hardware method here?

Yep.

> 
> Why not choose one or the other?

You are right, but there is a reason behind it of course.

The hardware has a free running counter that the SW can only read. This is
used when we do gettimex op. The rest ops, settime, adjfine and adjtime, we do
only in SW, since we can't control the HW free running counter.

Now, you are asking yourself, why the driver also update the HW? what does it
mean?

So, there is an HW machine which responsible for adding UTC timestamp on
R-SPAN mirror packets and there is no connection to the HW free running
counter. The SW can and should control this HW machine, from setting the UTC
time when doing settime for example. Or adjusting the frequency when doing
adjfine.

I hope it is more clear now.

> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-05 18:55         ` Shalom Toledo
@ 2019-06-06  2:37           ` Richard Cochran
  2019-06-06  9:11             ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-06  2:37 UTC (permalink / raw)
  To: Shalom Toledo
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On Wed, Jun 05, 2019 at 06:55:18PM +0000, Shalom Toledo wrote:
> On 05/06/2019 20:23, Richard Cochran wrote:
> > On Wed, Jun 05, 2019 at 11:30:06AM +0000, Shalom Toledo wrote:
> >> On 04/06/2019 17:17, Richard Cochran wrote:
> >>> On Mon, Jun 03, 2019 at 03:12:39PM +0300, Ido Schimmel wrote:
> >>>> From: Shalom Toledo <shalomt@mellanox.com>
> >>>>
> >>>> The MTUTC register configures the HW UTC counter.
> >>>
> >>> Why is this called the "UTC" counter?
> >>>
> >>> The PTP time scale is TAI.  Is this counter intended to reflect the
> >>> Linux CLOCK_REALTIME system time?
> >>
> >> Exactly. The hardware doesn't have the ability to convert the FRC to UTC, so
> >> we, as a driver, need to do it and align the hardware with this value.
> > 
> > What does "FRC" mean?
> 
> Free Running Counter

Okay, so then you want to convert it into TAI (for PTP) rather than UTC.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-05 19:28         ` Shalom Toledo
@ 2019-06-06  2:43           ` Richard Cochran
  2019-06-06  8:50             ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Richard Cochran @ 2019-06-06  2:43 UTC (permalink / raw)
  To: Shalom Toledo
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On Wed, Jun 05, 2019 at 07:28:38PM +0000, Shalom Toledo wrote:
> So, there is an HW machine which responsible for adding UTC timestamp on
> R-SPAN mirror packets and there is no connection to the HW free running
> counter.

If there is no connection, then the frequency adjustments to the HW
clock will not work as expected.

Perhaps the free running counter and the HW clock share the same clock
source?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-06  2:43           ` Richard Cochran
@ 2019-06-06  8:50             ` Shalom Toledo
  2019-06-06  8:57               ` Shalom Toledo
  0 siblings, 1 reply; 37+ messages in thread
From: Shalom Toledo @ 2019-06-06  8:50 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On 06/06/2019 5:43, Richard Cochran wrote:
> On Wed, Jun 05, 2019 at 07:28:38PM +0000, Shalom Toledo wrote:
>> So, there is an HW machine which responsible for adding UTC timestamp on
>> R-SPAN mirror packets and there is no connection to the HW free running
>> counter.
> 
> If there is no connection, then the frequency adjustments to the HW
> clock will not work as expected.

I mean that these are two different peaces of HW with different control
interfaces.

> 
> Perhaps the free running counter and the HW clock share the same clock
> source?

Yes, of course.

> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-06  8:50             ` Shalom Toledo
@ 2019-06-06  8:57               ` Shalom Toledo
  0 siblings, 0 replies; 37+ messages in thread
From: Shalom Toledo @ 2019-06-06  8:57 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On 06/06/2019 11:50, Shalom Toledo wrote:
> On 06/06/2019 5:43, Richard Cochran wrote:
>> On Wed, Jun 05, 2019 at 07:28:38PM +0000, Shalom Toledo wrote:
>>> So, there is an HW machine which responsible for adding UTC timestamp on
>>> R-SPAN mirror packets and there is no connection to the HW free running
>>> counter.
>>
>> If there is no connection, then the frequency adjustments to the HW
>> clock will not work as expected.
> 
> I mean that these are two different peaces of HW with different control
> interfaces.

pieces*

> 
>>
>> Perhaps the free running counter and the HW clock share the same clock
>> source?
> 
> Yes, of course.
> 
>>
>> Thanks,
>> Richard
>>
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-06  2:37           ` Richard Cochran
@ 2019-06-06  9:11             ` Shalom Toledo
  2019-06-06 10:12               ` Petr Machata
  0 siblings, 1 reply; 37+ messages in thread
From: Shalom Toledo @ 2019-06-06  9:11 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Petr Machata, mlxsw,
	Ido Schimmel

On 06/06/2019 5:37, Richard Cochran wrote:
> On Wed, Jun 05, 2019 at 06:55:18PM +0000, Shalom Toledo wrote:
>> On 05/06/2019 20:23, Richard Cochran wrote:
>>> On Wed, Jun 05, 2019 at 11:30:06AM +0000, Shalom Toledo wrote:
>>>> On 04/06/2019 17:17, Richard Cochran wrote:
>>>>> On Mon, Jun 03, 2019 at 03:12:39PM +0300, Ido Schimmel wrote:
>>>>>> From: Shalom Toledo <shalomt@mellanox.com>
>>>>>>
>>>>>> The MTUTC register configures the HW UTC counter.
>>>>>
>>>>> Why is this called the "UTC" counter?
>>>>>
>>>>> The PTP time scale is TAI.  Is this counter intended to reflect the
>>>>> Linux CLOCK_REALTIME system time?
>>>>
>>>> Exactly. The hardware doesn't have the ability to convert the FRC to UTC, so
>>>> we, as a driver, need to do it and align the hardware with this value.
>>>
>>> What does "FRC" mean?
>>
>> Free Running Counter
> 
> Okay, so then you want to convert it into TAI (for PTP) rather than UTC.

No, the HW interface is in UTC format. This is part of the HW machine that
responsible for adding the UTC time stamping on R-SPAN mirror packets.

> 
> Thanks,
> Richard
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register
  2019-06-06  9:11             ` Shalom Toledo
@ 2019-06-06 10:12               ` Petr Machata
  0 siblings, 0 replies; 37+ messages in thread
From: Petr Machata @ 2019-06-06 10:12 UTC (permalink / raw)
  To: Shalom Toledo
  Cc: Richard Cochran, Ido Schimmel, netdev, davem, Jiri Pirko, mlxsw,
	Ido Schimmel


Shalom Toledo <shalomt@mellanox.com> writes:

> On 06/06/2019 5:37, Richard Cochran wrote:
>> Okay, so then you want to convert it into TAI (for PTP) rather than UTC.
>
> No, the HW interface is in UTC format. This is part of the HW machine that
> responsible for adding the UTC time stamping on R-SPAN mirror packets.

The way I understand it is that UTC is a misnomer. We are certainly not
updating the list of leap seconds in the ASIC so that it knows how to
get to UTC. (To be clear, no such interface even exists.) But it's
called UTC in the documentation, so we want to keep the name for that
reason.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations
  2019-06-05 17:31       ` Richard Cochran
@ 2019-06-06 10:21         ` Petr Machata
  0 siblings, 0 replies; 37+ messages in thread
From: Petr Machata @ 2019-06-06 10:21 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Ido Schimmel, netdev, davem, Jiri Pirko, Shalom Toledo, mlxsw,
	Ido Schimmel


Richard Cochran <richardcochran@gmail.com> writes:

> On Wed, Jun 05, 2019 at 09:00:09AM +0000, Petr Machata wrote:
>> We don't build the PTP module at all unless CONFIG_PTP_1588_CLOCK is
>> enabled, and fall back to inline stubs unless it IS_REACHABLE. I believe
>> this should be OK.
>
> Please use "imply PTP_1588_CLOCK" in your kconfig, just like the other
> PTP drivers do.

All right.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test
  2019-06-03 12:12 ` [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test Ido Schimmel
  2019-06-04 17:05   ` Richard Cochran
@ 2019-06-07 11:15   ` Vladimir Oltean
  2019-06-08 10:44     ` Vladimir Oltean
  2019-06-11 13:54     ` Shalom Toledo
  1 sibling, 2 replies; 37+ messages in thread
From: Vladimir Oltean @ 2019-06-07 11:15 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, David S. Miller, Richard Cochran, jiri, shalomt, petrm,
	mlxsw, Ido Schimmel

On Mon, 3 Jun 2019 at 15:25, Ido Schimmel <idosch@idosch.org> wrote:
>
> From: Shalom Toledo <shalomt@mellanox.com>
>
> Test the PTP Physical Hardware Clock functionality using the "phc_ctl" (a
> part of "linuxptp").
>
> The test contains three sub-tests:
>   * "settime" test
>   * "adjtime" test
>   * "adjfreq" test
>
> "settime" test:
>   * set the PHC time to 0 seconds.
>   * wait for 120.5 seconds.
>   * check if PHC time equal to 120.XX seconds.
>
> "adjtime" test:
>   * set the PHC time to 0 seconds.
>   * adjust the time by 10 seconds.
>   * check if PHC time equal to 10.XX seconds.
>
> "adjfreq" test:
>   * adjust the PHC frequency to be 1% faster.
>   * set the PHC time to 0 seconds.
>   * wait for 100.5 seconds.
>   * check if PHC time equal to 101.XX seconds.
>
> Usage:
>   $ ./phc.sh /dev/ptp<X>
>
>   It is possible to run a subset of the tests, for example:
>     * To run only the "settime" test:
>       $ TESTS="settime" ./phc.sh /dev/ptp<X>
>
> Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
> Reviewed-by: Petr Machata <petrm@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  tools/testing/selftests/ptp/phc.sh | 166 +++++++++++++++++++++++++++++
>  1 file changed, 166 insertions(+)
>  create mode 100755 tools/testing/selftests/ptp/phc.sh
>
> diff --git a/tools/testing/selftests/ptp/phc.sh b/tools/testing/selftests/ptp/phc.sh
> new file mode 100755
> index 000000000000..ac6e5a6e1d3a
> --- /dev/null
> +++ b/tools/testing/selftests/ptp/phc.sh
> @@ -0,0 +1,166 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +ALL_TESTS="
> +       settime
> +       adjtime
> +       adjfreq
> +"
> +DEV=$1
> +
> +##############################################################################
> +# Sanity checks
> +
> +if [[ "$(id -u)" -ne 0 ]]; then
> +       echo "SKIP: need root privileges"
> +       exit 0
> +fi
> +
> +if [[ "$DEV" == "" ]]; then
> +       echo "SKIP: PTP device not provided"
> +       exit 0
> +fi
> +
> +require_command()
> +{
> +       local cmd=$1; shift
> +
> +       if [[ ! -x "$(command -v "$cmd")" ]]; then
> +               echo "SKIP: $cmd not installed"
> +               exit 1
> +       fi
> +}
> +
> +phc_sanity()
> +{
> +       phc_ctl $DEV get &> /dev/null
> +
> +       if [ $? != 0 ]; then
> +               echo "SKIP: unknown clock $DEV: No such device"
> +               exit 1
> +       fi
> +}
> +
> +require_command phc_ctl
> +phc_sanity
> +
> +##############################################################################
> +# Helpers
> +
> +# Exit status to return at the end. Set in case one of the tests fails.
> +EXIT_STATUS=0
> +# Per-test return value. Clear at the beginning of each test.
> +RET=0
> +
> +check_err()
> +{
> +       local err=$1
> +
> +       if [[ $RET -eq 0 && $err -ne 0 ]]; then
> +               RET=$err
> +       fi
> +}
> +
> +log_test()
> +{
> +       local test_name=$1
> +
> +       if [[ $RET -ne 0 ]]; then
> +               EXIT_STATUS=1
> +               printf "TEST: %-60s  [FAIL]\n" "$test_name"
> +               return 1
> +       fi
> +
> +       printf "TEST: %-60s  [ OK ]\n" "$test_name"
> +       return 0
> +}
> +
> +tests_run()
> +{
> +       local current_test
> +
> +       for current_test in ${TESTS:-$ALL_TESTS}; do
> +               $current_test
> +       done
> +}
> +
> +##############################################################################
> +# Tests
> +
> +settime_do()
> +{
> +       local res
> +
> +       res=$(phc_ctl $DEV set 0 wait 120.5 get 2> /dev/null \
> +               | awk '/clock time is/{print $5}' \
> +               | awk -F. '{print $1}')
> +
> +       (( res == 120 ))
> +}
> +
> +adjtime_do()
> +{
> +       local res
> +
> +       res=$(phc_ctl $DEV set 0 adj 10 get 2> /dev/null \
> +               | awk '/clock time is/{print $5}' \
> +               | awk -F. '{print $1}')
> +
> +       (( res == 10 ))
> +}
> +
> +adjfreq_do()
> +{
> +       local res
> +
> +       # Set the clock to be 1% faster
> +       res=$(phc_ctl $DEV freq 10000000 set 0 wait 100.5 get 2> /dev/null \
> +               | awk '/clock time is/{print $5}' \
> +               | awk -F. '{print $1}')
> +
> +       (( res == 101 ))
> +}
> +
> +##############################################################################
> +
> +cleanup()
> +{
> +       phc_ctl $DEV freq 0.0 &> /dev/null
> +       phc_ctl $DEV set &> /dev/null
> +}
> +
> +settime()
> +{
> +       RET=0
> +
> +       settime_do
> +       check_err $?
> +       log_test "settime"
> +       cleanup
> +}
> +
> +adjtime()
> +{
> +       RET=0
> +
> +       adjtime_do
> +       check_err $?
> +       log_test "adjtime"
> +       cleanup
> +}
> +
> +adjfreq()
> +{
> +       RET=0
> +
> +       adjfreq_do
> +       check_err $?
> +       log_test "adjfreq"
> +       cleanup
> +}
> +
> +trap cleanup EXIT
> +
> +tests_run
> +
> +exit $EXIT_STATUS
> --
> 2.20.1
>

Cool testing framework, thanks!
Some things to consider:
- Why the .5 in the wait commands?
- I suspect there's a huge margin of inaccuracy that the test is
missing by only looking at the 'seconds' portion of the PHC time after
the adjfreq operation (up to 10^9 - 1 ppb, in the worst case).

Tested-by: Vladimir Oltean <olteanv@gmail.com>

Regards,
-Vladimir

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test
  2019-06-07 11:15   ` Vladimir Oltean
@ 2019-06-08 10:44     ` Vladimir Oltean
  2019-06-11 13:54     ` Shalom Toledo
  1 sibling, 0 replies; 37+ messages in thread
From: Vladimir Oltean @ 2019-06-08 10:44 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, David S. Miller, Richard Cochran, jiri, shalomt, petrm,
	mlxsw, Ido Schimmel

On 6/7/19 2:15 PM, Vladimir Oltean wrote:
> On Mon, 3 Jun 2019 at 15:25, Ido Schimmel <idosch@idosch.org> wrote:
>>
>> From: Shalom Toledo <shalomt@mellanox.com>
>>
>> Test the PTP Physical Hardware Clock functionality using the "phc_ctl" (a
>> part of "linuxptp").
>>
>> The test contains three sub-tests:
>>    * "settime" test
>>    * "adjtime" test
>>    * "adjfreq" test
>>
>> "settime" test:
>>    * set the PHC time to 0 seconds.
>>    * wait for 120.5 seconds.
>>    * check if PHC time equal to 120.XX seconds.
>>
>> "adjtime" test:
>>    * set the PHC time to 0 seconds.
>>    * adjust the time by 10 seconds.
>>    * check if PHC time equal to 10.XX seconds.
>>
>> "adjfreq" test:
>>    * adjust the PHC frequency to be 1% faster.
>>    * set the PHC time to 0 seconds.
>>    * wait for 100.5 seconds.
>>    * check if PHC time equal to 101.XX seconds.
>>
>> Usage:
>>    $ ./phc.sh /dev/ptp<X>
>>
>>    It is possible to run a subset of the tests, for example:
>>      * To run only the "settime" test:
>>        $ TESTS="settime" ./phc.sh /dev/ptp<X>
>>
>> Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
>> Reviewed-by: Petr Machata <petrm@mellanox.com>
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> ---
>>   tools/testing/selftests/ptp/phc.sh | 166 +++++++++++++++++++++++++++++
>>   1 file changed, 166 insertions(+)
>>   create mode 100755 tools/testing/selftests/ptp/phc.sh
>>
>> diff --git a/tools/testing/selftests/ptp/phc.sh b/tools/testing/selftests/ptp/phc.sh
>> new file mode 100755
>> index 000000000000..ac6e5a6e1d3a
>> --- /dev/null
>> +++ b/tools/testing/selftests/ptp/phc.sh
>> @@ -0,0 +1,166 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +ALL_TESTS="
>> +       settime
>> +       adjtime
>> +       adjfreq
>> +"
>> +DEV=$1
>> +
>> +##############################################################################
>> +# Sanity checks
>> +
>> +if [[ "$(id -u)" -ne 0 ]]; then
>> +       echo "SKIP: need root privileges"
>> +       exit 0
>> +fi
>> +
>> +if [[ "$DEV" == "" ]]; then
>> +       echo "SKIP: PTP device not provided"
>> +       exit 0
>> +fi
>> +
>> +require_command()
>> +{
>> +       local cmd=$1; shift
>> +
>> +       if [[ ! -x "$(command -v "$cmd")" ]]; then
>> +               echo "SKIP: $cmd not installed"
>> +               exit 1
>> +       fi
>> +}
>> +
>> +phc_sanity()
>> +{
>> +       phc_ctl $DEV get &> /dev/null
>> +
>> +       if [ $? != 0 ]; then
>> +               echo "SKIP: unknown clock $DEV: No such device"
>> +               exit 1
>> +       fi
>> +}
>> +
>> +require_command phc_ctl
>> +phc_sanity
>> +
>> +##############################################################################
>> +# Helpers
>> +
>> +# Exit status to return at the end. Set in case one of the tests fails.
>> +EXIT_STATUS=0
>> +# Per-test return value. Clear at the beginning of each test.
>> +RET=0
>> +
>> +check_err()
>> +{
>> +       local err=$1
>> +
>> +       if [[ $RET -eq 0 && $err -ne 0 ]]; then
>> +               RET=$err
>> +       fi
>> +}
>> +
>> +log_test()
>> +{
>> +       local test_name=$1
>> +
>> +       if [[ $RET -ne 0 ]]; then
>> +               EXIT_STATUS=1
>> +               printf "TEST: %-60s  [FAIL]\n" "$test_name"
>> +               return 1
>> +       fi
>> +
>> +       printf "TEST: %-60s  [ OK ]\n" "$test_name"
>> +       return 0
>> +}
>> +
>> +tests_run()
>> +{
>> +       local current_test
>> +
>> +       for current_test in ${TESTS:-$ALL_TESTS}; do
>> +               $current_test
>> +       done
>> +}
>> +
>> +##############################################################################
>> +# Tests
>> +
>> +settime_do()
>> +{
>> +       local res
>> +
>> +       res=$(phc_ctl $DEV set 0 wait 120.5 get 2> /dev/null \
>> +               | awk '/clock time is/{print $5}' \
>> +               | awk -F. '{print $1}')
>> +
>> +       (( res == 120 ))
>> +}
>> +
>> +adjtime_do()
>> +{
>> +       local res
>> +
>> +       res=$(phc_ctl $DEV set 0 adj 10 get 2> /dev/null \
>> +               | awk '/clock time is/{print $5}' \
>> +               | awk -F. '{print $1}')
>> +
>> +       (( res == 10 ))
>> +}
>> +
>> +adjfreq_do()
>> +{
>> +       local res
>> +
>> +       # Set the clock to be 1% faster
>> +       res=$(phc_ctl $DEV freq 10000000 set 0 wait 100.5 get 2> /dev/null \
>> +               | awk '/clock time is/{print $5}' \
>> +               | awk -F. '{print $1}')
>> +
>> +       (( res == 101 ))
>> +}
>> +
>> +##############################################################################
>> +
>> +cleanup()
>> +{
>> +       phc_ctl $DEV freq 0.0 &> /dev/null
>> +       phc_ctl $DEV set &> /dev/null
>> +}
>> +
>> +settime()
>> +{
>> +       RET=0
>> +
>> +       settime_do
>> +       check_err $?
>> +       log_test "settime"
>> +       cleanup
>> +}
>> +
>> +adjtime()
>> +{
>> +       RET=0
>> +
>> +       adjtime_do
>> +       check_err $?
>> +       log_test "adjtime"
>> +       cleanup
>> +}
>> +
>> +adjfreq()
>> +{
>> +       RET=0
>> +
>> +       adjfreq_do
>> +       check_err $?
>> +       log_test "adjfreq"
>> +       cleanup
>> +}
>> +
>> +trap cleanup EXIT
>> +
>> +tests_run
>> +
>> +exit $EXIT_STATUS
>> --
>> 2.20.1
>>
> 
> Cool testing framework, thanks!
> Some things to consider:
> - Why the .5 in the wait commands?
> - I suspect there's a huge margin of inaccuracy that the test is
> missing by only looking at the 'seconds' portion of the PHC time after
> the adjfreq operation (up to 10^9 - 1 ppb, in the worst case).
> 
> Tested-by: Vladimir Oltean <olteanv@gmail.com>
> 
> Regards,
> -Vladimir
> 

Here, I was actually thinking about something like this:

check_with_tolerance()
{
	local res=$1
	local expected=$2
	local ppb=$3
	local expected_min=$(($expected - (($expected * $ppb) / 1000000000)))
	local expected_max=$(($expected + (($expected * $ppb) / 1000000000)))

	if [ $res -lt $expected_min ]; then
		printf "%d is more than %d ppb lower than expected %d (%d)\n" \
			$res $ppb $expected $expected_min
		return 1
	elif [ $res -gt $expected_max ]; then
		printf "%d is more than %d ppb higher than expected %d (%d)\n" \
			$res $ppb $expected $expected_max
		return 1;
	else
		printf "%d is within the +/-%d ppb tolerance of %d (%d - %d)\n" \
			$res $ppb $expected $expected_min $expected_max
		return 0;
	fi
}

settime_do()
{
	local res

	res=$(phc_ctl $DEV set 0 wait 120 get 2> /dev/null \
		 | awk '/clock time is/{print $5}' \
		 | awk -F. '{print $1 * 1000000000 + $2}')

	check_with_tolerance $res 120000000000 10000
}

adjtime_do()
{
	local res

	res=$(phc_ctl $DEV set 0 adj 10 get 2> /dev/null \
		 | awk '/clock time is/{print $5}' \
		 | awk -F. '{print $1 * 1000000000 + $2}')

	check_with_tolerance $res 10000000000 10000
}

adjfreq_do()
{
	local res

	# Set the clock to be 1% faster
	res=$(phc_ctl $DEV freq 10000000 set 0 wait 100 get 2> /dev/null \
		 | awk '/clock time is/{print $5}' \
		 | awk -F. '{print $1 * 1000000000 + $2}')

	check_with_tolerance $res 101000000000 10000
}


With the above changes:

SJA1105 hardware clock operations (not the timecounter ones that I 
submitted):

# ./phc.sh /dev/ptp1
119999611352 is within the +/-1000000 ppb tolerance of 120000000000 
(119880000000 - 120120000000)
TEST: settime                                                 [ OK ]
10001018472 is within the +/-1000000 ppb tolerance of 10000000000 
(9990000000 - 10010000000)
TEST: adjtime                                                 [ OK ]
100998300984 is within the +/-1000000 ppb tolerance of 101000000000 
(100899000000 - 101101000000)
TEST: adjfreq                                                 [ OK ]

But at a lower tolerance of 10000 ppb:

[root@OpenIL:~]# ./phc.sh /dev/ptp1
119998277344 is more than 10000 ppb lower than expected 120000000000 
(119998800000)
TEST: settime                                                 [FAIL]
10002033840 is more than 10000 ppb higher than expected 10000000000 
(10000100000)
TEST: adjtime                                                 [FAIL]
100998295304 is more than 10000 ppb lower than expected 101000000000 
(100998990000)
TEST: adjfreq                                                 [FAIL]

For reference, ptp_qoriq:

[root@OpenIL:~]# ./phc.sh /dev/ptp0
120000960470 is within the +/-10000 ppb tolerance of 120000000000 
(119998800000 - 120001200000)
TEST: settime                                                 [ OK ]
10000699770 is more than 10000 ppb higher than expected 10000000000 
(10000100000)
TEST: adjtime                                                 [FAIL]
101000211269 is within the +/-10000 ppb tolerance of 101000000000 
(100998990000 - 101001010000)
TEST: adjfreq                                                 [ OK ]

Regards,
-Vladimir

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test
  2019-06-07 11:15   ` Vladimir Oltean
  2019-06-08 10:44     ` Vladimir Oltean
@ 2019-06-11 13:54     ` Shalom Toledo
  1 sibling, 0 replies; 37+ messages in thread
From: Shalom Toledo @ 2019-06-11 13:54 UTC (permalink / raw)
  To: Vladimir Oltean, Ido Schimmel
  Cc: netdev, David S. Miller, Richard Cochran, Jiri Pirko,
	Petr Machata, mlxsw, Ido Schimmel

On 07/06/2019 14:15, Vladimir Oltean wrote:
> On Mon, 3 Jun 2019 at 15:25, Ido Schimmel <idosch@idosch.org> wrote:
>>
>> From: Shalom Toledo <shalomt@mellanox.com>
>>
>> Test the PTP Physical Hardware Clock functionality using the "phc_ctl" (a
>> part of "linuxptp").
>>
>> The test contains three sub-tests:
>>   * "settime" test
>>   * "adjtime" test
>>   * "adjfreq" test
>>
>> "settime" test:
>>   * set the PHC time to 0 seconds.
>>   * wait for 120.5 seconds.
>>   * check if PHC time equal to 120.XX seconds.
>>
>> "adjtime" test:
>>   * set the PHC time to 0 seconds.
>>   * adjust the time by 10 seconds.
>>   * check if PHC time equal to 10.XX seconds.
>>
>> "adjfreq" test:
>>   * adjust the PHC frequency to be 1% faster.
>>   * set the PHC time to 0 seconds.
>>   * wait for 100.5 seconds.
>>   * check if PHC time equal to 101.XX seconds.
>>
>> Usage:
>>   $ ./phc.sh /dev/ptp<X>
>>
>>   It is possible to run a subset of the tests, for example:
>>     * To run only the "settime" test:
>>       $ TESTS="settime" ./phc.sh /dev/ptp<X>
>>
>> Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
>> Reviewed-by: Petr Machata <petrm@mellanox.com>
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> ---
>>  tools/testing/selftests/ptp/phc.sh | 166 +++++++++++++++++++++++++++++
>>  1 file changed, 166 insertions(+)
>>  create mode 100755 tools/testing/selftests/ptp/phc.sh
>>
>> diff --git a/tools/testing/selftests/ptp/phc.sh b/tools/testing/selftests/ptp/phc.sh
>> new file mode 100755
>> index 000000000000..ac6e5a6e1d3a
>> --- /dev/null
>> +++ b/tools/testing/selftests/ptp/phc.sh
>> @@ -0,0 +1,166 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +ALL_TESTS="
>> +       settime
>> +       adjtime
>> +       adjfreq
>> +"
>> +DEV=$1
>> +
>> +##############################################################################
>> +# Sanity checks
>> +
>> +if [[ "$(id -u)" -ne 0 ]]; then
>> +       echo "SKIP: need root privileges"
>> +       exit 0
>> +fi
>> +
>> +if [[ "$DEV" == "" ]]; then
>> +       echo "SKIP: PTP device not provided"
>> +       exit 0
>> +fi
>> +
>> +require_command()
>> +{
>> +       local cmd=$1; shift
>> +
>> +       if [[ ! -x "$(command -v "$cmd")" ]]; then
>> +               echo "SKIP: $cmd not installed"
>> +               exit 1
>> +       fi
>> +}
>> +
>> +phc_sanity()
>> +{
>> +       phc_ctl $DEV get &> /dev/null
>> +
>> +       if [ $? != 0 ]; then
>> +               echo "SKIP: unknown clock $DEV: No such device"
>> +               exit 1
>> +       fi
>> +}
>> +
>> +require_command phc_ctl
>> +phc_sanity
>> +
>> +##############################################################################
>> +# Helpers
>> +
>> +# Exit status to return at the end. Set in case one of the tests fails.
>> +EXIT_STATUS=0
>> +# Per-test return value. Clear at the beginning of each test.
>> +RET=0
>> +
>> +check_err()
>> +{
>> +       local err=$1
>> +
>> +       if [[ $RET -eq 0 && $err -ne 0 ]]; then
>> +               RET=$err
>> +       fi
>> +}
>> +
>> +log_test()
>> +{
>> +       local test_name=$1
>> +
>> +       if [[ $RET -ne 0 ]]; then
>> +               EXIT_STATUS=1
>> +               printf "TEST: %-60s  [FAIL]\n" "$test_name"
>> +               return 1
>> +       fi
>> +
>> +       printf "TEST: %-60s  [ OK ]\n" "$test_name"
>> +       return 0
>> +}
>> +
>> +tests_run()
>> +{
>> +       local current_test
>> +
>> +       for current_test in ${TESTS:-$ALL_TESTS}; do
>> +               $current_test
>> +       done
>> +}
>> +
>> +##############################################################################
>> +# Tests
>> +
>> +settime_do()
>> +{
>> +       local res
>> +
>> +       res=$(phc_ctl $DEV set 0 wait 120.5 get 2> /dev/null \
>> +               | awk '/clock time is/{print $5}' \
>> +               | awk -F. '{print $1}')
>> +
>> +       (( res == 120 ))
>> +}
>> +
>> +adjtime_do()
>> +{
>> +       local res
>> +
>> +       res=$(phc_ctl $DEV set 0 adj 10 get 2> /dev/null \
>> +               | awk '/clock time is/{print $5}' \
>> +               | awk -F. '{print $1}')
>> +
>> +       (( res == 10 ))
>> +}
>> +
>> +adjfreq_do()
>> +{
>> +       local res
>> +
>> +       # Set the clock to be 1% faster
>> +       res=$(phc_ctl $DEV freq 10000000 set 0 wait 100.5 get 2> /dev/null \
>> +               | awk '/clock time is/{print $5}' \
>> +               | awk -F. '{print $1}')
>> +
>> +       (( res == 101 ))
>> +}
>> +
>> +##############################################################################
>> +
>> +cleanup()
>> +{
>> +       phc_ctl $DEV freq 0.0 &> /dev/null
>> +       phc_ctl $DEV set &> /dev/null
>> +}
>> +
>> +settime()
>> +{
>> +       RET=0
>> +
>> +       settime_do
>> +       check_err $?
>> +       log_test "settime"
>> +       cleanup
>> +}
>> +
>> +adjtime()
>> +{
>> +       RET=0
>> +
>> +       adjtime_do
>> +       check_err $?
>> +       log_test "adjtime"
>> +       cleanup
>> +}
>> +
>> +adjfreq()
>> +{
>> +       RET=0
>> +
>> +       adjfreq_do
>> +       check_err $?
>> +       log_test "adjfreq"
>> +       cleanup
>> +}
>> +
>> +trap cleanup EXIT
>> +
>> +tests_run
>> +
>> +exit $EXIT_STATUS
>> --
>> 2.20.1
>>
> 
> Cool testing framework, thanks!
> Some things to consider:
> - Why the .5 in the wait commands?

To make sure the clock get to the expected time. It's been tested on different
devices.

> - I suspect there's a huge margin of inaccuracy that the test is
> missing by only looking at the 'seconds' portion of the PHC time after
> the adjfreq operation (up to 10^9 - 1 ppb, in the worst case).

Correct, but this test does the work. It tests the basic functionality of PHC.

I'm just sending a v2 based on Richard comments, feel free to extended this
framework. All of us will get benefit from it.

> 
> Tested-by: Vladimir Oltean <olteanv@gmail.com>
> 
> Regards,
> -Vladimir
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2019-06-11 13:56 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 12:12 [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock Ido Schimmel
2019-06-03 12:12 ` [PATCH net-next 1/9] mlxsw: cmd: Free running clock PCI BAR and offsets via query firmware Ido Schimmel
2019-06-03 12:12 ` [PATCH net-next 2/9] mlxsw: core: Add a new interface for reading the hardware free running clock Ido Schimmel
2019-06-03 12:12 ` [PATCH net-next 3/9] mlxsw: pci: Query free running clock PCI BAR and offsets Ido Schimmel
2019-06-03 12:12 ` [PATCH net-next 4/9] mlxsw: reg: Add Management UTC Register Ido Schimmel
2019-06-04 14:17   ` Richard Cochran
2019-06-05 11:30     ` Shalom Toledo
2019-06-05 17:23       ` Richard Cochran
2019-06-05 18:55         ` Shalom Toledo
2019-06-06  2:37           ` Richard Cochran
2019-06-06  9:11             ` Shalom Toledo
2019-06-06 10:12               ` Petr Machata
2019-06-03 12:12 ` [PATCH net-next 5/9] mlxsw: reg: Add Management Pulse Per Second Register Ido Schimmel
2019-06-03 12:12 ` [PATCH net-next 6/9] ptp: ptp_clock: Publish scaled_ppm_to_ppb Ido Schimmel
2019-06-04 14:21   ` Richard Cochran
2019-06-05 11:46     ` Shalom Toledo
2019-06-03 12:12 ` [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations Ido Schimmel
2019-06-04 14:28   ` Richard Cochran
2019-06-05  6:30     ` Jiri Pirko
2019-06-05 17:24       ` Richard Cochran
2019-06-05 11:44     ` Shalom Toledo
2019-06-05 17:40       ` Richard Cochran
2019-06-05 19:28         ` Shalom Toledo
2019-06-06  2:43           ` Richard Cochran
2019-06-06  8:50             ` Shalom Toledo
2019-06-06  8:57               ` Shalom Toledo
2019-06-04 17:03   ` Richard Cochran
2019-06-05  9:00     ` Petr Machata
2019-06-05 17:31       ` Richard Cochran
2019-06-06 10:21         ` Petr Machata
2019-06-03 12:12 ` [PATCH net-next 8/9] mlxsw: spectrum: PTP physical hardware clock initialization Ido Schimmel
2019-06-03 12:12 ` [PATCH net-next 9/9] selftests: ptp: Add Physical Hardware Clock test Ido Schimmel
2019-06-04 17:05   ` Richard Cochran
2019-06-07 11:15   ` Vladimir Oltean
2019-06-08 10:44     ` Vladimir Oltean
2019-06-11 13:54     ` Shalom Toledo
2019-06-03 17:35 ` [PATCH net-next 0/9] mlxsw: Add support for physical hardware clock David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.