Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
v5: - rewritten the documentation - fixed doxygen headers
v4: - Dropped EEC_STATE reporting (TBD: DPLL subsystem) - moved recovered clock configuration to ethtool netlink
v3: - remove RTM_GETRCLKRANGE - return state of all possible pins in the RTM_GETRCLKSTATE - clarify documentation
v2: - improved documentation - fixed kdoc warning
RFC history: v2: - removed whitespace changes - fix issues reported by test robot v3: - Changed naming from SyncE to EEC - Clarify cover letter and commit message for patch 1 v4: - Removed sync_source and pin_idx info - Changed one structure to attributes - Added EEC_SRC_PORT flag to indicate that the EEC is synchronized to the recovered clock of a port that returns the state v5: - add EEC source as an optiona attribute - implement support for recovered clocks - align states returned by EEC to ITU-T G.781 v6: - fix EEC clock state reporting - add documentation - fix descriptions in code comments
Maciej Machnikowski (4): ice: add support detecting features based on netlist ethtool: Add ability to configure recovered clock for SyncE feature ice: add support for monitoring SyncE DPLL state ice: add support for recovered clocks
Documentation/networking/ethtool-netlink.rst | 62 ++++ drivers/net/ethernet/intel/ice/ice.h | 7 + .../net/ethernet/intel/ice/ice_adminq_cmd.h | 70 ++++- drivers/net/ethernet/intel/ice/ice_common.c | 224 +++++++++++++++ drivers/net/ethernet/intel/ice/ice_common.h | 20 +- drivers/net/ethernet/intel/ice/ice_devids.h | 3 + drivers/net/ethernet/intel/ice/ice_ethtool.c | 96 +++++++ drivers/net/ethernet/intel/ice/ice_lib.c | 6 +- drivers/net/ethernet/intel/ice/ice_ptp.c | 35 +++ drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 49 ++++ drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 36 +++ drivers/net/ethernet/intel/ice/ice_type.h | 1 + include/linux/ethtool.h | 9 + include/uapi/linux/ethtool_netlink.h | 21 ++ net/ethtool/Makefile | 3 +- net/ethtool/netlink.c | 20 ++ net/ethtool/netlink.h | 4 + net/ethtool/synce.c | 267 ++++++++++++++++++ 18 files changed, 929 insertions(+), 4 deletions(-) create mode 100644 net/ethtool/synce.c
Add new functions to check netlist of a given board for: - Recovered Clock device, - Clock Generation Unit, - Clock Multiplexer,
Initialize feature bits depending on detected components.
Signed-off-by: Maciej Machnikowski maciej.machnikowski@intel.com --- drivers/net/ethernet/intel/ice/ice.h | 2 + .../net/ethernet/intel/ice/ice_adminq_cmd.h | 7 +- drivers/net/ethernet/intel/ice/ice_common.c | 123 ++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_common.h | 9 ++ drivers/net/ethernet/intel/ice/ice_lib.c | 6 +- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 1 + drivers/net/ethernet/intel/ice/ice_type.h | 1 + 7 files changed, 147 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index b67ad51cbcc9..cb6b4c53584b 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -183,6 +183,8 @@
enum ice_feature { ICE_F_DSCP, + ICE_F_CGU, + ICE_F_PHY_RCLK, ICE_F_SMA_CTRL, ICE_F_MAX }; diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h index 4eef3488d86f..339c2a86f680 100644 --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h @@ -1297,6 +1297,8 @@ struct ice_aqc_link_topo_params { #define ICE_AQC_LINK_TOPO_NODE_TYPE_CAGE 6 #define ICE_AQC_LINK_TOPO_NODE_TYPE_MEZZ 7 #define ICE_AQC_LINK_TOPO_NODE_TYPE_ID_EEPROM 8 +#define ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL 9 +#define ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX 10 #define ICE_AQC_LINK_TOPO_NODE_CTX_S 4 #define ICE_AQC_LINK_TOPO_NODE_CTX_M \ (0xF << ICE_AQC_LINK_TOPO_NODE_CTX_S) @@ -1333,7 +1335,10 @@ struct ice_aqc_link_topo_addr { struct ice_aqc_get_link_topo { struct ice_aqc_link_topo_addr addr; u8 node_part_num; -#define ICE_AQC_GET_LINK_TOPO_NODE_NR_PCA9575 0x21 +#define ICE_AQC_GET_LINK_TOPO_NODE_NR_PCA9575 0x21 +#define ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032 0x24 +#define ICE_ACQ_GET_LINK_TOPO_NODE_NR_PKVL 0x31 +#define ICE_ACQ_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX 0x47 u8 rsvd[9]; };
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c index b3066d0fea8b..35903b282885 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -274,6 +274,79 @@ ice_aq_get_link_topo_handle(struct ice_port_info *pi, u8 node_type, return ice_aq_send_cmd(pi->hw, &desc, NULL, 0, cd); }
+/** + * ice_aq_get_netlist_node + * @hw: pointer to the hw struct + * @cmd: get_link_topo AQ structure + * @node_part_number: output node part number if node found + * @node_handle: output node handle parameter if node found + */ +enum ice_status +ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo *cmd, + u8 *node_part_number, u16 *node_handle) +{ + struct ice_aq_desc desc; + + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_get_link_topo); + desc.params.get_link_topo = *cmd; + + if (ice_aq_send_cmd(hw, &desc, NULL, 0, NULL)) + return ICE_ERR_NOT_SUPPORTED; + + if (node_handle) + *node_handle = + le16_to_cpu(desc.params.get_link_topo.addr.handle); + if (node_part_number) + *node_part_number = desc.params.get_link_topo.node_part_num; + + return ICE_SUCCESS; +} + +#define MAX_NETLIST_SIZE 10 +/** + * ice_find_netlist_node + * @hw: pointer to the hw struct + * @node_type_ctx: type of netlist node to look for + * @node_part_number: node part number to look for + * @node_handle: output parameter if node found - optional + * + * Find and return the node handle for a given node type and part number in the + * netlist. When found ICE_SUCCESS is returned, ICE_ERR_DOES_NOT_EXIST + * otherwise. If @node_handle provided, it would be set to found node handle. + */ +enum ice_status +ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx, u8 node_part_number, + u16 *node_handle) +{ + struct ice_aqc_get_link_topo cmd; + u8 rec_node_part_number; + enum ice_status status; + u16 rec_node_handle; + u8 idx; + + for (idx = 0; idx < MAX_NETLIST_SIZE; idx++) { + memset(&cmd, 0, sizeof(cmd)); + + cmd.addr.topo_params.node_type_ctx = + (node_type_ctx << ICE_AQC_LINK_TOPO_NODE_TYPE_S); + cmd.addr.topo_params.index = idx; + + status = ice_aq_get_netlist_node(hw, &cmd, + &rec_node_part_number, + &rec_node_handle); + if (status) + return status; + + if (rec_node_part_number == node_part_number) { + if (node_handle) + *node_handle = rec_node_handle; + return ICE_SUCCESS; + } + } + + return ICE_ERR_DOES_NOT_EXIST; +} + /** * ice_is_media_cage_present * @pi: port information structure @@ -5083,3 +5156,53 @@ bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw) } return false; } + +/** + * ice_is_phy_rclk_present_e810t + * @hw: pointer to the hw struct + * + * Check if the PHY Recovered Clock device is present in the netlist + */ +bool ice_is_phy_rclk_present_e810t(struct ice_hw *hw) +{ + if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL, + ICE_ACQ_GET_LINK_TOPO_NODE_NR_PKVL, NULL)) + return false; + + return true; +} + +/** + * ice_is_cgu_present_e810t + * @hw: pointer to the hw struct + * + * Check if the Clock Generation Unit (CGU) device is present in the netlist + */ +bool ice_is_cgu_present_e810t(struct ice_hw *hw) +{ + if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL, + ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032, + NULL)) { + hw->cgu_part_number = + ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032; + return true; + } + return false; +} + +/** + * ice_is_clock_mux_present_e810t + * @hw: pointer to the hw struct + * + * Check if the Clock Multiplexer device is present in the netlist + */ +bool ice_is_clock_mux_present_e810t(struct ice_hw *hw) +{ + if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX, + ICE_ACQ_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX, + NULL)) + return false; + + return true; +} + diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h index 65c1b3244264..b20a5c085246 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.h +++ b/drivers/net/ethernet/intel/ice/ice_common.h @@ -89,6 +89,12 @@ ice_aq_get_phy_caps(struct ice_port_info *pi, bool qual_mods, u8 report_mode, struct ice_aqc_get_phy_caps_data *caps, struct ice_sq_cd *cd); enum ice_status +ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo *cmd, + u8 *node_part_number, u16 *node_handle); +enum ice_status +ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx, u8 node_part_number, + u16 *node_handle); +enum ice_status ice_aq_list_caps(struct ice_hw *hw, void *buf, u16 buf_size, u32 *cap_count, enum ice_adminq_opc opc, struct ice_sq_cd *cd); enum ice_status @@ -206,4 +212,7 @@ bool ice_fw_supports_lldp_fltr_ctrl(struct ice_hw *hw); enum ice_status ice_lldp_fltr_add_remove(struct ice_hw *hw, u16 vsi_num, bool add); bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw); +bool ice_is_phy_rclk_present_e810t(struct ice_hw *hw); +bool ice_is_cgu_present_e810t(struct ice_hw *hw); +bool ice_is_clock_mux_present_e810t(struct ice_hw *hw); #endif /* _ICE_COMMON_H_ */ diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index 09a3297cd63c..18c30b2912e3 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -4188,8 +4188,12 @@ void ice_init_feature_support(struct ice_pf *pf) case ICE_DEV_ID_E810C_QSFP: case ICE_DEV_ID_E810C_SFP: ice_set_feature_support(pf, ICE_F_DSCP); - if (ice_is_e810t(&pf->hw)) + if (ice_is_clock_mux_present_e810t(&pf->hw)) ice_set_feature_support(pf, ICE_F_SMA_CTRL); + if (ice_is_phy_rclk_present_e810t(&pf->hw)) + ice_set_feature_support(pf, ICE_F_PHY_RCLK); + if (ice_is_cgu_present_e810t(&pf->hw)) + ice_set_feature_support(pf, ICE_F_CGU); break; default: break; diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index 29f947c0cd2e..aa257db36765 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -800,3 +800,4 @@ bool ice_is_pca9575_present(struct ice_hw *hw)
return !status && handle; } + diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h index 9e0c2923c62e..a9dc16641bd4 100644 --- a/drivers/net/ethernet/intel/ice/ice_type.h +++ b/drivers/net/ethernet/intel/ice/ice_type.h @@ -920,6 +920,7 @@ struct ice_hw { struct list_head rss_list_head; struct ice_mbx_snapshot mbx_snapshot; u16 io_expander_handle; + u8 cgu_part_number; };
/* Statistics collected by each port, VSI, VEB, and S-channel */
Add netlink ethtool messages: - ETHTOOL_MSG_RCLK_GET - ETHTOOL_MSG_RCLK_SET Required for controling basic SyncE functionality - configuration of recovered reference clocks from PHY port on netdevice supporting SyncE.
- ETHTOOL_MSG_RCLK_GET - will read the current state if pin index is given or will return allowed range if it's not
- ETHTOOL_MSG_RCLK_SET - will configure given recovered clock pin to output recovered clock
Co-developed-by: Arkadiusz Kubalewski arkadiusz.kubalewski@intel.com Signed-off-by: Arkadiusz Kubalewski arkadiusz.kubalewski@intel.com Signed-off-by: Maciej Machnikowski maciej.machnikowski@intel.com --- Documentation/networking/ethtool-netlink.rst | 62 +++++ include/linux/ethtool.h | 9 + include/uapi/linux/ethtool_netlink.h | 21 ++ net/ethtool/Makefile | 3 +- net/ethtool/netlink.c | 20 ++ net/ethtool/netlink.h | 4 + net/ethtool/synce.c | 267 +++++++++++++++++++ 7 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 net/ethtool/synce.c
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index 9d98e0511249..6399cdb3fdca 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -220,6 +220,8 @@ Userspace to kernel: ``ETHTOOL_MSG_PHC_VCLOCKS_GET`` get PHC virtual clocks info ``ETHTOOL_MSG_MODULE_SET`` set transceiver module parameters ``ETHTOOL_MSG_MODULE_GET`` get transceiver module parameters + ``ETHTOOL_RCLK_GET`` get recovered clock parameters + ``ETHTOOL_RCLK_SET`` set recovered clock parameters ===================================== =================================
Kernel to userspace: @@ -260,6 +262,8 @@ Kernel to userspace: ``ETHTOOL_MSG_STATS_GET_REPLY`` standard statistics ``ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY`` PHC virtual clocks info ``ETHTOOL_MSG_MODULE_GET_REPLY`` transceiver module parameters + ``ETHTOOL_MSG_RCLK_GET_REPLY`` reference recovered clock config + ``ETHTOOL_MSG_RCLK_NTF`` reference recovered clock config ======================================== =================================
``GET`` requests are sent by userspace applications to retrieve device @@ -1598,6 +1602,62 @@ For SFF-8636 modules, low power mode is forced by the host according to table For CMIS modules, low power mode is forced by the host according to table 6-12 in revision 5.0 of the specification.
+RCLK_GET +======== + +Get status of an output pin for PHY recovered frequency clock signal. + +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 + ====================================== ====== ========================== + +A device can support multiple recovered clock output pins. They typically are +connected to the EEC and are used as a frequency reference. +When a given pin on a given port is enabled, the PHY recovered frequency is +fed onto that pin. +Pins can be freely indexed and don't have to start with index 0. + +The ``ETHTOOL_A_RCLK_OUT_PIN_IDX`` is optional parameter. If present in +the RCLK_GET request, the ``ETHTOOL_A_RCLK_PIN_FLAGS`` will be returned in a +response, containing the state of the pin pointed by the index. + +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 the lowest recovered clock pin index. +``ETHTOOL_A_RCLK_RANGE_MAX_PIN`` is the highest recovered clock pin index. + +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 the index of the pin for which a change of +state is requested. +``ETHTOOL_A_RCLK_PIN_FLAGS`` if the ``ETHTOOL_RCLK_PIN_FLAGS_ENA`` flag +is set, the given output pin will be enabled, if not - the pin will be disabled + + + Request translation ===================
@@ -1699,4 +1759,6 @@ are netlink only. n/a ``ETHTOOL_MSG_PHC_VCLOCKS_GET`` n/a ``ETHTOOL_MSG_MODULE_GET`` n/a ``ETHTOOL_MSG_MODULE_SET`` + n/a ``ETHTOOL_MSG_RCLK_GET`` + n/a ``ETHTOOL_MSG_RCLK_SET`` =================================== ===================================== diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index a26f37a27167..e344e5153f9b 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -614,6 +614,9 @@ struct ethtool_module_power_mode_params { * plugged-in. * @set_module_power_mode: Set the power mode policy for the plug-in module * used by the network device. + * @get_rclk_range: Get range of valid Reference Clock input pins for the port. + * @get_rclk_state: Get state of Reference Clock input signal pin. + * @set_rclk_out: Enable/disable Reference Clock input signal pin. * * All operations are optional (i.e. the function pointer may be set * to %NULL) and callers must take this into account. Callers must @@ -750,6 +753,12 @@ struct ethtool_ops { int (*set_module_power_mode)(struct net_device *dev, const struct ethtool_module_power_mode_params *params, struct netlink_ext_ack *extack); + int (*get_rclk_range)(struct net_device *dev, u32 *min_idx, + u32 *max_idx, struct netlink_ext_ack *extack); + int (*get_rclk_state)(struct net_device *dev, u32 out_idx, + bool *ena, struct netlink_ext_ack *extack); + int (*set_rclk_out)(struct net_device *dev, u32 out_idx, bool ena, + struct netlink_ext_ack *extack); };
int ethtool_check_ops(const struct ethtool_ops *ops); diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index cca6e474a085..9a860f052a36 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -49,6 +49,8 @@ enum { ETHTOOL_MSG_PHC_VCLOCKS_GET, ETHTOOL_MSG_MODULE_GET, ETHTOOL_MSG_MODULE_SET, + ETHTOOL_MSG_RCLK_GET, + ETHTOOL_MSG_RCLK_SET,
/* add new constants above here */ __ETHTOOL_MSG_USER_CNT, @@ -94,6 +96,8 @@ enum { ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY, ETHTOOL_MSG_MODULE_GET_REPLY, ETHTOOL_MSG_MODULE_NTF, + ETHTOOL_MSG_RCLK_GET_REPLY, + ETHTOOL_MSG_RCLK_NTF,
/* add new constants above here */ __ETHTOOL_MSG_KERNEL_CNT, @@ -853,6 +857,23 @@ enum { ETHTOOL_A_MODULE_MAX = (__ETHTOOL_A_MODULE_CNT - 1) };
+/* REF CLK */ + +enum { + ETHTOOL_A_RCLK_UNSPEC, + ETHTOOL_A_RCLK_HEADER, /* nest - _A_HEADER_* */ + ETHTOOL_A_RCLK_OUT_PIN_IDX, /* u32 */ + ETHTOOL_A_RCLK_PIN_FLAGS, /* u32 */ + ETHTOOL_A_RCLK_PIN_MIN, /* u32 */ + ETHTOOL_A_RCLK_PIN_MAX, /* u32 */ + + /* add new constants above here */ + __ETHTOOL_A_RCLK_CNT, + ETHTOOL_A_RCLK_MAX = (__ETHTOOL_A_RCLK_CNT - 1) +}; + +#define ETHTOOL_RCLK_PIN_FLAGS_ENA (1 << 0) + /* generic netlink info */ #define ETHTOOL_GENL_NAME "ethtool" #define ETHTOOL_GENL_VERSION 1 diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile index b76432e70e6b..dd6de311a9c2 100644 --- a/net/ethtool/Makefile +++ b/net/ethtool/Makefile @@ -7,4 +7,5 @@ obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o \ linkstate.o debug.o wol.o features.o privflags.o rings.o \ channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \ - tunnels.o fec.o eeprom.o stats.o phc_vclocks.o module.o + tunnels.o fec.o eeprom.o stats.o phc_vclocks.o module.o \ + synce.o diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 23f32a995099..a88e4e91d015 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -285,6 +285,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = { [ETHTOOL_MSG_STATS_GET] = ðnl_stats_request_ops, [ETHTOOL_MSG_PHC_VCLOCKS_GET] = ðnl_phc_vclocks_request_ops, [ETHTOOL_MSG_MODULE_GET] = ðnl_module_request_ops, + [ETHTOOL_MSG_RCLK_GET] = ðnl_rclk_request_ops, };
static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb) @@ -597,6 +598,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = { [ETHTOOL_MSG_EEE_NTF] = ðnl_eee_request_ops, [ETHTOOL_MSG_FEC_NTF] = ðnl_fec_request_ops, [ETHTOOL_MSG_MODULE_NTF] = ðnl_module_request_ops, + [ETHTOOL_MSG_RCLK_NTF] = ðnl_rclk_request_ops, };
/* default notification handler */ @@ -692,6 +694,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = { [ETHTOOL_MSG_EEE_NTF] = ethnl_default_notify, [ETHTOOL_MSG_FEC_NTF] = ethnl_default_notify, [ETHTOOL_MSG_MODULE_NTF] = ethnl_default_notify, + [ETHTOOL_MSG_RCLK_NTF] = ethnl_default_notify, };
void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data) @@ -1021,6 +1024,23 @@ static const struct genl_ops ethtool_genl_ops[] = { .policy = ethnl_module_set_policy, .maxattr = ARRAY_SIZE(ethnl_module_set_policy) - 1, }, + { + .cmd = ETHTOOL_MSG_RCLK_GET, + .flags = GENL_UNS_ADMIN_PERM, + .doit = ethnl_default_doit, + .start = ethnl_default_start, + .dumpit = ethnl_default_dumpit, + .done = ethnl_default_done, + .policy = ethnl_rclk_get_policy, + .maxattr = ARRAY_SIZE(ethnl_rclk_get_policy) - 1, + }, + { + .cmd = ETHTOOL_MSG_RCLK_SET, + .flags = GENL_UNS_ADMIN_PERM, + .doit = ethnl_set_rclk, + .policy = ethnl_rclk_set_policy, + .maxattr = ARRAY_SIZE(ethnl_rclk_set_policy) - 1, + }, };
static const struct genl_multicast_group ethtool_nl_mcgrps[] = { diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index a779bbb0c524..cfc7c45513b1 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -340,6 +340,7 @@ extern const struct ethnl_request_ops ethnl_module_eeprom_request_ops; extern const struct ethnl_request_ops ethnl_stats_request_ops; extern const struct ethnl_request_ops ethnl_phc_vclocks_request_ops; extern const struct ethnl_request_ops ethnl_module_request_ops; +extern const struct ethnl_request_ops ethnl_rclk_request_ops;
extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1]; extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1]; @@ -378,6 +379,8 @@ extern const struct nla_policy ethnl_stats_get_policy[ETHTOOL_A_STATS_GROUPS + 1 extern const struct nla_policy ethnl_phc_vclocks_get_policy[ETHTOOL_A_PHC_VCLOCKS_HEADER + 1]; extern const struct nla_policy ethnl_module_get_policy[ETHTOOL_A_MODULE_HEADER + 1]; extern const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MODE_POLICY + 1]; +extern const struct nla_policy ethnl_rclk_get_policy[ETHTOOL_A_RCLK_OUT_PIN_IDX + 1]; +extern const struct nla_policy ethnl_rclk_set_policy[ETHTOOL_A_RCLK_PIN_FLAGS + 1];
int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info); int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info); @@ -397,6 +400,7 @@ int ethnl_tunnel_info_start(struct netlink_callback *cb); int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb); int ethnl_set_fec(struct sk_buff *skb, struct genl_info *info); int ethnl_set_module(struct sk_buff *skb, struct genl_info *info); +int ethnl_set_rclk(struct sk_buff *skb, struct genl_info *info);
extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN]; extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN]; diff --git a/net/ethtool/synce.c b/net/ethtool/synce.c new file mode 100644 index 000000000000..f4ebb4c57d4d --- /dev/null +++ b/net/ethtool/synce.c @@ -0,0 +1,267 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/ethtool.h> +#include "netlink.h" + +struct rclk_out_pin_info { + u32 idx; + bool valid; +}; + +struct rclk_request_data { + struct ethnl_req_info base; + struct rclk_out_pin_info out_pin; +}; + +struct rclk_pin_state_info { + u32 range_min; + u32 range_max; + u32 flags; + u32 idx; +}; + +struct rclk_reply_data { + struct ethnl_reply_data base; + struct rclk_pin_state_info pin_state; +}; + +#define RCLK_REPDATA(__reply_base) \ + container_of(__reply_base, struct rclk_reply_data, base) + +#define RCLK_REQDATA(__req_base) \ + container_of(__req_base, struct rclk_request_data, base) + +/* RCLK_GET */ + +const struct nla_policy +ethnl_rclk_get_policy[ETHTOOL_A_RCLK_OUT_PIN_IDX + 1] = { + [ETHTOOL_A_RCLK_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy), + [ETHTOOL_A_RCLK_OUT_PIN_IDX] = { .type = NLA_U32 }, +}; + +static int rclk_parse_request(struct ethnl_req_info *req_base, + struct nlattr **tb, + struct netlink_ext_ack *extack) +{ + struct rclk_request_data *req = RCLK_REQDATA(req_base); + + if (tb[ETHTOOL_A_RCLK_OUT_PIN_IDX]) { + req->out_pin.idx = nla_get_u32(tb[ETHTOOL_A_RCLK_OUT_PIN_IDX]); + req->out_pin.valid = true; + } + + return 0; +} + +static int rclk_state_get(struct net_device *dev, + struct rclk_reply_data *data, + struct netlink_ext_ack *extack, + u32 out_idx) +{ + const struct ethtool_ops *ops = dev->ethtool_ops; + bool pin_state; + int ret; + + if (!ops->get_rclk_state) + return -EOPNOTSUPP; + + ret = ops->get_rclk_state(dev, out_idx, &pin_state, extack); + if (ret) + return ret; + + data->pin_state.flags = pin_state ? ETHTOOL_RCLK_PIN_FLAGS_ENA : 0; + data->pin_state.idx = out_idx; + + return ret; +} + +static int rclk_range_get(struct net_device *dev, + struct rclk_reply_data *data, + struct netlink_ext_ack *extack) +{ + const struct ethtool_ops *ops = dev->ethtool_ops; + u32 min_idx, max_idx; + int ret; + + if (!ops->get_rclk_range) + return -EOPNOTSUPP; + + ret = ops->get_rclk_range(dev, &min_idx, &max_idx, extack); + if (ret) + return ret; + + data->pin_state.range_min = min_idx; + data->pin_state.range_max = max_idx; + + return ret; +} + +static int rclk_prepare_data(const struct ethnl_req_info *req_base, + struct ethnl_reply_data *reply_base, + struct genl_info *info) +{ + struct rclk_reply_data *reply = RCLK_REPDATA(reply_base); + struct rclk_request_data *request = RCLK_REQDATA(req_base); + struct netlink_ext_ack *extack = info ? info->extack : NULL; + struct net_device *dev = reply_base->dev; + int ret; + + memset(&reply->pin_state, 0, sizeof(reply->pin_state)); + ret = ethnl_ops_begin(dev); + if (ret < 0) + return ret; + + if (request->out_pin.valid) + ret = rclk_state_get(dev, reply, extack, + request->out_pin.idx); + else + ret = rclk_range_get(dev, reply, extack); + + ethnl_ops_complete(dev); + + return ret; +} + +static int rclk_fill_reply(struct sk_buff *skb, + const struct ethnl_req_info *req_base, + const struct ethnl_reply_data *reply_base) +{ + const struct rclk_reply_data *reply = RCLK_REPDATA(reply_base); + const struct rclk_request_data *request = RCLK_REQDATA(req_base); + + if (request->out_pin.valid) { + if (nla_put_u32(skb, ETHTOOL_A_RCLK_PIN_FLAGS, + reply->pin_state.flags)) + return -EMSGSIZE; + if (nla_put_u32(skb, ETHTOOL_A_RCLK_OUT_PIN_IDX, + reply->pin_state.idx)) + return -EMSGSIZE; + } else { + if (nla_put_u32(skb, ETHTOOL_A_RCLK_PIN_MIN, + reply->pin_state.range_min)) + return -EMSGSIZE; + if (nla_put_u32(skb, ETHTOOL_A_RCLK_PIN_MAX, + reply->pin_state.range_max)) + return -EMSGSIZE; + } + + return 0; +} + +static int rclk_reply_size(const struct ethnl_req_info *req_base, + const struct ethnl_reply_data *reply_base) +{ + const struct rclk_request_data *request = RCLK_REQDATA(req_base); + + if (request->out_pin.valid) + return nla_total_size(sizeof(u32)) + /* ETHTOOL_A_RCLK_PIN_FLAGS */ + nla_total_size(sizeof(u32)); /* ETHTOOL_A_RCLK_OUT_PIN_IDX */ + else + return nla_total_size(sizeof(u32)) + /* ETHTOOL_A_RCLK_PIN_MIN */ + nla_total_size(sizeof(u32)); /* ETHTOOL_A_RCLK_PIN_MAX */ +} + +const struct ethnl_request_ops ethnl_rclk_request_ops = { + .request_cmd = ETHTOOL_MSG_RCLK_GET, + .reply_cmd = ETHTOOL_MSG_RCLK_GET_REPLY, + .hdr_attr = ETHTOOL_A_RCLK_HEADER, + .req_info_size = sizeof(struct rclk_request_data), + .reply_data_size = sizeof(struct rclk_reply_data), + + .parse_request = rclk_parse_request, + .prepare_data = rclk_prepare_data, + .reply_size = rclk_reply_size, + .fill_reply = rclk_fill_reply, +}; + +/* RCLK SET */ + +const struct nla_policy +ethnl_rclk_set_policy[ETHTOOL_A_RCLK_PIN_FLAGS + 1] = { + [ETHTOOL_A_RCLK_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy), + [ETHTOOL_A_RCLK_OUT_PIN_IDX] = { .type = NLA_U32 }, + [ETHTOOL_A_RCLK_PIN_FLAGS] = { .type = NLA_U32 }, +}; + +static int rclk_set_state(struct net_device *dev, struct nlattr **tb, + bool *p_mod, struct netlink_ext_ack *extack) +{ + const struct ethtool_ops *ops = dev->ethtool_ops; + bool old_state, new_state; + u32 min_idx, max_idx; + u32 out_idx; + int ret; + + if (!tb[ETHTOOL_A_RCLK_PIN_FLAGS] && + !tb[ETHTOOL_A_RCLK_OUT_PIN_IDX]) + return 0; + + if (!ops->set_rclk_out || !ops->get_rclk_range) { + NL_SET_ERR_MSG_ATTR(extack, + tb[ETHTOOL_A_RCLK_PIN_FLAGS], + "Setting recovered clock state is not supported by this device"); + return -EOPNOTSUPP; + } + + ret = ops->get_rclk_range(dev, &min_idx, &max_idx, extack); + if (ret) + return ret; + + out_idx = nla_get_u32(tb[ETHTOOL_A_RCLK_OUT_PIN_IDX]); + if (out_idx < min_idx || out_idx > max_idx) { + NL_SET_ERR_MSG_ATTR(extack, + tb[ETHTOOL_A_RCLK_OUT_PIN_IDX], + "Requested recovered clock pin index is out of range"); + return -EINVAL; + } + + ret = ops->get_rclk_state(dev, out_idx, &old_state, extack); + if (ret < 0) + return ret; + + new_state = !!(nla_get_u32(tb[ETHTOOL_A_RCLK_PIN_FLAGS]) & + ETHTOOL_RCLK_PIN_FLAGS_ENA); + + /* If state changed - flag need for sending the notification */ + *p_mod = old_state != new_state; + + return ops->set_rclk_out(dev, out_idx, new_state, extack); +} + +int ethnl_set_rclk(struct sk_buff *skb, struct genl_info *info) +{ + struct ethnl_req_info req_info = {}; + struct nlattr **tb = info->attrs; + struct net_device *dev; + bool mod = false; + int ret; + + ret = ethnl_parse_header_dev_get(&req_info, tb[ETHTOOL_A_RCLK_HEADER], + genl_info_net(info), info->extack, + true); + if (ret < 0) + return ret; + dev = req_info.dev; + + rtnl_lock(); + ret = ethnl_ops_begin(dev); + if (ret < 0) + goto out_rtnl; + + ret = rclk_set_state(dev, tb, &mod, info->extack); + if (ret < 0) + goto out_ops; + + if (!mod) + goto out_ops; + + ethtool_notify(dev, ETHTOOL_MSG_RCLK_NTF, NULL); + +out_ops: + ethnl_ops_complete(dev); +out_rtnl: + rtnl_unlock(); + dev_put(dev); + return ret; +} +
Implement SyncE DPLL monitoring for E810-T devices. Poll loop will periodically check the state of the DPLL and cache it in the pf structure. State changes will be logged in the system log.
Signed-off-by: Maciej Machnikowski maciej.machnikowski@intel.com --- drivers/net/ethernet/intel/ice/ice.h | 5 ++ .../net/ethernet/intel/ice/ice_adminq_cmd.h | 34 +++++++++++++ drivers/net/ethernet/intel/ice/ice_common.c | 36 ++++++++++++++ drivers/net/ethernet/intel/ice/ice_common.h | 5 +- drivers/net/ethernet/intel/ice/ice_devids.h | 3 ++ drivers/net/ethernet/intel/ice/ice_ptp.c | 35 ++++++++++++++ drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 48 +++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 34 +++++++++++++ 8 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index cb6b4c53584b..2dcc8fd6dff5 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -607,6 +607,11 @@ struct ice_pf { #define ICE_VF_AGG_NODE_ID_START 65 #define ICE_MAX_VF_AGG_NODES 32 struct ice_agg_node vf_agg_node[ICE_MAX_VF_AGG_NODES]; + + enum ice_eec_state synce_dpll_state; + u8 synce_dpll_pin; + enum ice_eec_state ptp_dpll_state; + u8 ptp_dpll_pin; };
struct ice_netdev_priv { diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h index 339c2a86f680..11226af7a9a4 100644 --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h @@ -1808,6 +1808,36 @@ struct ice_aqc_add_rdma_qset_data { struct ice_aqc_add_tx_rdma_qset_entry rdma_qsets[]; };
+/* Get CGU DPLL status (direct 0x0C66) */ +struct ice_aqc_get_cgu_dpll_status { + u8 dpll_num; + u8 ref_state; +#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_LOS BIT(0) +#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_SCM BIT(1) +#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_CFM BIT(2) +#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_GST BIT(3) +#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_PFM BIT(4) +#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_ESYNC BIT(6) +#define ICE_AQC_GET_CGU_DPLL_STATUS_FAST_LOCK_EN BIT(7) + __le16 dpll_state; +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_LOCK BIT(0) +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO BIT(1) +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO_READY BIT(2) +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_FLHIT BIT(5) +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_PSLHIT BIT(7) +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SHIFT 8 +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SEL \ + ICE_M(0x1F, ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SHIFT) +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE_SHIFT 13 +#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE \ + ICE_M(0x7, ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE_SHIFT) + __le32 phase_offset_h; + __le32 phase_offset_l; + u8 eec_mode; + u8 rsvd[1]; + __le16 node_handle; +}; + /* Configure Firmware Logging Command (indirect 0xFF09) * Logging Information Read Response (indirect 0xFF10) * Note: The 0xFF10 command has no input parameters. @@ -2039,6 +2069,7 @@ struct ice_aq_desc { struct ice_aqc_fw_logging fw_logging; struct ice_aqc_get_clear_fw_log get_clear_fw_log; struct ice_aqc_download_pkg download_pkg; + struct ice_aqc_get_cgu_dpll_status get_cgu_dpll_status; struct ice_aqc_driver_shared_params drv_shared_params; struct ice_aqc_set_mac_lb set_mac_lb; struct ice_aqc_alloc_free_res_cmd sw_res_ctrl; @@ -2205,6 +2236,9 @@ enum ice_adminq_opc { ice_aqc_opc_update_pkg = 0x0C42, ice_aqc_opc_get_pkg_info_list = 0x0C43,
+ /* 1588/SyncE commands/events */ + ice_aqc_opc_get_cgu_dpll_status = 0x0C66, + ice_aqc_opc_driver_shared_params = 0x0C90,
/* Standalone Commands/Events */ diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c index 35903b282885..8069141ac105 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -4644,6 +4644,42 @@ ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid, return ice_status_to_errno(status); }
+/** + * ice_aq_get_cgu_dpll_status + * @hw: pointer to the HW struct + * @dpll_num: DPLL index + * @ref_state: Reference clock state + * @dpll_state: DPLL state + * @phase_offset: Phase offset in ps + * @eec_mode: EEC_mode + * + * Get CGU DPLL status (0x0C66) + */ +enum ice_status +ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state, + u16 *dpll_state, u64 *phase_offset, u8 *eec_mode) +{ + struct ice_aqc_get_cgu_dpll_status *cmd; + struct ice_aq_desc desc; + enum ice_status status; + + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_get_cgu_dpll_status); + cmd = &desc.params.get_cgu_dpll_status; + cmd->dpll_num = dpll_num; + + status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL); + if (!status) { + *ref_state = cmd->ref_state; + *dpll_state = le16_to_cpu(cmd->dpll_state); + *phase_offset = le32_to_cpu(cmd->phase_offset_h); + *phase_offset <<= 32; + *phase_offset += le32_to_cpu(cmd->phase_offset_l); + *eec_mode = cmd->eec_mode; + } + + return status; +} + /** * ice_replay_pre_init - replay pre initialization * @hw: pointer to the HW struct diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h index b20a5c085246..aaed388a40a8 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.h +++ b/drivers/net/ethernet/intel/ice/ice_common.h @@ -106,6 +106,7 @@ enum ice_status ice_aq_manage_mac_write(struct ice_hw *hw, const u8 *mac_addr, u8 flags, struct ice_sq_cd *cd); bool ice_is_e810(struct ice_hw *hw); +bool ice_is_e810t(struct ice_hw *hw); enum ice_status ice_clear_pf_cfg(struct ice_hw *hw); enum ice_status ice_aq_set_phy_cfg(struct ice_hw *hw, struct ice_port_info *pi, @@ -162,6 +163,9 @@ ice_cfg_vsi_rdma(struct ice_port_info *pi, u16 vsi_handle, u16 tc_bitmap, int ice_ena_vsi_rdma_qset(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u16 *rdma_qset, u16 num_qsets, u32 *qset_teid); +enum ice_status +ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state, + u16 *dpll_state, u64 *phase_offset, u8 *eec_mode); int ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid, u16 *q_id); @@ -189,7 +193,6 @@ ice_stat_update40(struct ice_hw *hw, u32 reg, bool prev_stat_loaded, void ice_stat_update32(struct ice_hw *hw, u32 reg, bool prev_stat_loaded, u64 *prev_stat, u64 *cur_stat); -bool ice_is_e810t(struct ice_hw *hw); enum ice_status ice_sched_query_elem(struct ice_hw *hw, u32 node_teid, struct ice_aqc_txsched_elem_data *buf); diff --git a/drivers/net/ethernet/intel/ice/ice_devids.h b/drivers/net/ethernet/intel/ice/ice_devids.h index 61dd2f18dee8..0b654d417d29 100644 --- a/drivers/net/ethernet/intel/ice/ice_devids.h +++ b/drivers/net/ethernet/intel/ice/ice_devids.h @@ -58,4 +58,7 @@ /* Intel(R) Ethernet Connection E822-L 1GbE */ #define ICE_DEV_ID_E822L_SGMII 0x189A
+#define ICE_SUBDEV_ID_E810T 0x000E +#define ICE_SUBDEV_ID_E810T2 0x000F + #endif /* _ICE_DEVIDS_H_ */ diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index bf7247c6f58e..bb502c19d53a 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -1766,6 +1766,36 @@ static void ice_ptp_tx_tstamp_cleanup(struct ice_ptp_tx *tx) } }
+static void ice_handle_cgu_state(struct ice_pf *pf) +{ + enum ice_eec_state cgu_state; + u8 pin; + + cgu_state = ice_get_zl_dpll_state(&pf->hw, ICE_CGU_DPLL_SYNCE, &pin); + if (pf->synce_dpll_state != cgu_state) { + pf->synce_dpll_state = cgu_state; + pf->synce_dpll_pin = pin; + + dev_warn(ice_pf_to_dev(pf), + "<DPLL%i> state changed to: %d, pin %d", + ICE_CGU_DPLL_SYNCE, + pf->synce_dpll_state, + pin); + } + + cgu_state = ice_get_zl_dpll_state(&pf->hw, ICE_CGU_DPLL_PTP, &pin); + if (pf->ptp_dpll_state != cgu_state) { + pf->ptp_dpll_state = cgu_state; + pf->ptp_dpll_pin = pin; + + dev_warn(ice_pf_to_dev(pf), + "<DPLL%i> state changed to: %d, pin %d", + ICE_CGU_DPLL_PTP, + pf->ptp_dpll_state, + pin); + } +} + static void ice_ptp_periodic_work(struct kthread_work *work) { struct ice_ptp *ptp = container_of(work, struct ice_ptp, work.work); @@ -1774,6 +1804,10 @@ static void ice_ptp_periodic_work(struct kthread_work *work) if (!test_bit(ICE_FLAG_PTP, pf->flags)) return;
+ if (ice_is_feature_supported(pf, ICE_F_CGU) && + pf->hw.func_caps.ts_func_info.src_tmr_owned) + ice_handle_cgu_state(pf); + ice_ptp_update_cached_phctime(pf);
ice_ptp_tx_tstamp_cleanup(&pf->ptp.port.tx); @@ -1958,3 +1992,4 @@ void ice_ptp_release(struct ice_pf *pf)
dev_info(ice_pf_to_dev(pf), "Removed PTP clock\n"); } + diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index aa257db36765..b4300bf3e4ce 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -375,6 +375,54 @@ static int ice_ptp_port_cmd_e810(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd) return 0; }
+/** + * ice_get_zl_dpll_state - get the state of the DPLL + * @hw: pointer to the hw struct + * @dpll_idx: Index of internal DPLL unit + * @pin: pointer to a buffer for returning currently active pin + * + * This function will read the state of the DPLL(dpll_idx). If optional + * parameter pin is given it'll be used to retrieve currently active pin. + * + * Return: state of the DPLL + */ +enum ice_eec_state +ice_get_zl_dpll_state(struct ice_hw *hw, u8 dpll_idx, u8 *pin) +{ + enum ice_status status; + u64 phase_offset; + u16 dpll_state; + u8 ref_state; + u8 eec_mode; + + if (dpll_idx >= ICE_CGU_DPLL_MAX) + return ICE_EEC_STATE_INVALID; + + status = ice_aq_get_cgu_dpll_status(hw, dpll_idx, &ref_state, + &dpll_state, &phase_offset, + &eec_mode); + if (status) + return ICE_EEC_STATE_INVALID; + + if (pin) { + /* current ref pin in dpll_state_refsel_status_X register */ + *pin = (dpll_state & + ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SEL) >> + ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SHIFT; + } + + if (dpll_state & ICE_AQC_GET_CGU_DPLL_STATUS_STATE_LOCK) { + if (dpll_state & ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO_READY) + return ICE_EEC_STATE_LOCKED_HO_ACQ; + else + return ICE_EEC_STATE_LOCKED; + } else if ((dpll_state & ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO) && + (dpll_state & ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO_READY)) { + return ICE_EEC_STATE_HOLDOVER; + } + return ICE_EEC_STATE_FREERUN; +} + /* Device agnostic functions * * The following functions implement useful behavior to hide the differences diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index b2984b5c22c1..28b04ec40bae 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -12,6 +12,18 @@ enum ice_ptp_tmr_cmd { READ_TIME };
+enum ice_eec_state { + ICE_EEC_STATE_INVALID = 0, /* state is not valid */ + ICE_EEC_STATE_FREERUN, /* clock is free-running */ + ICE_EEC_STATE_LOCKED, /* clock is locked to the reference, + * but the holdover memory is not valid + */ + ICE_EEC_STATE_LOCKED_HO_ACQ, /* clock is locked to the reference + * and holdover memory is valid + */ + ICE_EEC_STATE_HOLDOVER, /* clock is in holdover mode */ +}; + /* Increment value to generate nanoseconds in the GLTSYN_TIME_L register for * the E810 devices. Based off of a PLL with an 812.5 MHz frequency. */ @@ -33,6 +45,8 @@ int ice_ptp_init_phy_e810(struct ice_hw *hw); int ice_read_sma_ctrl_e810t(struct ice_hw *hw, u8 *data); int ice_write_sma_ctrl_e810t(struct ice_hw *hw, u8 data); bool ice_is_pca9575_present(struct ice_hw *hw); +enum ice_eec_state +ice_get_zl_dpll_state(struct ice_hw *hw, u8 dpll_idx, u8 *pin);
#define PFTSYN_SEM_BYTES 4
@@ -98,4 +112,24 @@ bool ice_is_pca9575_present(struct ice_hw *hw); #define ICE_SMA_MAX_BIT_E810T 7 #define ICE_PCA9575_P1_OFFSET 8
+enum ice_e810t_cgu_dpll { + ICE_CGU_DPLL_SYNCE, + ICE_CGU_DPLL_PTP, + ICE_CGU_DPLL_MAX +}; + +enum ice_e810t_cgu_pins { + REF0P, + REF0N, + REF1P, + REF1N, + REF2P, + REF2N, + REF3P, + REF3N, + REF4P, + REF4N, + NUM_E810T_CGU_PINS +}; + #endif /* _ICE_PTP_HW_H_ */
Implement ethtool netlink functions for handling recovered clocks configuration on ice driver: - ETHTOOL_MSG_RCLK_SET - ETHTOOL_MSG_RCLK_GET
Co-developed-by: Arkadiusz Kubalewski arkadiusz.kubalewski@intel.com Signed-off-by: Arkadiusz Kubalewski arkadiusz.kubalewski@intel.com Signed-off-by: Maciej Machnikowski maciej.machnikowski@intel.com --- .../net/ethernet/intel/ice/ice_adminq_cmd.h | 29 ++++++ drivers/net/ethernet/intel/ice/ice_common.c | 65 +++++++++++++ drivers/net/ethernet/intel/ice/ice_common.h | 6 ++ drivers/net/ethernet/intel/ice/ice_ethtool.c | 96 +++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 2 + 5 files changed, 198 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h index 11226af7a9a4..aed03200bb99 100644 --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h @@ -1281,6 +1281,31 @@ struct ice_aqc_set_mac_lb { u8 reserved[15]; };
+/* Set PHY recovered clock output (direct 0x0630) */ +struct ice_aqc_set_phy_rec_clk_out { + u8 phy_output; + u8 port_num; + u8 flags; +#define ICE_AQC_SET_PHY_REC_CLK_OUT_OUT_EN BIT(0) +#define ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT 0xFF + u8 rsvd; + __le32 freq; + u8 rsvd2[6]; + __le16 node_handle; +}; + +/* Get PHY recovered clock output (direct 0x0631) */ +struct ice_aqc_get_phy_rec_clk_out { + u8 phy_output; + u8 port_num; + u8 flags; +#define ICE_AQC_GET_PHY_REC_CLK_OUT_OUT_EN BIT(0) + u8 rsvd; + __le32 freq; + u8 rsvd2[6]; + __le16 node_handle; +}; + struct ice_aqc_link_topo_params { u8 lport_num; u8 lport_num_valid; @@ -2033,6 +2058,8 @@ struct ice_aq_desc { struct ice_aqc_get_phy_caps get_phy; struct ice_aqc_set_phy_cfg set_phy; struct ice_aqc_restart_an restart_an; + struct ice_aqc_set_phy_rec_clk_out set_phy_rec_clk_out; + struct ice_aqc_get_phy_rec_clk_out get_phy_rec_clk_out; struct ice_aqc_gpio read_write_gpio; struct ice_aqc_sff_eeprom read_write_sff_param; struct ice_aqc_set_port_id_led set_port_id_led; @@ -2188,6 +2215,8 @@ enum ice_adminq_opc { ice_aqc_opc_get_link_status = 0x0607, ice_aqc_opc_set_event_mask = 0x0613, ice_aqc_opc_set_mac_lb = 0x0620, + ice_aqc_opc_set_phy_rec_clk_out = 0x0630, + ice_aqc_opc_get_phy_rec_clk_out = 0x0631, ice_aqc_opc_get_link_topo = 0x06E0, ice_aqc_opc_set_port_id_led = 0x06E9, ice_aqc_opc_set_gpio = 0x06EC, diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c index 8069141ac105..29d302ea1e56 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -5242,3 +5242,68 @@ bool ice_is_clock_mux_present_e810t(struct ice_hw *hw) return true; }
+/** + * ice_aq_set_phy_rec_clk_out - set RCLK phy out + * @hw: pointer to the HW struct + * @phy_output: PHY reference clock output pin + * @enable: GPIO state to be applied + * @freq: PHY output frequency + * + * Set CGU reference priority (0x0630) + * Return 0 on success or negative value on failure. + */ +enum ice_status +ice_aq_set_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, bool enable, + u32 *freq) +{ + struct ice_aqc_set_phy_rec_clk_out *cmd; + struct ice_aq_desc desc; + enum ice_status status; + + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_set_phy_rec_clk_out); + cmd = &desc.params.set_phy_rec_clk_out; + cmd->phy_output = phy_output; + cmd->port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT; + cmd->flags = enable & ICE_AQC_SET_PHY_REC_CLK_OUT_OUT_EN; + cmd->freq = cpu_to_le32(*freq); + + status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL); + if (!status) + *freq = le32_to_cpu(cmd->freq); + + return status; +} + +/** + * ice_aq_get_phy_rec_clk_out + * @hw: pointer to the HW struct + * @phy_output: PHY reference clock output pin + * @port_num: Port number + * @flags: PHY flags + * @freq: PHY output frequency + * + * Get PHY recovered clock output (0x0631) + */ +enum ice_status +ice_aq_get_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, u8 *port_num, + u8 *flags, u32 *freq) +{ + struct ice_aqc_get_phy_rec_clk_out *cmd; + struct ice_aq_desc desc; + enum ice_status status; + + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_get_phy_rec_clk_out); + cmd = &desc.params.get_phy_rec_clk_out; + cmd->phy_output = phy_output; + cmd->port_num = *port_num; + + status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL); + if (!status) { + *port_num = cmd->port_num; + *flags = cmd->flags; + *freq = le32_to_cpu(cmd->freq); + } + + return status; +} + diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h index aaed388a40a8..8a99c8364173 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.h +++ b/drivers/net/ethernet/intel/ice/ice_common.h @@ -166,6 +166,12 @@ ice_ena_vsi_rdma_qset(struct ice_port_info *pi, u16 vsi_handle, u8 tc, enum ice_status ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state, u16 *dpll_state, u64 *phase_offset, u8 *eec_mode); +enum ice_status +ice_aq_set_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, bool enable, + u32 *freq); +enum ice_status +ice_aq_get_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, u8 *port_num, + u8 *flags, u32 *freq); int ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid, u16 *q_id); diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index 5af2faaa21e1..2ae58f23c845 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -4076,6 +4076,99 @@ ice_get_module_eeprom(struct net_device *netdev, return 0; }
+/** + * ice_get_rclk_range - get range of recovered clock indices + * @netdev: network interface device structure + * @min_idx: min rclk index + * @max_idx: max rclk index + * @extack: netlink extended ack + */ +static int +ice_get_rclk_range(struct net_device *netdev, u32 *min_idx, u32 *max_idx, + struct netlink_ext_ack *extack) +{ + struct ice_netdev_priv *np = netdev_priv(netdev); + struct ice_vsi *vsi = np->vsi; + struct ice_pf *pf = vsi->back; + + if (!ice_is_feature_supported(pf, ICE_F_CGU)) + return -EOPNOTSUPP; + + *min_idx = 0; + *max_idx = ICE_RCLK_PIN_MAX; + + return 0; +} + +/** + * ice_get_rclk_state - get state of a recovered frequency output pin + * @netdev: network interface device structure + * @out_idx: index of a questioned pin + * @ena: returned state of a pin + * @extack: netlink extended ack + */ +static int +ice_get_rclk_state(struct net_device *netdev, u32 out_idx, + bool *ena, struct netlink_ext_ack *extack) +{ + u8 port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT, flags; + struct ice_netdev_priv *np = netdev_priv(netdev); + struct ice_vsi *vsi = np->vsi; + struct ice_pf *pf = vsi->back; + u32 freq; + int ret; + + if (!ice_is_feature_supported(pf, ICE_F_CGU)) + return -EOPNOTSUPP; + + if (out_idx > ICE_RCLK_PIN_MAX) + return -EINVAL; + + ret = ice_aq_get_phy_rec_clk_out(&pf->hw, out_idx, + &port_num, &flags, &freq); + if (ret) + return ret; + + if (flags & ICE_AQC_GET_PHY_REC_CLK_OUT_OUT_EN) + *ena = true; + else + *ena = false; + + return ret; +} + +/** + * ice_set_rclk_out - enable/disable recovered clock redirection to the + * output pin + * @netdev: network interface device structure + * @out_idx: index of pin being configured + * @ena: requested state of a pin + * @extack: netlink extended ack + */ +static int +ice_set_rclk_out(struct net_device *netdev, u32 out_idx, bool ena, + struct netlink_ext_ack *extack) +{ + struct ice_netdev_priv *np = netdev_priv(netdev); + struct ice_vsi *vsi = np->vsi; + struct ice_pf *pf = vsi->back; + enum ice_status ret; + u32 freq; + + if (!ice_is_feature_supported(pf, ICE_F_CGU)) + return -EOPNOTSUPP; + + if (out_idx > ICE_RCLK_PIN_MAX) + return -EINVAL; + + ret = ice_aq_set_phy_rec_clk_out(&pf->hw, out_idx, + ena, &freq); + if (ret) + return ret; + + return ret; +} + static const struct ethtool_ops ice_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE | @@ -4121,6 +4214,9 @@ static const struct ethtool_ops ice_ethtool_ops = { .set_fecparam = ice_set_fecparam, .get_module_info = ice_get_module_info, .get_module_eeprom = ice_get_module_eeprom, + .get_rclk_range = ice_get_rclk_range, + .get_rclk_state = ice_get_rclk_state, + .set_rclk_out = ice_set_rclk_out, };
static const struct ethtool_ops ice_ethtool_safe_mode_ops = { diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index 28b04ec40bae..865ca680b62e 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -132,4 +132,6 @@ enum ice_e810t_cgu_pins { NUM_E810T_CGU_PINS };
+#define ICE_RCLK_PIN_MAX (REF1N - REF1P) + #endif /* _ICE_PTP_HW_H_ */
On Fri, 10 Dec 2021 14:45:46 +0100 Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
You missed CCing Vadim. I guess Ccing the right people may be right up there with naming things as the hardest things in SW development..
Anyway, Vadim - do you have an ETA on the first chunk of the PLL work?
-----Original Message----- From: Jakub Kicinski kuba@kernel.org Sent: Friday, December 10, 2021 5:17 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux- kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, 10 Dec 2021 14:45:46 +0100 Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
You missed CCing Vadim. I guess Ccing the right people may be right up there with naming things as the hardest things in SW development..
Anyway, Vadim - do you have an ETA on the first chunk of the PLL work?
Sounds about right :) thanks for adding Vadim!
-----Original Message----- From: Machnikowski, Maciej maciej.machnikowski@intel.com Sent: poniedziałek, 13 grudnia 2021 09:54 To: Jakub Kicinski kuba@kernel.org Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux-kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: RE: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
-----Original Message----- From: Jakub Kicinski kuba@kernel.org Sent: Friday, December 10, 2021 5:17 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux- kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, 10 Dec 2021 14:45:46 +0100 Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
You missed CCing Vadim. I guess Ccing the right people may be right up there with naming things as the hardest things in SW development..
Anyway, Vadim - do you have an ETA on the first chunk of the PLL work?
Sounds about right :) thanks for adding Vadim!
Good day Vadim,
Can we help on the new PLL interfaces? I can start some works related to that, although would need a guidance from the expert. Where to place it? What in-kernel interfaces to use? Any other high level tips that could be useful? Or if you already started some work, could you please share some information?
On 15.12.2021 12:14, Kubalewski, Arkadiusz wrote:
-----Original Message----- From: Machnikowski, Maciej maciej.machnikowski@intel.com Sent: poniedziałek, 13 grudnia 2021 09:54 To: Jakub Kicinski kuba@kernel.org Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux-kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: RE: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
-----Original Message----- From: Jakub Kicinski kuba@kernel.org Sent: Friday, December 10, 2021 5:17 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux- kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, 10 Dec 2021 14:45:46 +0100 Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
You missed CCing Vadim. I guess Ccing the right people may be right up there with naming things as the hardest things in SW development..
Anyway, Vadim - do you have an ETA on the first chunk of the PLL work?
Sounds about right :) thanks for adding Vadim!
Good day Vadim,
Can we help on the new PLL interfaces? I can start some works related to that, although would need a guidance from the expert. Where to place it? What in-kernel interfaces to use? Any other high level tips that could be useful? Or if you already started some work, could you please share some information?
Hi!
I'm going to publish RFC till the end of the week and we will be able to continue discussion via this mailing list. I think that netlink is a good option for in-kernel interface and is easy to implement.
On 15.12.2021 12:14, Kubalewski, Arkadiusz wrote:
-----Original Message----- From: Machnikowski, Maciej maciej.machnikowski@intel.com Sent: poniedziałek, 13 grudnia 2021 09:54 To: Jakub Kicinski kuba@kernel.org Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux-kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: RE: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
-----Original Message----- From: Jakub Kicinski kuba@kernel.org Sent: Friday, December 10, 2021 5:17 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux- kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, 10 Dec 2021 14:45:46 +0100 Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
You missed CCing Vadim. I guess Ccing the right people may be right up there with naming things as the hardest things in SW development..
Anyway, Vadim - do you have an ETA on the first chunk of the PLL work?
Sounds about right :) thanks for adding Vadim!
Good day Vadim,
Can we help on the new PLL interfaces? I can start some works related to that, although would need a guidance from the expert. Where to place it? What in-kernel interfaces to use? Any other high level tips that could be useful? Or if you already started some work, could you please share some information?
Hi!
I'm going to publish RFC till the end of the week and we will be able to continue discussion via this mailing list. I think that netlink is a good option for in-kernel interface and is easy to implement.
Oh, that sounds great! Thank you!
On 15.12.2021 12:14, Kubalewski, Arkadiusz wrote:
-----Original Message----- From: Machnikowski, Maciej maciej.machnikowski@intel.com Sent: poniedziałek, 13 grudnia 2021 09:54 To: Jakub Kicinski kuba@kernel.org Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux-kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: RE: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
-----Original Message----- From: Jakub Kicinski kuba@kernel.org Sent: Friday, December 10, 2021 5:17 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kubalewski, Arkadiusz arkadiusz.kubalewski@intel.com; richardcochran@gmail.com; Byagowi, Ahmad abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; linux- kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com; petrm@nvidia.com; Vadim Fedorenko vfedorenko@novek.ru Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, 10 Dec 2021 14:45:46 +0100 Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
You missed CCing Vadim. I guess Ccing the right people may be right up there with naming things as the hardest things in SW development..
Anyway, Vadim - do you have an ETA on the first chunk of the PLL work?
Sounds about right :) thanks for adding Vadim!
Good day Vadim,
Can we help on the new PLL interfaces? I can start some works related to that, although would need a guidance from the expert. Where to place it? What in-kernel interfaces to use? Any other high level tips that could be useful? Or if you already started some work, could you please share some information?
Hi!
I'm going to publish RFC till the end of the week and we will be able to continue discussion via this mailing list. I think that netlink is a good option for in-kernel interface and is easy to implement.
Oh, that sounds great! Thank you!
Good day Vadim,
Hope you are well! I have been trying to find a new RFC on mailing list, but could not find it. I guess, it was not submitted yet? Could you please share your current schedule for this task? Or maybe we could help somehow?
On Fri, Dec 10, 2021 at 02:45:46PM +0100, Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
The dependency is the other way around. It doesn't make sense to add APIs to configure the inputs of an object that doesn't exist. First add the EEC object, then we can talk about APIs to configure its inputs from netdevs.
With these four patches alone, user space doesn't know how many EECs there are in the system, it doesn't know the mapping from netdev to EEC, it doesn't know the state of the EEC, it doesn't know which source is chosen in case more than one source is enabled. Patch #3 tries to work around it by having ice print to kernel log, when the information should really be exposed via the EEC object.
+ dev_warn(ice_pf_to_dev(pf), + "<DPLL%i> state changed to: %d, pin %d", + ICE_CGU_DPLL_SYNCE, + pf->synce_dpll_state, + pin);
Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
When the EEC object materializes, we might find out that this API needs to be changed / reworked / removed, but we won't be able to do that given it's uAPI. I don't know where the confidence that it won't happen stems from when there are so many question marks around this new object.
v5:
- rewritten the documentation
- fixed doxygen headers
v4:
- Dropped EEC_STATE reporting (TBD: DPLL subsystem)
- moved recovered clock configuration to ethtool netlink
v3:
- remove RTM_GETRCLKRANGE
- return state of all possible pins in the RTM_GETRCLKSTATE
- clarify documentation
v2:
- improved documentation
- fixed kdoc warning
RFC history: v2:
- removed whitespace changes
- fix issues reported by test robot
v3:
- Changed naming from SyncE to EEC
- Clarify cover letter and commit message for patch 1
v4:
- Removed sync_source and pin_idx info
- Changed one structure to attributes
- Added EEC_SRC_PORT flag to indicate that the EEC is synchronized to the recovered clock of a port that returns the state
v5:
- add EEC source as an optiona attribute
- implement support for recovered clocks
- align states returned by EEC to ITU-T G.781
v6:
- fix EEC clock state reporting
- add documentation
- fix descriptions in code comments
Maciej Machnikowski (4): ice: add support detecting features based on netlist ethtool: Add ability to configure recovered clock for SyncE feature ice: add support for monitoring SyncE DPLL state ice: add support for recovered clocks
Documentation/networking/ethtool-netlink.rst | 62 ++++ drivers/net/ethernet/intel/ice/ice.h | 7 + .../net/ethernet/intel/ice/ice_adminq_cmd.h | 70 ++++- drivers/net/ethernet/intel/ice/ice_common.c | 224 +++++++++++++++ drivers/net/ethernet/intel/ice/ice_common.h | 20 +- drivers/net/ethernet/intel/ice/ice_devids.h | 3 + drivers/net/ethernet/intel/ice/ice_ethtool.c | 96 +++++++ drivers/net/ethernet/intel/ice/ice_lib.c | 6 +- drivers/net/ethernet/intel/ice/ice_ptp.c | 35 +++ drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 49 ++++ drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 36 +++ drivers/net/ethernet/intel/ice/ice_type.h | 1 + include/linux/ethtool.h | 9 + include/uapi/linux/ethtool_netlink.h | 21 ++ net/ethtool/Makefile | 3 +- net/ethtool/netlink.c | 20 ++ net/ethtool/netlink.h | 4 + net/ethtool/synce.c | 267 ++++++++++++++++++ 18 files changed, 929 insertions(+), 4 deletions(-) create mode 100644 net/ethtool/synce.c
-- 2.26.3
-----Original Message----- From: Ido Schimmel idosch@idosch.org Sent: Sunday, December 12, 2021 12:48 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, Dec 10, 2021 at 02:45:46PM +0100, Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
The dependency is the other way around. It doesn't make sense to add APIs to configure the inputs of an object that doesn't exist. First add the EEC object, then we can talk about APIs to configure its inputs from netdevs.
This API configures frequency outputs of the PTY layer of a PHY/integrated MAC. It does not configure any inputs nor it interacts with the EEC. The goal of it is to expose the clock to the piece that requires it as a reference one (a DPLL/FPGA/anything else).
I don't agree with the statement that we must have EEC object first, as we can already configure different frequency sources using different subsystems. The source of signal should be separated from its consumer.
With these four patches alone, user space doesn't know how many EECs there are in the system, it doesn't know the mapping from netdev to EEC, it doesn't know the state of the EEC, it doesn't know which source is chosen in case more than one source is enabled. Patch #3 tries to work around it by having ice print to kernel log, when the information should really be exposed via the EEC object.
The goal of them is to add API for recovered clocks - not for EECs. This part is there for observability and will still be there when EEC is in place. Those will need to be addressed by the DPLL subsystem.
dev_warn(ice_pf_to_dev(pf),"<DPLL%i> state changed to: %d, pin %d",ICE_CGU_DPLL_SYNCE,pf->synce_dpll_state,pin);Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
When the EEC object materializes, we might find out that this API needs to be changed / reworked / removed, but we won't be able to do that given it's uAPI. I don't know where the confidence that it won't happen stems from when there are so many question marks around this new object.
This API follows the functionality of other frequency outputs that exist in the kernel, like PTP period file for frequency output of PTP clock or other GPIOs. I highly doubt it'll change - we may extend it to add mapping between pins, but like I indicated - this will not always be known to SW.
Regards Maciek
On Wed, Dec 15, 2021 at 12:13:47PM +0000, Machnikowski, Maciej wrote:
-----Original Message----- From: Ido Schimmel idosch@idosch.org Sent: Sunday, December 12, 2021 12:48 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, Dec 10, 2021 at 02:45:46PM +0100, Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
The dependency is the other way around. It doesn't make sense to add APIs to configure the inputs of an object that doesn't exist. First add the EEC object, then we can talk about APIs to configure its inputs from netdevs.
This API configures frequency outputs of the PTY layer of a PHY/integrated MAC. It does not configure any inputs nor it interacts with the EEC. The goal of it is to expose the clock to the piece that requires it as a reference one (a DPLL/FPGA/anything else).
My fundamental issue with these patches is that instead of abstracting the hardware from user space they give user space direct control over it.
This approach has the advantage of keeping the kernel relatively simple and fitting more use cases than just EEC, but these are also its disadvantages. Complexity needs to live somewhere and if this complexity is related to the abstraction of hardware, then it should live in the kernel and not in user space. We should strive to come up with an API that does the same thing regardless of the underlying hardware implementation.
Look at the proposed API, it basically says "Make the clock recovered from eth0 available on pin 1". If user space issues this command on different systems, it will mean different things, based on the underlying design of the hardware and the connection of the pin: To "DPLL/FPGA/anything else".
Contrast that with an API that says "Set the source of EEC X to the clock recovered from eth0". This API is well defined and does the same thing across different systems.
Lets assume that these patches are merged as-is and tomorrow we get another implementation of these two ethtool operations in a different driver. We can't tell if these are used to feed the recovered clock into an EEC like ice does or enable some other use case that we never intended to enable.
Even if all the implementations use this API to feed the EEC, consider how difficult it is going to be for user space to use it. Ideally, user space should be able to query the state of the EEC and its source via an EEC object in a single command. With the current approach in which we have some amorphic object called "DPLL" that is only aware of pins and not netdevs, it's going to be very hard. User space will see that the DPLL is locked to the clock fed via pin 1. How user space is supposed to understand what is the source of this clock? Issue RCLK_GET dump and check for matching pin index that is enabled? User space has no reason to do it given that it doesn't even know that the source is a netdev.
I don't agree with the statement that we must have EEC object first, as we can already configure different frequency sources using different subsystems.
Regardless of all the above technical arguments, I think that these patches should not be merged now based on common sense alone. Not only these patches are of very limited use without an EEC object, they also prevent us from making changes to the API when such an object is introduced.
The source of signal should be separated from its consumer.
If it is completely separated (despite being hardwired on the board), then user space does not know how the signal is used when it issues the command. Is this signal fed into an EEC that controls that transmission rate of other netdev? Is this signal fed into an FPGA that blinks a led?
With these four patches alone, user space doesn't know how many EECs there are in the system, it doesn't know the mapping from netdev to EEC, it doesn't know the state of the EEC, it doesn't know which source is chosen in case more than one source is enabled. Patch #3 tries to work around it by having ice print to kernel log, when the information should really be exposed via the EEC object.
The goal of them is to add API for recovered clocks - not for EECs.
What do you mean by "not for EECs"? The file is called "net/ethtool/synce.c", if the signal is not being fed into an EEC then into what? It is unclear what kind of back doors this API will open.
This part is there for observability and will still be there when EEC is in place. Those will need to be addressed by the DPLL subsystem.
If it is it only meant for observability, then why these messages are emitted as warnings to the kernel log? Regardless, the user API should be designed with observability in mind so that you wouldn't need to rely on prints to the kernel log.
dev_warn(ice_pf_to_dev(pf),"<DPLL%i> state changed to: %d, pin %d",ICE_CGU_DPLL_SYNCE,pf->synce_dpll_state,pin);Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
When the EEC object materializes, we might find out that this API needs to be changed / reworked / removed, but we won't be able to do that given it's uAPI. I don't know where the confidence that it won't happen stems from when there are so many question marks around this new object.
This API follows the functionality of other frequency outputs that exist in the kernel, like PTP period file for frequency output of PTP clock or other GPIOs. I highly doubt it'll change - we may extend it to add mapping between pins, but like I indicated - this will not always be known to SW.
Regards Maciek
-----Original Message----- From: Ido Schimmel idosch@idosch.org Sent: Monday, December 20, 2021 2:51 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Wed, Dec 15, 2021 at 12:13:47PM +0000, Machnikowski, Maciej wrote:
-----Original Message----- From: Ido Schimmel idosch@idosch.org Sent: Sunday, December 12, 2021 12:48 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com Subject: Re: [PATCH v5 net-next 0/4] Add ethtool interface for RClocks
On Fri, Dec 10, 2021 at 02:45:46PM +0100, Maciej Machnikowski wrote:
Synchronous Ethernet networks use a physical layer clock to syntonize the frequency across different network elements.
Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet Equipment Clock (EEC) and have the ability to synchronize to reference frequency sources.
This patch series is a prerequisite for EEC object and adds ability to enable recovered clocks in the physical layer of the netdev object. Recovered clocks can be used as one of the reference signal by the EEC.
The dependency is the other way around. It doesn't make sense to add APIs to configure the inputs of an object that doesn't exist. First add the EEC object, then we can talk about APIs to configure its inputs from netdevs.
This API configures frequency outputs of the PTY layer of a PHY/integrated MAC. It does not configure any inputs nor it interacts with the EEC. The goal of it is to expose the clock to the piece that requires it as a reference one (a DPLL/FPGA/anything else).
My fundamental issue with these patches is that instead of abstracting the hardware from user space they give user space direct control over it.
Unfortunately, I don't see any other way to address the use cases that I listed otherwise. I'm a big fan of simple solutions, but only when they address all use cases that are known at the time of defining them.
This approach has the advantage of keeping the kernel relatively simple and fitting more use cases than just EEC, but these are also its disadvantages. Complexity needs to live somewhere and if this complexity is related to the abstraction of hardware, then it should live in the kernel and not in user space. We should strive to come up with an API that does the same thing regardless of the underlying hardware implementation.
Look at the proposed API, it basically says "Make the clock recovered from eth0 available on pin 1". If user space issues this command on different systems, it will mean different things, based on the underlying design of the hardware and the connection of the pin: To "DPLL/FPGA/anything else".
The link to the right DPLL input can be easily added later by adding the new attribute to those commands. This has a big advantage over the other model, as it is scalable to multiple different devices serviced by different drivers, as it doesn't have the issue we have with linking objects in kernel...
Contrast that with an API that says "Set the source of EEC X to the clock recovered from eth0". This API is well defined and does the same thing across different systems.
Lets assume that these patches are merged as-is and tomorrow we get another implementation of these two ethtool operations in a different driver. We can't tell if these are used to feed the recovered clock into an EEC like ice does or enable some other use case that we never intended to enable.
...that issue is caused by the randomness and scalability of that infrastructure. At the time system boots it enumerates devices and initializes them in "random" order depending on the current system configuration which makes it impossible to code any co-driver dependencies, as it's impossible to guess what identifier the kernel assigned to the other device that's connected to the same DPLL.
Even if all the implementations use this API to feed the EEC, consider how difficult it is going to be for user space to use it. Ideally, user space should be able to query the state of the EEC and its source via an EEC object in a single command. With the current approach in which we have some amorphic object called "DPLL" that is only aware of pins and not netdevs, it's going to be very hard. User space will see that the DPLL is locked to the clock fed via pin 1. How user space is supposed to understand what is the source of this clock? Issue RCLK_GET dump and check for matching pin index that is enabled? User space has no reason to do it given that it doesn't even know that the source is a netdev.
Userspace tool will always know what ports it works with - it will come from the config file, as - even if you have a dozen ports linked to a single DPLL you may decide that you don't want to send ESMC messages on some of them - just like we do with ptp4l.
The amorphic DPLL device has much more uses that a dedicated EEC object and if we know what pin the DPLL uses and what is the link between the RCLK output and the DPLL input it'll be very easy to figure out what's going on.
The dump will also not be needed, as we know who we set as the best source after receiving the ESMC packets from neighbors.
I don't agree with the statement that we must have EEC object first, as we can already configure different frequency sources using different subsystems.
Regardless of all the above technical arguments, I think that these patches should not be merged now based on common sense alone. Not only these patches are of very limited use without an EEC object, they also prevent us from making changes to the API when such an object is introduced.
And I see a big value of merging them in as that would enable upstreaming of the userspace tools that will explain a lot and close many gaps.
The source of signal should be separated from its consumer.
If it is completely separated (despite being hardwired on the board), then user space does not know how the signal is used when it issues the command. Is this signal fed into an EEC that controls that transmission rate of other netdev? Is this signal fed into an FPGA that blinks a led?
It's the same thing with frequency pins of the PTP subsystem. Userspace tool can enable them, but has no idea what's on the other end of them.
With these four patches alone, user space doesn't know how many EECs there are in the system, it doesn't know the mapping from netdev to EEC, it doesn't know the state of the EEC, it doesn't know which source is chosen in case more than one source is enabled. Patch #3 tries to work around it by having ice print to kernel log, when the information should really be exposed via the EEC object.
The goal of them is to add API for recovered clocks - not for EECs.
What do you mean by "not for EECs"? The file is called "net/ethtool/synce.c", if the signal is not being fed into an EEC then into what? It is unclear what kind of back doors this API will open.
Can be a DPLL of the radio part of the device, or different devices that expects frequency reference. If it makes it better we can move that to net/ethtool/rclk.c.
This part is there for observability and will still be there when EEC is in place. Those will need to be addressed by the DPLL subsystem.
If it is it only meant for observability, then why these messages are emitted as warnings to the kernel log? Regardless, the user API should be designed with observability in mind so that you wouldn't need to rely on prints to the kernel log.
You're right - need to change that to dev_info. And yes - in the final form userspace tools will pull EEC's info from the DPLL subsystem - this prints the state of 2 DPLLs and is designed for debuggability.
dev_warn(ice_pf_to_dev(pf),"<DPLL%i> state changed to: %d, pin %d",ICE_CGU_DPLL_SYNCE,pf->synce_dpll_state,pin);Further work is required to add the DPLL subsystem, link it to the netdev object and create API to read the EEC DPLL state.
When the EEC object materializes, we might find out that this API needs to be changed / reworked / removed, but we won't be able to do that given it's uAPI. I don't know where the confidence that it won't happen stems from when there are so many question marks around this new object.
This API follows the functionality of other frequency outputs that exist in the kernel, like PTP period file for frequency output of PTP clock or other GPIOs. I highly doubt it'll change - we may extend it to add
mapping
between pins, but like I indicated - this will not always be known to SW.
Regards Maciek
linux-kselftest-mirror@lists.linaro.org
-
Ido Schimmel -
Jakub Kicinski -
Kubalewski, Arkadiusz -
Machnikowski, Maciej -
Maciej Machnikowski
-
Vadim Fedorenko