From c19888267a467d778731e207d4cf37336130cd7d Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Tue, 24 Mar 2026 15:02:08 +0200 Subject: [PATCH 1/3] Improve DPLL pin commands printing --- addons/intel/clock-chain.go | 2 +- pkg/dpll-netlink/dpll-uapi.go | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/addons/intel/clock-chain.go b/addons/intel/clock-chain.go index c4306c15..046d0ee2 100644 --- a/addons/intel/clock-chain.go +++ b/addons/intel/clock-chain.go @@ -606,7 +606,7 @@ func batchPinSet(commands []dpll.PinParentDeviceCtl) error { //nolint:errcheck defer conn.Close() for _, command := range commands { - glog.Infof("DPLL pin command %#v", command) + glog.Infof("DPLL pin command %s", command.String()) b, err := dpll.EncodePinControl(command) if err != nil { return err diff --git a/pkg/dpll-netlink/dpll-uapi.go b/pkg/dpll-netlink/dpll-uapi.go index 19bdb8b7..eb4e8d7c 100644 --- a/pkg/dpll-netlink/dpll-uapi.go +++ b/pkg/dpll-netlink/dpll-uapi.go @@ -394,6 +394,59 @@ func GetPinDirection(d uint32) string { return "" } +// String returns a concise debug representation aligned with PinParentDeviceHR +// field semantics (direction/state as names when known). +func (p PinControl) String() string { + var b strings.Builder + fmt.Fprintf(&b, "parentID=%d", p.PinParentID) + if p.Direction != nil { + if d := GetPinDirection(*p.Direction); d != "" { + fmt.Fprintf(&b, " direction=%s", d) + } else { + fmt.Fprintf(&b, " direction=%d", *p.Direction) + } + } + if p.Prio != nil { + fmt.Fprintf(&b, " prio=%d", *p.Prio) + } + if p.State != nil { + if s := GetPinState(*p.State); s != "" { + fmt.Fprintf(&b, " state=%s", s) + } else { + fmt.Fprintf(&b, " state=%d", *p.State) + } + } + return b.String() +} + +// String returns a concise debug representation of the pin parent device control request. +func (p PinParentDeviceCtl) String() string { + var b strings.Builder + b.WriteString("PinParentDeviceCtl{") + fmt.Fprintf(&b, "id=%d", p.ID) + if p.Frequency != nil { + fmt.Fprintf(&b, " frequency=%d", *p.Frequency) + } + if p.PhaseAdjust != nil { + fmt.Fprintf(&b, " phaseAdjust=%d", *p.PhaseAdjust) + } + if p.EsyncFrequency != nil { + fmt.Fprintf(&b, " esyncFrequency=%d", *p.EsyncFrequency) + } + if len(p.PinParentCtl) > 0 { + b.WriteString(" pinParent=[") + for i := range p.PinParentCtl { + if i > 0 { + b.WriteString(", ") + } + b.WriteString(p.PinParentCtl[i].String()) + } + b.WriteString("]") + } + b.WriteString("}") + return b.String() +} + // Defines pin capabilities const ( PinCapNone = 0 From dca904f59ec8476f21d4fb14474ee39dfb5ecdc4 Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Tue, 24 Mar 2026 16:31:59 +0200 Subject: [PATCH 2/3] add SDP22 channel assignment --- addons/intel/clock-chain.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/addons/intel/clock-chain.go b/addons/intel/clock-chain.go index 046d0ee2..52df707f 100644 --- a/addons/intel/clock-chain.go +++ b/addons/intel/clock-chain.go @@ -362,6 +362,11 @@ func (c *ClockChain) EnableE810Outputs() error { glog.Errorf("failed to set SMA2 pin via sysfs: %s", err) } } else { + // This is to assign channel 2 to SDP22. The period command below will fail without it + err = pinConfig.applyPinSet(c.LeadingNIC.Name, pinSet{"SDP22": "2 2"}) + if err != nil { + glog.Errorf("failed to set SDP22 pin via sysfs: %s", err) + } sma2Cmds := c.DpllPins.GetCommandsForPluginPinSet(c.LeadingNIC.DpllClockID, map[string]string{"SMA2": "2 2"}) err = c.DpllPins.ApplyPinCommands(sma2Cmds) if err != nil { From e0754828608e02273edf5aa7de4a3b2bbf589e48 Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Wed, 25 Mar 2026 10:51:38 +0200 Subject: [PATCH 3/3] fix unit tests for pin compatibility --- addons/intel/clock-chain_test.go | 7 ++++--- addons/intel/e810_test.go | 2 +- addons/intel/mock_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/addons/intel/clock-chain_test.go b/addons/intel/clock-chain_test.go index 446adb99..0aba0669 100644 --- a/addons/intel/clock-chain_test.go +++ b/addons/intel/clock-chain_test.go @@ -22,6 +22,7 @@ func Test_ProcessProfileTbcClockChain(t *testing.T) { // EnableE810Outputs is called for the leading NIC (ens4f0) - needs specific paths mockFS.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) mockFS.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + // mock applyPinSet does not hit the filesystem; only the period write from EnableE810Outputs is real. mockFS.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) mockPinConfig, restorePins := setupMockPinConfig() @@ -41,7 +42,7 @@ func Test_ProcessProfileTbcClockChain(t *testing.T) { assert.Equal(t, ClockTypeTBC, ccData.Type, "identified a wrong clock type") assert.Equal(t, uint64(5799633565432596414), ccData.LeadingNIC.DpllClockID, "identified a wrong clock ID ") assert.Equal(t, "ens4f1", ccData.LeadingNIC.UpstreamPort, "wrong upstream port") - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) assert.NotNil(t, mockPinSet.commands, "DPLL commands should have been issued") assert.Greater(t, len(mockPinSet.commands), 0, "should have DPLL pin commands") @@ -102,7 +103,7 @@ func Test_ProcessProfileTtscClockChain(t *testing.T) { assert.Equal(t, uint64(5799633565432596414), ccData.LeadingNIC.DpllClockID, "identified a wrong clock ID ") assert.Equal(t, "ens4f1", ccData.LeadingNIC.UpstreamPort, "wrong upstream port") assert.NotNil(t, mockPinSet.commands, "Ensure some pins were set") - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) // Test holdover entry @@ -146,7 +147,7 @@ func Test_SetPinDefaults_AllNICs(t *testing.T) { // Initialize the clock chain with multiple NICs err = OnPTPConfigChangeE810(nil, profile) assert.NoError(t, err) - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) // Verify we have the expected clock chain structure diff --git a/addons/intel/e810_test.go b/addons/intel/e810_test.go index 1f80758b..1237a4a1 100644 --- a/addons/intel/e810_test.go +++ b/addons/intel/e810_test.go @@ -127,7 +127,7 @@ func Test_ProcessProfileTBCNoPhaseInputs(t *testing.T) { err = p.OnPTPConfigChange(d, profile) assert.NoError(t, err) - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) // Verify that clockChain was initialized (SetPinDefaults is called as part of InitClockChain) diff --git a/addons/intel/mock_test.go b/addons/intel/mock_test.go index 0f13dd12..ddc3a142 100644 --- a/addons/intel/mock_test.go +++ b/addons/intel/mock_test.go @@ -362,6 +362,30 @@ func setupMockDPLLPins(pins ...*dpll.PinInfo) (*mockedDPLLPins, func()) { return mock, func() { DpllPins = orig } } +// expandPinsForPluginYAMLCompatibility adds in-memory PinInfo clones so tests can keep legacy +// plugin YAML keys (SMA2, U.FL1, U.FL2) while dpll-pins.json reports boardLabel "SMA2/U.FL2" +// and no separate U.FL1 DPLL pin (tests only; production JSON and YAML unchanged). +func expandPinsForPluginYAMLCompatibility(pins []*dpll.PinInfo) []*dpll.PinInfo { + out := make([]*dpll.PinInfo, 0, len(pins)+32) + out = append(out, pins...) + for _, p := range pins { + if p.BoardLabel == "SMA2/U.FL2" { + s2 := *p + s2.BoardLabel = "SMA2" + out = append(out, &s2) + u2 := *p + u2.BoardLabel = "U.FL2" + out = append(out, &u2) + } + if p.BoardLabel == "SMA1" && p.ModuleName == "ice" { + u1 := *p + u1.BoardLabel = "U.FL1" + out = append(out, &u1) + } + } + return out +} + func setupMockDPLLPinsFromJSON(path string) (*mockedDPLLPins, func()) { //nolint: unparam // it may be used for other pin files in the future it doesn't make the code overly complex pins := []dpll.PinInfo{} data, err := os.ReadFile(path) @@ -375,6 +399,7 @@ func setupMockDPLLPinsFromJSON(path string) (*mockedDPLLPins, func()) { //nolint for i := range pins { ptrs[i] = &pins[i] } + ptrs = expandPinsForPluginYAMLCompatibility(ptrs) return setupMockDPLLPins(ptrs...) }