[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230809155022.132a69a7@kernel.org>
Date: Wed, 9 Aug 2023 15:50:22 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Sai Krishna <saikrishnag@...vell.com>
Cc: <davem@...emloft.net>, <edumazet@...gle.com>, <pabeni@...hat.com>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<sgoutham@...vell.com>, <gakula@...vell.com>,
<sbhatta@...vell.com>, <hkelam@...vell.com>,
<richardcochran@...il.com>,
Naveen Mamindlapalli <naveenm@...vell.com>
Subject: Re: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp counter
atomic update feature
On Mon, 7 Aug 2023 19:35:35 +0530 Sai Krishna wrote:
> Some of the newer silicon versions in CN10K series supports a feature
> where in the current PTP timestamp in HW can be updated atomically
> without losing any cpu cycles unlike read/modify/write register.
> This patch uses this feature so that PTP accuracy can be improved
> while adjusting the master offset in HW. There is no need for SW
> timecounter when using this feature. So removed references to SW
> timecounter wherever appropriate.
> -#include "ptp.h"
> #include "mbox.h"
> #include "rvu.h"
> +#include "ptp.h"
If you reorder the includes - maybe put them in alphabetical order?
> static bool cn10k_ptp_errata(struct ptp *ptp)
> {
> - if (ptp->pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_PTP ||
> - ptp->pdev->subsystem_device == PCI_SUBSYS_DEVID_CNF10K_A_PTP)
> + if ((is_ptp_dev_cn10ka(ptp) &&
> + ((ptp->pdev->revision & 0x0F) == 0x0 || (ptp->pdev->revision & 0x0F) == 0x1)) ||
> + (is_ptp_dev_cnf10ka(ptp) &&
> + ((ptp->pdev->revision & 0x0F) == 0x0 || (ptp->pdev->revision & 0x0F) == 0x1)))
Please refactor the revision check to avoid these long lines repeating
the same logic
> return true;
> +
> return false;
> }
>
> -static bool is_ptp_tsfmt_sec_nsec(struct ptp *ptp)
> +static bool is_tstmp_atomic_update_supported(struct rvu *rvu)
> {
> - if (ptp->pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_PTP ||
> - ptp->pdev->subsystem_device == PCI_SUBSYS_DEVID_CNF10K_A_PTP)
> - return true;
> - return false;
> + struct ptp *ptp = rvu->ptp;
> + struct pci_dev *pdev;
> +
> + if (is_rvu_otx2(rvu))
> + return false;
> +
> + pdev = ptp->pdev;
> +
> + /* On older silicon variants of CN10K, atomic update feature
> + * is not available.
> + */
> + if ((pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_PTP &&
> + (pdev->revision & 0x0F) == 0x0) ||
> + (pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_PTP &&
> + (pdev->revision & 0x0F) == 0x1) ||
> + (pdev->subsystem_device == PCI_SUBSYS_DEVID_CNF10K_A_PTP &&
> + (pdev->revision & 0x0F) == 0x0) ||
> + (pdev->subsystem_device == PCI_SUBSYS_DEVID_CNF10K_A_PTP &&
> + (pdev->revision & 0x0F) == 0x1))
why are you not using cn10k_ptp_errata() here?
> + return false;
> +
> + return true;
> }
> -static int otx2_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
> +static int otx2_ptp_tc_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
> {
> struct otx2_ptp *ptp = container_of(ptp_info, struct otx2_ptp,
> ptp_info);
> struct otx2_nic *pfvf = ptp->nic;
>
> + if (!ptp->nic)
> + return -ENODEV;
Is this check related to the rest of the patch?
> mutex_lock(&pfvf->mbox.lock);
> timecounter_adjtime(&ptp->time_counter, delta);
> mutex_unlock(&pfvf->mbox.lock);
--
pw-bot: cr
Powered by blists - more mailing lists