From: Wojciech Drewek <[email protected]>
Date: Wed, 21 Aug 2024 14:15:32 +0200
From: Jacob Keller <[email protected]>
Implement support for reading the PHC time indirectly via the
VIRTCHNL_OP_1588_PTP_GET_TIME operation.
[...]
+/**
+ * iavf_queue_ptp_cmd - Queue PTP command for sending over virtchnl
+ * @adapter: private adapter structure
+ * @cmd: the command structure to send
+ *
+ * Queue the given command structure into the PTP virtchnl command queue tos
+ * end to the PF.
+ */
+static void iavf_queue_ptp_cmd(struct iavf_adapter *adapter,
+ struct iavf_ptp_aq_cmd *cmd)
+{
+ mutex_lock(&adapter->ptp.aq_cmd_lock);
+ list_add_tail(&cmd->list, &adapter->ptp.aq_cmds);
+ mutex_unlock(&adapter->ptp.aq_cmd_lock);
+
+ adapter->aq_required |= IAVF_FLAG_AQ_SEND_PTP_CMD;
+ mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
Are you sure you need delayed_work here? delayed_work is used only when
you need to run it after a delay. If the delay is always 0, then you
only need work_struct and queue_work().
+}
+
+/**
+ * iavf_send_phc_read - Send request to read PHC time
[...]
+static int iavf_ptp_gettimex64(struct ptp_clock_info *info,
+ struct timespec64 *ts,
+ struct ptp_system_timestamp *sts)
+{
+ struct iavf_adapter *adapter = iavf_clock_to_adapter(info);
+
+ if (!adapter->ptp.initialized)
+ return -ENODEV;
Why is it -ENODEV here, but -EOPNOTSUPP several functions above, are you
sure these codes are the ones expected by the upper layers?
+
+ return iavf_read_phc_indirect(adapter, ts, sts);
+}
[...]
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.h
b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
index c2ed24cef926..0bb4bddc1495 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_ptp.h
+++ b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
@@ -6,9 +6,13 @@
#include "iavf_types.h"
+#define iavf_clock_to_adapter(info) \
+ container_of_const(info, struct iavf_adapter, ptp.info)
It's only used in one file, are you sure you need it here in the header?
Or it will be used in later patches?
[...]
+void iavf_virtchnl_send_ptp_cmd(struct iavf_adapter *adapter)
+{
+ struct device *dev = &adapter->pdev->dev;
+ struct iavf_ptp_aq_cmd *cmd;
+ int err;
+
+ if (!adapter->ptp.initialized) {
BTW does it make sense to introduce ptp.initialized since you can always
check ptp.clock for being %NULL and it will be the same?
+ /* This shouldn't be possible to hit, since no messages should
+ * be queued if PTP is not initialized.
+ */
+ pci_err(adapter->pdev, "PTP is not initialized\n");
+ adapter->aq_required &= ~IAVF_FLAG_AQ_SEND_PTP_CMD;
+ return;
+ }
+
+ mutex_lock(&adapter->ptp.aq_cmd_lock);
+ cmd = list_first_entry_or_null(&adapter->ptp.aq_cmds,
+ struct iavf_ptp_aq_cmd, list);
+ if (!cmd) {
+ /* no further PTP messages to send */
+ adapter->aq_required &= ~IAVF_FLAG_AQ_SEND_PTP_CMD;
+ goto out_unlock;
+ }
+
+ if (adapter->current_op != VIRTCHNL_OP_UNKNOWN) {
+ /* bail because we already have a command pending */
+ dev_err(dev, "Cannot send PTP command %d, command %d pending\n",
pci_err()
+ cmd->v_opcode, adapter->current_op);
+ goto out_unlock;
+ }
+
+ err = iavf_send_pf_msg(adapter, cmd->v_opcode, cmd->msg, cmd->msglen);
+ if (!err) {
+ /* Command was sent without errors, so we can remove it from
+ * the list and discard it.
+ */
+ list_del(&cmd->list);
+ kfree(cmd);
+ } else {
+ /* We failed to send the command, try again next cycle */
+ dev_warn(dev, "Failed to send PTP command %d\n", cmd->v_opcode);
pci_err() I'd say.
+ }
+
+ if (list_empty(&adapter->ptp.aq_cmds))
+ /* no further PTP messages to send */
+ adapter->aq_required &= ~IAVF_FLAG_AQ_SEND_PTP_CMD;
+
+out_unlock:
+ mutex_unlock(&adapter->ptp.aq_cmd_lock);
+}
+
/**
* iavf_print_link_message - print link up or down
* @adapter: adapter structure
@@ -2093,6 +2151,39 @@ static void iavf_activate_fdir_filters(struct
iavf_adapter *adapter)
adapter->aq_required |= IAVF_FLAG_AQ_ADD_FDIR_FILTER;
}
+/**
+ * iavf_virtchnl_ptp_get_time - Respond to VIRTCHNL_OP_1588_PTP_GET_TIME
+ * @adapter: private adapter structure
+ * @data: the message from the PF
+ * @len: length of the message from the PF
+ *
+ * Handle the VIRTCHNL_OP_1588_PTP_GET_TIME message from the PF. This message
+ * is sent by the PF in response to the same op as a request from the VF.
+ * Extract the 64bit nanoseconds time from the message and store it in
+ * cached_phc_time. Then, notify any thread that is waiting for the update via
+ * the wait queue.
+ */
+static void iavf_virtchnl_ptp_get_time(struct iavf_adapter *adapter,
+ void *data, u16 len)
+{
+ struct virtchnl_phc_time *msg;
+
+ if (len == sizeof(*msg)) {
+ msg = (struct virtchnl_phc_time *)data;
Redundant cast.
+ } else {
+ dev_err_once(&adapter->pdev->dev,
+ "Invalid VIRTCHNL_OP_1588_PTP_GET_TIME from PF. Got
size %u, expected %zu\n",
+ len, sizeof(*msg));
+ return;
+ }
struct virtchnl_phc_time *msg = data;
if (len != sizeof(*msg))
// error path
adapter->ptp.cached ...
IOW there's no point in this complex if-else.
+
+ adapter->ptp.cached_phc_time = msg->time;
+ adapter->ptp.cached_phc_updated = jiffies;
+ adapter->ptp.phc_time_ready = true;
+
+ wake_up(&adapter->ptp.phc_time_waitqueue);
+}
Thanks,
Olek
