* [PATCH net-next v4 0/3] ice: lighter locking for PTP time reading
@ 2024-03-25 23:20 ` Michal Schmidt
0 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jesse Brandeburg, Tony Nguyen, Richard Cochran, netdev,
Jacob Keller, Jiri Pirko, Arkadiusz Kubalewski, Karol Kolacinski,
Marcin Szycik, Przemek Kitszel, Temerkhanov, Sergey
This series removes the use of the heavy-weight PTP hardware semaphore
in the gettimex64 path. Instead, serialization of access to the time
register is done using a host-side spinlock. The timer hardware is
shared between PFs on the PCI adapter, so the spinlock must be shared
between ice_pf instances too.
Replacing the PTP hardware semaphore entirely with a mutex is also
possible and you can see it done in my git branch[1], but I am not
posting those patches yet to keep the scope of this series limited.
[1] https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
v4:
- Patch 1: Use named GENMASK macros and FIELD_PREP.
v3:
- Longer variable name ("a" -> "adapter").
- Propagate xarray error in ice_adapter_get with ERR_PTR.
- Added kernel-doc comments for ice_adapter_{get,put}.
v2:
- Patch 1: Rely on xarray's own lock. (Suggested by Jiri Pirko)
- Patch 2: Do not use *_irqsave with ptp_gltsyn_time_lock, as it's used
only in process contexts.
Michal Schmidt (3):
ice: add ice_adapter for shared data across PFs on the same NIC
ice: avoid the PTP hardware semaphore in gettimex64 path
ice: fold ice_ptp_read_time into ice_ptp_gettimex64
drivers/net/ethernet/intel/ice/Makefile | 3 +-
drivers/net/ethernet/intel/ice/ice.h | 2 +
drivers/net/ethernet/intel/ice/ice_adapter.c | 116 +++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_adapter.h | 28 +++++
drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
drivers/net/ethernet/intel/ice/ice_ptp.c | 33 +-----
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +
7 files changed, 163 insertions(+), 30 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
--
2.43.2
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-wired-lan] [PATCH net-next v4 0/3] ice: lighter locking for PTP time reading
@ 2024-03-25 23:20 ` Michal Schmidt
0 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev, Richard Cochran,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Tony Nguyen, Przemek Kitszel, Jacob Keller
This series removes the use of the heavy-weight PTP hardware semaphore
in the gettimex64 path. Instead, serialization of access to the time
register is done using a host-side spinlock. The timer hardware is
shared between PFs on the PCI adapter, so the spinlock must be shared
between ice_pf instances too.
Replacing the PTP hardware semaphore entirely with a mutex is also
possible and you can see it done in my git branch[1], but I am not
posting those patches yet to keep the scope of this series limited.
[1] https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
v4:
- Patch 1: Use named GENMASK macros and FIELD_PREP.
v3:
- Longer variable name ("a" -> "adapter").
- Propagate xarray error in ice_adapter_get with ERR_PTR.
- Added kernel-doc comments for ice_adapter_{get,put}.
v2:
- Patch 1: Rely on xarray's own lock. (Suggested by Jiri Pirko)
- Patch 2: Do not use *_irqsave with ptp_gltsyn_time_lock, as it's used
only in process contexts.
Michal Schmidt (3):
ice: add ice_adapter for shared data across PFs on the same NIC
ice: avoid the PTP hardware semaphore in gettimex64 path
ice: fold ice_ptp_read_time into ice_ptp_gettimex64
drivers/net/ethernet/intel/ice/Makefile | 3 +-
drivers/net/ethernet/intel/ice/ice.h | 2 +
drivers/net/ethernet/intel/ice/ice_adapter.c | 116 +++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_adapter.h | 28 +++++
drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
drivers/net/ethernet/intel/ice/ice_ptp.c | 33 +-----
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +
7 files changed, 163 insertions(+), 30 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
--
2.43.2
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-25 23:20 ` Michal Schmidt
-1 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jesse Brandeburg, Tony Nguyen, Richard Cochran, netdev,
Jacob Keller, Jiri Pirko, Arkadiusz Kubalewski, Karol Kolacinski,
Marcin Szycik, Przemek Kitszel, Temerkhanov, Sergey
There is a need for synchronization between ice PFs on the same physical
adapter.
Add a "struct ice_adapter" for holding data shared between PFs of the
same multifunction PCI device. The struct is refcounted - each ice_pf
holds a reference to it.
Its first use will be for PTP. I expect it will be useful also to
improve the ugliness that is ice_prot_id_tbl.
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/intel/ice/Makefile | 3 +-
drivers/net/ethernet/intel/ice/ice.h | 2 +
drivers/net/ethernet/intel/ice/ice_adapter.c | 114 +++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_adapter.h | 22 ++++
drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
5 files changed, 148 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index cddd82d4ca0f..4fa09c321440 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -36,7 +36,8 @@ ice-y := ice_main.o \
ice_repr.o \
ice_tc_lib.o \
ice_fwlog.o \
- ice_debugfs.o
+ ice_debugfs.o \
+ ice_adapter.o
ice-$(CONFIG_PCI_IOV) += \
ice_sriov.o \
ice_virtchnl.o \
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 365c03d1c462..1ffecbdd361a 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -77,6 +77,7 @@
#include "ice_gnss.h"
#include "ice_irq.h"
#include "ice_dpll.h"
+#include "ice_adapter.h"
#define ICE_BAR0 0
#define ICE_REQ_DESC_MULTIPLE 32
@@ -544,6 +545,7 @@ struct ice_agg_node {
struct ice_pf {
struct pci_dev *pdev;
+ struct ice_adapter *adapter;
struct devlink_region *nvm_region;
struct devlink_region *sram_region;
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
new file mode 100644
index 000000000000..f00ab998e853
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: Copyright Red Hat
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/xarray.h>
+#include "ice_adapter.h"
+
+static DEFINE_XARRAY(ice_adapters);
+
+/* PCI bus number is 8 bits. Slot is 5 bits. Domain can have the rest. */
+#define INDEX_FIELD_DOMAIN GENMASK(BITS_PER_LONG - 1, 13)
+#define INDEX_FIELD_BUS GENMASK(12, 5)
+#define INDEX_FIELD_SLOT GENMASK(4, 0)
+
+static unsigned long ice_adapter_index(const struct pci_dev *pdev)
+{
+ unsigned int domain = pci_domain_nr(pdev->bus);
+
+ WARN_ON(domain > FIELD_MAX(INDEX_FIELD_DOMAIN));
+
+ return FIELD_PREP(INDEX_FIELD_DOMAIN, domain) |
+ FIELD_PREP(INDEX_FIELD_BUS, pdev->bus->number) |
+ FIELD_PREP(INDEX_FIELD_SLOT, PCI_SLOT(pdev->devfn));
+}
+
+static struct ice_adapter *ice_adapter_new(void)
+{
+ struct ice_adapter *adapter;
+
+ adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
+ if (!adapter)
+ return NULL;
+
+ refcount_set(&adapter->refcount, 1);
+
+ return adapter;
+}
+
+static void ice_adapter_free(struct ice_adapter *adapter)
+{
+ kfree(adapter);
+}
+
+DEFINE_FREE(ice_adapter_free, struct ice_adapter*, if (_T) ice_adapter_free(_T))
+
+/**
+ * ice_adapter_get - Get a shared ice_adapter structure.
+ * @pdev: Pointer to the pci_dev whose driver is getting the ice_adapter.
+ *
+ * Gets a pointer to a shared ice_adapter structure. Physical functions (PFs)
+ * of the same multi-function PCI device share one ice_adapter structure.
+ * The ice_adapter is reference-counted. The PF driver must use ice_adapter_put
+ * to release its reference.
+ *
+ * Context: Process, may sleep.
+ * Return: Pointer to ice_adapter on success.
+ * ERR_PTR() on error. -ENOMEM is the only possible error.
+ */
+struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev)
+{
+ struct ice_adapter *ret, __free(ice_adapter_free) *adapter = NULL;
+ unsigned long index = ice_adapter_index(pdev);
+
+ adapter = ice_adapter_new();
+ if (!adapter)
+ return ERR_PTR(-ENOMEM);
+
+ xa_lock(&ice_adapters);
+ ret = __xa_cmpxchg(&ice_adapters, index, NULL, adapter, GFP_KERNEL);
+ if (xa_is_err(ret)) {
+ ret = ERR_PTR(xa_err(ret));
+ goto unlock;
+ }
+ if (ret) {
+ refcount_inc(&ret->refcount);
+ goto unlock;
+ }
+ ret = no_free_ptr(adapter);
+unlock:
+ xa_unlock(&ice_adapters);
+ return ret;
+}
+
+/**
+ * ice_adapter_put - Release a reference to the shared ice_adapter structure.
+ * @pdev: Pointer to the pci_dev whose driver is releasing the ice_adapter.
+ *
+ * Releases the reference to ice_adapter previously obtained with
+ * ice_adapter_get.
+ *
+ * Context: Any.
+ */
+void ice_adapter_put(const struct pci_dev *pdev)
+{
+ unsigned long index = ice_adapter_index(pdev);
+ struct ice_adapter *adapter;
+
+ xa_lock(&ice_adapters);
+ adapter = xa_load(&ice_adapters, index);
+ if (WARN_ON(!adapter))
+ goto unlock;
+
+ if (!refcount_dec_and_test(&adapter->refcount))
+ goto unlock;
+
+ WARN_ON(__xa_erase(&ice_adapters, index) != adapter);
+ ice_adapter_free(adapter);
+unlock:
+ xa_unlock(&ice_adapters);
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
new file mode 100644
index 000000000000..cb5a02eb24c1
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* SPDX-FileCopyrightText: Copyright Red Hat */
+
+#ifndef _ICE_ADAPTER_H_
+#define _ICE_ADAPTER_H_
+
+#include <linux/refcount_types.h>
+
+struct pci_dev;
+
+/**
+ * struct ice_adapter - PCI adapter resources shared across PFs
+ * @refcount: Reference count. struct ice_pf objects hold the references.
+ */
+struct ice_adapter {
+ refcount_t refcount;
+};
+
+struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev);
+void ice_adapter_put(const struct pci_dev *pdev);
+
+#endif /* _ICE_ADAPTER_H */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 33a164fa325a..7a75d14d8e9d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5093,6 +5093,7 @@ static int
ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
{
struct device *dev = &pdev->dev;
+ struct ice_adapter *adapter;
struct ice_pf *pf;
struct ice_hw *hw;
int err;
@@ -5145,7 +5146,12 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
pci_set_master(pdev);
+ adapter = ice_adapter_get(pdev);
+ if (IS_ERR(adapter))
+ return PTR_ERR(adapter);
+
pf->pdev = pdev;
+ pf->adapter = adapter;
pci_set_drvdata(pdev, pf);
set_bit(ICE_DOWN, pf->state);
/* Disable service task until DOWN bit is cleared */
@@ -5196,6 +5202,7 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
err_load:
ice_deinit(pf);
err_init:
+ ice_adapter_put(pdev);
pci_disable_device(pdev);
return err;
}
@@ -5302,6 +5309,7 @@ static void ice_remove(struct pci_dev *pdev)
ice_setup_mc_magic_wake(pf);
ice_set_wake(pf);
+ ice_adapter_put(pdev);
pci_disable_device(pdev);
}
--
2.43.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-wired-lan] [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC
@ 2024-03-25 23:20 ` Michal Schmidt
0 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev, Richard Cochran,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Tony Nguyen, Przemek Kitszel, Jacob Keller
There is a need for synchronization between ice PFs on the same physical
adapter.
Add a "struct ice_adapter" for holding data shared between PFs of the
same multifunction PCI device. The struct is refcounted - each ice_pf
holds a reference to it.
Its first use will be for PTP. I expect it will be useful also to
improve the ugliness that is ice_prot_id_tbl.
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/intel/ice/Makefile | 3 +-
drivers/net/ethernet/intel/ice/ice.h | 2 +
drivers/net/ethernet/intel/ice/ice_adapter.c | 114 +++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_adapter.h | 22 ++++
drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
5 files changed, 148 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index cddd82d4ca0f..4fa09c321440 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -36,7 +36,8 @@ ice-y := ice_main.o \
ice_repr.o \
ice_tc_lib.o \
ice_fwlog.o \
- ice_debugfs.o
+ ice_debugfs.o \
+ ice_adapter.o
ice-$(CONFIG_PCI_IOV) += \
ice_sriov.o \
ice_virtchnl.o \
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 365c03d1c462..1ffecbdd361a 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -77,6 +77,7 @@
#include "ice_gnss.h"
#include "ice_irq.h"
#include "ice_dpll.h"
+#include "ice_adapter.h"
#define ICE_BAR0 0
#define ICE_REQ_DESC_MULTIPLE 32
@@ -544,6 +545,7 @@ struct ice_agg_node {
struct ice_pf {
struct pci_dev *pdev;
+ struct ice_adapter *adapter;
struct devlink_region *nvm_region;
struct devlink_region *sram_region;
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
new file mode 100644
index 000000000000..f00ab998e853
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: Copyright Red Hat
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/xarray.h>
+#include "ice_adapter.h"
+
+static DEFINE_XARRAY(ice_adapters);
+
+/* PCI bus number is 8 bits. Slot is 5 bits. Domain can have the rest. */
+#define INDEX_FIELD_DOMAIN GENMASK(BITS_PER_LONG - 1, 13)
+#define INDEX_FIELD_BUS GENMASK(12, 5)
+#define INDEX_FIELD_SLOT GENMASK(4, 0)
+
+static unsigned long ice_adapter_index(const struct pci_dev *pdev)
+{
+ unsigned int domain = pci_domain_nr(pdev->bus);
+
+ WARN_ON(domain > FIELD_MAX(INDEX_FIELD_DOMAIN));
+
+ return FIELD_PREP(INDEX_FIELD_DOMAIN, domain) |
+ FIELD_PREP(INDEX_FIELD_BUS, pdev->bus->number) |
+ FIELD_PREP(INDEX_FIELD_SLOT, PCI_SLOT(pdev->devfn));
+}
+
+static struct ice_adapter *ice_adapter_new(void)
+{
+ struct ice_adapter *adapter;
+
+ adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
+ if (!adapter)
+ return NULL;
+
+ refcount_set(&adapter->refcount, 1);
+
+ return adapter;
+}
+
+static void ice_adapter_free(struct ice_adapter *adapter)
+{
+ kfree(adapter);
+}
+
+DEFINE_FREE(ice_adapter_free, struct ice_adapter*, if (_T) ice_adapter_free(_T))
+
+/**
+ * ice_adapter_get - Get a shared ice_adapter structure.
+ * @pdev: Pointer to the pci_dev whose driver is getting the ice_adapter.
+ *
+ * Gets a pointer to a shared ice_adapter structure. Physical functions (PFs)
+ * of the same multi-function PCI device share one ice_adapter structure.
+ * The ice_adapter is reference-counted. The PF driver must use ice_adapter_put
+ * to release its reference.
+ *
+ * Context: Process, may sleep.
+ * Return: Pointer to ice_adapter on success.
+ * ERR_PTR() on error. -ENOMEM is the only possible error.
+ */
+struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev)
+{
+ struct ice_adapter *ret, __free(ice_adapter_free) *adapter = NULL;
+ unsigned long index = ice_adapter_index(pdev);
+
+ adapter = ice_adapter_new();
+ if (!adapter)
+ return ERR_PTR(-ENOMEM);
+
+ xa_lock(&ice_adapters);
+ ret = __xa_cmpxchg(&ice_adapters, index, NULL, adapter, GFP_KERNEL);
+ if (xa_is_err(ret)) {
+ ret = ERR_PTR(xa_err(ret));
+ goto unlock;
+ }
+ if (ret) {
+ refcount_inc(&ret->refcount);
+ goto unlock;
+ }
+ ret = no_free_ptr(adapter);
+unlock:
+ xa_unlock(&ice_adapters);
+ return ret;
+}
+
+/**
+ * ice_adapter_put - Release a reference to the shared ice_adapter structure.
+ * @pdev: Pointer to the pci_dev whose driver is releasing the ice_adapter.
+ *
+ * Releases the reference to ice_adapter previously obtained with
+ * ice_adapter_get.
+ *
+ * Context: Any.
+ */
+void ice_adapter_put(const struct pci_dev *pdev)
+{
+ unsigned long index = ice_adapter_index(pdev);
+ struct ice_adapter *adapter;
+
+ xa_lock(&ice_adapters);
+ adapter = xa_load(&ice_adapters, index);
+ if (WARN_ON(!adapter))
+ goto unlock;
+
+ if (!refcount_dec_and_test(&adapter->refcount))
+ goto unlock;
+
+ WARN_ON(__xa_erase(&ice_adapters, index) != adapter);
+ ice_adapter_free(adapter);
+unlock:
+ xa_unlock(&ice_adapters);
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
new file mode 100644
index 000000000000..cb5a02eb24c1
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* SPDX-FileCopyrightText: Copyright Red Hat */
+
+#ifndef _ICE_ADAPTER_H_
+#define _ICE_ADAPTER_H_
+
+#include <linux/refcount_types.h>
+
+struct pci_dev;
+
+/**
+ * struct ice_adapter - PCI adapter resources shared across PFs
+ * @refcount: Reference count. struct ice_pf objects hold the references.
+ */
+struct ice_adapter {
+ refcount_t refcount;
+};
+
+struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev);
+void ice_adapter_put(const struct pci_dev *pdev);
+
+#endif /* _ICE_ADAPTER_H */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 33a164fa325a..7a75d14d8e9d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5093,6 +5093,7 @@ static int
ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
{
struct device *dev = &pdev->dev;
+ struct ice_adapter *adapter;
struct ice_pf *pf;
struct ice_hw *hw;
int err;
@@ -5145,7 +5146,12 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
pci_set_master(pdev);
+ adapter = ice_adapter_get(pdev);
+ if (IS_ERR(adapter))
+ return PTR_ERR(adapter);
+
pf->pdev = pdev;
+ pf->adapter = adapter;
pci_set_drvdata(pdev, pf);
set_bit(ICE_DOWN, pf->state);
/* Disable service task until DOWN bit is cleared */
@@ -5196,6 +5202,7 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
err_load:
ice_deinit(pf);
err_init:
+ ice_adapter_put(pdev);
pci_disable_device(pdev);
return err;
}
@@ -5302,6 +5309,7 @@ static void ice_remove(struct pci_dev *pdev)
ice_setup_mc_magic_wake(pf);
ice_set_wake(pf);
+ ice_adapter_put(pdev);
pci_disable_device(pdev);
}
--
2.43.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-25 23:20 ` Michal Schmidt
-1 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jesse Brandeburg, Tony Nguyen, Richard Cochran, netdev,
Jacob Keller, Jiri Pirko, Arkadiusz Kubalewski, Karol Kolacinski,
Marcin Szycik, Przemek Kitszel, Temerkhanov, Sergey
The PTP hardware semaphore (PFTSYN_SEM) is used to synchronize
operations that program the PTP timers. The operations involve issuing
commands to the sideband queue. The E810 does not have a hardware
sideband queue, so the admin queue is used. The admin queue is slow.
I have observed delays in hundreds of milliseconds waiting for
ice_sq_done.
When phc2sys reads the time from the ice PTP clock and PFTSYN_SEM is
held by a task performing one of the slow operations, ice_ptp_lock can
easily time out. phc2sys gets -EBUSY and the kernel prints:
ice 0000:XX:YY.0: PTP failed to get time
These messages appear once every few seconds, causing log spam.
The E810 datasheet recommends an algorithm for reading the upper 64 bits
of the GLTSYN_TIME register. It matches what's implemented in
ice_ptp_read_src_clk_reg. It is robust against wrap-around, but not
necessarily against the concurrent setting of the register (with
GLTSYN_CMD_{INIT,ADJ}_TIME commands). Perhaps that's why
ice_ptp_gettimex64 also takes PFTSYN_SEM.
The race with time setters can be prevented without relying on the PTP
hardware semaphore. Using the "ice_adapter" from the previous patch,
we can have a common spinlock for the PFs that share the clock hardware.
It will protect the reading and writing to the GLTSYN_TIME register.
The writing is performed indirectly, by the hardware, as a result of
the driver writing GLTSYN_CMD_SYNC in ice_ptp_exec_tmr_cmd. I wasn't
sure if the ice_flush there is enough to make sure GLTSYN_TIME has been
updated, but it works well in my testing.
My test code can be seen here:
https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
It consists of:
- kernel threads reading the time in a busy loop and looking at the
deltas between consecutive values, reporting new maxima.
- a shell script that sets the time repeatedly;
- a bpftrace probe to produce a histogram of the measured deltas.
Without the spinlock ptp_gltsyn_time_lock, it is easy to see tearing.
Deltas in the [2G, 4G) range appear in the histograms.
With the spinlock added, there is no tearing and the biggest delta I saw
was in the range [1M, 2M), that is under 2 ms.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_adapter.c | 2 ++
drivers/net/ethernet/intel/ice/ice_adapter.h | 6 ++++++
drivers/net/ethernet/intel/ice/ice_ptp.c | 8 +-------
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +++
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
index f00ab998e853..52d15ef7f4b1 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -6,6 +6,7 @@
#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include <linux/xarray.h>
#include "ice_adapter.h"
@@ -35,6 +36,7 @@ static struct ice_adapter *ice_adapter_new(void)
if (!adapter)
return NULL;
+ spin_lock_init(&adapter->ptp_gltsyn_time_lock);
refcount_set(&adapter->refcount, 1);
return adapter;
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
index cb5a02eb24c1..9d11014ec02f 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -4,15 +4,21 @@
#ifndef _ICE_ADAPTER_H_
#define _ICE_ADAPTER_H_
+#include <linux/spinlock_types.h>
#include <linux/refcount_types.h>
struct pci_dev;
/**
* struct ice_adapter - PCI adapter resources shared across PFs
+ * @ptp_gltsyn_time_lock: Spinlock protecting access to the GLTSYN_TIME
+ * register of the PTP clock.
* @refcount: Reference count. struct ice_pf objects hold the references.
*/
struct ice_adapter {
+ /* For access to the GLTSYN_TIME register */
+ spinlock_t ptp_gltsyn_time_lock;
+
refcount_t refcount;
};
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index c11eba07283c..0875f37add78 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -374,6 +374,7 @@ ice_ptp_read_src_clk_reg(struct ice_pf *pf, struct ptp_system_timestamp *sts)
u8 tmr_idx;
tmr_idx = ice_get_ptp_src_clock_index(hw);
+ guard(spinlock)(&pf->adapter->ptp_gltsyn_time_lock);
/* Read the system timestamp pre PHC read */
ptp_read_system_prets(sts);
@@ -1925,15 +1926,8 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct timespec64 *ts,
struct ptp_system_timestamp *sts)
{
struct ice_pf *pf = ptp_info_to_pf(info);
- struct ice_hw *hw = &pf->hw;
-
- if (!ice_ptp_lock(hw)) {
- dev_err(ice_pf_to_dev(pf), "PTP failed to get time\n");
- return -EBUSY;
- }
ice_ptp_read_time(pf, ts, sts);
- ice_ptp_unlock(hw);
return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 187ce9b54e1a..2b9423a173bb 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -274,6 +274,9 @@ void ice_ptp_src_cmd(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd)
*/
static void ice_ptp_exec_tmr_cmd(struct ice_hw *hw)
{
+ struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
+
+ guard(spinlock)(&pf->adapter->ptp_gltsyn_time_lock);
wr32(hw, GLTSYN_CMD_SYNC, SYNC_EXEC_CMD);
ice_flush(hw);
}
--
2.43.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-wired-lan] [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path
@ 2024-03-25 23:20 ` Michal Schmidt
0 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev, Richard Cochran,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Tony Nguyen, Przemek Kitszel, Jacob Keller
The PTP hardware semaphore (PFTSYN_SEM) is used to synchronize
operations that program the PTP timers. The operations involve issuing
commands to the sideband queue. The E810 does not have a hardware
sideband queue, so the admin queue is used. The admin queue is slow.
I have observed delays in hundreds of milliseconds waiting for
ice_sq_done.
When phc2sys reads the time from the ice PTP clock and PFTSYN_SEM is
held by a task performing one of the slow operations, ice_ptp_lock can
easily time out. phc2sys gets -EBUSY and the kernel prints:
ice 0000:XX:YY.0: PTP failed to get time
These messages appear once every few seconds, causing log spam.
The E810 datasheet recommends an algorithm for reading the upper 64 bits
of the GLTSYN_TIME register. It matches what's implemented in
ice_ptp_read_src_clk_reg. It is robust against wrap-around, but not
necessarily against the concurrent setting of the register (with
GLTSYN_CMD_{INIT,ADJ}_TIME commands). Perhaps that's why
ice_ptp_gettimex64 also takes PFTSYN_SEM.
The race with time setters can be prevented without relying on the PTP
hardware semaphore. Using the "ice_adapter" from the previous patch,
we can have a common spinlock for the PFs that share the clock hardware.
It will protect the reading and writing to the GLTSYN_TIME register.
The writing is performed indirectly, by the hardware, as a result of
the driver writing GLTSYN_CMD_SYNC in ice_ptp_exec_tmr_cmd. I wasn't
sure if the ice_flush there is enough to make sure GLTSYN_TIME has been
updated, but it works well in my testing.
My test code can be seen here:
https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
It consists of:
- kernel threads reading the time in a busy loop and looking at the
deltas between consecutive values, reporting new maxima.
- a shell script that sets the time repeatedly;
- a bpftrace probe to produce a histogram of the measured deltas.
Without the spinlock ptp_gltsyn_time_lock, it is easy to see tearing.
Deltas in the [2G, 4G) range appear in the histograms.
With the spinlock added, there is no tearing and the biggest delta I saw
was in the range [1M, 2M), that is under 2 ms.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_adapter.c | 2 ++
drivers/net/ethernet/intel/ice/ice_adapter.h | 6 ++++++
drivers/net/ethernet/intel/ice/ice_ptp.c | 8 +-------
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +++
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
index f00ab998e853..52d15ef7f4b1 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -6,6 +6,7 @@
#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include <linux/xarray.h>
#include "ice_adapter.h"
@@ -35,6 +36,7 @@ static struct ice_adapter *ice_adapter_new(void)
if (!adapter)
return NULL;
+ spin_lock_init(&adapter->ptp_gltsyn_time_lock);
refcount_set(&adapter->refcount, 1);
return adapter;
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
index cb5a02eb24c1..9d11014ec02f 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -4,15 +4,21 @@
#ifndef _ICE_ADAPTER_H_
#define _ICE_ADAPTER_H_
+#include <linux/spinlock_types.h>
#include <linux/refcount_types.h>
struct pci_dev;
/**
* struct ice_adapter - PCI adapter resources shared across PFs
+ * @ptp_gltsyn_time_lock: Spinlock protecting access to the GLTSYN_TIME
+ * register of the PTP clock.
* @refcount: Reference count. struct ice_pf objects hold the references.
*/
struct ice_adapter {
+ /* For access to the GLTSYN_TIME register */
+ spinlock_t ptp_gltsyn_time_lock;
+
refcount_t refcount;
};
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index c11eba07283c..0875f37add78 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -374,6 +374,7 @@ ice_ptp_read_src_clk_reg(struct ice_pf *pf, struct ptp_system_timestamp *sts)
u8 tmr_idx;
tmr_idx = ice_get_ptp_src_clock_index(hw);
+ guard(spinlock)(&pf->adapter->ptp_gltsyn_time_lock);
/* Read the system timestamp pre PHC read */
ptp_read_system_prets(sts);
@@ -1925,15 +1926,8 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct timespec64 *ts,
struct ptp_system_timestamp *sts)
{
struct ice_pf *pf = ptp_info_to_pf(info);
- struct ice_hw *hw = &pf->hw;
-
- if (!ice_ptp_lock(hw)) {
- dev_err(ice_pf_to_dev(pf), "PTP failed to get time\n");
- return -EBUSY;
- }
ice_ptp_read_time(pf, ts, sts);
- ice_ptp_unlock(hw);
return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 187ce9b54e1a..2b9423a173bb 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -274,6 +274,9 @@ void ice_ptp_src_cmd(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd)
*/
static void ice_ptp_exec_tmr_cmd(struct ice_hw *hw)
{
+ struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
+
+ guard(spinlock)(&pf->adapter->ptp_gltsyn_time_lock);
wr32(hw, GLTSYN_CMD_SYNC, SYNC_EXEC_CMD);
ice_flush(hw);
}
--
2.43.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-25 23:20 ` Michal Schmidt
-1 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jesse Brandeburg, Tony Nguyen, Richard Cochran, netdev,
Jacob Keller, Jiri Pirko, Arkadiusz Kubalewski, Karol Kolacinski,
Marcin Szycik, Przemek Kitszel, Temerkhanov, Sergey
This is a cleanup. It is unnecessary to have this function just to call
another function.
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 0875f37add78..0f17fc1181d2 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1166,26 +1166,6 @@ static void ice_ptp_reset_cached_phctime(struct ice_pf *pf)
ice_ptp_mark_tx_tracker_stale(&pf->ptp.port.tx);
}
-/**
- * ice_ptp_read_time - Read the time from the device
- * @pf: Board private structure
- * @ts: timespec structure to hold the current time value
- * @sts: Optional parameter for holding a pair of system timestamps from
- * the system clock. Will be ignored if NULL is given.
- *
- * This function reads the source clock registers and stores them in a timespec.
- * However, since the registers are 64 bits of nanoseconds, we must convert the
- * result to a timespec before we can return.
- */
-static void
-ice_ptp_read_time(struct ice_pf *pf, struct timespec64 *ts,
- struct ptp_system_timestamp *sts)
-{
- u64 time_ns = ice_ptp_read_src_clk_reg(pf, sts);
-
- *ts = ns_to_timespec64(time_ns);
-}
-
/**
* ice_ptp_write_init - Set PHC time to provided value
* @pf: Board private structure
@@ -1926,9 +1906,10 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct timespec64 *ts,
struct ptp_system_timestamp *sts)
{
struct ice_pf *pf = ptp_info_to_pf(info);
+ u64 time_ns;
- ice_ptp_read_time(pf, ts, sts);
-
+ time_ns = ice_ptp_read_src_clk_reg(pf, sts);
+ *ts = ns_to_timespec64(time_ns);
return 0;
}
--
2.43.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
@ 2024-03-25 23:20 ` Michal Schmidt
0 siblings, 0 replies; 19+ messages in thread
From: Michal Schmidt @ 2024-03-25 23:20 UTC (permalink / raw)
To: intel-wired-lan
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev, Richard Cochran,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Tony Nguyen, Przemek Kitszel, Jacob Keller
This is a cleanup. It is unnecessary to have this function just to call
another function.
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 0875f37add78..0f17fc1181d2 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1166,26 +1166,6 @@ static void ice_ptp_reset_cached_phctime(struct ice_pf *pf)
ice_ptp_mark_tx_tracker_stale(&pf->ptp.port.tx);
}
-/**
- * ice_ptp_read_time - Read the time from the device
- * @pf: Board private structure
- * @ts: timespec structure to hold the current time value
- * @sts: Optional parameter for holding a pair of system timestamps from
- * the system clock. Will be ignored if NULL is given.
- *
- * This function reads the source clock registers and stores them in a timespec.
- * However, since the registers are 64 bits of nanoseconds, we must convert the
- * result to a timespec before we can return.
- */
-static void
-ice_ptp_read_time(struct ice_pf *pf, struct timespec64 *ts,
- struct ptp_system_timestamp *sts)
-{
- u64 time_ns = ice_ptp_read_src_clk_reg(pf, sts);
-
- *ts = ns_to_timespec64(time_ns);
-}
-
/**
* ice_ptp_write_init - Set PHC time to provided value
* @pf: Board private structure
@@ -1926,9 +1906,10 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct timespec64 *ts,
struct ptp_system_timestamp *sts)
{
struct ice_pf *pf = ptp_info_to_pf(info);
+ u64 time_ns;
- ice_ptp_read_time(pf, ts, sts);
-
+ time_ns = ice_ptp_read_src_clk_reg(pf, sts);
+ *ts = ns_to_timespec64(time_ns);
return 0;
}
--
2.43.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v4 0/3] ice: lighter locking for PTP time reading
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
` (3 preceding siblings ...)
(?)
@ 2024-03-26 9:51 ` Ivan Vecera
-1 siblings, 0 replies; 19+ messages in thread
From: Ivan Vecera @ 2024-03-26 9:51 UTC (permalink / raw)
To: Michal Schmidt, intel-wired-lan
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev, Richard Cochran,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Tony Nguyen, Przemek Kitszel, Jacob Keller
On 26. 03. 24 0:20, Michal Schmidt wrote:
> This series removes the use of the heavy-weight PTP hardware semaphore
> in the gettimex64 path. Instead, serialization of access to the time
> register is done using a host-side spinlock. The timer hardware is
> shared between PFs on the PCI adapter, so the spinlock must be shared
> between ice_pf instances too.
>
> Replacing the PTP hardware semaphore entirely with a mutex is also
> possible and you can see it done in my git branch[1], but I am not
> posting those patches yet to keep the scope of this series limited.
>
> [1] https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
>
> v4:
> - Patch 1: Use named GENMASK macros and FIELD_PREP.
>
> v3:
> - Longer variable name ("a" -> "adapter").
> - Propagate xarray error in ice_adapter_get with ERR_PTR.
> - Added kernel-doc comments for ice_adapter_{get,put}.
>
> v2:
> - Patch 1: Rely on xarray's own lock. (Suggested by Jiri Pirko)
> - Patch 2: Do not use *_irqsave with ptp_gltsyn_time_lock, as it's used
> only in process contexts.
>
>
> Michal Schmidt (3):
> ice: add ice_adapter for shared data across PFs on the same NIC
> ice: avoid the PTP hardware semaphore in gettimex64 path
> ice: fold ice_ptp_read_time into ice_ptp_gettimex64
>
> drivers/net/ethernet/intel/ice/Makefile | 3 +-
> drivers/net/ethernet/intel/ice/ice.h | 2 +
> drivers/net/ethernet/intel/ice/ice_adapter.c | 116 +++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_adapter.h | 28 +++++
> drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
> drivers/net/ethernet/intel/ice/ice_ptp.c | 33 +-----
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +
> 7 files changed, 163 insertions(+), 30 deletions(-)
> create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
> create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
>
Reviewed-by: Ivan Vecera <ivecera@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-26 14:46 ` Sai Krishna Gajula
-1 siblings, 0 replies; 19+ messages in thread
From: Sai Krishna Gajula @ 2024-03-26 14:46 UTC (permalink / raw)
To: Michal Schmidt, intel-wired-lan@lists.osuosl.org
Cc: Jesse Brandeburg, Tony Nguyen, Richard Cochran,
netdev@vger.kernel.org, Jacob Keller, Jiri Pirko,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Przemek Kitszel, Temerkhanov, Sergey
> -----Original Message-----
> From: Michal Schmidt <mschmidt@redhat.com>
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>; Tony Nguyen
> <anthony.l.nguyen@intel.com>; Richard Cochran
> <richardcochran@gmail.com>; netdev@vger.kernel.org; Jacob Keller
> <jacob.e.keller@intel.com>; Jiri Pirko <jiri@resnulli.us>; Arkadiusz Kubalewski
> <arkadiusz.kubalewski@intel.com>; Karol Kolacinski
> <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>;
> Przemek Kitszel <przemyslaw.kitszel@intel.com>; Temerkhanov, Sergey
> <sergey.temerkhanov@intel.com>
> Subject: [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into
> ice_ptp_gettimex64
>
> This is a cleanup. It is unnecessary to have this function just to call another
> function.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 0875f37add78..0f17fc1181d2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1166,26 +1166,6 @@ static void ice_ptp_reset_cached_phctime(struct
> ice_pf *pf)
> ice_ptp_mark_tx_tracker_stale(&pf->ptp.port.tx);
> }
>
> -/**
> - * ice_ptp_read_time - Read the time from the device
> - * @pf: Board private structure
> - * @ts: timespec structure to hold the current time value
> - * @sts: Optional parameter for holding a pair of system timestamps from
> - * the system clock. Will be ignored if NULL is given.
> - *
> - * This function reads the source clock registers and stores them in a
> timespec.
> - * However, since the registers are 64 bits of nanoseconds, we must convert
> the
> - * result to a timespec before we can return.
> - */
> -static void
> -ice_ptp_read_time(struct ice_pf *pf, struct timespec64 *ts,
> - struct ptp_system_timestamp *sts)
> -{
> - u64 time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> -
> - *ts = ns_to_timespec64(time_ns);
> -}
> -
> /**
> * ice_ptp_write_init - Set PHC time to provided value
> * @pf: Board private structure
> @@ -1926,9 +1906,10 @@ ice_ptp_gettimex64(struct ptp_clock_info *info,
> struct timespec64 *ts,
> struct ptp_system_timestamp *sts)
> {
> struct ice_pf *pf = ptp_info_to_pf(info);
> + u64 time_ns;
>
> - ice_ptp_read_time(pf, ts, sts);
> -
> + time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> + *ts = ns_to_timespec64(time_ns);
> return 0;
> }
>
> --
> 2.43.2
>
Reviewed-by: Sai Krishna <saikrishnag@marvell.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
@ 2024-03-26 14:46 ` Sai Krishna Gajula
0 siblings, 0 replies; 19+ messages in thread
From: Sai Krishna Gajula @ 2024-03-26 14:46 UTC (permalink / raw)
To: Michal Schmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Arkadiusz Kubalewski, Karol Kolacinski,
Marcin Szycik, Tony Nguyen, Przemek Kitszel, Jacob Keller
> -----Original Message-----
> From: Michal Schmidt <mschmidt@redhat.com>
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>; Tony Nguyen
> <anthony.l.nguyen@intel.com>; Richard Cochran
> <richardcochran@gmail.com>; netdev@vger.kernel.org; Jacob Keller
> <jacob.e.keller@intel.com>; Jiri Pirko <jiri@resnulli.us>; Arkadiusz Kubalewski
> <arkadiusz.kubalewski@intel.com>; Karol Kolacinski
> <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>;
> Przemek Kitszel <przemyslaw.kitszel@intel.com>; Temerkhanov, Sergey
> <sergey.temerkhanov@intel.com>
> Subject: [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into
> ice_ptp_gettimex64
>
> This is a cleanup. It is unnecessary to have this function just to call another
> function.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 0875f37add78..0f17fc1181d2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1166,26 +1166,6 @@ static void ice_ptp_reset_cached_phctime(struct
> ice_pf *pf)
> ice_ptp_mark_tx_tracker_stale(&pf->ptp.port.tx);
> }
>
> -/**
> - * ice_ptp_read_time - Read the time from the device
> - * @pf: Board private structure
> - * @ts: timespec structure to hold the current time value
> - * @sts: Optional parameter for holding a pair of system timestamps from
> - * the system clock. Will be ignored if NULL is given.
> - *
> - * This function reads the source clock registers and stores them in a
> timespec.
> - * However, since the registers are 64 bits of nanoseconds, we must convert
> the
> - * result to a timespec before we can return.
> - */
> -static void
> -ice_ptp_read_time(struct ice_pf *pf, struct timespec64 *ts,
> - struct ptp_system_timestamp *sts)
> -{
> - u64 time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> -
> - *ts = ns_to_timespec64(time_ns);
> -}
> -
> /**
> * ice_ptp_write_init - Set PHC time to provided value
> * @pf: Board private structure
> @@ -1926,9 +1906,10 @@ ice_ptp_gettimex64(struct ptp_clock_info *info,
> struct timespec64 *ts,
> struct ptp_system_timestamp *sts)
> {
> struct ice_pf *pf = ptp_info_to_pf(info);
> + u64 time_ns;
>
> - ice_ptp_read_time(pf, ts, sts);
> -
> + time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> + *ts = ns_to_timespec64(time_ns);
> return 0;
> }
>
> --
> 2.43.2
>
Reviewed-by: Sai Krishna <saikrishnag@marvell.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [Intel-wired-lan] [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-29 4:57 ` Pucha, HimasekharX Reddy
-1 siblings, 0 replies; 19+ messages in thread
From: Pucha, HimasekharX Reddy @ 2024-03-29 4:57 UTC (permalink / raw)
To: mschmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Kubalewski, Arkadiusz, Kolacinski, Karol,
Marcin Szycik, Nguyen, Anthony L, Kitszel, Przemyslaw,
Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Michal Schmidt
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jiri Pirko <jiri@resnulli.us>; Temerkhanov, Sergey <sergey.temerkhanov@intel.com>; netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC
>
> There is a need for synchronization between ice PFs on the same physical adapter.
>
> Add a "struct ice_adapter" for holding data shared between PFs of the same multifunction PCI device. The struct is refcounted - each ice_pf holds a reference to it.
>
> Its first use will be for PTP. I expect it will be useful also to improve the ugliness that is ice_prot_id_tbl.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/Makefile | 3 +-
> drivers/net/ethernet/intel/ice/ice.h | 2 +
> drivers/net/ethernet/intel/ice/ice_adapter.c | 114 +++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_adapter.h | 22 ++++
> drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
> 5 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
> create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC
@ 2024-03-29 4:57 ` Pucha, HimasekharX Reddy
0 siblings, 0 replies; 19+ messages in thread
From: Pucha, HimasekharX Reddy @ 2024-03-29 4:57 UTC (permalink / raw)
To: mschmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Kubalewski, Arkadiusz, Kolacinski, Karol,
Marcin Szycik, Nguyen, Anthony L, Kitszel, Przemyslaw,
Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Michal Schmidt
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jiri Pirko <jiri@resnulli.us>; Temerkhanov, Sergey <sergey.temerkhanov@intel.com>; netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC
>
> There is a need for synchronization between ice PFs on the same physical adapter.
>
> Add a "struct ice_adapter" for holding data shared between PFs of the same multifunction PCI device. The struct is refcounted - each ice_pf holds a reference to it.
>
> Its first use will be for PTP. I expect it will be useful also to improve the ugliness that is ice_prot_id_tbl.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/Makefile | 3 +-
> drivers/net/ethernet/intel/ice/ice.h | 2 +
> drivers/net/ethernet/intel/ice/ice_adapter.c | 114 +++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_adapter.h | 22 ++++
> drivers/net/ethernet/intel/ice/ice_main.c | 8 ++
> 5 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
> create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [Intel-wired-lan] [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-29 5:01 ` Pucha, HimasekharX Reddy
-1 siblings, 0 replies; 19+ messages in thread
From: Pucha, HimasekharX Reddy @ 2024-03-29 5:01 UTC (permalink / raw)
To: mschmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Kubalewski, Arkadiusz, Kolacinski, Karol,
Marcin Szycik, Nguyen, Anthony L, Kitszel, Przemyslaw,
Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Michal Schmidt
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jiri Pirko <jiri@resnulli.us>; Temerkhanov, Sergey <sergey.temerkhanov@intel.com>; netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path
>
> The PTP hardware semaphore (PFTSYN_SEM) is used to synchronize operations that program the PTP timers. The operations involve issuing commands to the sideband queue. The E810 does not have a hardware sideband queue, so the admin queue is used. The admin queue is slow.
> I have observed delays in hundreds of milliseconds waiting for ice_sq_done.
>
> When phc2sys reads the time from the ice PTP clock and PFTSYN_SEM is held by a task performing one of the slow operations, ice_ptp_lock can easily time out. phc2sys gets -EBUSY and the kernel prints:
> ice 0000:XX:YY.0: PTP failed to get time These messages appear once every few seconds, causing log spam.
>
> The E810 datasheet recommends an algorithm for reading the upper 64 bits of the GLTSYN_TIME register. It matches what's implemented in ice_ptp_read_src_clk_reg. It is robust against wrap-around, but not necessarily against the concurrent setting of the register (with GLTSYN_CMD_{INIT,ADJ}_TIME commands). Perhaps that's why
> ice_ptp_gettimex64 also takes PFTSYN_SEM.
>
> The race with time setters can be prevented without relying on the PTP hardware semaphore. Using the "ice_adapter" from the previous patch, we can have a common spinlock for the PFs that share the clock hardware.
> It will protect the reading and writing to the GLTSYN_TIME register.
> The writing is performed indirectly, by the hardware, as a result of the driver writing GLTSYN_CMD_SYNC in ice_ptp_exec_tmr_cmd. I wasn't sure if the ice_flush there is enough to make sure GLTSYN_TIME has been updated, but it works well in my testing.
>
> My test code can be seen here:
> https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
> It consists of:
> - kernel threads reading the time in a busy loop and looking at the
> deltas between consecutive values, reporting new maxima.
> - a shell script that sets the time repeatedly;
> - a bpftrace probe to produce a histogram of the measured deltas.
> Without the spinlock ptp_gltsyn_time_lock, it is easy to see tearing.
> Deltas in the [2G, 4G) range appear in the histograms.
> With the spinlock added, there is no tearing and the biggest delta I saw was in the range [1M, 2M), that is under 2 ms.
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_adapter.c | 2 ++ drivers/net/ethernet/intel/ice/ice_adapter.h | 6 ++++++
> drivers/net/ethernet/intel/ice/ice_ptp.c | 8 +-------
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +++
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path
@ 2024-03-29 5:01 ` Pucha, HimasekharX Reddy
0 siblings, 0 replies; 19+ messages in thread
From: Pucha, HimasekharX Reddy @ 2024-03-29 5:01 UTC (permalink / raw)
To: mschmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Kubalewski, Arkadiusz, Kolacinski, Karol,
Marcin Szycik, Nguyen, Anthony L, Kitszel, Przemyslaw,
Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Michal Schmidt
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jiri Pirko <jiri@resnulli.us>; Temerkhanov, Sergey <sergey.temerkhanov@intel.com>; netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path
>
> The PTP hardware semaphore (PFTSYN_SEM) is used to synchronize operations that program the PTP timers. The operations involve issuing commands to the sideband queue. The E810 does not have a hardware sideband queue, so the admin queue is used. The admin queue is slow.
> I have observed delays in hundreds of milliseconds waiting for ice_sq_done.
>
> When phc2sys reads the time from the ice PTP clock and PFTSYN_SEM is held by a task performing one of the slow operations, ice_ptp_lock can easily time out. phc2sys gets -EBUSY and the kernel prints:
> ice 0000:XX:YY.0: PTP failed to get time These messages appear once every few seconds, causing log spam.
>
> The E810 datasheet recommends an algorithm for reading the upper 64 bits of the GLTSYN_TIME register. It matches what's implemented in ice_ptp_read_src_clk_reg. It is robust against wrap-around, but not necessarily against the concurrent setting of the register (with GLTSYN_CMD_{INIT,ADJ}_TIME commands). Perhaps that's why
> ice_ptp_gettimex64 also takes PFTSYN_SEM.
>
> The race with time setters can be prevented without relying on the PTP hardware semaphore. Using the "ice_adapter" from the previous patch, we can have a common spinlock for the PFs that share the clock hardware.
> It will protect the reading and writing to the GLTSYN_TIME register.
> The writing is performed indirectly, by the hardware, as a result of the driver writing GLTSYN_CMD_SYNC in ice_ptp_exec_tmr_cmd. I wasn't sure if the ice_flush there is enough to make sure GLTSYN_TIME has been updated, but it works well in my testing.
>
> My test code can be seen here:
> https://gitlab.com/mschmidt2/linux/-/commits/ice-ptp-host-side-lock-10
> It consists of:
> - kernel threads reading the time in a busy loop and looking at the
> deltas between consecutive values, reporting new maxima.
> - a shell script that sets the time repeatedly;
> - a bpftrace probe to produce a histogram of the measured deltas.
> Without the spinlock ptp_gltsyn_time_lock, it is easy to see tearing.
> Deltas in the [2G, 4G) range appear in the histograms.
> With the spinlock added, there is no tearing and the biggest delta I saw was in the range [1M, 2M), that is under 2 ms.
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_adapter.c | 2 ++ drivers/net/ethernet/intel/ice/ice_adapter.h | 6 ++++++
> drivers/net/ethernet/intel/ice/ice_ptp.c | 8 +-------
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 +++
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-29 5:03 ` Pucha, HimasekharX Reddy
-1 siblings, 0 replies; 19+ messages in thread
From: Pucha, HimasekharX Reddy @ 2024-03-29 5:03 UTC (permalink / raw)
To: mschmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Kubalewski, Arkadiusz, Kolacinski, Karol,
Marcin Szycik, Nguyen, Anthony L, Kitszel, Przemyslaw,
Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Michal Schmidt
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jiri Pirko <jiri@resnulli.us>; Temerkhanov, Sergey <sergey.temerkhanov@intel.com>; netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
>
> This is a cleanup. It is unnecessary to have this function just to call another function.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
@ 2024-03-29 5:03 ` Pucha, HimasekharX Reddy
0 siblings, 0 replies; 19+ messages in thread
From: Pucha, HimasekharX Reddy @ 2024-03-29 5:03 UTC (permalink / raw)
To: mschmidt, intel-wired-lan@lists.osuosl.org
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev@vger.kernel.org,
Richard Cochran, Kubalewski, Arkadiusz, Kolacinski, Karol,
Marcin Szycik, Nguyen, Anthony L, Kitszel, Przemyslaw,
Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Michal Schmidt
> Sent: Tuesday, March 26, 2024 4:51 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Jiri Pirko <jiri@resnulli.us>; Temerkhanov, Sergey <sergey.temerkhanov@intel.com>; netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; Marcin Szycik <marcin.szycik@linux.intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
>
> This is a cleanup. It is unnecessary to have this function just to call another function.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
@ 2024-03-29 8:35 ` Kalesh Anakkur Purayil
-1 siblings, 0 replies; 19+ messages in thread
From: Kalesh Anakkur Purayil @ 2024-03-29 8:35 UTC (permalink / raw)
To: Michal Schmidt
Cc: intel-wired-lan, Jesse Brandeburg, Tony Nguyen, Richard Cochran,
netdev, Jacob Keller, Jiri Pirko, Arkadiusz Kubalewski,
Karol Kolacinski, Marcin Szycik, Przemek Kitszel,
Temerkhanov, Sergey
[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]
On Tue, Mar 26, 2024 at 6:07 AM Michal Schmidt <mschmidt@redhat.com> wrote:
>
> This is a cleanup. It is unnecessary to have this function just to call
> another function.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 0875f37add78..0f17fc1181d2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1166,26 +1166,6 @@ static void ice_ptp_reset_cached_phctime(struct ice_pf *pf)
> ice_ptp_mark_tx_tracker_stale(&pf->ptp.port.tx);
> }
>
> -/**
> - * ice_ptp_read_time - Read the time from the device
> - * @pf: Board private structure
> - * @ts: timespec structure to hold the current time value
> - * @sts: Optional parameter for holding a pair of system timestamps from
> - * the system clock. Will be ignored if NULL is given.
> - *
> - * This function reads the source clock registers and stores them in a timespec.
> - * However, since the registers are 64 bits of nanoseconds, we must convert the
> - * result to a timespec before we can return.
> - */
> -static void
> -ice_ptp_read_time(struct ice_pf *pf, struct timespec64 *ts,
> - struct ptp_system_timestamp *sts)
> -{
> - u64 time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> -
> - *ts = ns_to_timespec64(time_ns);
> -}
> -
> /**
> * ice_ptp_write_init - Set PHC time to provided value
> * @pf: Board private structure
> @@ -1926,9 +1906,10 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct timespec64 *ts,
> struct ptp_system_timestamp *sts)
> {
> struct ice_pf *pf = ptp_info_to_pf(info);
> + u64 time_ns;
>
> - ice_ptp_read_time(pf, ts, sts);
> -
> + time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> + *ts = ns_to_timespec64(time_ns);
> return 0;
> }
>
> --
> 2.43.2
>
>
--
Regards,
Kalesh A P
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64
@ 2024-03-29 8:35 ` Kalesh Anakkur Purayil
0 siblings, 0 replies; 19+ messages in thread
From: Kalesh Anakkur Purayil @ 2024-03-29 8:35 UTC (permalink / raw)
To: Michal Schmidt
Cc: Jiri Pirko, Temerkhanov, Sergey, netdev, Richard Cochran,
Arkadiusz Kubalewski, Karol Kolacinski, Marcin Szycik,
Tony Nguyen, Przemek Kitszel, Jacob Keller, intel-wired-lan
[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]
On Tue, Mar 26, 2024 at 6:07 AM Michal Schmidt <mschmidt@redhat.com> wrote:
>
> This is a cleanup. It is unnecessary to have this function just to call
> another function.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 25 +++---------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 0875f37add78..0f17fc1181d2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1166,26 +1166,6 @@ static void ice_ptp_reset_cached_phctime(struct ice_pf *pf)
> ice_ptp_mark_tx_tracker_stale(&pf->ptp.port.tx);
> }
>
> -/**
> - * ice_ptp_read_time - Read the time from the device
> - * @pf: Board private structure
> - * @ts: timespec structure to hold the current time value
> - * @sts: Optional parameter for holding a pair of system timestamps from
> - * the system clock. Will be ignored if NULL is given.
> - *
> - * This function reads the source clock registers and stores them in a timespec.
> - * However, since the registers are 64 bits of nanoseconds, we must convert the
> - * result to a timespec before we can return.
> - */
> -static void
> -ice_ptp_read_time(struct ice_pf *pf, struct timespec64 *ts,
> - struct ptp_system_timestamp *sts)
> -{
> - u64 time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> -
> - *ts = ns_to_timespec64(time_ns);
> -}
> -
> /**
> * ice_ptp_write_init - Set PHC time to provided value
> * @pf: Board private structure
> @@ -1926,9 +1906,10 @@ ice_ptp_gettimex64(struct ptp_clock_info *info, struct timespec64 *ts,
> struct ptp_system_timestamp *sts)
> {
> struct ice_pf *pf = ptp_info_to_pf(info);
> + u64 time_ns;
>
> - ice_ptp_read_time(pf, ts, sts);
> -
> + time_ns = ice_ptp_read_src_clk_reg(pf, sts);
> + *ts = ns_to_timespec64(time_ns);
> return 0;
> }
>
> --
> 2.43.2
>
>
--
Regards,
Kalesh A P
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-03-29 8:35 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-25 23:20 [PATCH net-next v4 0/3] ice: lighter locking for PTP time reading Michal Schmidt
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
2024-03-25 23:20 ` [PATCH net-next v4 1/3] ice: add ice_adapter for shared data across PFs on the same NIC Michal Schmidt
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
2024-03-29 4:57 ` Pucha, HimasekharX Reddy
2024-03-29 4:57 ` Pucha, HimasekharX Reddy
2024-03-25 23:20 ` [PATCH net-next v4 2/3] ice: avoid the PTP hardware semaphore in gettimex64 path Michal Schmidt
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
2024-03-29 5:01 ` Pucha, HimasekharX Reddy
2024-03-29 5:01 ` Pucha, HimasekharX Reddy
2024-03-25 23:20 ` [PATCH net-next v4 3/3] ice: fold ice_ptp_read_time into ice_ptp_gettimex64 Michal Schmidt
2024-03-25 23:20 ` [Intel-wired-lan] " Michal Schmidt
2024-03-26 14:46 ` Sai Krishna Gajula
2024-03-26 14:46 ` [Intel-wired-lan] " Sai Krishna Gajula
2024-03-29 5:03 ` Pucha, HimasekharX Reddy
2024-03-29 5:03 ` Pucha, HimasekharX Reddy
2024-03-29 8:35 ` Kalesh Anakkur Purayil
2024-03-29 8:35 ` [Intel-wired-lan] " Kalesh Anakkur Purayil
2024-03-26 9:51 ` [Intel-wired-lan] [PATCH net-next v4 0/3] ice: lighter locking for PTP time reading Ivan Vecera
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.