| Message ID | 20221214081038.1720-1-muhammad.husaini.zulkifli@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net,v1] igc: Fix PPS delta between two synchronized end-points | expand |
On 12/14/2022 10:10, Muhammad Husaini Zulkifli wrote: > From: Christopher S Hall <christopher.s.hall@intel.com> > > This patch fix the pulse per second output delta between > two synchronized end-points. > > Based on Intel Discrete I225 Software User Manual Section > 4.2.15 TimeSync Auxiliary Control Register, ST0[Bit 4] and > ST1[Bit 7] must be set to ensure that clock output will be > toggles based on frequency value defined. This is to ensure > that output of the PPS is aligned with the clock. > > How to test: > > 1) Running time synchronization on both end points. > Ex: ptp4l --step_threshold=1 -m -f gPTP.cfg -i <interface name> > > 2) Configure PPS output using below command for both end-points > Ex: SDP0 on I225 REV4 SKU variant > > ./testptp -d /dev/ptp0 -L 0,2 > ./testptp -d /dev/ptp0 -p 1000000000 > > 3) Measure the output using analyzer for both end-points > > Fixes: 87938851b6ef ("igc: enable auxiliary PHC functions for the i225") > Signed-off-by: Christopher S Hall <christopher.s.hall@intel.com> > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > --- > drivers/net/ethernet/intel/igc/igc_defines.h | 2 ++ > drivers/net/ethernet/intel/igc/igc_ptp.c | 10 ++++++---- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h > index a7b22639cfcd..e9747ec5ac0b 100644 > --- a/drivers/net/ethernet/intel/igc/igc_defines.h > +++ b/drivers/net/ethernet/intel/igc/igc_defines.h > @@ -475,7 +475,9 @@ > #define IGC_TSAUXC_EN_TT0 BIT(0) /* Enable target time 0. */ > #define IGC_TSAUXC_EN_TT1 BIT(1) /* Enable target time 1. */ > #define IGC_TSAUXC_EN_CLK0 BIT(2) /* Enable Configurable Frequency Clock 0. */ > +#define IGC_TSAUXC_ST0 BIT(4) /* Start Clock 0 Toggle on Target Time 0. */ > #define IGC_TSAUXC_EN_CLK1 BIT(5) /* Enable Configurable Frequency Clock 1. */ > +#define IGC_TSAUXC_ST1 BIT(7) /* Start Clock 1 Toggle on Target Time 1. */ > #define IGC_TSAUXC_EN_TS0 BIT(8) /* Enable hardware timestamp 0. */ > #define IGC_TSAUXC_AUTT0 BIT(9) /* Auxiliary Timestamp Taken. */ > #define IGC_TSAUXC_EN_TS1 BIT(10) /* Enable hardware timestamp 0. */ > diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c > index 8dbb9f903ca7..c34734d432e0 100644 > --- a/drivers/net/ethernet/intel/igc/igc_ptp.c > +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c > @@ -322,7 +322,7 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp, > ts = ns_to_timespec64(ns); > if (rq->perout.index == 1) { > if (use_freq) { > - tsauxc_mask = IGC_TSAUXC_EN_CLK1; > + tsauxc_mask = IGC_TSAUXC_EN_CLK1 | IGC_TSAUXC_ST1; > tsim_mask = 0; > } else { > tsauxc_mask = IGC_TSAUXC_EN_TT1; > @@ -333,7 +333,7 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp, > freqout = IGC_FREQOUT1; > } else { > if (use_freq) { > - tsauxc_mask = IGC_TSAUXC_EN_CLK0; > + tsauxc_mask = IGC_TSAUXC_EN_CLK0 | IGC_TSAUXC_ST0; > tsim_mask = 0; > } else { > tsauxc_mask = IGC_TSAUXC_EN_TT0; > @@ -347,10 +347,12 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp, > tsauxc = rd32(IGC_TSAUXC); > tsim = rd32(IGC_TSIM); > if (rq->perout.index == 1) { > - tsauxc &= ~(IGC_TSAUXC_EN_TT1 | IGC_TSAUXC_EN_CLK1); > + tsauxc &= ~(IGC_TSAUXC_EN_TT1 | IGC_TSAUXC_EN_CLK1 | > + IGC_TSAUXC_ST1); > tsim &= ~IGC_TSICR_TT1; > } else { > - tsauxc &= ~(IGC_TSAUXC_EN_TT0 | IGC_TSAUXC_EN_CLK0); > + tsauxc &= ~(IGC_TSAUXC_EN_TT0 | IGC_TSAUXC_EN_CLK0 | > + IGC_TSAUXC_ST0); > tsim &= ~IGC_TSICR_TT0; > } > if (on) { Acked-by: Sasha Neftin <sasha.neftin@intel.com>
On 12/14/2022 10:10, Muhammad Husaini Zulkifli wrote: > From: Christopher S Hall <christopher.s.hall@intel.com> > > This patch fix the pulse per second output delta between > two synchronized end-points. > > Based on Intel Discrete I225 Software User Manual Section > 4.2.15 TimeSync Auxiliary Control Register, ST0[Bit 4] and > ST1[Bit 7] must be set to ensure that clock output will be > toggles based on frequency value defined. This is to ensure > that output of the PPS is aligned with the clock. > > How to test: > > 1) Running time synchronization on both end points. > Ex: ptp4l --step_threshold=1 -m -f gPTP.cfg -i <interface name> > > 2) Configure PPS output using below command for both end-points > Ex: SDP0 on I225 REV4 SKU variant > > ./testptp -d /dev/ptp0 -L 0,2 > ./testptp -d /dev/ptp0 -p 1000000000 > > 3) Measure the output using analyzer for both end-points > > Fixes: 87938851b6ef ("igc: enable auxiliary PHC functions for the i225") > Signed-off-by: Christopher S Hall <christopher.s.hall@intel.com> > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > --- > drivers/net/ethernet/intel/igc/igc_defines.h | 2 ++ > drivers/net/ethernet/intel/igc/igc_ptp.c | 10 ++++++---- > 2 files changed, 8 insertions(+), 4 deletions(-) Tested-by: Naama Meir <naamax.meir@linux.intel.com>
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h index a7b22639cfcd..e9747ec5ac0b 100644 --- a/drivers/net/ethernet/intel/igc/igc_defines.h +++ b/drivers/net/ethernet/intel/igc/igc_defines.h @@ -475,7 +475,9 @@ #define IGC_TSAUXC_EN_TT0 BIT(0) /* Enable target time 0. */ #define IGC_TSAUXC_EN_TT1 BIT(1) /* Enable target time 1. */ #define IGC_TSAUXC_EN_CLK0 BIT(2) /* Enable Configurable Frequency Clock 0. */ +#define IGC_TSAUXC_ST0 BIT(4) /* Start Clock 0 Toggle on Target Time 0. */ #define IGC_TSAUXC_EN_CLK1 BIT(5) /* Enable Configurable Frequency Clock 1. */ +#define IGC_TSAUXC_ST1 BIT(7) /* Start Clock 1 Toggle on Target Time 1. */ #define IGC_TSAUXC_EN_TS0 BIT(8) /* Enable hardware timestamp 0. */ #define IGC_TSAUXC_AUTT0 BIT(9) /* Auxiliary Timestamp Taken. */ #define IGC_TSAUXC_EN_TS1 BIT(10) /* Enable hardware timestamp 0. */ diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c index 8dbb9f903ca7..c34734d432e0 100644 --- a/drivers/net/ethernet/intel/igc/igc_ptp.c +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c @@ -322,7 +322,7 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp, ts = ns_to_timespec64(ns); if (rq->perout.index == 1) { if (use_freq) { - tsauxc_mask = IGC_TSAUXC_EN_CLK1; + tsauxc_mask = IGC_TSAUXC_EN_CLK1 | IGC_TSAUXC_ST1; tsim_mask = 0; } else { tsauxc_mask = IGC_TSAUXC_EN_TT1; @@ -333,7 +333,7 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp, freqout = IGC_FREQOUT1; } else { if (use_freq) { - tsauxc_mask = IGC_TSAUXC_EN_CLK0; + tsauxc_mask = IGC_TSAUXC_EN_CLK0 | IGC_TSAUXC_ST0; tsim_mask = 0; } else { tsauxc_mask = IGC_TSAUXC_EN_TT0; @@ -347,10 +347,12 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp, tsauxc = rd32(IGC_TSAUXC); tsim = rd32(IGC_TSIM); if (rq->perout.index == 1) { - tsauxc &= ~(IGC_TSAUXC_EN_TT1 | IGC_TSAUXC_EN_CLK1); + tsauxc &= ~(IGC_TSAUXC_EN_TT1 | IGC_TSAUXC_EN_CLK1 | + IGC_TSAUXC_ST1); tsim &= ~IGC_TSICR_TT1; } else { - tsauxc &= ~(IGC_TSAUXC_EN_TT0 | IGC_TSAUXC_EN_CLK0); + tsauxc &= ~(IGC_TSAUXC_EN_TT0 | IGC_TSAUXC_EN_CLK0 | + IGC_TSAUXC_ST0); tsim &= ~IGC_TSICR_TT0; } if (on) {