From: Ido Schimmel <idosch@idosch.org> To: Maciej Machnikowski <maciej.machnikowski@intel.com> Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, arkadiusz.kubalewski@intel.com, richardcochran@gmail.com, abyagowi@fb.com, 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, petrm@nvidia.com Subject: Re: [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature Date: Thu, 2 Dec 2021 14:43:39 +0200 [thread overview] Message-ID: <Yai/e5jz3NZAg0pm@shredder> (raw) In-Reply-To: <20211201180208.640179-3-maciej.machnikowski@intel.com> On Wed, Dec 01, 2021 at 07:02:06PM +0100, Maciej Machnikowski wrote: > +RCLK_GET > +======== > + > +Get status of an output pin for PHY recovered frequency clock. > + > +Request contents: > + > + ====================================== ====== ========================== > + ``ETHTOOL_A_RCLK_HEADER`` nested request header > + ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` u32 index of a pin > + ====================================== ====== ========================== > + > +Kernel response contents: > + > + ====================================== ====== ========================== > + ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` u32 index of a pin > + ``ETHTOOL_A_RCLK_PIN_FLAGS`` u32 state of a pin > + ``ETHTOOL_A_RCLK_RANGE_MIN_PIN`` u32 min index of RCLK pins > + ``ETHTOOL_A_RCLK_RANGE_MAX_PIN`` u32 max index of RCLK pins > + ====================================== ====== ========================== > + > +Supported device can have mulitple reference recover clock pins available s/mulitple/multiple/ > +to be used as source of frequency for a DPLL. > +Once a pin on given port is enabled. The PHY recovered frequency is being > +fed onto that pin, and can be used by DPLL to synchonize with its signal. s/synchonize/synchronize/ Please run a spell checker on documentation > +Pins don't have to start with index equal 0 - device can also have different > +external sources pins. > + > +The ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is optional parameter. If present in > +the RCLK_GET request, the ``ETHTOOL_A_RCLK_PIN_ENABLED`` is provided in a The `ETHTOOL_A_RCLK_PIN_ENABLED` attribute is no where to be found in this submission > +response, it contatins state of the pin pointed by the index. Values are: s/contatins/contains/ > + > +.. kernel-doc:: include/uapi/linux/ethtool.h > + :identifiers: ethtool_rclk_pin_state This structure is also no where to be found > + > +If ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is not present in the RCLK_GET request, > +the range of available pins is returned: > +``ETHTOOL_A_RCLK_RANGE_MIN_PIN`` is lowest possible index of a pin available > +for recovering frequency from PHY. > +``ETHTOOL_A_RCLK_RANGE_MAX_PIN`` is highest possible index of a pin available > +for recovering frequency from PHY. > + > +RCLK_SET > +========== > + > +Set status of an output pin for PHY recovered frequency clock. > + > +Request contents: > + > + ====================================== ====== ======================== > + ``ETHTOOL_A_RCLK_HEADER`` nested request header > + ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` u32 index of a pin > + ``ETHTOOL_A_RCLK_PIN_FLAGS`` u32 requested state > + ====================================== ====== ======================== > + > +``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is a index of a pin for which the change of > +state is requested. Values of ``ETHTOOL_A_RCLK_PIN_ENABLED`` are: > + > +.. kernel-doc:: include/uapi/linux/ethtool.h > + :identifiers: ethtool_rclk_pin_state Same. Looking at the diagram from the previous submission [1]: ┌──────────┬──────────┐ │ RX │ TX │ 1 │ ports │ ports │ 1 ───►├─────┐ │ ├─────► 2 │ │ │ │ 2 ───►├───┐ │ │ ├─────► 3 │ │ │ │ │ 3 ───►├─┐ │ │ │ ├─────► │ ▼ ▼ ▼ │ │ │ ────── │ │ │ \____/ │ │ └──┼──┼────┴──────────┘ 1│ 2│ ▲ RCLK out│ │ │ TX CLK in ▼ ▼ │ ┌─────────────┴───┐ │ │ │ SEC │ │ │ └─────────────────┘ Given a netdev (1, 2 or 3 in the diagram), the RCLK_SET message allows me to redirect the frequency recovered from this netdev to the EEC via either pin 1, pin 2 or both. Given a netdev, the RCLK_GET message allows me to query the range of pins (RCLK out 1-2 in the diagram) through which the frequency can be fed into the EEC. Questions: 1. The query for all the above netdevs will return the same range of pins. How does user space know that these are the same pins? That is, how does user space know that RCLK_SET message to redirect the frequency recovered from netdev 1 to pin 1 will be overridden by the same message but for netdev 2? 2. How does user space know the mapping between a netdev and an EEC? That is, how does user space know that RCLK_SET message for netdev 1 will cause the Tx frequency of netdev 2 to change according to the frequency recovered from netdev 1? 3. If user space sends two RCLK_SET messages to redirect the frequency recovered from netdev 1 to RCLK out 1 and from netdev 2 to RCLK out 2, how does it know which recovered frequency is actually used an input to the EEC? 4. Why these pins are represented as attributes of a netdev and not as attributes of the EEC? That is, why are they represented as output pins of the PHY as opposed to input pins of the EEC? 5. What is the problem with the following model? - The EEC is a separate object with following attributes: * State: Invalid / Freerun / Locked / etc * Sources: Netdev / external / etc * Potentially more - Notifications are emitted to user space when the state of the EEC changes. Drivers will either poll the state from the device or get interrupts - The mapping from netdev to EEC is queried via ethtool [1] https://lore.kernel.org/netdev/20211110114448.2792314-1-maciej.machnikowski@intel.com/
WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@idosch.org> To: intel-wired-lan@osuosl.org Subject: [Intel-wired-lan] [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature Date: Thu, 2 Dec 2021 14:43:39 +0200 [thread overview] Message-ID: <Yai/e5jz3NZAg0pm@shredder> (raw) In-Reply-To: <20211201180208.640179-3-maciej.machnikowski@intel.com> On Wed, Dec 01, 2021 at 07:02:06PM +0100, Maciej Machnikowski wrote: > +RCLK_GET > +======== > + > +Get status of an output pin for PHY recovered frequency clock. > + > +Request contents: > + > + ====================================== ====== ========================== > + ``ETHTOOL_A_RCLK_HEADER`` nested request header > + ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` u32 index of a pin > + ====================================== ====== ========================== > + > +Kernel response contents: > + > + ====================================== ====== ========================== > + ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` u32 index of a pin > + ``ETHTOOL_A_RCLK_PIN_FLAGS`` u32 state of a pin > + ``ETHTOOL_A_RCLK_RANGE_MIN_PIN`` u32 min index of RCLK pins > + ``ETHTOOL_A_RCLK_RANGE_MAX_PIN`` u32 max index of RCLK pins > + ====================================== ====== ========================== > + > +Supported device can have mulitple reference recover clock pins available s/mulitple/multiple/ > +to be used as source of frequency for a DPLL. > +Once a pin on given port is enabled. The PHY recovered frequency is being > +fed onto that pin, and can be used by DPLL to synchonize with its signal. s/synchonize/synchronize/ Please run a spell checker on documentation > +Pins don't have to start with index equal 0 - device can also have different > +external sources pins. > + > +The ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is optional parameter. If present in > +the RCLK_GET request, the ``ETHTOOL_A_RCLK_PIN_ENABLED`` is provided in a The `ETHTOOL_A_RCLK_PIN_ENABLED` attribute is no where to be found in this submission > +response, it contatins state of the pin pointed by the index. Values are: s/contatins/contains/ > + > +.. kernel-doc:: include/uapi/linux/ethtool.h > + :identifiers: ethtool_rclk_pin_state This structure is also no where to be found > + > +If ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is not present in the RCLK_GET request, > +the range of available pins is returned: > +``ETHTOOL_A_RCLK_RANGE_MIN_PIN`` is lowest possible index of a pin available > +for recovering frequency from PHY. > +``ETHTOOL_A_RCLK_RANGE_MAX_PIN`` is highest possible index of a pin available > +for recovering frequency from PHY. > + > +RCLK_SET > +========== > + > +Set status of an output pin for PHY recovered frequency clock. > + > +Request contents: > + > + ====================================== ====== ======================== > + ``ETHTOOL_A_RCLK_HEADER`` nested request header > + ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` u32 index of a pin > + ``ETHTOOL_A_RCLK_PIN_FLAGS`` u32 requested state > + ====================================== ====== ======================== > + > +``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is a index of a pin for which the change of > +state is requested. Values of ``ETHTOOL_A_RCLK_PIN_ENABLED`` are: > + > +.. kernel-doc:: include/uapi/linux/ethtool.h > + :identifiers: ethtool_rclk_pin_state Same. Looking at the diagram from the previous submission [1]: ??????????????????????? ? RX ? TX ? 1 ? ports ? ports ? 1 ??????????? ? ??????? 2 ? ? ? ? 2 ????????? ? ? ??????? 3 ? ? ? ? ? 3 ??????? ? ? ? ??????? ? ? ? ? ? ? ? ?????? ? ? ? \____/ ? ? ??????????????????????? 1? 2? ? RCLK out? ? ? TX CLK in ? ? ? ??????????????????? ? ? ? SEC ? ? ? ??????????????????? Given a netdev (1, 2 or 3 in the diagram), the RCLK_SET message allows me to redirect the frequency recovered from this netdev to the EEC via either pin 1, pin 2 or both. Given a netdev, the RCLK_GET message allows me to query the range of pins (RCLK out 1-2 in the diagram) through which the frequency can be fed into the EEC. Questions: 1. The query for all the above netdevs will return the same range of pins. How does user space know that these are the same pins? That is, how does user space know that RCLK_SET message to redirect the frequency recovered from netdev 1 to pin 1 will be overridden by the same message but for netdev 2? 2. How does user space know the mapping between a netdev and an EEC? That is, how does user space know that RCLK_SET message for netdev 1 will cause the Tx frequency of netdev 2 to change according to the frequency recovered from netdev 1? 3. If user space sends two RCLK_SET messages to redirect the frequency recovered from netdev 1 to RCLK out 1 and from netdev 2 to RCLK out 2, how does it know which recovered frequency is actually used an input to the EEC? 4. Why these pins are represented as attributes of a netdev and not as attributes of the EEC? That is, why are they represented as output pins of the PHY as opposed to input pins of the EEC? 5. What is the problem with the following model? - The EEC is a separate object with following attributes: * State: Invalid / Freerun / Locked / etc * Sources: Netdev / external / etc * Potentially more - Notifications are emitted to user space when the state of the EEC changes. Drivers will either poll the state from the device or get interrupts - The mapping from netdev to EEC is queried via ethtool [1] https://lore.kernel.org/netdev/20211110114448.2792314-1-maciej.machnikowski at intel.com/
next prev parent reply other threads:[~2021-12-02 12:43 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-01 18:02 [PATCH v4 net-next 0/4] Add ethtool interface for SyncE Maciej Machnikowski 2021-12-01 18:02 ` [Intel-wired-lan] " Maciej Machnikowski 2021-12-01 18:02 ` [PATCH v4 net-next 1/4] ice: add support detecting features based on netlist Maciej Machnikowski 2021-12-01 18:02 ` [Intel-wired-lan] " Maciej Machnikowski 2021-12-01 18:02 ` [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature Maciej Machnikowski 2021-12-01 18:02 ` [Intel-wired-lan] " Maciej Machnikowski 2021-12-02 11:48 ` kernel test robot 2021-12-02 11:48 ` kernel test robot 2021-12-02 11:48 ` [Intel-wired-lan] " kernel test robot 2021-12-02 12:43 ` Ido Schimmel [this message] 2021-12-02 12:43 ` Ido Schimmel 2021-12-02 15:17 ` Machnikowski, Maciej 2021-12-02 15:17 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-02 16:35 ` Ido Schimmel 2021-12-02 16:35 ` [Intel-wired-lan] " Ido Schimmel 2021-12-02 17:20 ` Machnikowski, Maciej 2021-12-02 17:20 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-03 14:26 ` Petr Machata 2021-12-03 14:26 ` [Intel-wired-lan] " Petr Machata 2021-12-03 14:55 ` Machnikowski, Maciej 2021-12-03 14:55 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-03 15:58 ` Ido Schimmel 2021-12-03 15:58 ` [Intel-wired-lan] " Ido Schimmel 2021-12-03 16:26 ` Petr Machata 2021-12-03 16:26 ` [Intel-wired-lan] " Petr Machata 2021-12-03 16:50 ` Machnikowski, Maciej 2021-12-03 16:50 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-03 18:44 ` Petr Machata 2021-12-03 18:44 ` [Intel-wired-lan] " Petr Machata 2021-12-03 19:07 ` Machnikowski, Maciej 2021-12-03 19:07 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-06 14:40 ` Petr Machata 2021-12-06 14:40 ` [Intel-wired-lan] " Petr Machata 2021-12-06 15:09 ` Machnikowski, Maciej 2021-12-06 15:09 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-06 16:00 ` Petr Machata 2021-12-06 16:00 ` [Intel-wired-lan] " Petr Machata 2021-12-06 18:49 ` Machnikowski, Maciej 2021-12-06 18:49 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-03 15:45 ` Ido Schimmel 2021-12-03 15:45 ` [Intel-wired-lan] " Ido Schimmel 2021-12-03 16:18 ` Machnikowski, Maciej 2021-12-03 16:18 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-03 18:21 ` Petr Machata 2021-12-03 18:21 ` [Intel-wired-lan] " Petr Machata 2021-12-05 12:24 ` Ido Schimmel 2021-12-05 12:24 ` [Intel-wired-lan] " Ido Schimmel 2021-12-06 9:15 ` Machnikowski, Maciej 2021-12-06 9:15 ` [Intel-wired-lan] " Machnikowski, Maciej 2021-12-01 18:02 ` [PATCH v4 net-next 3/4] ice: add support for monitoring SyncE DPLL state Maciej Machnikowski 2021-12-01 18:02 ` [Intel-wired-lan] " Maciej Machnikowski 2021-12-01 18:02 ` [PATCH v4 net-next 4/4] ice: add support for SyncE recovered clocks Maciej Machnikowski 2021-12-01 18:02 ` [Intel-wired-lan] " Maciej Machnikowski 2021-12-02 1:56 ` Jakub Kicinski 2021-12-02 1:56 ` [Intel-wired-lan] " Jakub Kicinski 2021-12-02 12:29 [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature kernel test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Yai/e5jz3NZAg0pm@shredder \ --to=idosch@idosch.org \ --cc=abyagowi@fb.com \ --cc=anthony.l.nguyen@intel.com \ --cc=arkadiusz.kubalewski@intel.com \ --cc=davem@davemloft.net \ --cc=intel-wired-lan@lists.osuosl.org \ --cc=kuba@kernel.org \ --cc=linux-kselftest@vger.kernel.org \ --cc=maciej.machnikowski@intel.com \ --cc=michael.chan@broadcom.com \ --cc=mkubecek@suse.cz \ --cc=netdev@vger.kernel.org \ --cc=petrm@nvidia.com \ --cc=richardcochran@gmail.com \ --cc=saeed@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.