From 0391c4b75e90bde89d49aea2c5c591365e91d47d Mon Sep 17 00:00:00 2001 From: Cristiano Ruschel Marques Dias Date: Tue, 8 Oct 2024 21:38:53 -0700 Subject: [PATCH] Add max frequency parameter (#400) Summary: WHAT? Add max frequency parameter to ts2phc command WHY? Avoid issues where device reports wrong max adjustment value (for instance, driver issues) Considerations - Why math.Min(maxFreq, deviceFreq)?? - I want to use the most restrictive value always, so that if device reports lower value, using this configuration does not cause invalid clock steps. Reviewed By: leoleovich Differential Revision: D63832053 --- cmd/ptpcheck/cmd/ts2phc.go | 11 +++++++---- phc/pps_source.go | 15 ++++++++++----- phc/pps_source_test.go | 37 +++++++++++++++++++++++++++---------- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/cmd/ptpcheck/cmd/ts2phc.go b/cmd/ptpcheck/cmd/ts2phc.go index b1ac9586..ccbe50b8 100644 --- a/cmd/ptpcheck/cmd/ts2phc.go +++ b/cmd/ptpcheck/cmd/ts2phc.go @@ -33,6 +33,7 @@ var ( firstStepTS2PHCFlag time.Duration srcPinTS2PHCFlag uint dstPinTS2PHCFlag uint + maxFreqTS2PHCFlag float64 ) func init() { @@ -40,9 +41,10 @@ func init() { ts2phcCmd.Flags().StringVarP(&srcDeviceTS2PHCFlag, "source", "s", "/dev/ptp_tcard", "Source for Time of Day (ToD) data. Only PHC devices are supported at the moment") ts2phcCmd.Flags().StringVarP(&dstDeviceTS2PHCFlag, "destination", "d", "eth0", "PHC to be synchronized. The clock may be identified by its character device (like /dev/ptp0) or its associated network interface (like eth0).") ts2phcCmd.Flags().DurationVarP(&intervalTS2PHCFlag, "interval", "i", time.Second, "Interval between syncs in nanosseconds") - ts2phcCmd.Flags().DurationVarP(&firstStepTS2PHCFlag, "first_step", "f", 20*time.Microsecond, "The maximum offset, specified in seconds, that the servo will correct by changing the clock frequency instead of stepping the clock. This is only applied on the first update. When set to 0.0, the servo will not step the clock on start. The default is 0.00002 (20 microseconds).") - ts2phcCmd.Flags().UintVarP(&srcPinTS2PHCFlag, "out-pin", "n", phc.DefaultTs2PhcIndex, "output pin number of the PPS signal on source device. Defaults to Pin 3") - ts2phcCmd.Flags().UintVarP(&dstPinTS2PHCFlag, "in-pin", "o", phc.DefaultTs2PhcSinkIndex, "input pin number of the PPS signal on destination device. Defaults to Pin 3") + ts2phcCmd.Flags().DurationVarP(&firstStepTS2PHCFlag, "first_step", "f", 20*time.Microsecond, "The maximum offset, specified in seconds, that the servo will correct by changing the clock frequency instead of stepping the clock. This is only applied on the first update. When set to 0.0, the servo will not step the clock on start.") + ts2phcCmd.Flags().UintVarP(&srcPinTS2PHCFlag, "out-pin", "n", phc.DefaultTs2PhcIndex, "output pin number of the PPS signal on source device.") + ts2phcCmd.Flags().UintVarP(&dstPinTS2PHCFlag, "in-pin", "o", phc.DefaultTs2PhcSinkIndex, "input pin number of the PPS signal on destination device. (default 0)") + ts2phcCmd.Flags().Float64VarP(&maxFreqTS2PHCFlag, "max_frequency", "m", 0, "maximum frequency in parts per billion (PPB) that the servo will correct by changing the clock frequency instead of stepping the clock. If unset, uses the maximum frequency reported by the PHC device.") } func ts2phcRun(srcDevicePath string, dstDeviceName string, interval time.Duration, stepth time.Duration, srcPinIndex uint) error { @@ -58,7 +60,8 @@ func ts2phcRun(srcDevicePath string, dstDeviceName string, interval time.Duratio if err != nil { return fmt.Errorf("error setting target device as PPS sink: %w", err) } - pi, err := phc.NewPiServo(interval, stepth, dstDevice) + + pi, err := phc.NewPiServo(interval, stepth, dstDevice, maxFreqTS2PHCFlag) if err != nil { return fmt.Errorf("error getting servo: %w", err) } diff --git a/phc/pps_source.go b/phc/pps_source.go index e4474c01..53e8a11a 100644 --- a/phc/pps_source.go +++ b/phc/pps_source.go @@ -62,6 +62,7 @@ const ( ppsStartDelay = 2 defaultPollerInterval = 40 * time.Millisecond PPSPollMaxAttempts = 20 + defaultMaxFreqAdj = 500000.0 ) // ServoController abstracts away servo @@ -180,8 +181,8 @@ func (ppsSource *PPSSource) Timestamp() (*time.Time, error) { return &currTime, nil } -// NewPiServo returns a servo.PiServo object configure for synchronizing the given device -func NewPiServo(interval time.Duration, stepth time.Duration, device FrequencyGetter) (*servo.PiServo, error) { +// NewPiServo returns a servo.PiServo object configure for synchronizing the given device. maxFreq 0 is equivalent to no maxFreq +func NewPiServo(interval time.Duration, stepth time.Duration, device FrequencyGetter, maxFreq float64) (*servo.PiServo, error) { servoCfg := servo.DefaultServoConfig() if stepth != 0 { // allow stepping clock on first update @@ -197,10 +198,14 @@ func NewPiServo(interval time.Duration, stepth time.Duration, device FrequencyGe pi := servo.NewPiServo(servoCfg, servo.DefaultPiServoCfg(), -freq) pi.SyncInterval(interval.Seconds()) - maxFreq, err := device.MaxFreqAdjPPB() - if err != nil { - maxFreq = float64(DefaultMaxClockFreqPPB) + if maxFreq == 0 { + maxFreq, err = device.MaxFreqAdjPPB() + if err != nil { + log.Printf("unable to get max frequency adjustment from device, using default: %f", defaultMaxFreqAdj) + maxFreq = defaultMaxFreqAdj + } } + pi.SetMaxFreq(maxFreq) return pi, nil diff --git a/phc/pps_source_test.go b/phc/pps_source_test.go index 8a9503af..eac73884 100644 --- a/phc/pps_source_test.go +++ b/phc/pps_source_test.go @@ -343,17 +343,17 @@ func TestNewPiServo(t *testing.T) { mockFrequencyGetter := NewMockFrequencyGetter(ctrl) gomock.InOrder( mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil), - mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(2.0, nil), + mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(3.0, nil), ) - servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter) + servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 0.0) require.NoError(t, err) require.Equal(t, int64(1), servo.Servo.FirstStepThreshold) require.Equal(t, true, servo.Servo.FirstUpdate) require.Equal(t, -1.0, servo.MeanFreq()) require.Equal(t, "INIT", servo.GetState().String()) - require.Equal(t, 2.0, servo.GetMaxFreq()) + require.Equal(t, 3.0, servo.GetMaxFreq()) } func TestNewPiServoFreqPPBError(t *testing.T) { @@ -364,21 +364,21 @@ func TestNewPiServoFreqPPBError(t *testing.T) { mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, fmt.Errorf("error")), ) - _, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter) + _, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 0.0) require.Error(t, err) } -func TestNewPiServoDefaultMaxFreq(t *testing.T) { +func TestNewPiServoMaxFreqError(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() mockFrequencyGetter := NewMockFrequencyGetter(ctrl) gomock.InOrder( mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil), - mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(2.0, fmt.Errorf("error")), + mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(12345.0, fmt.Errorf("error")), ) - servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter) + servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 0.0) require.NoError(t, err) require.Equal(t, int64(1), servo.Servo.FirstStepThreshold) @@ -388,22 +388,39 @@ func TestNewPiServoDefaultMaxFreq(t *testing.T) { require.Equal(t, 500000.0, servo.GetMaxFreq()) } +func TestNewPiServoUseMaxFreq(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockFrequencyGetter := NewMockFrequencyGetter(ctrl) + gomock.InOrder( + mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil), + ) + + servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 2.0) + + require.NoError(t, err) + require.Equal(t, int64(1), servo.Servo.FirstStepThreshold) + require.Equal(t, true, servo.Servo.FirstUpdate) + require.Equal(t, -1.0, servo.MeanFreq()) + require.Equal(t, "INIT", servo.GetState().String()) + require.Equal(t, 2.0, servo.GetMaxFreq()) +} + func TestNewPiServoNoFirstStep(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() mockFrequencyGetter := NewMockFrequencyGetter(ctrl) gomock.InOrder( mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil), - mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(2.0, fmt.Errorf("error")), ) - servo, err := NewPiServo(time.Duration(1), time.Duration(0), mockFrequencyGetter) + servo, err := NewPiServo(time.Duration(1), time.Duration(0), mockFrequencyGetter, 2.0) require.NoError(t, err) require.Equal(t, false, servo.Servo.FirstUpdate) require.Equal(t, -1.0, servo.MeanFreq()) require.Equal(t, "INIT", servo.GetState().String()) - require.Equal(t, 500000.0, servo.GetMaxFreq()) + require.Equal(t, 2.0, servo.GetMaxFreq()) } func TestPollLatestPPSEvent_SuccessfulPollWithEvent(t *testing.T) {