Add max frequency parameter

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

@@ -33,16 +33,18 @@ var (
 	firstStepTS2PHCFlag time.Duration
 	srcPinTS2PHCFlag    uint
 	dstPinTS2PHCFlag    uint
+	maxFreqTS2PHCFlag   float64
 )
 
 func init() {
 	RootCmd.AddCommand(ts2phcCmd)
 	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)
 	}

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

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) {