-----Original Message----- From: Ido Schimmel idosch@idosch.org Sent: Wednesday, October 27, 2021 5:11 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; richardcochran@gmail.com; abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; kuba@kernel.org; linux-kselftest@vger.kernel.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com Subject: Re: [RFC v5 net-next 2/5] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status
On Wed, Oct 27, 2021 at 01:16:22PM +0000, Machnikowski, Maciej wrote:
-----Original Message----- From: Ido Schimmel idosch@idosch.org Sent: Wednesday, October 27, 2021 9:10 AM To: Machnikowski, Maciej maciej.machnikowski@intel.com Subject: Re: [RFC v5 net-next 2/5] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status
On Tue, Oct 26, 2021 at 07:31:43PM +0200, Maciej Machnikowski wrote:
+/* SyncE section */
+enum if_eec_state {
- IF_EEC_STATE_INVALID = 0,
- IF_EEC_STATE_FREERUN,
- IF_EEC_STATE_LOCKED,
- IF_EEC_STATE_LOCKED_HO_ACQ,
Is this referring to "Locked mode, acquiring holdover: This is a temporary mode, when coming from free-run, to acquire holdover memory." ?
Locked HO ACQ means locked and holdover acquired. It's the state that allows transferring to the holdover state. Locked means that we locked our frequency and started acquiring the holdover memory.
So that's a transient state, right? FWIW, I find it weird to call such a state "LOCKED".
It seems ice isn't using it, so maybe drop it? Can be added later in case we have a driver that can report it
I'll update the driver in the next revision
You mean update it to use "IF_EEC_STATE_LOCKED_HO_ACQ" instead of "IF_EEC_STATE_LOCKED"?
Rather report them separately - as there's a value in having info about both of them. LOCKED_HO_ACQ can be forced into forced holdover, while the LOCKED will revert to freerun.
Regardless, would be good to document these values.
Noted! :)
There is also "Locked mode, holdover acquired: This is a steady state mode, entered when holdover memory is acquired." But I assume that's "IF_EEC_STATE_LOCKED"
- IF_EEC_STATE_HOLDOVER,
+};