Messages in this thread | | | Date | Wed, 9 Aug 2023 15:50:22 -0700 | From | Jakub Kicinski <> | 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
| |